| Message ID | 20150803104309.GB4033@e104818-lin.cambridge.arm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Mon, Aug 03, 2015 at 11:43:09AM +0100, Catalin Marinas wrote: > The simplest would be to add a kmemleak_not_leak() annotation in > scsi_init_request(), though you would hide real leaks (if any). > > A better way could be to inform kmemleak of these pages, something like > below (compile-tested only): Both versions sound reasonable to me, but I'd prefer the second one if it works. -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, Aug 03, 2015 at 02:33:27PM +0100, Christoph Hellwig wrote: > On Mon, Aug 03, 2015 at 11:43:09AM +0100, Catalin Marinas wrote: > > The simplest would be to add a kmemleak_not_leak() annotation in > > scsi_init_request(), though you would hide real leaks (if any). > > > > A better way could be to inform kmemleak of these pages, something like > > below (compile-tested only): > > Both versions sound reasonable to me, but I'd prefer the second one if > it works. I don't currently have a system where scsi_init_request() is used. But I hacked loop_init_request (and added a loop_exit_request) to allocate a dummy structure. Without this patch, I indeed get a few hundred kmemleak reports which disappear once I apply it.
On 08/03/2015 03:43 AM, Catalin Marinas wrote: > The pages allocated for struct request contain pointers to other slab > allocations (via ops->init_request). Since kmemleak does not track/scan > page allocations, the slab objects will be reported as leaks (false > positives). This patch adds kmemleak callbacks to allow tracking of such > pages. > > Signed-off-by: Catalin Marinas<catalin.marinas@arm.com> > Reported-by: Bart Van Assche<bart.vanassche@sandisk.com> That patch works fine on my test setup, hence: Tested-by: Bart Van Assche<bart.vanassche@sandisk.com> Thanks ! Bart. -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, Aug 03, 2015 at 06:05:59PM +0100, Bart Van Assche wrote: > On 08/03/2015 03:43 AM, Catalin Marinas wrote: > > The pages allocated for struct request contain pointers to other slab > > allocations (via ops->init_request). Since kmemleak does not track/scan > > page allocations, the slab objects will be reported as leaks (false > > positives). This patch adds kmemleak callbacks to allow tracking of such > > pages. > > > > Signed-off-by: Catalin Marinas<catalin.marinas@arm.com> > > Reported-by: Bart Van Assche<bart.vanassche@sandisk.com> > > That patch works fine on my test setup, hence: > > Tested-by: Bart Van Assche<bart.vanassche@sandisk.com> Thanks for testing. Cc'ing the block layer maintainer since the patches touches this part. Jens, are you ok with the patch posted below? If yes, would you like me to re-post or you're happy to pick it up from the list: http://lkml.kernel.org/r/20150803104309.GB4033@e104818-lin.cambridge.arm.com Thanks.
diff --git a/block/blk-mq.c b/block/blk-mq.c index 7d842db59699..c5fe656fe244 100644 --- a/block/blk-mq.c +++ b/block/blk-mq.c @@ -9,6 +9,7 @@ #include <linux/backing-dev.h> #include <linux/bio.h> #include <linux/blkdev.h> +#include <linux/kmemleak.h> #include <linux/mm.h> #include <linux/init.h> #include <linux/slab.h> @@ -1448,6 +1449,11 @@ static void blk_mq_free_rq_map(struct blk_mq_tag_set *set, while (!list_empty(&tags->page_list)) { page = list_first_entry(&tags->page_list, struct page, lru); list_del_init(&page->lru); + /* + * Remove kmemleak object previously allocated in + * blk_mq_init_rq_map(). + */ + kmemleak_free(page_address(page)); __free_pages(page, page->private); } @@ -1520,6 +1526,11 @@ static struct blk_mq_tags *blk_mq_init_rq_map(struct blk_mq_tag_set *set, list_add_tail(&page->lru, &tags->page_list); p = page_address(page); + /* + * Allow kmemleak to scan these pages as they contain pointers + * to additional allocations like via ops->init_request(). + */ + kmemleak_alloc(p, order_to_size(this_order), 1, GFP_KERNEL); entries_per_page = order_to_size(this_order) / rq_size; to_do = min(entries_per_page, set->queue_depth - i); left -= to_do * rq_size;