On 6/17/26 07:32, Harry Yoo wrote:
>> Here we would get a sheaf with kmalloc_nolock() so that's ok even if it's
>> later freed by someone else by kfree(), right.
>>
>>> + }
>>> +
>>> + empty = alloc_empty_sheaf(s, gfp, alloc_flags);
>>>
>>> if (!empty)
>>> goto fail;
>>>
>>> if (!local_trylock(&s->cpu_sheaves->lock)) {
>>> - barn_put_empty_sheaf(barn, empty);
>>> + __free_empty_sheaf(s, empty, allow_spin);
>>
>> Well we could still use the barn with allow_spin == true.
>
> Initially I did
>
> if (!__barn_put_empty_sheaf(barn, empty, allow_spin))
> __free_empty_sheaf(s, empty, allow_spin);
>
> but I ended up just calling __free_empty_sheaf() because it's
> pretty rare to hit... no strong opinion though.
Yeah we can leave it simpler as it's rare.
>> But more crucially, here we might be freeing with kfree_nolock() a sheaf
>> from the barn previously allocated with kmalloc()?
>
> Well, we don't release and reacquire the local lock when we got an empty
> sheaf from the barn, so it doesn't free the sheaf from the barn?
>
> That was indeed very subtle and I got confused :D.
>
> When we free a sheaf in this function, it's always allocated in current
> context?
Indeed! It's fine then.
>> Maybe we need to track if it's the case and defer-free it or something.
>>
>> Also maybe there could be a wrapper kfree_maybe_nolock() (~better name?)
>> That means "I want to kfree safely in kfree_nolock() context something that
>> MIGHT have been kmalloc()"
>> And maybe depending on the debugging options that make kmalloc() ->
>> kfree_nolock() incompatible, if those are not enabled, it wouldn't have to
>> defer, but proceed normally?
>
> But I really like the idea of supporting kmalloc() -> kfree_nolock(),
> and I think it's worth exploring that.
At least my confusion potentially lead to something good then :)
> Thanks!
>
