[PATCH] staging: lustre: check result of register_shrinker

Aliaksei Karaliou akaraliou.dev at gmail.com
Mon Dec 4 18:42:12 UTC 2017


On 12/04/2017 11:40 AM, Dan Carpenter wrote:
> On Sun, Dec 03, 2017 at 07:59:07PM +0300, ak wrote:
>> Thank you for your extensive comments.
>>
>> I've also thought about adding more protection into unregister_shrinker(),
>> but not sure how to properly organize the patch set, because there will be
>> three patches:
>>      * change in mm/vmscan that adds protection and sanitizer.
>>      * fixed change for Lustre driver
>>      * there also two explicit usages of shrinker->nr_deferred in drivers -
>>         good idea to fix too.
>> All patches have different lists of maintainers, and second and third depend
>> on first one. And I don't like to send them separately.
>> So, I'm going to at least prepend this patch with mm/vmscan one.
> Fix the style for the Lustre patch and resend.  Then patch
> unregister_shrinker().  Then remove the checks.
>
> The unregister_shrinker() changes seem like a good idea, but I haven't
> really looked at it.  It might be more involved than it seems.
>
> regards,
> dan carpenter
Thanks for the comments too.
I'll send patch with accumulated fixes.
>>   }
>> diff --git a/drivers/staging/lustre/lustre/obdclass/lu_object.c b/drivers/staging/lustre/lustre/obdclass/lu_object.c
>> index b938a3f9d50a..9e0256ca2079 100644
>> --- a/drivers/staging/lustre/lustre/obdclass/lu_object.c
>> +++ b/drivers/staging/lustre/lustre/obdclass/lu_object.c
>> @@ -1951,7 +1951,7 @@ int lu_global_init(void)
>>   	 * inode, one for ea. Unfortunately setting this high value results in
>>   	 * lu_object/inode cache consuming all the memory.
>>   	 */
>> -	register_shrinker(&lu_site_shrinker);
>> +	result = register_shrinker(&lu_site_shrinker);
> There should be some error handling if the register fails. 
Yeah, I think so, but it seems that it is out of scope of this patch.
The whole negative branch in the mainline kernel looks broken (IMHO).
In mainline Lustre's git there is a reworked version of upper function 
obdclass_init(),
which at least calls lu_global_fini() before exiting `module_init` on 
further failure,
but yeah, still lacks proper cleanup inside lu_global_initcall().
I'll add one more patch in a patch-set so that maintainers may decide 
what to do with that.

Best regards,
     Aliaksei.


More information about the devel mailing list