Re: [PATCH v5 02/12] dm: Use bioset's front_pad for dm_rq_clone_bio_info

[Tejun Heo <tj@xxxxxxxxxx>]

Hello,

On Mon, Aug 06, 2012 at 03:08:31PM -0700, Kent Overstreet wrote:
> 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@xxxxxxxxxx>
> ---
>  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;
>  };
...
> @@ -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;

I do like this approach much better but this isn't something
super-obvious.  Can we please explain what's going on?  Especially,
the comment above dm_rq_clone_bio_info is outright misleading now.

Can someone more familiar review this one?  Alasdir, Mike?

Also, how was this tested?

Thanks.

-- 
tejun

--
dm-devel mailing list
dm-devel@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/dm-devel