| Message ID | CAJVOszD16vfES8e6NcanBJ7P4HHmp635Ln7VVVyraaPnm0jriA@mail.gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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. -- 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 http://vger.kernel.org/majordomo-info.html
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.
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.
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(&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 <catalin.marinas@arm.com> -- 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 http://vger.kernel.org/majordomo-info.html
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(&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 <catalin.marinas@arm.com> The patch appears to work here as well. Tested-by: Larry Finger@lwfinger.net Thanks, Larry -- 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 http://vger.kernel.org/majordomo-info.html
On Tue, Jun 7, 2016 at 3:18 PM, Larry Finger <Larry.Finger@lwfinger.net> 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(&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 <catalin.marinas@arm.com> > > > The patch appears to work here as well. > > Tested-by: Larry Finger@lwfinger.net > > Thanks, > > Larry > Works for me too. Tested-by: David Drysdale <drysdale@google.com> -- 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 http://vger.kernel.org/majordomo-info.html
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);