Re: kmemleak report after 9082e87bfbf8 ("block: remove struct bio_batch")
On Tue, Jun 7, 2016 at 3:18 PM, Larry Fingerwrote: > On 06/07/2016 04:39 AM, Catalin Marinas wrote: >> >> On Mon, Jun 06, 2016 at 12:09:49PM -0500, Shaun Tancheff wrote: >>> >>> 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(); >>> >>> @@ -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; >>> } >> >> >> This patch appears to fix the memory leak on my machine. >> >> Tested-by: Catalin Marinas > > > The patch appears to work here as well. > > Tested-by: Larry fin...@lwfinger.net > > Thanks, > > Larry > Works for me too. Tested-by: David Drysdale
Re: kmemleak report after 9082e87bfbf8 ("block: remove struct bio_batch")
On Tue, Jun 7, 2016 at 3:18 PM, Larry Finger wrote: > On 06/07/2016 04:39 AM, Catalin Marinas wrote: >> >> On Mon, Jun 06, 2016 at 12:09:49PM -0500, Shaun Tancheff wrote: >>> >>> 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(); >>> >>> @@ -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; >>> } >> >> >> This patch appears to fix the memory leak on my machine. >> >> Tested-by: Catalin Marinas > > > The patch appears to work here as well. > > Tested-by: Larry fin...@lwfinger.net > > Thanks, > > Larry > Works for me too. Tested-by: David Drysdale
Re: kmemleak report after 9082e87bfbf8 ("block: remove struct bio_batch")
On 06/07/2016 04:39 AM, Catalin Marinas wrote: On Mon, Jun 06, 2016 at 12:09:49PM -0500, Shaun Tancheff wrote: 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(); @@ -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; } This patch appears to fix the memory leak on my machine. Tested-by: Catalin MarinasThe patch appears to work here as well. Tested-by: Larry fin...@lwfinger.net Thanks, Larry
Re: kmemleak report after 9082e87bfbf8 ("block: remove struct bio_batch")
On 06/07/2016 04:39 AM, Catalin Marinas wrote: On Mon, Jun 06, 2016 at 12:09:49PM -0500, Shaun Tancheff wrote: 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(); @@ -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; } This patch appears to fix the memory leak on my machine. Tested-by: Catalin Marinas The patch appears to work here as well. Tested-by: Larry fin...@lwfinger.net Thanks, Larry
Re: kmemleak report after 9082e87bfbf8 ("block: remove struct bio_batch")
On Mon, Jun 06, 2016 at 12:09:49PM -0500, Shaun Tancheff wrote: > 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(); > > @@ -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; > } This patch appears to fix the memory leak on my machine. Tested-by: Catalin Marinas
Re: kmemleak report after 9082e87bfbf8 ("block: remove struct bio_batch")
On Mon, Jun 06, 2016 at 12:09:49PM -0500, Shaun Tancheff wrote: > 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(); > > @@ -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; > } This patch appears to fix the memory leak on my machine. Tested-by: Catalin Marinas
Re: kmemleak report after 9082e87bfbf8 ("block: remove struct bio_batch")
On Mon, Jun 06, 2016 at 11:06:05PM -0500, Larry Finger wrote: > The leak is definitely not related to mkfs. At the moment, my system has > been up about 26 hours, and has generated 162 of these leaks without ever > doing a single mkfs. In addition, the box say idle for almost 12 of those > hours. That makes sense only if you are using ext4 that's doing discards/write_zero at runtime. Either way, can you please give the patch from Shaun a try?
Re: kmemleak report after 9082e87bfbf8 ("block: remove struct bio_batch")
On Mon, Jun 06, 2016 at 11:06:05PM -0500, Larry Finger wrote: > The leak is definitely not related to mkfs. At the moment, my system has > been up about 26 hours, and has generated 162 of these leaks without ever > doing a single mkfs. In addition, the box say idle for almost 12 of those > hours. That makes sense only if you are using ext4 that's doing discards/write_zero at runtime. Either way, can you please give the patch from Shaun a try?
Re: kmemleak report after 9082e87bfbf8 ("block: remove struct bio_batch")
On 06/06/2016 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). The leak is definitely not related to mkfs. At the moment, my system has been up about 26 hours, and has generated 162 of these leaks without ever doing a single mkfs. In addition, the box say idle for almost 12 of those hours. Larry
Re: kmemleak report after 9082e87bfbf8 ("block: remove struct bio_batch")
On 06/06/2016 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). The leak is definitely not related to mkfs. At the moment, my system has been up about 26 hours, and has generated 162 of these leaks without ever doing a single mkfs. In addition, the box say idle for almost 12 of those hours. Larry
Re: kmemleak report after 9082e87bfbf8 ("block: remove struct bio_batch")
On 06/06/2016 11:27 AM, Christoph Hellwig wrote: On Mon, Jun 06, 2016 at 12:09:49PM -0500, Shaun Tancheff wrote: 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). Yeah, that makes sense - oddly enough submit_bio_wait doesn't do a bio_put. Still not sure why I don't see the leaks after repeated mkfs.xfs runs, though. Because some of the users (blkdev_issue_flush()) need to inspect the bio after completion. -- Jens Axboe
Re: kmemleak report after 9082e87bfbf8 ("block: remove struct bio_batch")
On 06/06/2016 11:27 AM, Christoph Hellwig wrote: On Mon, Jun 06, 2016 at 12:09:49PM -0500, Shaun Tancheff wrote: 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). Yeah, that makes sense - oddly enough submit_bio_wait doesn't do a bio_put. Still not sure why I don't see the leaks after repeated mkfs.xfs runs, though. Because some of the users (blkdev_issue_flush()) need to inspect the bio after completion. -- Jens Axboe
Re: kmemleak report after 9082e87bfbf8 ("block: remove struct bio_batch")
On Mon, Jun 06, 2016 at 07:27:18PM +0200, Christoph Hellwig wrote: > On Mon, Jun 06, 2016 at 12:09:49PM -0500, Shaun Tancheff wrote: > > 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). > > Yeah, that makes sense - oddly enough submit_bio_wait doesn't do a > bio_put. Still not sure why I don't see the leaks after repeated > mkfs.xfs runs, though. You can force more kmemleak scans via: echo scan > /sys/kernel/debug/kmemleak In my case, the leaks were reported for ext4 and appeared during boot, no need for mkfs. But kmemleak favours false negatives more than positives (otherwise it would be pretty unusable), so you don't always hit them. -- Catalin
Re: kmemleak report after 9082e87bfbf8 ("block: remove struct bio_batch")
On Mon, Jun 06, 2016 at 07:27:18PM +0200, Christoph Hellwig wrote: > On Mon, Jun 06, 2016 at 12:09:49PM -0500, Shaun Tancheff wrote: > > 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). > > Yeah, that makes sense - oddly enough submit_bio_wait doesn't do a > bio_put. Still not sure why I don't see the leaks after repeated > mkfs.xfs runs, though. You can force more kmemleak scans via: echo scan > /sys/kernel/debug/kmemleak In my case, the leaks were reported for ext4 and appeared during boot, no need for mkfs. But kmemleak favours false negatives more than positives (otherwise it would be pretty unusable), so you don't always hit them. -- Catalin
Re: kmemleak report after 9082e87bfbf8 ("block: remove struct bio_batch")
On Mon, Jun 06, 2016 at 12:09:49PM -0500, Shaun Tancheff wrote: > 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). Yeah, that makes sense - oddly enough submit_bio_wait doesn't do a bio_put. Still not sure why I don't see the leaks after repeated mkfs.xfs runs, though.
Re: kmemleak report after 9082e87bfbf8 ("block: remove struct bio_batch")
On Mon, Jun 06, 2016 at 12:09:49PM -0500, Shaun Tancheff wrote: > 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). Yeah, that makes sense - oddly enough submit_bio_wait doesn't do a bio_put. Still not sure why I don't see the leaks after repeated mkfs.xfs runs, though.
Re: kmemleak report after 9082e87bfbf8 ("block: remove struct bio_batch")
On Mon, Jun 6, 2016 at 11:12 AM, Catalin Marinaswrote: > > 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 0x880262cbe900 (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 0x880036488600 (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 majord...@vger.kernel.org > More majordomo info at >
Re: kmemleak report after 9082e87bfbf8 ("block: remove struct bio_batch")
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 0x880262cbe900 (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 0x880036488600 (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 majord...@vger.kernel.org > More majordomo info at >
Re: kmemleak report after 9082e87bfbf8 ("block: remove struct bio_batch")
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 0x880262cbe900 (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 0x880036488600 (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
Re: kmemleak report after 9082e87bfbf8 ("block: remove struct bio_batch")
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 0x880262cbe900 (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 0x880036488600 (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
Re: kmemleak report after 9082e87bfbf8 ("block: remove struct bio_batch")
Hi Catalin, 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. Can all of you who have reported the issue take a look at their slabinfo files and check if you can confirm that observation?
Re: kmemleak report after 9082e87bfbf8 ("block: remove struct bio_batch")
Hi Catalin, 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. Can all of you who have reported the issue take a look at their slabinfo files and check if you can confirm that observation?
kmemleak report after 9082e87bfbf8 ("block: remove struct bio_batch")
Hi Christoph, I tried enabling kmemleak on 4.7-rc2 on an x86 host (macbook pro running Debian sid) and I get some kmemleak reports every few minutes coming from the block layer. Reverting commit 9082e87bfbf8 ("block: remove struct bio_batch") makes them go away: unreferenced object 0x880077859c00 (size 256): comm "upowerd", pid 1185, jiffies 4295046823 (age 1874.852s) 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 e4 21 49 02 88 ff ff ..!I 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 It's coming from various processes, not just upowerd. I haven't got the chance to dig further but you may have a better idea. Thanks. -- Catalin
kmemleak report after 9082e87bfbf8 ("block: remove struct bio_batch")
Hi Christoph, I tried enabling kmemleak on 4.7-rc2 on an x86 host (macbook pro running Debian sid) and I get some kmemleak reports every few minutes coming from the block layer. Reverting commit 9082e87bfbf8 ("block: remove struct bio_batch") makes them go away: unreferenced object 0x880077859c00 (size 256): comm "upowerd", pid 1185, jiffies 4295046823 (age 1874.852s) 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 e4 21 49 02 88 ff ff ..!I 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 It's coming from various processes, not just upowerd. I haven't got the chance to dig further but you may have a better idea. Thanks. -- Catalin