Re: [PATCH] staging: lustre: check result of register_shrinker

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

 



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.
_______________________________________________
devel mailing list
devel@xxxxxxxxxxxxxxxxxxxxxx
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel



[Index of Archives]     [Linux Driver Backports]     [DMA Engine]     [Linux GPIO]     [Linux SPI]     [Video for Linux]     [Linux USB Devel]     [Linux Coverity]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Yosemite Backpacking]
  Powered by Linux