From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752502AbcFFRKP (ORCPT ); Mon, 6 Jun 2016 13:10:15 -0400 Received: from mx0a-00003501.pphosted.com ([67.231.144.15]:55815 "EHLO mx0a-000cda01.pphosted.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1751837AbcFFRKK convert rfc822-to-8bit (ORCPT ); Mon, 6 Jun 2016 13:10:10 -0400 Authentication-Results: seagate.com; dkim=pass header.s="google" header.d=seagate.com MIME-Version: 1.0 In-Reply-To: <20160606161245.GC29910@e104818-lin.cambridge.arm.com> References: <20160606112620.GA29910@e104818-lin.cambridge.arm.com> <20160606141334.GA6579@lst.de> <20160606161245.GC29910@e104818-lin.cambridge.arm.com> From: Shaun Tancheff Date: Mon, 6 Jun 2016 12:09:49 -0500 Message-ID: Subject: Re: kmemleak report after 9082e87bfbf8 ("block: remove struct bio_batch") To: Catalin Marinas Cc: Christoph Hellwig , linux-block@vger.kernel.org, LKML , Jens Axboe , Larry.Finger@lwfinger.net, bart.vanassche@sandisk.com, drysdale@google.com Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8BIT X-Proofpoint-PolicyRoute: Outbound X-Proofpoint-Virus-Version: vendor=fsecure engine=2.50.10432:,, definitions=2016-06-06_07:,, signatures=0 X-Proofpoint-Spam-Details: rule=notspam policy=default score=0 priorityscore=1501 suspectscore=6 malwarescore=0 phishscore=0 bulkscore=0 spamscore=0 clxscore=1011 impostorscore=0 lowpriorityscore=0 adultscore=0 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.0.1-1604210000 definitions=main-1606060193 X-Proofpoint-Spam-Policy: Default Domain Policy Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Mon, Jun 6, 2016 at 11:12 AM, Catalin Marinas wrote: > > 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: > > 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); > } > > > 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 > -- > To unsubscribe from this list: send the line "unsubscribe linux-block" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at https://urldefense.proofpoint.com/v2/url?u=http-3A__vger.kernel.org_majordomo-2Dinfo.html&d=CwIBAg&c=IGDlg0lD0b-nebmJJ0Kp8A&r=Wg5NqlNlVTT7Ugl8V50qIHLe856QW0qfG3WVYGOrWzA&m=uHYaMtk0CBsO1MWg4alX8kHC6bvXsmWCR9wL8nrX28E&s=4EHwF1z6acQYr9R052hraaTE1KUf6IiXcEmd-CSLV48&e= I'm pretty sure it is missing a bio_put() after submit_bio_wait(). Please excuse the hack-y patch but I think you need to do something like this ... (Note tabs eaten by gmail). diff --git a/block/blk-lib.c b/block/blk-lib.c index 23d7f30..9e29dc3 100644 --- a/block/blk-lib.c +++ b/block/blk-lib.c @@ -113,6 +113,7 @@ int blkdev_issue_discard(struct block_device *bdev, sector_t sector, ret = submit_bio_wait(type, bio); if (ret == -EOPNOTSUPP) ret = 0; + bio_put(bio); } blk_finish_plug(&plug); @@ -165,8 +166,10 @@ int blkdev_issue_write_same(struct block_device *bdev, sector_t sector, } } - if (bio) + if (bio) { ret = submit_bio_wait(REQ_WRITE | REQ_WRITE_SAME, bio); + bio_put(bio); + } return ret != -EOPNOTSUPP ? ret : 0; } EXPORT_SYMBOL(blkdev_issue_write_same); @@ -206,8 +209,11 @@ static int __blkdev_issue_zeroout(struct block_device *bdev, sector_t sector, } } - if (bio) - return submit_bio_wait(WRITE, bio); + if (bio) { + ret = submit_bio_wait(WRITE, bio); + bio_put(bio); + return ret; + } return 0; } -- Shaun Tancheff