Re: [PATCH v5 27/45] vmw_balloon: dynamically allocate the vmw-balloon shrinker

[Qi Zheng <zhengqi.arch@bytedance.com>]

Hi Nadav,

On 2023/8/24 23:28, Nadav Amit wrote:
> 
> 
>> On Aug 23, 2023, at 8:42 PM, Qi Zheng <zhengqi.arch@bytedance.com> wrote:
>>
>> In preparation for implementing lockless slab shrink, use new APIs to
>> dynamically allocate the vmw-balloon shrinker, so that it can be freed
>> asynchronously via RCU. Then it doesn't need to wait for RCU read-side
>> critical section when releasing the struct vmballoon.
>>
>> And we can simply exit vmballoon_init() when registering the shrinker
>> fails. So the shrinker_registered indication is redundant, just remove it.
> 
> ...
> 
> Ugh. We should have already moved to OOM notifier instead...
> 
>> static void vmballoon_unregister_shrinker(struct vmballoon *b)
>> {
>> -	if (b->shrinker_registered)
>> -		unregister_shrinker(&b->shrinker);
>> -	b->shrinker_registered = false;
>> +	shrinker_free(b->shrinker);
>> }
> 
> If the patch goes through another iteration, please add:
> 
> 	b->shrinker = NULL;
> 
> Not that this is a real issue, but I prefer it so in order to more easily
> identify UAF if the function is called elsewhere.

OK, will add it.

> 
> Otherwise, LGTM. Thanks.

Thanks for your review!

Qi

>