[dm-devel] dm: disable WRITE SAME if it fails

[alex chen]

On 2014/6/3 2:36, Mike Snitzer wrote:
> On Sat, May 31 2014 at 11:05am -0400,
> Alasdair G Kergon <agk at redhat.com> wrote:
> 
>> On Sat, May 31, 2014 at 04:51:30PM +0800, alex chen wrote:
>>> The original commit f84cb8a46a771f36a04a02c61ea635c968ed5f6a("dm mpath:
>>> disable WRITE SAME if it fails") disables WRITE SAME in the DM multipath
>>> device if it fails, but when the DM linear device stacks ontop of the
>>> multipath device it doesn't help.
>>> this patch adds DM linear end_io method to catch WRITE SAME errors and
>>> disables WRITE SAME in the DM linear device's queue_limits if an
>>> underlying device disables it.
>>
>> How does your patch address striped targets?
>>  
>> Shouldn't this code be taken out of mpath and moved to dm.c and applied to all
>> targets (both bio and rq-based, at least where WRITE SAME is supported)?
> 
> Alex,
> 
> I've implemented what Alasdair and I have been suggesting.
> Can you please test this untested patch?
> 
> Thanks,
> Mike
> 

I tested this patch, it is OK.
Test cases is as follow:
CASE 1:
dm linear device(dm-23): ocfs2-fs;
multipath device(dm-9);
scsi device(sdr,sdv);

After WRITE SAME IO fails:
linux-MOLybD:~ # cat /sys/block/dm-23/queue/write_same_max_bytes
0
linux-MOLybD:~ # cat /sys/block/dm-9/queue/write_same_max_bytes
0
linux-MOLybD:~ # cat /sys/block/sdr/queue/write_same_max_bytes
0
linux-MOLybD:~ # cat /sys/block/sdv/queue/write_same_max_bytes
0

Result: pass

Tested-by: Alex Chen <alex.chen at huawei.com>
> ---
>  drivers/md/dm-mpath.c |   11 +----------
>  drivers/md/dm.c       |   16 ++++++++++++++++
>  2 files changed, 17 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/md/dm-mpath.c b/drivers/md/dm-mpath.c
> index fa0f6cb..9eb6dfd 100644
> --- a/drivers/md/dm-mpath.c
> +++ b/drivers/md/dm-mpath.c
> @@ -1240,17 +1240,8 @@ static int do_end_io(struct multipath *m, struct request *clone,
>  	if (!error && !clone->errors)
>  		return 0;	/* I/O complete */
>  
> -	if (noretry_error(error)) {
> -		if ((clone->cmd_flags & REQ_WRITE_SAME) &&
> -		    !clone->q->limits.max_write_same_sectors) {
> -			struct queue_limits *limits;
> -
> -			/* device doesn't really support WRITE SAME, disable it */
> -			limits = dm_get_queue_limits(dm_table_get_md(m->ti->table));
> -			limits->max_write_same_sectors = 0;
> -		}
> +	if (noretry_error(error))
>  		return error;
> -	}
>  
>  	if (mpio->pgpath)
>  		fail_path(mpio->pgpath);
> diff --git a/drivers/md/dm.c b/drivers/md/dm.c
> index 97940fc..c37581c 100644
> --- a/drivers/md/dm.c
> +++ b/drivers/md/dm.c
> @@ -755,6 +755,14 @@ static void dec_pending(struct dm_io *io, int error)
>  	}
>  }
>  
> +static void disable_write_same(struct mapped_device *md)
> +{
> +	struct queue_limits *limits = dm_get_queue_limits(md);
> +
> +	/* device doesn't really support WRITE SAME, disable it */
> +	limits->max_write_same_sectors = 0;
> +}
> +
>  static void clone_endio(struct bio *bio, int error)
>  {
>  	int r = 0;
> @@ -783,6 +791,10 @@ static void clone_endio(struct bio *bio, int error)
>  		}
>  	}
>  
> +	if (r < 0 && (bio->bi_rw & REQ_WRITE_SAME) &&
> +	    !bdev_get_queue(bio->bi_bdev)->limits.max_write_same_sectors)
> +		disable_write_same(md);
> +
>  	free_tio(md, tio);
>  	dec_pending(io, error);
>  }
> @@ -977,6 +989,10 @@ static void dm_done(struct request *clone, int error, bool mapped)
>  			r = rq_end_io(tio->ti, clone, error, &tio->info);
>  	}
>  
> +	if (r < 0 && (clone->cmd_flags & REQ_WRITE_SAME) &&
> +	    !clone->q->limits.max_write_same_sectors)
> +		disable_write_same(tio->md);
> +
>  	if (r <= 0)
>  		/* The target wants to complete the I/O */
>  		dm_end_request(clone, r);
> 
> .
>