On 2014/6/3 2:36, Mike Snitzer wrote:
> On Sat, May 31 2014 at 11:05am -0400,
> Alasdair G Kergon <agk@xxxxxxxxxx> 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@xxxxxxxxxx>
> ---
> 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);
>
> .
>
--
dm-devel mailing list
dm-devel@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/dm-devel
