Re: [PATCH 7/7] dm mpath: remove extra nesting in map function when path exists

[Hannes Reinecke <hare@suse.de>]

On 02/03/2014 09:28 PM, Mike Snitzer wrote:
> Return early for case when no path exists, this eliminates the need for
> extra nesting for the case when a path exists (the common case).
> 
> Signed-off-by: Mike Snitzer <snitzer@redhat.com>
> ---
>  drivers/md/dm-mpath.c | 51 +++++++++++++++++++++++++++------------------------
>  1 file changed, 27 insertions(+), 24 deletions(-)
> 
> diff --git a/drivers/md/dm-mpath.c b/drivers/md/dm-mpath.c
> index 971fbec..272e7d4 100644
> --- a/drivers/md/dm-mpath.c
> +++ b/drivers/md/dm-mpath.c
> @@ -392,32 +392,35 @@ static int multipath_map(struct dm_target *ti, struct request *clone,
>  
>  	pgpath = m->current_pgpath;
>  
> -	if (pgpath) {
> -		if (__pgpath_busy(pgpath))
> -			goto out_unlock;
> +	if (!pgpath) {
> +		if (!__must_push_back(m))
> +			r = -EIO;	/* Failed */
> +		goto out_unlock;
> +	}
> +
> +	if (__pgpath_busy(pgpath))
> +		goto out_unlock;
>  
> -		if (pg_ready(m)) {
> -			if (set_mapinfo(m, map_context) < 0)
> -				/* ENOMEM, requeue */
> -				goto out_unlock;
> -
> -			bdev = pgpath->path.dev->bdev;
> -			clone->q = bdev_get_queue(bdev);
> -			clone->rq_disk = bdev->bd_disk;
> -			clone->cmd_flags |= REQ_FAILFAST_TRANSPORT;
> -			mpio = map_context->ptr;
> -			mpio->pgpath = pgpath;
> -			mpio->nr_bytes = nr_bytes;
> -			if (pgpath->pg->ps.type->start_io)
> -				pgpath->pg->ps.type->start_io(&pgpath->pg->ps,
> -							      &pgpath->path,
> -							      nr_bytes);
> -			r = DM_MAPIO_REMAPPED;
> +	if (pg_ready(m)) {
> +		if (set_mapinfo(m, map_context) < 0)
> +			/* ENOMEM, requeue */
>  			goto out_unlock;
> -		}
> -		__pg_init_all_paths(m, 0);
> -	} else if (!__must_push_back(m))
> -		r = -EIO;	/* Failed */
> +
> +		bdev = pgpath->path.dev->bdev;
> +		clone->q = bdev_get_queue(bdev);
> +		clone->rq_disk = bdev->bd_disk;
> +		clone->cmd_flags |= REQ_FAILFAST_TRANSPORT;
> +		mpio = map_context->ptr;
> +		mpio->pgpath = pgpath;
> +		mpio->nr_bytes = nr_bytes;
> +		if (pgpath->pg->ps.type->start_io)
> +			pgpath->pg->ps.type->start_io(&pgpath->pg->ps,
> +						      &pgpath->path,
> +						      nr_bytes);
> +		r = DM_MAPIO_REMAPPED;
> +		goto out_unlock;
> +	}
> +	__pg_init_all_paths(m, 0);
>  
>  out_unlock:
>  	spin_unlock_irqrestore(&m->lock, flags);
> 
But then you should be going full circle and move the now somewhat
lonely statement '__pg_init_all_paths' to the top, inverting the
if (pg_ready(m))
statement.

I'll be redoing the patchset (yet again)

Cheers,

Hannes
-- 
Dr. Hannes Reinecke		      zSeries & Storage
hare@suse.de			      +49 911 74053 688
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: J. Hawn, J. Guild, F. Imendörffer, HRB 16746 (AG Nürnberg)