Commit 9ae3b3f5 authored by Jens Axboe's avatar Jens Axboe

block: provide bio_uninit() free freeing integrity/task associations

Wen reports significant memory leaks with DIF and O_DIRECT:

"With nvme devive + T10 enabled, On a system it has 256GB and started
logging /proc/meminfo & /proc/slabinfo for every minute and in an hour
it increased by 15968128 kB or ~15+GB.. Approximately 256 MB / minute
leaking.

/proc/meminfo | grep SUnreclaim...

SUnreclaim:      6752128 kB
SUnreclaim:      6874880 kB
SUnreclaim:      7238080 kB
....
SUnreclaim:     22307264 kB
SUnreclaim:     22485888 kB
SUnreclaim:     22720256 kB

When testcases with T10 enabled call into __blkdev_direct_IO_simple,
code doesn't free memory allocated by bio_integrity_alloc. The patch
fixes the issue. HTX has been run with +60 hours without failure."

Since __blkdev_direct_IO_simple() allocates the bio on the stack, it
doesn't go through the regular bio free. This means that any ancillary
data allocated with the bio through the stack is not freed. Hence, we
can leak the integrity data associated with the bio, if the device is
using DIF/DIX.

Fix this by providing a bio_uninit() and export it, so that we can use
it to free this data. Note that this is a minimal fix for this issue.
Any current user of bio's that are allocated outside of
bio_alloc_bioset() suffers from this issue, most notably some drivers.
We will fix those in a more comprehensive patch for 4.13. This also
means that the commit marked as being fixed by this isn't the real
culprit, it's just the most obvious one out there.

Fixes: 542ff7bf ("block: new direct I/O implementation")
Reported-by: default avatarWen Xiong <wenxiong@linux.vnet.ibm.com>
Reviewed-by: default avatarChristoph Hellwig <hch@lst.de>
Signed-off-by: default avatarJens Axboe <axboe@kernel.dk>
parent ebef7368
...@@ -240,20 +240,21 @@ struct bio_vec *bvec_alloc(gfp_t gfp_mask, int nr, unsigned long *idx, ...@@ -240,20 +240,21 @@ struct bio_vec *bvec_alloc(gfp_t gfp_mask, int nr, unsigned long *idx,
return bvl; return bvl;
} }
static void __bio_free(struct bio *bio) void bio_uninit(struct bio *bio)
{ {
bio_disassociate_task(bio); bio_disassociate_task(bio);
if (bio_integrity(bio)) if (bio_integrity(bio))
bio_integrity_free(bio); bio_integrity_free(bio);
} }
EXPORT_SYMBOL(bio_uninit);
static void bio_free(struct bio *bio) static void bio_free(struct bio *bio)
{ {
struct bio_set *bs = bio->bi_pool; struct bio_set *bs = bio->bi_pool;
void *p; void *p;
__bio_free(bio); bio_uninit(bio);
if (bs) { if (bs) {
bvec_free(bs->bvec_pool, bio->bi_io_vec, BVEC_POOL_IDX(bio)); bvec_free(bs->bvec_pool, bio->bi_io_vec, BVEC_POOL_IDX(bio));
...@@ -271,6 +272,11 @@ static void bio_free(struct bio *bio) ...@@ -271,6 +272,11 @@ static void bio_free(struct bio *bio)
} }
} }
/*
* Users of this function have their own bio allocation. Subsequently,
* they must remember to pair any call to bio_init() with bio_uninit()
* when IO has completed, or when the bio is released.
*/
void bio_init(struct bio *bio, struct bio_vec *table, void bio_init(struct bio *bio, struct bio_vec *table,
unsigned short max_vecs) unsigned short max_vecs)
{ {
...@@ -297,7 +303,7 @@ void bio_reset(struct bio *bio) ...@@ -297,7 +303,7 @@ void bio_reset(struct bio *bio)
{ {
unsigned long flags = bio->bi_flags & (~0UL << BIO_RESET_BITS); unsigned long flags = bio->bi_flags & (~0UL << BIO_RESET_BITS);
__bio_free(bio); bio_uninit(bio);
memset(bio, 0, BIO_RESET_BYTES); memset(bio, 0, BIO_RESET_BYTES);
bio->bi_flags = flags; bio->bi_flags = flags;
......
...@@ -263,7 +263,10 @@ __blkdev_direct_IO_simple(struct kiocb *iocb, struct iov_iter *iter, ...@@ -263,7 +263,10 @@ __blkdev_direct_IO_simple(struct kiocb *iocb, struct iov_iter *iter,
kfree(vecs); kfree(vecs);
if (unlikely(bio.bi_error)) if (unlikely(bio.bi_error))
return bio.bi_error; ret = bio.bi_error;
bio_uninit(&bio);
return ret; return ret;
} }
......
...@@ -426,6 +426,7 @@ extern void bio_advance(struct bio *, unsigned); ...@@ -426,6 +426,7 @@ extern void bio_advance(struct bio *, unsigned);
extern void bio_init(struct bio *bio, struct bio_vec *table, extern void bio_init(struct bio *bio, struct bio_vec *table,
unsigned short max_vecs); unsigned short max_vecs);
extern void bio_uninit(struct bio *);
extern void bio_reset(struct bio *); extern void bio_reset(struct bio *);
void bio_chain(struct bio *, struct bio *); void bio_chain(struct bio *, struct bio *);
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment