Re: [PATCH --resend 1/2] md: disable WRITE SAME if it fails for linear/raid0

[NeilBrown <neilb@suse.com>]

[-- Attachment #1: Type: text/plain, Size: 2568 bytes --]

On Mon, Feb 13 2017, Shaohua Li wrote:

> This makes md do the same thing as dm for write same IO failure. Please
> see 7eee4ae(dm: disable WRITE SAME if it fails) for details why we need
> this.
>
> Also reported here: https://bugzilla.kernel.org/show_bug.cgi?id=118581
>
> Signed-off-by: Shaohua Li <shli@fb.com>
> ---
>  drivers/md/linear.c |  6 +++++-
>  drivers/md/md.c     | 45 +++++++++++++++++++++++++++++++++++++++++++++
>  drivers/md/md.h     |  2 ++
>  drivers/md/raid0.c  |  6 +++++-
>  4 files changed, 57 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/md/linear.c b/drivers/md/linear.c
> index 26a73b2..bebc834 100644
> --- a/drivers/md/linear.c
> +++ b/drivers/md/linear.c
> @@ -291,7 +291,11 @@ static void linear_make_request(struct mddev *mddev, struct bio *bio)
>  				trace_block_bio_remap(bdev_get_queue(split->bi_bdev),
>  						      split, disk_devt(mddev->gendisk),
>  						      bio_sector);
> -			generic_make_request(split);
> +			if (bio_op(split) == REQ_OP_WRITE_SAME)
> +				generic_make_request(md_writesame_setup(mddev,
> +								split));
> +			else
> +				generic_make_request(split);
>  		}
>  	} while (split != bio);
>  	return;
> diff --git a/drivers/md/md.c b/drivers/md/md.c
> index 13020e5..7354f0b 100644
> --- a/drivers/md/md.c
> +++ b/drivers/md/md.c
> @@ -312,6 +312,51 @@ static blk_qc_t md_make_request(struct request_queue *q, struct bio *bio)
>  	return BLK_QC_T_NONE;
>  }
>  
> +struct md_writesame_data {
> +	struct bio *orig_bio;
> +	struct mddev *mddev;
> +	struct bio cloned_bio;
> +};
> +
> +static void md_writesame_endio(struct bio *bio)
> +{
> +	struct md_writesame_data *data = bio->bi_private;
> +
> +	if (bio->bi_error == -EREMOTEIO &&
> +	    !bdev_get_queue(bio->bi_bdev)->limits.max_write_same_sectors)
> +		data->mddev->queue->limits.max_write_same_sectors = 0;

What would be *really* nice is if a block device could send a
reconfigure message to its 'holder' (bd_holder).  This could include
device size changes and, for this, changes to max_write_same_sectors.
There are probably other changes that can usefully be propagated.

But for this patch, wouldn't it be easier, and maybe more efficient, to
do

  if (bio_op(split) == REQ_OP_WRITE_SAME &&
      !bdev_get_queue(bio->bi_bdev)->limits.max_write_same_sectors)
           mddev->queue->limits.max_write_same_sectors = 0;
  generic_make_request(split);

???
If there is some reason that can't work, then the patch as it stands
look OK to me.

Thanks,
NeilBrown

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 832 bytes --]