From patchwork Mon Jun 6 16:12:46 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Catalin Marinas X-Patchwork-Id: 69428 Delivered-To: patch@linaro.org Received: by 10.140.106.246 with SMTP id e109csp1558636qgf; Mon, 6 Jun 2016 09:13:01 -0700 (PDT) X-Received: by 10.107.140.132 with SMTP id o126mr23658594iod.70.1465229577682; Mon, 06 Jun 2016 09:12:57 -0700 (PDT) Return-Path: Received: from vger.kernel.org (vger.kernel.org. [209.132.180.67]) by mx.google.com with ESMTP id yr6si1393952pab.245.2016.06.06.09.12.57; Mon, 06 Jun 2016 09:12:57 -0700 (PDT) Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67; Authentication-Results: mx.google.com; spf=pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752315AbcFFQMy (ORCPT + 31 others); Mon, 6 Jun 2016 12:12:54 -0400 Received: from foss.arm.com ([217.140.101.70]:37649 "EHLO foss.arm.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751631AbcFFQMw (ORCPT ); Mon, 6 Jun 2016 12:12:52 -0400 Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.72.51.249]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id 81E6349; Mon, 6 Jun 2016 09:13:24 -0700 (PDT) Received: from e104818-lin.cambridge.arm.com (e104818-lin.cambridge.arm.com [10.1.203.148]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id EF3443F5C3; Mon, 6 Jun 2016 09:12:48 -0700 (PDT) Date: Mon, 6 Jun 2016 17:12:46 +0100 From: Catalin Marinas To: Christoph Hellwig Cc: linux-block@vger.kernel.org, linux-kernel@vger.kernel.org, Jens Axboe , Larry.Finger@lwfinger.net, bart.vanassche@sandisk.com, drysdale@google.com Subject: Re: kmemleak report after 9082e87bfbf8 ("block: remove struct bio_batch") Message-ID: <20160606161245.GC29910@e104818-lin.cambridge.arm.com> References: <20160606112620.GA29910@e104818-lin.cambridge.arm.com> <20160606141334.GA6579@lst.de> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <20160606141334.GA6579@lst.de> User-Agent: Mutt/1.5.23 (2014-03-12) Sender: linux-kernel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Mon, Jun 06, 2016 at 04:13:34PM +0200, Christoph Hellwig wrote: > I've got a few reports of this over the weekend, but it still doesn't > make much sense to me. > > Could it be that kmemleak can't deal with the bio_batch logic? I've > tried to look at the various bio and biovec number entries in > /proc/slabinfo, and while they keep changing a bit during the > system runtime there doesn't seem to be a persistent increase > even after lots of mkfs calls. I think the reported leaks settle after about 10-20min (2-3 kmemleak periodic scans), so checking /proc/slabinfo may not be sufficient if the leak is not growing. The leaks also do not seem to disappear, otherwise kmemleak would no longer report them (e.g. after kfree, even if they had been previously reported). What kmemleak reports is objects for which it cannot find a pointer (to anywhere inside that object; e.g. list_heads are handled). False positives are indeed present sometimes but for cases where pointers are stored in non-tracked objects like alloc_pages(). It seems that this leak reports always come in pairs. The first one: unreferenced object 0xffff880262cbe900 (size 256): comm "NetworkManager", pid 516, jiffies 4294895670 (age 2479.340s) hex dump (first 32 bytes): 00 00 00 00 00 00 00 00 c0 f3 ab 8a 00 88 ff ff ................ 02 20 00 20 00 00 00 00 11 00 00 00 00 00 00 00 . . ............ backtrace: [] kmem_cache_alloc+0xfe/0x250 [] mempool_alloc+0x72/0x190 [] bio_alloc_bioset+0xb6/0x240 [] next_bio+0x1f/0x50 [] blkdev_issue_zeroout+0xea/0x1d0 [] ext4_issue_zeroout+0x40/0x50 [ext4] [] ext4_ext_map_blocks+0x144d/0x1bb0 [ext4] [] release_pages+0x254/0x310 [] __pagevec_release+0x2a/0x40 [] mpage_prepare_extent_to_map+0x227/0x2c0 [ext4] [] ext4_map_blocks+0x173/0x5d0 [ext4] [] ext4_writepages+0x700/0xd40 [ext4] [] legitimize_mnt+0xe/0x50 [] kmem_cache_alloc+0xfe/0x250 [] __filemap_fdatawrite_range+0xc5/0x100 [] filemap_write_and_wait_range+0x33/0x70 is the first mempool_alloc() in bio_alloc_bioset() for struct bio and front_pad. The second report: unreferenced object 0xffff880036488600 (size 256): comm "NetworkManager", pid 516, jiffies 4294895670 (age 2479.348s) hex dump (first 32 bytes): 80 39 08 00 00 ea ff ff 00 10 00 00 00 00 00 00 .9.............. 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ................ backtrace: [] kmem_cache_alloc+0xfe/0x250 [] bvec_alloc+0x57/0xe0 [] bio_alloc_bioset+0x16f/0x240 [] next_bio+0x1f/0x50 [] blkdev_issue_zeroout+0xea/0x1d0 [] ext4_issue_zeroout+0x40/0x50 [ext4] [] ext4_ext_map_blocks+0x144d/0x1bb0 [ext4] [] release_pages+0x254/0x310 [] __pagevec_release+0x2a/0x40 [] mpage_prepare_extent_to_map+0x227/0x2c0 [ext4] [] ext4_map_blocks+0x173/0x5d0 [ext4] [] ext4_writepages+0x700/0xd40 [ext4] [] legitimize_mnt+0xe/0x50 [] kmem_cache_alloc+0xfe/0x250 [] __filemap_fdatawrite_range+0xc5/0x100 [] filemap_write_and_wait_range+0x33/0x70 is for the struct bio_vec allocation in bvec_alloc() (the one going via kmem_cache_alloc). IIUC, the bio object above allocated via next_bio() -> bio_alloc_bioset() is returned to __blkdev_issue_zeroout() which eventually submits them either directly for the last one or via next_bio(). Regarding bio chaining, I can't figure out what the first bio allocated in __blkdev_issue_zeroout() is chained to since bio == NULL initially. Subsequent next_bio() allocations are linked to the previous ones via bio_chain() but somehow kmemleak loses track of the first one, hence the subsequent bios are reported as leaks. That's unless the chaining should be the other way around: Also confusing is that chaining is done via bio->bi_private, however this is overridden in other places like submit_bio_wait(). However, since I don't fully understand this code, this chaining may not even be essential to struct bio freeing (and I'm investigating the wrong path). -- Catalin Tested-by: Catalin Marinas diff --git a/block/blk-lib.c b/block/blk-lib.c index 23d7f301a196..3bf78b7b74cc 100644 --- a/block/blk-lib.c +++ b/block/blk-lib.c @@ -15,7 +15,7 @@ static struct bio *next_bio(struct bio *bio, int rw, unsigned int nr_pages, struct bio *new = bio_alloc(gfp, nr_pages); if (bio) { - bio_chain(bio, new); + bio_chain(new, bio); submit_bio(rw, bio); }