dm: Use bioset's front_pad for dm_rq_clone_bio_info
Previously, dm_rq_clone_bio_info needed to be freed by the bio's
destructor to avoid a memory leak in the blk_rq_prep_clone() error path. This gets rid of a memory allocation and means we can kill dm_rq_bio_destructor. Change-Id: I4ed2b9dbd1dffd44ad15bc3de89d3c3b6bc7714a Signed-off-by: Kent Overstreet <koverstreet@google.com> --- drivers/md/dm.c | 31 +++++-------------------------- 1 file changed, 5 insertions(+), 26 deletions(-) diff --git a/drivers/md/dm.c b/drivers/md/dm.c index 40b7735..4014696 100644 --- a/drivers/md/dm.c +++ b/drivers/md/dm.c @@ -92,6 +92,7 @@ struct dm_rq_target_io { struct dm_rq_clone_bio_info { struct bio *orig; struct dm_rq_target_io *tio; + struct bio clone; }; union map_info *dm_get_mapinfo(struct bio *bio) @@ -467,16 +468,6 @@ static void free_rq_tio(struct dm_rq_target_io *tio) mempool_free(tio, tio->md->tio_pool); } -static struct dm_rq_clone_bio_info *alloc_bio_info(struct mapped_device *md) -{ - return mempool_alloc(md->io_pool, GFP_ATOMIC); -} - -static void free_bio_info(struct dm_rq_clone_bio_info *info) -{ - mempool_free(info, info->tio->md->io_pool); -} - static int md_in_flight(struct mapped_device *md) { return atomic_read(&md->pending[READ]) + @@ -1438,30 +1429,17 @@ void dm_dispatch_request(struct request *rq) } EXPORT_SYMBOL_GPL(dm_dispatch_request); -static void dm_rq_bio_destructor(struct bio *bio) -{ - struct dm_rq_clone_bio_info *info = bio->bi_private; - struct mapped_device *md = info->tio->md; - - free_bio_info(info); - bio_free(bio, md->bs); -} - static int dm_rq_bio_constructor(struct bio *bio, struct bio *bio_orig, void *data) { struct dm_rq_target_io *tio = data; - struct mapped_device *md = tio->md; - struct dm_rq_clone_bio_info *info = alloc_bio_info(md); - - if (!info) - return -ENOMEM; + struct dm_rq_clone_bio_info *info = + container_of(bio, struct dm_rq_clone_bio_info, clone); info->orig = bio_orig; info->tio = tio; bio->bi_end_io = end_clone_bio; bio->bi_private = info; - bio->bi_destructor = dm_rq_bio_destructor; return 0; } @@ -2696,7 +2674,8 @@ struct dm_md_mempools *dm_alloc_md_mempools(unsigned type, unsigned integrity) if (!pools->tio_pool) goto free_io_pool_and_out; - pools->bs = bioset_create(pool_size, 0); + pools->bs = bioset_create(pool_size, + offsetof(struct dm_rq_clone_bio_info, orig)); if (!pools->bs) goto free_tio_pool_and_out; -- 1.7.9.3.327.g2980b -- dm-devel mailing list dm-devel@redhat.com https://www.redhat.com/mailman/listinfo/dm-devel |
dm: Use bioset's front_pad for dm_rq_clone_bio_info
On 05/26/12 05:25, Kent Overstreet wrote:
> struct dm_rq_clone_bio_info { > struct bio *orig; > struct dm_rq_target_io *tio; > + struct bio clone; > }; ... > - pools->bs = bioset_create(pool_size, 0); > + pools->bs = bioset_create(pool_size, > + offsetof(struct dm_rq_clone_bio_info, orig)); > if (!pools->bs) > goto free_tio_pool_and_out; Did you mean offset of *clone*? "offsetof(struct dm_rq_clone_bio_info, orig)" is zero. And now _rq_bio_info_cache has no user and can be removed. Otherwise, I think it's nicer than the previous version. Thank you. -- Jun'ichi Nomura, NEC Corporation -- dm-devel mailing list dm-devel@redhat.com https://www.redhat.com/mailman/listinfo/dm-devel |
dm: Use bioset's front_pad for dm_rq_clone_bio_info
On 05/28/12 09:57, Jun'ichi Nomura wrote:
> On 05/26/12 05:25, Kent Overstreet wrote: >> - pools->bs = bioset_create(pool_size, 0); >> + pools->bs = bioset_create(pool_size, >> + offsetof(struct dm_rq_clone_bio_info, orig)); >> if (!pools->bs) >> goto free_tio_pool_and_out; > > Did you mean offset of *clone*? > "offsetof(struct dm_rq_clone_bio_info, orig)" is zero. Additional comment: frontpad is not necessary if type is DM_TYPE_BIO_BASED. Please check conditional switches done in the same function for pools->tio_pool. -- Jun'ichi Nomura, NEC Corporation -- dm-devel mailing list dm-devel@redhat.com https://www.redhat.com/mailman/listinfo/dm-devel |
dm: Use bioset's front_pad for dm_rq_clone_bio_info
Previously, dm_rq_clone_bio_info needed to be freed by the bio's
destructor to avoid a memory leak in the blk_rq_prep_clone() error path. This gets rid of a memory allocation and means we can kill dm_rq_bio_destructor. Signed-off-by: Kent Overstreet <koverstreet@google.com> --- drivers/md/dm.c | 31 +++++-------------------------- 1 files changed, 5 insertions(+), 26 deletions(-) diff --git a/drivers/md/dm.c b/drivers/md/dm.c index 40b7735..4014696 100644 --- a/drivers/md/dm.c +++ b/drivers/md/dm.c @@ -92,6 +92,7 @@ struct dm_rq_target_io { struct dm_rq_clone_bio_info { struct bio *orig; struct dm_rq_target_io *tio; + struct bio clone; }; union map_info *dm_get_mapinfo(struct bio *bio) @@ -467,16 +468,6 @@ static void free_rq_tio(struct dm_rq_target_io *tio) mempool_free(tio, tio->md->tio_pool); } -static struct dm_rq_clone_bio_info *alloc_bio_info(struct mapped_device *md) -{ - return mempool_alloc(md->io_pool, GFP_ATOMIC); -} - -static void free_bio_info(struct dm_rq_clone_bio_info *info) -{ - mempool_free(info, info->tio->md->io_pool); -} - static int md_in_flight(struct mapped_device *md) { return atomic_read(&md->pending[READ]) + @@ -1438,30 +1429,17 @@ void dm_dispatch_request(struct request *rq) } EXPORT_SYMBOL_GPL(dm_dispatch_request); -static void dm_rq_bio_destructor(struct bio *bio) -{ - struct dm_rq_clone_bio_info *info = bio->bi_private; - struct mapped_device *md = info->tio->md; - - free_bio_info(info); - bio_free(bio, md->bs); -} - static int dm_rq_bio_constructor(struct bio *bio, struct bio *bio_orig, void *data) { struct dm_rq_target_io *tio = data; - struct mapped_device *md = tio->md; - struct dm_rq_clone_bio_info *info = alloc_bio_info(md); - - if (!info) - return -ENOMEM; + struct dm_rq_clone_bio_info *info = + container_of(bio, struct dm_rq_clone_bio_info, clone); info->orig = bio_orig; info->tio = tio; bio->bi_end_io = end_clone_bio; bio->bi_private = info; - bio->bi_destructor = dm_rq_bio_destructor; return 0; } @@ -2696,7 +2674,8 @@ struct dm_md_mempools *dm_alloc_md_mempools(unsigned type, unsigned integrity) if (!pools->tio_pool) goto free_io_pool_and_out; - pools->bs = bioset_create(pool_size, 0); + pools->bs = bioset_create(pool_size, + offsetof(struct dm_rq_clone_bio_info, orig)); if (!pools->bs) goto free_tio_pool_and_out; -- 1.7.7.3 -- dm-devel mailing list dm-devel@redhat.com https://www.redhat.com/mailman/listinfo/dm-devel |
dm: Use bioset's front_pad for dm_rq_clone_bio_info
Previously, dm_rq_clone_bio_info needed to be freed by the bio's
destructor to avoid a memory leak in the blk_rq_prep_clone() error path. This gets rid of a memory allocation and means we can kill dm_rq_bio_destructor. Signed-off-by: Kent Overstreet <koverstreet@google.com> --- drivers/md/dm.c | 31 +++++-------------------------- 1 files changed, 5 insertions(+), 26 deletions(-) diff --git a/drivers/md/dm.c b/drivers/md/dm.c index 40b7735..4014696 100644 --- a/drivers/md/dm.c +++ b/drivers/md/dm.c @@ -92,6 +92,7 @@ struct dm_rq_target_io { struct dm_rq_clone_bio_info { struct bio *orig; struct dm_rq_target_io *tio; + struct bio clone; }; union map_info *dm_get_mapinfo(struct bio *bio) @@ -467,16 +468,6 @@ static void free_rq_tio(struct dm_rq_target_io *tio) mempool_free(tio, tio->md->tio_pool); } -static struct dm_rq_clone_bio_info *alloc_bio_info(struct mapped_device *md) -{ - return mempool_alloc(md->io_pool, GFP_ATOMIC); -} - -static void free_bio_info(struct dm_rq_clone_bio_info *info) -{ - mempool_free(info, info->tio->md->io_pool); -} - static int md_in_flight(struct mapped_device *md) { return atomic_read(&md->pending[READ]) + @@ -1438,30 +1429,17 @@ void dm_dispatch_request(struct request *rq) } EXPORT_SYMBOL_GPL(dm_dispatch_request); -static void dm_rq_bio_destructor(struct bio *bio) -{ - struct dm_rq_clone_bio_info *info = bio->bi_private; - struct mapped_device *md = info->tio->md; - - free_bio_info(info); - bio_free(bio, md->bs); -} - static int dm_rq_bio_constructor(struct bio *bio, struct bio *bio_orig, void *data) { struct dm_rq_target_io *tio = data; - struct mapped_device *md = tio->md; - struct dm_rq_clone_bio_info *info = alloc_bio_info(md); - - if (!info) - return -ENOMEM; + struct dm_rq_clone_bio_info *info = + container_of(bio, struct dm_rq_clone_bio_info, clone); info->orig = bio_orig; info->tio = tio; bio->bi_end_io = end_clone_bio; bio->bi_private = info; - bio->bi_destructor = dm_rq_bio_destructor; return 0; } @@ -2696,7 +2674,8 @@ struct dm_md_mempools *dm_alloc_md_mempools(unsigned type, unsigned integrity) if (!pools->tio_pool) goto free_io_pool_and_out; - pools->bs = bioset_create(pool_size, 0); + pools->bs = bioset_create(pool_size, + offsetof(struct dm_rq_clone_bio_info, orig)); if (!pools->bs) goto free_tio_pool_and_out; -- 1.7.7.3 -- dm-devel mailing list dm-devel@redhat.com https://www.redhat.com/mailman/listinfo/dm-devel |
dm: Use bioset's front_pad for dm_rq_clone_bio_info
Hi Kent, Tejun
On 9 August 2012 09:57, Kent Overstreet <koverstreet@google.com> wrote: >> Also, how was this tested? > > Well, AFAICT the only request based dm target is multipath, and from the > documentation I've seen it doesn't appear to work without multipath > hardware, or at least I haven't seen it documented how. So, unless > there's another user I missed it's not been tested. Multipath can be tested quite easily with a loopback scsi target, you don't require specialized hardware. The easiest way to do this would probably be the built in LIO target + open_iscsi initiator. I haven't attempted running this current version of the patch series but I haven't run into issues with bcache+multipath in the past. > >> >> Thanks. >> >> -- >> tejun > -- > To unsubscribe from this list: send the line "unsubscribe linux-kernel" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > Please read the FAQ at http://www.tux.org/lkml/ -- CTO | Orion Virtualisation Solutions | www.orionvm.com.au Phone: 1300 56 99 52 | Mobile: 0428 754 846 -- dm-devel mailing list dm-devel@redhat.com https://www.redhat.com/mailman/listinfo/dm-devel |
dm: Use bioset's front_pad for dm_rq_clone_bio_info
On 08/07/12 07:08, Kent Overstreet wrote:
> struct dm_rq_clone_bio_info { > struct bio *orig; > struct dm_rq_target_io *tio; > + struct bio clone; > }; ... > - pools->bs = bioset_create(pool_size, 0); > + pools->bs = bioset_create(pool_size, > + offsetof(struct dm_rq_clone_bio_info, orig)); > if (!pools->bs) > goto free_tio_pool_and_out; Shouldn't this be offsetof(struct dm_rq_clone_bio_info, clone)? -- Jun'ichi Nomura, NEC Corporation -- dm-devel mailing list dm-devel@redhat.com https://www.redhat.com/mailman/listinfo/dm-devel |
dm: Use bioset's front_pad for dm_rq_clone_bio_info
Previously, dm_rq_clone_bio_info needed to be freed by the bio's
destructor to avoid a memory leak in the blk_rq_prep_clone() error path. This gets rid of a memory allocation and means we can kill dm_rq_bio_destructor. v6: Fix comment on struct dm_rq_clone_bio_info, per Tejun Signed-off-by: Kent Overstreet <koverstreet@google.com> CC: Alasdair Kergon <agk@redhat.com> --- drivers/md/dm.c | 39 +++++++++++---------------------------- 1 file changed, 11 insertions(+), 28 deletions(-) diff --git a/drivers/md/dm.c b/drivers/md/dm.c index 0c3d6dd..5ed9779 100644 --- a/drivers/md/dm.c +++ b/drivers/md/dm.c @@ -86,12 +86,17 @@ struct dm_rq_target_io { }; /* - * For request-based dm. - * One of these is allocated per bio. + * For request-based dm - the bio clones we allocate are embedded in these + * structs. + * + * We allocate these with bio_alloc_bioset, using the front_pad parameter when + * the bioset is created - this means the bio has to come at the end of the + * struct. */ struct dm_rq_clone_bio_info { struct bio *orig; struct dm_rq_target_io *tio; + struct bio clone; }; union map_info *dm_get_mapinfo(struct bio *bio) @@ -467,16 +472,6 @@ static void free_rq_tio(struct dm_rq_target_io *tio) mempool_free(tio, tio->md->tio_pool); } -static struct dm_rq_clone_bio_info *alloc_bio_info(struct mapped_device *md) -{ - return mempool_alloc(md->io_pool, GFP_ATOMIC); -} - -static void free_bio_info(struct dm_rq_clone_bio_info *info) -{ - mempool_free(info, info->tio->md->io_pool); -} - static int md_in_flight(struct mapped_device *md) { return atomic_read(&md->pending[READ]) + @@ -1460,30 +1455,17 @@ void dm_dispatch_request(struct request *rq) } EXPORT_SYMBOL_GPL(dm_dispatch_request); -static void dm_rq_bio_destructor(struct bio *bio) -{ - struct dm_rq_clone_bio_info *info = bio->bi_private; - struct mapped_device *md = info->tio->md; - - free_bio_info(info); - bio_free(bio, md->bs); -} - static int dm_rq_bio_constructor(struct bio *bio, struct bio *bio_orig, void *data) { struct dm_rq_target_io *tio = data; - struct mapped_device *md = tio->md; - struct dm_rq_clone_bio_info *info = alloc_bio_info(md); - - if (!info) - return -ENOMEM; + struct dm_rq_clone_bio_info *info = + container_of(bio, struct dm_rq_clone_bio_info, clone); info->orig = bio_orig; info->tio = tio; bio->bi_end_io = end_clone_bio; bio->bi_private = info; - bio->bi_destructor = dm_rq_bio_destructor; return 0; } @@ -2718,7 +2700,8 @@ struct dm_md_mempools *dm_alloc_md_mempools(unsigned type, unsigned integrity) if (!pools->tio_pool) goto free_io_pool_and_out; - pools->bs = bioset_create(pool_size, 0); + pools->bs = bioset_create(pool_size, + offsetof(struct dm_rq_clone_bio_info, clone)); if (!pools->bs) goto free_tio_pool_and_out; -- 1.7.12 -- dm-devel mailing list dm-devel@redhat.com https://www.redhat.com/mailman/listinfo/dm-devel |
dm: Use bioset's front_pad for dm_rq_clone_bio_info
Previously, dm_rq_clone_bio_info needed to be freed by the bio's
destructor to avoid a memory leak in the blk_rq_prep_clone() error path. This gets rid of a memory allocation and means we can kill dm_rq_bio_destructor. The _rq_bio_info_cache kmem cache is unused now and needs to be deleted, but due to the way io_pool is used and overloaded this looks not quite trivial so I'm leaving it for a later patch. v6: Fix comment on struct dm_rq_clone_bio_info, per Tejun Signed-off-by: Kent Overstreet <koverstreet@google.com> CC: Alasdair Kergon <agk@redhat.com> Acked-by: Tejun Heo <tj@kernel.org> --- drivers/md/dm.c | 44 ++++++++++++++++---------------------------- 1 file changed, 16 insertions(+), 28 deletions(-) diff --git a/drivers/md/dm.c b/drivers/md/dm.c index 0c3d6dd..9206781 100644 --- a/drivers/md/dm.c +++ b/drivers/md/dm.c @@ -86,12 +86,17 @@ struct dm_rq_target_io { }; /* - * For request-based dm. - * One of these is allocated per bio. + * For request-based dm - the bio clones we allocate are embedded in these + * structs. + * + * We allocate these with bio_alloc_bioset, using the front_pad parameter when + * the bioset is created - this means the bio has to come at the end of the + * struct. */ struct dm_rq_clone_bio_info { struct bio *orig; struct dm_rq_target_io *tio; + struct bio clone; }; union map_info *dm_get_mapinfo(struct bio *bio) @@ -211,6 +216,11 @@ struct dm_md_mempools { static struct kmem_cache *_io_cache; static struct kmem_cache *_tio_cache; static struct kmem_cache *_rq_tio_cache; + +/* + * Unused now, and needs to be deleted. But since io_pool is overloaded and it's + * still used for _io_cache, I'm leaving this for a later cleanup + */ static struct kmem_cache *_rq_bio_info_cache; static int __init local_init(void) @@ -467,16 +477,6 @@ static void free_rq_tio(struct dm_rq_target_io *tio) mempool_free(tio, tio->md->tio_pool); } -static struct dm_rq_clone_bio_info *alloc_bio_info(struct mapped_device *md) -{ - return mempool_alloc(md->io_pool, GFP_ATOMIC); -} - -static void free_bio_info(struct dm_rq_clone_bio_info *info) -{ - mempool_free(info, info->tio->md->io_pool); -} - static int md_in_flight(struct mapped_device *md) { return atomic_read(&md->pending[READ]) + @@ -1460,30 +1460,17 @@ void dm_dispatch_request(struct request *rq) } EXPORT_SYMBOL_GPL(dm_dispatch_request); -static void dm_rq_bio_destructor(struct bio *bio) -{ - struct dm_rq_clone_bio_info *info = bio->bi_private; - struct mapped_device *md = info->tio->md; - - free_bio_info(info); - bio_free(bio, md->bs); -} - static int dm_rq_bio_constructor(struct bio *bio, struct bio *bio_orig, void *data) { struct dm_rq_target_io *tio = data; - struct mapped_device *md = tio->md; - struct dm_rq_clone_bio_info *info = alloc_bio_info(md); - - if (!info) - return -ENOMEM; + struct dm_rq_clone_bio_info *info = + container_of(bio, struct dm_rq_clone_bio_info, clone); info->orig = bio_orig; info->tio = tio; bio->bi_end_io = end_clone_bio; bio->bi_private = info; - bio->bi_destructor = dm_rq_bio_destructor; return 0; } @@ -2718,7 +2705,8 @@ struct dm_md_mempools *dm_alloc_md_mempools(unsigned type, unsigned integrity) if (!pools->tio_pool) goto free_io_pool_and_out; - pools->bs = bioset_create(pool_size, 0); + pools->bs = bioset_create(pool_size, + offsetof(struct dm_rq_clone_bio_info, clone)); if (!pools->bs) goto free_tio_pool_and_out; -- 1.7.12 -- dm-devel mailing list dm-devel@redhat.com https://www.redhat.com/mailman/listinfo/dm-devel |
dm: Use bioset's front_pad for dm_rq_clone_bio_info
Previously, dm_rq_clone_bio_info needed to be freed by the bio's
destructor to avoid a memory leak in the blk_rq_prep_clone() error path. This gets rid of a memory allocation and means we can kill dm_rq_bio_destructor. The _rq_bio_info_cache kmem cache is unused now and needs to be deleted, but due to the way io_pool is used and overloaded this looks not quite trivial so I'm leaving it for a later patch. v6: Fix comment on struct dm_rq_clone_bio_info, per Tejun Signed-off-by: Kent Overstreet <koverstreet@google.com> CC: Alasdair Kergon <agk@redhat.com> Acked-by: Tejun Heo <tj@kernel.org> --- drivers/md/dm.c | 44 ++++++++++++++++---------------------------- 1 file changed, 16 insertions(+), 28 deletions(-) diff --git a/drivers/md/dm.c b/drivers/md/dm.c index f43aaf6..f2eb730 100644 --- a/drivers/md/dm.c +++ b/drivers/md/dm.c @@ -86,12 +86,17 @@ struct dm_rq_target_io { }; /* - * For request-based dm. - * One of these is allocated per bio. + * For request-based dm - the bio clones we allocate are embedded in these + * structs. + * + * We allocate these with bio_alloc_bioset, using the front_pad parameter when + * the bioset is created - this means the bio has to come at the end of the + * struct. */ struct dm_rq_clone_bio_info { struct bio *orig; struct dm_rq_target_io *tio; + struct bio clone; }; union map_info *dm_get_mapinfo(struct bio *bio) @@ -211,6 +216,11 @@ struct dm_md_mempools { static struct kmem_cache *_io_cache; static struct kmem_cache *_tio_cache; static struct kmem_cache *_rq_tio_cache; + +/* + * Unused now, and needs to be deleted. But since io_pool is overloaded and it's + * still used for _io_cache, I'm leaving this for a later cleanup + */ static struct kmem_cache *_rq_bio_info_cache; static int __init local_init(void) @@ -467,16 +477,6 @@ static void free_rq_tio(struct dm_rq_target_io *tio) mempool_free(tio, tio->md->tio_pool); } -static struct dm_rq_clone_bio_info *alloc_bio_info(struct mapped_device *md) -{ - return mempool_alloc(md->io_pool, GFP_ATOMIC); -} - -static void free_bio_info(struct dm_rq_clone_bio_info *info) -{ - mempool_free(info, info->tio->md->io_pool); -} - static int md_in_flight(struct mapped_device *md) { return atomic_read(&md->pending[READ]) + @@ -1460,30 +1460,17 @@ void dm_dispatch_request(struct request *rq) } EXPORT_SYMBOL_GPL(dm_dispatch_request); -static void dm_rq_bio_destructor(struct bio *bio) -{ - struct dm_rq_clone_bio_info *info = bio->bi_private; - struct mapped_device *md = info->tio->md; - - free_bio_info(info); - bio_free(bio, md->bs); -} - static int dm_rq_bio_constructor(struct bio *bio, struct bio *bio_orig, void *data) { struct dm_rq_target_io *tio = data; - struct mapped_device *md = tio->md; - struct dm_rq_clone_bio_info *info = alloc_bio_info(md); - - if (!info) - return -ENOMEM; + struct dm_rq_clone_bio_info *info = + container_of(bio, struct dm_rq_clone_bio_info, clone); info->orig = bio_orig; info->tio = tio; bio->bi_end_io = end_clone_bio; bio->bi_private = info; - bio->bi_destructor = dm_rq_bio_destructor; return 0; } @@ -2718,7 +2705,8 @@ struct dm_md_mempools *dm_alloc_md_mempools(unsigned type, unsigned integrity) if (!pools->tio_pool) goto free_io_pool_and_out; - pools->bs = bioset_create(pool_size, 0); + pools->bs = bioset_create(pool_size, + offsetof(struct dm_rq_clone_bio_info, clone)); if (!pools->bs) goto free_tio_pool_and_out; -- 1.7.12 -- dm-devel mailing list dm-devel@redhat.com https://www.redhat.com/mailman/listinfo/dm-devel |
| All times are GMT. The time now is 08:47 AM. |
VBulletin, Copyright ©2000 - 2013, Jelsoft Enterprises Ltd.
Content Relevant URLs by vBSEO ©2007, Crawlability, Inc.