[PATCH] dm linear: disable WRITE SAME if it fails

[PATCH] dm linear: disable WRITE SAME if it fails

[alex chen]

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.

Before this patch:

dm linear device(dm-20): ocfs2-fs;
multipath device(dm-6);
scsi device(sdt,sdu);

After a WRITE SAME bio fails, the DM linear device does not disable the
WRITE SAME, although the underlying device(DM multipath device) has
disabled WRITE SAME.

cat /sys/block/dm-20/queue/write_same_max_bytes
33553920
cat /sys/block/dm-6/queue/write_same_max_bytes
0
cat /sys/block/sdt/queue/write_same_max_bytes
0
cat /sys/block/sdu/queue/write_same_max_bytes
0

After this patch:

dm linear device(dm-20): ocfs2-fs;
multipath device(dm-6);
scsi device(sdt,sdu);

After a WRITE SAME bio fails, the underlying device(DM multipath
device) disables WRITE SAME and then the DM linear device also disables
WRITE SAME.

cat /sys/block/dm-20/queue/write_same_max_bytes
0
cat /sys/block/dm-6/queue/write_same_max_bytes
0
cat /sys/block/sdt/queue/write_same_max_bytes
0
cat /sys/block/sdu/queue/write_same_max_bytes
0

Signed-off-by: alex.chen <alex.chen@huawei.com>
---
 drivers/md/dm-linear.c | 26 +++++++++++++++++++++++++-
 drivers/md/dm.c        |  2 +-
 2 files changed, 26 insertions(+), 2 deletions(-)

diff --git a/drivers/md/dm-linear.c b/drivers/md/dm-linear.c
index 53e848c..eaf86a5 100644
--- a/drivers/md/dm-linear.c
+++ b/drivers/md/dm-linear.c
@@ -96,6 +96,29 @@ static int linear_map(struct dm_target *ti, struct
bio *bio)
 	return DM_MAPIO_REMAPPED;
 }

+static int linear_end_io(struct dm_target *ti, struct bio *bio, int error)
+{
+	struct linear_c *lc = ti->private;
+	struct block_device *bdev;
+	struct queue_limits *limits;
+	struct queue_limits *bdev_limits;
+
+	if (!error)
+		return error;
+
+	if (unlikely(bio->bi_rw & REQ_WRITE_SAME)) {
+		bdev = lc->dev->bdev;
+		bdev_limits = &bdev->bd_disk->queue->limits;
+
+		limits = dm_get_queue_limits(dm_table_get_md(ti->table));
+
+		if ((!bdev_limits->max_write_same_sectors) &&
+				limits->max_write_same_sectors)
+			limits->max_write_same_sectors = 0;
+	}
+
+	return error;
+}
 static void linear_status(struct dm_target *ti, status_type_t type,
 			  unsigned status_flags, char *result, unsigned maxlen)
 {
@@ -155,11 +178,12 @@ static int linear_iterate_devices(struct dm_target
*ti,

 static struct target_type linear_target = {
 	.name   = "linear",
-	.version = {1, 2, 1},
+	.version = {1, 2, 2},
 	.module = THIS_MODULE,
 	.ctr    = linear_ctr,
 	.dtr    = linear_dtr,
 	.map    = linear_map,
+	.end_io = linear_end_io,
 	.status = linear_status,
 	.ioctl  = linear_ioctl,
 	.merge  = linear_merge,
diff --git a/drivers/md/dm.c b/drivers/md/dm.c
index 8c53b09..4337025 100644
--- a/drivers/md/dm.c
+++ b/drivers/md/dm.c
@@ -770,7 +770,7 @@ static void clone_endio(struct bio *bio, int error)

 	if (endio) {
 		r = endio(tio->ti, bio, error);
-		if (r < 0 || r == DM_ENDIO_REQUEUE)
+		if (r <= 0 || r == DM_ENDIO_REQUEUE)
 			/*
 			 * error and requeue request are handled
 			 * in dec_pending().
-- 
1.8.4.3

_______________________________________________
vims.openeuler mailing list
vims.openeuler@huawei.com
http://rnd-openeuler.huawei.com/mailman/listinfo/vims.openeuler

.

Re: [PATCH] dm linear: disable WRITE SAME if it fails

[alex chen]

Who can help me review my patch? ths.

On 2014/2/19 12:14, 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.
> 
> Before this patch:
> 
> dm linear device(dm-20): ocfs2-fs;
> multipath device(dm-6);
> scsi device(sdt,sdu);
> 
> After a WRITE SAME bio fails, the DM linear device does not disable the
> WRITE SAME, although the underlying device(DM multipath device) has
> disabled WRITE SAME.
> 
> cat /sys/block/dm-20/queue/write_same_max_bytes
> 33553920
> cat /sys/block/dm-6/queue/write_same_max_bytes
> 0
> cat /sys/block/sdt/queue/write_same_max_bytes
> 0
> cat /sys/block/sdu/queue/write_same_max_bytes
> 0
> 
> After this patch:
> 
> dm linear device(dm-20): ocfs2-fs;
> multipath device(dm-6);
> scsi device(sdt,sdu);
> 
> After a WRITE SAME bio fails, the underlying device(DM multipath
> device) disables WRITE SAME and then the DM linear device also disables
> WRITE SAME.
> 
> cat /sys/block/dm-20/queue/write_same_max_bytes
> 0
> cat /sys/block/dm-6/queue/write_same_max_bytes
> 0
> cat /sys/block/sdt/queue/write_same_max_bytes
> 0
> cat /sys/block/sdu/queue/write_same_max_bytes
> 0
> 
> Signed-off-by: alex.chen <alex.chen@huawei.com>
> ---
>  drivers/md/dm-linear.c | 26 +++++++++++++++++++++++++-
>  drivers/md/dm.c        |  2 +-
>  2 files changed, 26 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/md/dm-linear.c b/drivers/md/dm-linear.c
> index 53e848c..eaf86a5 100644
> --- a/drivers/md/dm-linear.c
> +++ b/drivers/md/dm-linear.c
> @@ -96,6 +96,29 @@ static int linear_map(struct dm_target *ti, struct
> bio *bio)
>  	return DM_MAPIO_REMAPPED;
>  }
> 
> +static int linear_end_io(struct dm_target *ti, struct bio *bio, int error)
> +{
> +	struct linear_c *lc = ti->private;
> +	struct block_device *bdev;
> +	struct queue_limits *limits;
> +	struct queue_limits *bdev_limits;
> +
> +	if (!error)
> +		return error;
> +
> +	if (unlikely(bio->bi_rw & REQ_WRITE_SAME)) {
> +		bdev = lc->dev->bdev;
> +		bdev_limits = &bdev->bd_disk->queue->limits;
> +
> +		limits = dm_get_queue_limits(dm_table_get_md(ti->table));
> +
> +		if ((!bdev_limits->max_write_same_sectors) &&
> +				limits->max_write_same_sectors)
> +			limits->max_write_same_sectors = 0;
> +	}
> +
> +	return error;
> +}
>  static void linear_status(struct dm_target *ti, status_type_t type,
>  			  unsigned status_flags, char *result, unsigned maxlen)
>  {
> @@ -155,11 +178,12 @@ static int linear_iterate_devices(struct dm_target
> *ti,
> 
>  static struct target_type linear_target = {
>  	.name   = "linear",
> -	.version = {1, 2, 1},
> +	.version = {1, 2, 2},
>  	.module = THIS_MODULE,
>  	.ctr    = linear_ctr,
>  	.dtr    = linear_dtr,
>  	.map    = linear_map,
> +	.end_io = linear_end_io,
>  	.status = linear_status,
>  	.ioctl  = linear_ioctl,
>  	.merge  = linear_merge,
> diff --git a/drivers/md/dm.c b/drivers/md/dm.c
> index 8c53b09..4337025 100644
> --- a/drivers/md/dm.c
> +++ b/drivers/md/dm.c
> @@ -770,7 +770,7 @@ static void clone_endio(struct bio *bio, int error)
> 
>  	if (endio) {
>  		r = endio(tio->ti, bio, error);
> -		if (r < 0 || r == DM_ENDIO_REQUEUE)
> +		if (r <= 0 || r == DM_ENDIO_REQUEUE)
>  			/*
>  			 * error and requeue request are handled
>  			 * in dec_pending().
> 

Re: dm linear: disable WRITE SAME if it fails

[Mike Snitzer]

On Wed, Mar 05 2014 at  2:30am -0500,
alex chen <alex.chen@huawei.com> wrote:

> Who can help me review my patch? ths.

I can review it next week.  You raise a relevant issue.  But I don't
want to see each target having to implement this WRITE_SAME disable.

Anyway, This is 3.15 material.  I'm dealing with more urgent issues for
3.14 at the moment.

Mike

[PATCH] dm linear: disable WRITE SAME if it fails

[alex chen]

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.

Before this patch:

dm linear device(dm-20): ocfs2-fs;
multipath device(dm-6);
scsi device(sdt,sdu);

After a WRITE SAME bio fails, the DM linear device does not disable the
WRITE SAME, although the underlying device(DM multipath device) has
disabled WRITE SAME.

cat /sys/block/dm-20/queue/write_same_max_bytes
33553920
cat /sys/block/dm-6/queue/write_same_max_bytes
0
cat /sys/block/sdt/queue/write_same_max_bytes
0
cat /sys/block/sdu/queue/write_same_max_bytes
0

After this patch:

dm linear device(dm-20): ocfs2-fs;
multipath device(dm-6);
scsi device(sdt,sdu);

After a WRITE SAME bio fails, the underlying device(DM multipath
device) disables WRITE SAME and then the DM linear device also disables
WRITE SAME.

cat /sys/block/dm-20/queue/write_same_max_bytes
0
cat /sys/block/dm-6/queue/write_same_max_bytes
0
cat /sys/block/sdt/queue/write_same_max_bytes
0
cat /sys/block/sdu/queue/write_same_max_bytes
0

Signed-off-by: alex.chen <alex.chen@huawei.com>
---
 drivers/md/dm-linear.c | 26 +++++++++++++++++++++++++-
 drivers/md/dm.c        |  2 +-
 2 files changed, 26 insertions(+), 2 deletions(-)

diff --git a/drivers/md/dm-linear.c b/drivers/md/dm-linear.c
index 53e848c..eaf86a5 100644
--- a/drivers/md/dm-linear.c
+++ b/drivers/md/dm-linear.c
@@ -96,6 +96,29 @@ static int linear_map(struct dm_target *ti, struct
bio *bio)
 	return DM_MAPIO_REMAPPED;
 }

+static int linear_end_io(struct dm_target *ti, struct bio *bio, int error)
+{
+	struct linear_c *lc = ti->private;
+	struct block_device *bdev;
+	struct queue_limits *limits;
+	struct queue_limits *bdev_limits;
+
+	if (!error)
+		return error;
+
+	if (unlikely(bio->bi_rw & REQ_WRITE_SAME)) {
+		bdev = lc->dev->bdev;
+		bdev_limits = &bdev->bd_disk->queue->limits;
+
+		limits = dm_get_queue_limits(dm_table_get_md(ti->table));
+
+		if ((!bdev_limits->max_write_same_sectors) &&
+				limits->max_write_same_sectors)
+			limits->max_write_same_sectors = 0;
+	}
+
+	return error;
+}
 static void linear_status(struct dm_target *ti, status_type_t type,
 			  unsigned status_flags, char *result, unsigned maxlen)
 {
@@ -155,11 +178,12 @@ static int linear_iterate_devices(struct dm_target
*ti,

 static struct target_type linear_target = {
 	.name   = "linear",
-	.version = {1, 2, 1},
+	.version = {1, 2, 2},
 	.module = THIS_MODULE,
 	.ctr    = linear_ctr,
 	.dtr    = linear_dtr,
 	.map    = linear_map,
+	.end_io = linear_end_io,
 	.status = linear_status,
 	.ioctl  = linear_ioctl,
 	.merge  = linear_merge,
diff --git a/drivers/md/dm.c b/drivers/md/dm.c
index 8c53b09..4337025 100644
--- a/drivers/md/dm.c
+++ b/drivers/md/dm.c
@@ -770,7 +770,7 @@ static void clone_endio(struct bio *bio, int error)

 	if (endio) {
 		r = endio(tio->ti, bio, error);
-		if (r < 0 || r == DM_ENDIO_REQUEUE)
+		if (r <= 0 || r == DM_ENDIO_REQUEUE)
 			/*
 			 * error and requeue request are handled
 			 * in dec_pending().
-- 
1.8.4.3

Re: [PATCH] dm linear: disable WRITE SAME if it fails

[Alasdair G Kergon]

On Sat, May 31, 2014 at 04:51:30PM +0800, alex chen wrote:
> this patch adds DM linear end_io method 

No.  If linear needs to hook end_io something has gone seriously wrong.
Find the underlying source of the problem and fix that instead.

Alasdair

Re: [PATCH] dm linear: disable WRITE SAME if it fails

[Alasdair G Kergon]

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)?

Alasdair

dm: disable WRITE SAME if it fails

[Mike Snitzer]

On Sat, May 31 2014 at 11:05am -0400,
Alasdair G Kergon <agk@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

---
 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);

Re: dm: disable WRITE SAME if it fails

[Mike Snitzer]

On Mon, Jun 02 2014 at  2:36pm -0400,
Mike Snitzer <snitzer@redhat.com> wrote:

> On Sat, May 31 2014 at 11:05am -0400,
> Alasdair G Kergon <agk@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?

FYI, I pushed a revised version of this patch to linux-next, please see:
https://git.kernel.org/cgit/linux/kernel/git/device-mapper/linux-dm.git/commit/?h=for-next&id=f314e7fff61dad0e9ed1c602bfd62cf722d476f7

Re: dm: disable WRITE SAME if it fails

[Martin K. Petersen]

>>>>> "Mike" == Mike Snitzer <snitzer@redhat.com> writes:

>> > 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)?

Mike> FYI, I pushed a revised version of this patch to linux-next,

I haven't tested the patch but I definitely agree with this approach.

Acked-by: Martin K. Petersen <martin.petersen@oracle.com>

-- 
Martin K. Petersen	Oracle Linux Engineering

Re: dm: disable WRITE SAME if it fails

[Mike Snitzer]

On Tue, Jun 03 2014 at  6:59pm -0400,
Martin K. Petersen <martin.petersen@oracle.com> wrote:

> >>>>> "Mike" == Mike Snitzer <snitzer@redhat.com> writes:
> 
> >> > 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)?
> 
> Mike> FYI, I pushed a revised version of this patch to linux-next,
> 
> I haven't tested the patch but I definitely agree with this approach.
> 
> Acked-by: Martin K. Petersen <martin.petersen@oracle.com>

Thanks, added.

Re: 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@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@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);
> 
> .
> 

Re: dm: disable WRITE SAME if it fails

[Mike Snitzer]

On Wed, Jun 04 2014 at 12:03am -0400,
alex chen <alex.chen@huawei.com> wrote:

> On 2014/6/3 2:36, Mike Snitzer wrote:
> > On Sat, May 31 2014 at 11:05am -0400,
> > Alasdair G Kergon <agk@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@huawei.com>

OK, thanks.. FYI, as I sahred before: I updated it slightly to check for
the specific -EREMOTEIO error code that SCSI returns for this case, see:

https://git.kernel.org/cgit/linux/kernel/git/device-mapper/linux-dm.git/commit/?h=for-next&id=7eee4ae2dbb2be0a15a4406718806e48b18ba831

Re: dm: disable WRITE SAME if it fails

[alex chen]

On 2014/6/4 21:52, Mike Snitzer wrote:
> On Wed, Jun 04 2014 at 12:03am -0400,
> alex chen <alex.chen@huawei.com> wrote:
> 
>> On 2014/6/3 2:36, Mike Snitzer wrote:
>>> On Sat, May 31 2014 at 11:05am -0400,
>>> Alasdair G Kergon <agk@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@huawei.com>
> 
> OK, thanks.. FYI, as I sahred before: I updated it slightly to check for
> the specific -EREMOTEIO error code that SCSI returns for this case, see:
> 
> https://git.kernel.org/cgit/linux/kernel/git/device-mapper/linux-dm.git/commit/?h=for-next&id=7eee4ae2dbb2be0a15a4406718806e48b18ba831
> 

OK，I've tested the new patch, the result was also pass.

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

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

Re: dm: disable WRITE SAME if it fails

[Mike Snitzer]

On Wed, Jun 04 2014 at  9:52pm -0400,
alex chen <alex.chen@huawei.com> wrote:

> On 2014/6/4 21:52, Mike Snitzer wrote:
> > 
> > OK, thanks.. FYI, as I sahred before: I updated it slightly to check for
> > the specific -EREMOTEIO error code that SCSI returns for this case, see:
> > 
> > https://git.kernel.org/cgit/linux/kernel/git/device-mapper/linux-dm.git/commit/?h=for-next&id=7eee4ae2dbb2be0a15a4406718806e48b18ba831
> > 
> 
> OK，I've tested the new patch, the result was also pass.

Great, thanks.

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