Re: [PATCH 3/8] execmem: rework execmem_cache_free()

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Mon, Jul 07, 2025 at 11:06:25AM -0400, Liam R. Howlett wrote:
> * Mike Rapoport <rppt@xxxxxxxxxx> [250707 07:32]:
> > On Mon, Jul 07, 2025 at 01:11:02PM +0200, Peter Zijlstra wrote:
> > > 
> > > 	err = __execmem_cache_free(&mas, ptr, GFP_KERNEL | __GFP_NORETRY);
> > > 	if (err) {
> > > 		mas_store_gfp(&mas, pending_free_set(ptr), GFP_KERNEL);
> > > 		execmem_cache.pending_free_cnt++;
> > > 		schedule_delayed_work(&execmem_cache_free_work, FREE_DELAY);
> > > 		return true;
> > > 	}
> > > 
> > > 	schedule_work(&execmem_cache_clean_work);
> > > 	return true;
> > > }
> > > 
> > > And now I have to ask what happens if mas_store_gfp() returns an error?
> > 
> > AFAIU it won't. mas points to exact slot we've got the area from, nothing else
> > can modify the tree because of the mutex, so that mas_store_gfp()
> > essentially updates the value at an existing entry.
> > 
> > I'll add a comment about it.
> > 
> > Added @Liam to make sure I'm not saying nonsense :)
> > 
> 
> Yes, if there is already a node with a value with the same range, there
> will be no allocations that will happen, so it'll just change the
> pointer for you.  This is a slot store operation.
> 
> But, if it's possible to have no entries (an empty tree, or a single
> value at 0), you will most likely allocate a node to store it, which is
> 256B.
> 
> I don't think this is a concern in this particular case though as you
> are searching for an entry and storing, so it needs to exist.  So
> really, the only scenario here is if you store 1 - ULONG_MAX (without
> having expanded a root node) or 0 - ULONG_MAX, and that seems invalid.

Thanks for clarification, Liam!
The tree cannot be empty at that point and if it has a single value, it
won't be at 0, I'm quite sure no architecture has execmem areas at 0.

 
> Thanks,
> Liam

-- 
Sincerely yours,
Mike.




[Index of Archives]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Big List of Linux Books]

  Powered by Linux