Messages in this thread | | | Date | Thu, 24 Sep 2009 09:07:47 +0800 | From | Li Zefan <> | Subject | Re: [PATCH] Add missing blk_trace_remove_sysfs to be in pair with blk_trace_init_sysfs |
| |
>>> Subject: [PATCH] Add missing blk_trace_remove_sysfs to be in pair with >>> blk_trace_init_sysfs >>> From: Zdenek Kabelac <zkabelac@redhat.com> >>> >>> Adds missing blk_trace_remove_sysfs() to be in pair with >>> blk_trace_init_sysfs() introduced in commit >>> 1d54ad6da9192fed5dd3b60224d9f2dfea0dcd82. >>> >>> Problem was noticed via kmemleak backtrace when some sysfs entries >>> were note properly destroyed during device removal: >>> >> Thanks for reporting and fixing this! >> >>> @@ -465,6 +466,7 @@ void blk_unregister_queue(struct gendisk *disk) >>> >>> kobject_uevent(&q->kobj, KOBJ_REMOVE); >>> kobject_del(&q->kobj); >>> + blk_trace_remove_sysfs(disk_to_dev(disk)); >> This should be moved outside of 'if'. >> > > I was not really sure about the proper place - if it could be placed > before if() or after the if(){} - as I've not checked in depth
Just use the reverse order against blk_register_queue() should be fine.
> connection between kobj and sysfs. It's somewhat unclear why all the > kobject operation are only within this if(){} block - so I've thought > there is some reason... > IMHO only elv_unregister_queue() should be probably in the if(){} block. >
Seems it's a bug to put kobject_put(dev->kobj) in the if block.
I created a stacked device (mdadm) and kmemleak still reported leaks even after I fixed the blktrace issue. And then I moved kobejct_put() outside the if, no more leaks reports.
> Feel free to update/fix. >
| |