dm: reduce memory overhead of DM devices
Subsystems (2)
cover dm: reduce memory overhead of DM devices
This series of patches reduce the memory overhead of device-mapper device mainly by removing unused mempool for bio vecs.
DM creates per-device bioset to ensure forward progress under low memory situation and bioset always includes mempool for bvec.
However, with the introduction of immutable bvec by Kent Overstreet, dm core now uses bio_clone_fast for creating a clone bio, and no longer needs bvecs for it.
For example, when you create 10,000 bio-based DM devices and 1,000 request-based DM devices, memory usage of biovec under no load is:
# grep biovec /proc/slabinfo biovec-256 418068 418068 4096 ... biovec-128 0 0 2048 ... biovec-64 0 0 1024 ... biovec-16 0 0 256 ...
With this patch series applied, the usage becomes:
# grep biovec /proc/slabinfo biovec-256 116 116 4096 ... biovec-128 0 0 2048 ... biovec-64 0 0 1024 ... biovec-16 0 0 256 ...
So 4096 * (418068 - 116) = 1.6GB of memory is saved in this example.
Jun'ichi Nomura (4):
dm: remove nr_iovecs parameter from alloc_tio()
block: use bio_clone_fast() in blk_rq_prep_clone()
block: add bioset_nobvec_create()
dm: use bioset_nobvec_create()
block/bio.c | 61 ++++++++++++++++++++++++++++++++++++++--------------- block/blk-core.c | 2 +- drivers/md/dm.c | 15 +++++-------- include/linux/bio.h | 1 + 4 files changed, 51 insertions(+), 28 deletions(-)
Signature · 4 lines
1.9.3 -- Jun'ichi Nomura, NEC Corporation
PATCH 1/4 dm: remove nr_iovecs parameter from alloc_tio()
alloc_tio() allocates a bio for clone. It takes the number of bvecs to allocate for the clone-bio.
However, with the introduction of bio_clone_fast() in v3.14, we no longer need to allocate bvecs and nr_iovecs is always 0.
__clone_and_map_simple_bio() looks like passing non-zero nr_iovecs, but its value is always within the range of inline bvecs and no allocation actually happens. If allocation happened, BUG_ON() in __bio_clone_fast() would trigger.
This patch removes the nr_iovecs parameter from alloc_tio() to prevent possible future mis-use of the interface.
--- drivers/md/dm.c | 13 ++++--------- 1 file changed, 4 insertions(+), 9 deletions(-)
drivers/md/dm.c +4 -9
@@ -1249,13 +1249,13 @@ static void clone_bio(struct dm_target_io *tio, struct bio *bio, }static struct dm_target_io *alloc_tio (struct clone_info *ci, -struct dm_target *ti,int nr_iovecs, +struct dm_target *ti,unsigned target_bio_nr) {struct dm_target_io *tio;struct bio *clone; - clone =bio_alloc_bioset (GFP_NOIO , nr_iovecs, ci->md->bs); + clone =bio_alloc_bioset (GFP_NOIO ,0 , ci->md->bs); tio = container_of(clone, struct dm_target_io, clone); tio->io = ci->io;
@@ -1269,16 +1269,11 @@ static void __clone_and_map_simple_bio(struct clone_info *ci,struct dm_target *ti, unsigned target_bio_nr, unsigned *len) { - struct dm_target_io *tio = alloc_tio(ci, ti, ci->bio->bi_max_vecs, target_bio_nr); + struct dm_target_io *tio = alloc_tio(ci, ti, target_bio_nr);struct bio *clone = &tio->clone; tio->len_ptr = len; -/* -* Discard requests require the bio's inline iovecs be initialized. -* ci->bio->bi_max_vecs is BIO_INLINE_VECS anyway, for both flush -* and discard, so no need for concern about wasted bvec allocations. -*/ __bio_clone_fast (clone, ci->bio);if (len)bio_setup_sector (clone, ci->sector, *len);
@@ -1322,7 +1317,7 @@ static void __clone_and_map_data_bio(struct clone_info *ci, struct dm_target *ti num_target_bios = ti->num_write_bios (ti, bio);for (target_bio_nr =0 ; target_bio_nr < num_target_bios; target_bio_nr++) { - tio =alloc_tio (ci, ti,0 , target_bio_nr); + tio =alloc_tio (ci, ti, target_bio_nr); tio->len_ptr = len;clone_bio (tio, bio, sector, *len);__map_bio (tio);
Signature · 1 line
1.9.3
Re: [PATCH 1/4] dm: remove nr_iovecs parameter from alloc_tio()
Quoted 11 lines · Junichi Nomura
alloc_tio()allocates a bio for clone. It takes the number of bvecs to allocate for the clone-bio.
However, with the introduction ofbio_clone_fast()in v3.14, we no longer need to allocate bvecs andnr_iovecsis always 0.
__clone_and_map_simple_bio()looks like passing non-zeronr_iovecs, but its value is always within the range of inline bvecs and no allocation actually happens. If allocation happened,BUG_ON()in__bio_clone_fast()would trigger.
Quoted 4 lines
This patch removes the
nr_iovecsparameter fromalloc_tio()to prevent possible future mis-use of the interface.Signed-off-by Jun'ichi Nomura <[email protected]>
I picked this patch up and staged in linux-dm.git's for-next, see:
https://git.kernel.org/cgit/linux/kernel/git/device-mapper/linux-dm.git/commit/?h=for-next&id=8d59090cdc0ea91ffc390f78d0406baf8f66ae38
PATCH 2/4 block: use bio_clone_fast() in blk_rq_prep_clone()
In reply to Junichi NomuraRequest cloning clones bios in the request to track the completion of each bio.
For that purpose, we can use bio_clone_fast() instead of bio_clone() to avoid unnecessary allocation and copy of bvecs.
This patch reduces memory footprint of request-based device-mapper (about 1-4KB for each request) and is a preparation for further reduction of memory usage by removing unused bvec mempool.
--- block/blk-core.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
block/blk-core.c +1 -1
@@ -2931,7 +2931,7 @@ int blk_rq_prep_clone(struct request *rq, struct request *rq_src,blk_rq_init (NULL , rq);__rq_for_each_bio (bio_src, rq_src) { - bio =bio_clone_bioset (bio_src, gfp_mask, bs); + bio =bio_clone_fast (bio_src, gfp_mask, bs);if (!bio) goto free_and_out;
Signature · 1 line
1.9.3
PATCH 3/4 block: add bioset_nobvec_create()
In reply to Junichi NomuraUsers of bio_clone_fast() do not need its own bvecs. Allocating bvec mempool for such users is waste of memory.
This variant allows them to create bioset without bvec mempool.
--- block/bio.c | 61 ++++++++++++++++++++++++++++++++++++++--------------- include/linux/bio.h | 1 + 2 files changed, 45 insertions(+), 17 deletions(-)
block/bio.c +44 -17
@@ -428,6 +428,9 @@ struct bio *bio_alloc_bioset(gfp_t gfp_mask, int nr_iovecs, struct bio_set *bs) front_pad =0 ; inline_vecs = nr_iovecs; }else { +/* should not use nobvec bioset for nr_iovecs > 0 */ +if (WARN_ON_ONCE (!bs->bvec_pool && nr_iovecs >0 )) +return NULL ;/* * generic_make_request() converts recursion to iteration; this * means if we're running beneath it, any bios we allocate and
@@ -1900,20 +1903,9 @@ void bioset_free(struct bio_set *bs) }EXPORT_SYMBOL (bioset_free); -/** -* bioset_create - Create a bio_set -* @pool_size: Number of bio and bio_vecs to cache in the mempool -* @front_pad: Number of bytes to allocate in front of the returned bio -* -* Description: -* Set up a bio_set to be used with @bio_alloc_bioset. Allows the caller -* to ask for a number of bytes to be allocated in front of the bio. -* Front pad allocation is useful for embedding the bio inside -* another structure, to avoid allocating extra data to go with the bio. -* Note that the bio must be embedded at the END of that structure always, -* or things will break badly. -*/ -struct bio_set *bioset_create (unsigned int pool_size,unsigned int front_pad) +static struct bio_set *__bioset_create (unsigned int pool_size, +unsigned int front_pad, +unsigned int create_bvec_pool) {unsigned int back_pad =BIO_INLINE_VECS *sizeof (struct bio_vec );struct bio_set *bs;
@@ -1938,9 +1930,11 @@ struct bio_set *bioset_create(unsigned int pool_size, unsigned int front_pad)if (!bs->bio_pool) goto bad; - bs->bvec_pool =biovec_create_pool (pool_size); -if (!bs->bvec_pool) - goto bad; +if (create_bvec_pool) { + bs->bvec_pool =biovec_create_pool (pool_size); +if (!bs->bvec_pool) + goto bad; + } bs->rescue_workqueue =alloc_workqueue ("bioset" ,WQ_MEM_RECLAIM ,0 );if (!bs->rescue_workqueue)
@@ -1951,8 +1945,41 @@ bad:bioset_free (bs);return NULL ; } + +/** +* bioset_create - Create a bio_set +* @pool_size: Number of bio and bio_vecs to cache in the mempool +* @front_pad: Number of bytes to allocate in front of the returned bio +* +* Description: +* Set up a bio_set to be used with @bio_alloc_bioset. Allows the caller +* to ask for a number of bytes to be allocated in front of the bio. +* Front pad allocation is useful for embedding the bio inside +* another structure, to avoid allocating extra data to go with the bio. +* Note that the bio must be embedded at the END of that structure always, +* or things will break badly. +*/ +struct bio_set *bioset_create (unsigned int pool_size,unsigned int front_pad) +{ +return __bioset_create (pool_size, front_pad,1 ); +}EXPORT_SYMBOL (bioset_create); +/** +* bioset_nobvec_create - Create a bio_set without bio_vec mempool +* @pool_size: Number of bio to cache in the mempool +* @front_pad: Number of bytes to allocate in front of the returned bio +* +* Description: +* Same functionality as bioset_create() except that mempool is not +* created for bio_vecs. Saving some memory for bio_clone_fast() users. +*/ +struct bio_set *bioset_nobvec_create (unsigned int pool_size,unsigned int front_pad) +{ +return __bioset_create (pool_size, front_pad,0 ); +} +EXPORT_SYMBOL (bioset_nobvec_create); +#ifdef CONFIG_BLK_CGROUP /** * bio_associate_current - associate a bio with %current
include/linux/bio.h +1 -0
@@ -342,6 +342,7 @@ static inline struct bio *bio_next_split(struct bio *bio, int sectors, }extern struct bio_set *bioset_create (unsigned int ,unsigned int ); +extern struct bio_set *bioset_nobvec_create (unsigned int ,unsigned int );extern void bioset_free (struct bio_set *);extern mempool_t *biovec_create_pool (int pool_entries);
Signature · 1 line
1.9.3
Re: [PATCH 3/4] block: add bioset_nobvec_create()
Quoted 29 lines · Junichi Nomura
Users of
bio_clone_fast()do not need its own bvecs. Allocating bvec mempool for such users is waste of memory.This variant allows them to create bioset without bvec mempool.
Signed-off-by Jun'ichi Nomura <[email protected]>--- block/bio.c | 61 ++++++++++++++++++++++++++++++++++++++--------------- include/linux/bio.h | 1 + 2 files changed, 45 insertions(+), 17 deletions(-)block/bio.c +0
-10@@ -1900,20 +1903,9 @@ void bioset_free(struct bio_set *bs) }EXPORT_SYMBOL (bioset_free); -/** -* bioset_create - Create a bio_set -* @pool_size: Number of bio and bio_vecs to cache in the mempool -* @front_pad: Number of bytes to allocate in front of the returned bio -* -* Description: -* Set up a bio_set to be used with @bio_alloc_bioset. Allows the caller -* to ask for a number of bytes to be allocated in front of the bio. -* Front pad allocation is useful for embedding the bio inside -* another structure, to avoid allocating extra data to go with the bio.
block/bio.c void bioset_free(struct bio_set *bs) · line 1911
Quoted 10 lines
block/bio.c +3
-4-* Note that the bio must be embedded at the END of that structure always, -* or things will break badly. -*/ -struct bio_set *bioset_create (unsigned int pool_size,unsigned int front_pad) +static struct bio_set *__bioset_create (unsigned int pool_size, +unsigned int front_pad, +unsigned int create_bvec_pool) {unsigned int back_pad =BIO_INLINE_VECS *sizeof (struct bio_vec );struct bio_set *bs;
Any reason not to use bool for create_bvec_pool?
Quoted 32 lines
block/bio.c +26
-0@@ -1951,8 +1945,41 @@ bad:bioset_free (bs);return NULL ; } + +/** +* bioset_create - Create a bio_set +* @pool_size: Number of bio and bio_vecs to cache in the mempool +* @front_pad: Number of bytes to allocate in front of the returned bio +* +* Description: +* Set up a bio_set to be used with @bio_alloc_bioset. Allows the caller +* to ask for a number of bytes to be allocated in front of the bio. +* Front pad allocation is useful for embedding the bio inside +* another structure, to avoid allocating extra data to go with the bio. +* Note that the bio must be embedded at the END of that structure always, +* or things will break badly. +*/ +struct bio_set *bioset_create (unsigned int pool_size,unsigned int front_pad) +{ +return __bioset_create (pool_size, front_pad,1 ); +}EXPORT_SYMBOL (bioset_create); +/** +* bioset_nobvec_create - Create a bio_set without bio_vec mempool +* @pool_size: Number of bio to cache in the mempool +* @front_pad: Number of bytes to allocate in front of the returned bio +* +* Description: +* Same functionality as bioset_create() except that mempool is not +* created for bio_vecs. Saving some memory for bio_clone_fast() users.
block/bio.c bad: · line 1985
Quoted 10 lines
block/bio.c +7
-0+*/ +struct bio_set *bioset_nobvec_create (unsigned int pool_size,unsigned int front_pad) +{ +return __bioset_create (pool_size, front_pad,0 ); +} +EXPORT_SYMBOL (bioset_nobvec_create); +#ifdef CONFIG_BLK_CGROUP /** * bio_associate_current - associate a bio with %current
And use true and false accordingly...
include/linux/bio.h static inline struct bio *bio_next_split(struct bio *bio, int sectors, · line 347
Quoted 11 lines
include/linux/bio.h +1
-0@@ -342,6 +342,7 @@ static inline struct bio *bio_next_split(struct bio *bio, int sectors, }extern struct bio_set *bioset_create (unsigned int ,unsigned int ); +extern struct bio_set *bioset_nobvec_create (unsigned int ,unsigned int );extern void bioset_free (struct bio_set *);extern mempool_t *biovec_create_pool (int pool_entries);
Also, I prefer the function name: bioset_create_nobvec (to keep it in the "bioset_create" family).
Otherwise, please feel free to add to v2:
PATCH 4/4 dm: use bioset_nobvec_create()
In reply to Junichi NomuraSince dm core uses bio_clone_fast() for both bio-based and request-based DM device, there is no need for bvec mempool.
With this patch, on arch with 4KB page for example, memory usage will be reduced by 64KB for each bio-based DM device and 1MB for each request-based DM device.
--- drivers/md/dm.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/md/dm.c +1 -1
@@ -2895,7 +2895,7 @@ struct dm_md_mempools *dm_alloc_md_mempools(unsigned type, unsigned integrity, uif (!pools->io_pool) goto out; - pools->bs =bioset_create (pool_size, front_pad); + pools->bs =bioset_nobvec_create (pool_size, front_pad);if (!pools->bs) goto out;
Signature · 1 line
1.9.3
Quoted 18 lines · Junichi Nomura
This series of patches reduce the memory overhead of device-mapper device mainly by removing unused mempool for bio vecs.
DM creates per-device bioset to ensure forward progress under low memory situation and bioset always includes mempool for bvec.
However, with the introduction of immutable bvec by Kent Overstreet, dm core now uses
bio_clone_fastfor creating a clone bio, and no longer needs bvecs for it.For example, when you create 10,000 bio-based DM devices and 1,000 request-based DM devices, memory usage of biovec under no load is:
# grep biovec /proc/slabinfo biovec-256 418068 418068 4096 ... biovec-128 0 0 2048 ... biovec-64 0 0 1024 ... biovec-16 0 0 256 ...
Quoted 8 lines
With this patch series applied, the usage becomes:
# grep biovec /proc/slabinfo biovec-256 116 116 4096 ... biovec-128 0 0 2048 ... biovec-64 0 0 1024 ... biovec-16 0 0 256 ...So 4096 * (418068 - 116) = 1.6GB of memory is saved in this example.
Very nice Junichi!
Kent definitely left us some low hanging fruit that offers a huge win (and fixes the really long-standing problem of excessive memory reserves for each request-based DM device!)
Jens, this is obviously an awkward patchset given it touches both block and DM (block more so than DM). I'd be fine with you pulling it into block (or vice-versa if you're OK with me staging it for 3.18 via linux-dm.git with your Acked-by).
Either way, this is all to say, once my feedback to patch 3 is taken into account, for the set:
Quoted 32 lines · Mike Snitzer
Quoted 18 lines · Junichi Nomura
This series of patches reduce the memory overhead of device-mapper device mainly by removing unused mempool for bio vecs.
DM creates per-device bioset to ensure forward progress under low memory situation and bioset always includes mempool for bvec.
However, with the introduction of immutable bvec by Kent Overstreet, dm core now uses
bio_clone_fastfor creating a clone bio, and no longer needs bvecs for it.For example, when you create 10,000 bio-based DM devices and 1,000 request-based DM devices, memory usage of biovec under no load is:
# grep biovec /proc/slabinfo biovec-256 418068 418068 4096 ... biovec-128 0 0 2048 ... biovec-64 0 0 1024 ... biovec-16 0 0 256 ...Quoted 8 lines
With this patch series applied, the usage becomes:
# grep biovec /proc/slabinfo biovec-256 116 116 4096 ... biovec-128 0 0 2048 ... biovec-64 0 0 1024 ... biovec-16 0 0 256 ...So 4096 * (418068 - 116) = 1.6GB of memory is saved in this example.
Very nice Junichi!
Quoted 8 lines
Kent definitely left us some low hanging fruit that offers a huge win (and fixes the really long-standing problem of excessive memory reserves for each request-based DM device!)
Jens, this is obviously an awkward patchset given it touches both block and DM (block more so than DM). I'd be fine with you pulling it into block (or vice-versa if you're OK with me staging it for 3.18 via linux-dm.git with your Acked-by).
I picked up this patchset, refactored the patches based on my earlier suggested s/bioset_nobvec_create/bioset_create_nobvec/ rename and tweaked headers a bit. I've staged the result in the for-next branch of linux-dm.git for v3.18 inclusion, see:
https://git.kernel.org/cgit/linux/kernel/git/device-mapper/linux-dm.git/log/?h=for-next
b83bb63 dm: remove nr_iovecs parameter from alloc_tio() b875215 block: use bio_clone_fast() in blk_rq_prep_clone() cd595c9 block: add bioset_create_nobvec()
3611594 dm: use bioset_create_nobvec()
Jens, I can easily yield this patchset to you if you care. Or I can backfill your Acked-by in the above 2 block commits (I obviously won't just send block changes upstream without your OK!).
Please advise, thanks!
Mike
Jens Axboe
maintainerQuoted 50 lines · Mike Snitzer
Quoted 32 lines · Mike Snitzer
Quoted 18 lines · Junichi Nomura
This series of patches reduce the memory overhead of device-mapper device mainly by removing unused mempool for bio vecs.
DM creates per-device bioset to ensure forward progress under low memory situation and bioset always includes mempool for bvec.
However, with the introduction of immutable bvec by Kent Overstreet, dm core now uses
bio_clone_fastfor creating a clone bio, and no longer needs bvecs for it.For example, when you create 10,000 bio-based DM devices and 1,000 request-based DM devices, memory usage of biovec under no load is:
# grep biovec /proc/slabinfo biovec-256 418068 418068 4096 ... biovec-128 0 0 2048 ... biovec-64 0 0 1024 ... biovec-16 0 0 256 ...Quoted 8 lines
With this patch series applied, the usage becomes:
# grep biovec /proc/slabinfo biovec-256 116 116 4096 ... biovec-128 0 0 2048 ... biovec-64 0 0 1024 ... biovec-16 0 0 256 ...So 4096 * (418068 - 116) = 1.6GB of memory is saved in this example.
Very nice Junichi!
Quoted 8 lines
Kent definitely left us some low hanging fruit that offers a huge win (and fixes the really long-standing problem of excessive memory reserves for each request-based DM device!)
Jens, this is obviously an awkward patchset given it touches both block and DM (block more so than DM). I'd be fine with you pulling it into block (or vice-versa if you're OK with me staging it for 3.18 via linux-dm.git with your Acked-by).
I picked up this patchset, refactored the patches based on my earlier suggested s/bioset_nobvec_create/bioset_create_nobvec/ rename and tweaked headers a bit. I've staged the result in the for-next branch of linux-dm.git for v3.18 inclusion, see:
https://git.kernel.org/cgit/linux/kernel/git/device-mapper/linux-dm.git/log/?h=for-next
Quoted 8 lines
b83bb63 dm: remove
nr_iovecsparameter fromalloc_tio()b875215 block: usebio_clone_fast()inblk_rq_prep_clone()cd595c9 block: addbioset_create_nobvec()
3611594 dm: usebioset_create_nobvec()Jens, I can easily yield this patchset to you if you care. Or I can backfill your Acked-by in the above 2 block commits (I obviously won't just send block changes upstream without your OK!).
My only worry is the switch to fast clones, apart from that, it's pretty basic. But we are pretty close to the 3.18 merge window, and as such, I'd prefer to let it simmer for a release.
Signature · 1 line
Jens Axboe
Quoted 7 lines · Jens Axboe
Quoted 3 lines
Jens, I can easily yield this patchset to you if you care. Or I can backfill your Acked-by in the above 2 block commits (I obviously won't just send block changes upstream without your OK!).
My only worry is the switch to fast clones, apart from that, it's pretty basic. But we are pretty close to the 3.18 merge window, and as such, I'd prefer to let it simmer for a release.
Fine with me. fast clones is all we need (thanks to Kent's immutable biovec work), but waiting until 3.19 will give us plenty of time to verify all is well.
PATCH v2 1/4 dm: remove nr_iovecs parameter from alloc_tio()
In reply to Mike SnitzerFrom: Junichi Nomura <[email protected]>
alloc_tio() uses bio_alloc_bioset() to allocate a clone-bio for a bio. alloc_tio() takes the number of bvecs to allocate for the clone-bio. However, with v3.14's immutable biovec changes DM now uses __bio_clone_fast() and no longer needs to allocate bvecs.
In practice, the 'nr_iovecs' passed to alloc_tio() is always effectively
0. __clone_and_map_simple_bio() looked like it was passing non-zero nr_iovecs, but its value was always within the range of inline bvecs and no allocation actually happened. If allocation happened, the BUG_ON() in __bio_clone_fast() would've triggered.
Remove the nr_iovecs parameter from alloc_tio() to prevent possible future bio_alloc_bioset() mis-use of a new bioset interface that will no longer allow bvecs to be allocated.
Also fix extra whitespace before the __bio_clone_fast() call in __clone_and_map_simple_bio().
--- drivers/md/dm.c | 15 +++++---------- 1 file changed, 5 insertions(+), 10 deletions(-)
drivers/md/dm.c +5 -10
@@ -1372,13 +1372,13 @@ static void clone_bio(struct dm_target_io *tio, struct bio *bio, }static struct dm_target_io *alloc_tio (struct clone_info *ci, -struct dm_target *ti,int nr_iovecs, +struct dm_target *ti,unsigned target_bio_nr) {struct dm_target_io *tio;struct bio *clone; - clone =bio_alloc_bioset (GFP_NOIO , nr_iovecs, ci->md->bs); + clone =bio_alloc_bioset (GFP_NOIO ,0 , ci->md->bs); tio = container_of(clone, struct dm_target_io, clone); tio->io = ci->io;
@@ -1392,17 +1392,12 @@ static void __clone_and_map_simple_bio(struct clone_info *ci,struct dm_target *ti, unsigned target_bio_nr, unsigned *len) { - struct dm_target_io *tio = alloc_tio(ci, ti, ci->bio->bi_max_vecs, target_bio_nr); + struct dm_target_io *tio = alloc_tio(ci, ti, target_bio_nr);struct bio *clone = &tio->clone; tio->len_ptr = len; -/* -* Discard requests require the bio's inline iovecs be initialized. -* ci->bio->bi_max_vecs is BIO_INLINE_VECS anyway, for both flush -* and discard, so no need for concern about wasted bvec allocations. -*/ -__bio_clone_fast (clone, ci->bio); +__bio_clone_fast (clone, ci->bio);if (len)bio_setup_sector (clone, ci->sector, *len);
@@ -1445,7 +1440,7 @@ static void __clone_and_map_data_bio(struct clone_info *ci, struct dm_target *ti num_target_bios = ti->num_write_bios (ti, bio);for (target_bio_nr =0 ; target_bio_nr < num_target_bios; target_bio_nr++) { - tio =alloc_tio (ci, ti,0 , target_bio_nr); + tio =alloc_tio (ci, ti, target_bio_nr); tio->len_ptr = len;clone_bio (tio, bio, sector, *len);__map_bio (tio);
Signature · 1 line
1.9.3
PATCH v2 2/4 block: use bio_clone_fast() in blk_rq_prep_clone()
From: Junichi Nomura <[email protected]>
Request cloning clones bios in the request to track the completion of each bio.
For that purpose, we can use bio_clone_fast() instead of bio_clone() to avoid unnecessary allocation and copy of bvecs.
This patch reduces memory footprint of request-based device-mapper (about 1-4KB for each request) and is a preparation for further reduction of memory usage by removing unused bvec mempool.
--- block/blk-core.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
block/blk-core.c +1 -1
@@ -2931,7 +2931,7 @@ int blk_rq_prep_clone(struct request *rq, struct request *rq_src,blk_rq_init (NULL , rq);__rq_for_each_bio (bio_src, rq_src) { - bio =bio_clone_bioset (bio_src, gfp_mask, bs); + bio =bio_clone_fast (bio_src, gfp_mask, bs);if (!bio) goto free_and_out;
Signature · 1 line
1.9.3
PATCH v2 3/4 block: add bioset_create_nobvec()
In reply to Mike SnitzerFrom: Junichi Nomura <[email protected]>
Users of bio_clone_fast() do not want bios with their own bvecs. Allocating a bvec mempool as part of the bioset intended for such users is a waste of memory.
bioset_create_nobvec() creates a bioset that doesn't have the bvec mempool.
--- block/bio.c | 61 ++++++++++++++++++++++++++++++++++++++--------------- include/linux/bio.h | 1 + 2 files changed, 45 insertions(+), 17 deletions(-)
block/bio.c +44 -17
@@ -428,6 +428,9 @@ struct bio *bio_alloc_bioset(gfp_t gfp_mask, int nr_iovecs, struct bio_set *bs) front_pad =0 ; inline_vecs = nr_iovecs; }else { +/* should not use nobvec bioset for nr_iovecs > 0 */ +if (WARN_ON_ONCE (!bs->bvec_pool && nr_iovecs >0 )) +return NULL ;/* * generic_make_request() converts recursion to iteration; this * means if we're running beneath it, any bios we allocate and
@@ -1900,20 +1903,9 @@ void bioset_free(struct bio_set *bs) }EXPORT_SYMBOL (bioset_free); -/** -* bioset_create - Create a bio_set -* @pool_size: Number of bio and bio_vecs to cache in the mempool -* @front_pad: Number of bytes to allocate in front of the returned bio -* -* Description: -* Set up a bio_set to be used with @bio_alloc_bioset. Allows the caller -* to ask for a number of bytes to be allocated in front of the bio. -* Front pad allocation is useful for embedding the bio inside -* another structure, to avoid allocating extra data to go with the bio. -* Note that the bio must be embedded at the END of that structure always, -* or things will break badly. -*/ -struct bio_set *bioset_create (unsigned int pool_size,unsigned int front_pad) +static struct bio_set *__bioset_create (unsigned int pool_size, +unsigned int front_pad, +bool create_bvec_pool) {unsigned int back_pad =BIO_INLINE_VECS *sizeof (struct bio_vec );struct bio_set *bs;
@@ -1938,9 +1930,11 @@ struct bio_set *bioset_create(unsigned int pool_size, unsigned int front_pad)if (!bs->bio_pool) goto bad; - bs->bvec_pool =biovec_create_pool (pool_size); -if (!bs->bvec_pool) - goto bad; +if (create_bvec_pool) { + bs->bvec_pool =biovec_create_pool (pool_size); +if (!bs->bvec_pool) + goto bad; + } bs->rescue_workqueue =alloc_workqueue ("bioset" ,WQ_MEM_RECLAIM ,0 );if (!bs->rescue_workqueue)
@@ -1951,8 +1945,41 @@ bad:bioset_free (bs);return NULL ; } + +/** +* bioset_create - Create a bio_set +* @pool_size: Number of bio and bio_vecs to cache in the mempool +* @front_pad: Number of bytes to allocate in front of the returned bio +* +* Description: +* Set up a bio_set to be used with @bio_alloc_bioset. Allows the caller +* to ask for a number of bytes to be allocated in front of the bio. +* Front pad allocation is useful for embedding the bio inside +* another structure, to avoid allocating extra data to go with the bio. +* Note that the bio must be embedded at the END of that structure always, +* or things will break badly. +*/ +struct bio_set *bioset_create (unsigned int pool_size,unsigned int front_pad) +{ +return __bioset_create (pool_size, front_pad, true); +}EXPORT_SYMBOL (bioset_create); +/** +* bioset_create_nobvec - Create a bio_set without bio_vec mempool +* @pool_size: Number of bio to cache in the mempool +* @front_pad: Number of bytes to allocate in front of the returned bio +* +* Description: +* Same functionality as bioset_create() except that mempool is not +* created for bio_vecs. Saving some memory for bio_clone_fast() users. +*/ +struct bio_set *bioset_create_nobvec (unsigned int pool_size,unsigned int front_pad) +{ +return __bioset_create (pool_size, front_pad, false); +} +EXPORT_SYMBOL (bioset_create_nobvec); +#ifdef CONFIG_BLK_CGROUP /** * bio_associate_current - associate a bio with %current
include/linux/bio.h +1 -0
@@ -342,6 +342,7 @@ static inline struct bio *bio_next_split(struct bio *bio, int sectors, }extern struct bio_set *bioset_create (unsigned int ,unsigned int ); +extern struct bio_set *bioset_create_nobvec (unsigned int ,unsigned int );extern void bioset_free (struct bio_set *);extern mempool_t *biovec_create_pool (int pool_entries);
Signature · 1 line
1.9.3
PATCH v2 4/4 dm: use bioset_create_nobvec()
In reply to Mike SnitzerFrom: Junichi Nomura <[email protected]>
Since DM core uses bio_clone_fast() for both bio-based and request-based DM devices there is no need for DM's bioset to have a bvec mempool.
With this patch, on arch with 4KB page for example, memory usage will be reduced by 64KB for each bio-based DM device and 1MB for each request-based DM device.
For example, when you create 10,000 bio-based DM devices and 1,000 request-based DM devices, memory usage of biovec under no load is:
# grep biovec /proc/slabinfo
biovec-256 418068 418068 4096 ... biovec-128 0 0 2048 ... biovec-64 0 0 1024 ... biovec-16 0 0 256 ...
With this patch series applied, the usage becomes:
# grep biovec /proc/slabinfo
biovec-256 116 116 4096 ... biovec-128 0 0 2048 ... biovec-64 0 0 1024 ... biovec-16 0 0 256 ...
So 4096 * (418068 - 116) = 1.6GB of memory is saved in this example.
--- drivers/md/dm.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/md/dm.c +1 -1
@@ -3021,7 +3021,7 @@ struct dm_md_mempools *dm_alloc_md_mempools(unsigned type, unsigned integrity, uif (!pools->io_pool) goto out; - pools->bs =bioset_create (pool_size, front_pad); + pools->bs =bioset_nobvec_create (pool_size, front_pad);if (!pools->bs) goto out;
Signature · 1 line
1.9.3
Re: [PATCH v2 4/4] dm: use bioset_create_nobvec()
drivers/md/dm.c struct dm_md_mempools *dm_alloc_md_mempools(unsigned type, unsigned integrity, u · line 3026
Quoted 8 lines · Mike Snitzer
drivers/md/dm.c +1
-1@@ -3021,7 +3021,7 @@ struct dm_md_mempools *dm_alloc_md_mempools(unsigned type, unsigned integrity, uif (!pools->io_pool) goto out; - pools->bs =bioset_create (pool_size, front_pad); + pools->bs =bioset_nobvec_create (pool_size, front_pad);if (!pools->bs) goto out;
Hi Mike,
thank you for the rename. It's better.
But we have to s/nobvec_create/create_nobvec/ here, too.
Signature · 1 line
Jun'ichi Nomura, NEC Corporation
Re: [PATCH v2 4/4] dm: use bioset_create_nobvec()
Quoted 12 lines · Junichi Nomura
drivers/md/dm.c struct dm_md_mempools *dm_alloc_md_mempools(unsigned type, unsigned integrity, u · line 3026
Quoted 8 lines · Mike Snitzer
drivers/md/dm.c +1
-1@@ -3021,7 +3021,7 @@ struct dm_md_mempools *dm_alloc_md_mempools(unsigned type, unsigned integrity, uif (!pools->io_pool) goto out; - pools->bs =bioset_create (pool_size, front_pad); + pools->bs =bioset_nobvec_create (pool_size, front_pad);if (!pools->bs) goto out;Hi Mike,
Quoted 2 lines
thank you for the rename. It's better.
But we have to s/nobvec_create/create_nobvec/ here, too.
Oops, thanks for letting me know. I've fixed it and rebased linux-dm.git's for-next branch ontop of linux-block's for-3.18/core since Jens staged your 2 block patches.
Quoted 5 lines · Junichi Nomura
This series of patches reduce the memory overhead of device-mapper device mainly by removing unused mempool for bio vecs.
DM creates per-device bioset to ensure forward progress under low memory situation and bioset always includes mempool for bvec.
Thanks, this looks like a good step forward.
I'd still love to go all the way and avoid any bio cloning. I can see two ways to do this:
1) similar to the original block layer multipathing with a callback that allows the submitter to hook into the completion path at blk_update_request time.
2) by not completing the io chain in req_bio_endio, similar to how we do it for requests that are part of a flush sequence using the REQ_FLUSH_SEQ.
While the first seems like the cleaner architecture it might be worth to look into the second version given that 1) wasn't well received last time, and 2) already works nicely for a very similar use case. If we end up implementing 1) it should also be able to replace the current flag based completion hack for the flush sequence.