[dm-devel] [PATCH] block: do not return -EOPNOTSUPP only when issue a discard request

[dm-devel] [PATCH] block: do not return -EOPNOTSUPP only when issue a discard request

[jiangyiwen]

commit 8af1954d172a("blkdev: Do not return -EOPNOTSUPP if discard
is supported") only solve the situation of discard, because When
applications issue a discard request to device, they can't expect
deterministic behaviour. However, WRITE SAME should not ignore error
with EOPNOTSUPP, because if applications issue WRITE SAME requests to
device, it should return deterministic results to applications
according to the T10 standard, or else it will cause an inconsistent
state between upper layer and bottom layer.

So ignoring error with EOPNOTSUPP only in discard situation.

Signed-off-by: Yiwen Jiang <jiangyiwen@huawei.com>
---
 block/blk-lib.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/block/blk-lib.c b/block/blk-lib.c
index 9ebf653..e4f02f1 100644
--- a/block/blk-lib.c
+++ b/block/blk-lib.c
@@ -19,7 +19,9 @@ static void bio_batch_end_io(struct bio *bio)
 {
 	struct bio_batch *bb = bio->bi_private;

-	if (bio->bi_error && bio->bi_error != -EOPNOTSUPP)
+	/* ignore -EOPNOTSUPP only when issue a discard request */
+	if (bio->bi_error && (!(bio->bi_rw & REQ_DISCARD) ||
+			(bio->bi_error != -EOPNOTSUPP)))
 		bb->error = bio->bi_error;
 	if (atomic_dec_and_test(&bb->done))
 		complete(bb->wait);
-- 
1.8.4.3

Re: [dm-devel] [PATCH] block: do not return -EOPNOTSUPP only when issue a discard request

[Lukáš Czerner]

[-- Attachment #1: Type: TEXT/PLAIN, Size: 1834 bytes --]

On Wed, 3 Feb 2016, jiangyiwen wrote:

> Date: Wed, 3 Feb 2016 17:54:36 +0800
> From: jiangyiwen <jiangyiwen@huawei.com>
> To: axboe@kernel.dk, linux-kernel@vger.kernel.org
> Cc: dm-devel@redhat.com, martin.petersen@oracle.com,
>     Lukáš Czerner <lczerner@redhat.com>, xuejiufei@huawei.com,
>     "Qijiang (Joseph, Euler)" <joseph.qi@huawei.com>
> Subject: [dm-devel] [PATCH] block: do not return -EOPNOTSUPP only when issue a
>      discard request
> 
> commit 8af1954d172a("blkdev: Do not return -EOPNOTSUPP if discard
> is supported") only solve the situation of discard, because When
> applications issue a discard request to device, they can't expect
> deterministic behaviour. However, WRITE SAME should not ignore error
> with EOPNOTSUPP, because if applications issue WRITE SAME requests to
> device, it should return deterministic results to applications
> according to the T10 standard, or else it will cause an inconsistent
> state between upper layer and bottom layer.
> 
> So ignoring error with EOPNOTSUPP only in discard situation.
> 
> Signed-off-by: Yiwen Jiang <jiangyiwen@huawei.com>

Looks good.

Reviewed-by: Lukas Czerner <lczerner@redhat.com>

Thanks!
-Lukas

> ---
>  block/blk-lib.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/block/blk-lib.c b/block/blk-lib.c
> index 9ebf653..e4f02f1 100644
> --- a/block/blk-lib.c
> +++ b/block/blk-lib.c
> @@ -19,7 +19,9 @@ static void bio_batch_end_io(struct bio *bio)
>  {
>  	struct bio_batch *bb = bio->bi_private;
> 
> -	if (bio->bi_error && bio->bi_error != -EOPNOTSUPP)
> +	/* ignore -EOPNOTSUPP only when issue a discard request */
> +	if (bio->bi_error && (!(bio->bi_rw & REQ_DISCARD) ||
> +			(bio->bi_error != -EOPNOTSUPP)))
>  		bb->error = bio->bi_error;
>  	if (atomic_dec_and_test(&bb->done))
>  		complete(bb->wait);
> 

Re: [dm-devel] [PATCH] block: do not return -EOPNOTSUPP only when issue a discard request

[Martin K. Petersen]

>>>>> "Yiwen" == jiangyiwen  <jiangyiwen@huawei.com> writes:

Yiwen> commit 8af1954d172a("blkdev: Do not return -EOPNOTSUPP if discard
Yiwen> is supported") only solve the situation of discard, because When
Yiwen> applications issue a discard request to device, they can't expect
Yiwen> deterministic behaviour. However, WRITE SAME should not ignore
Yiwen> error with EOPNOTSUPP, because if applications issue WRITE SAME
Yiwen> requests to device, it should return deterministic results to
Yiwen> applications according to the T10 standard, or else it will cause
Yiwen> an inconsistent state between upper layer and bottom layer.

Yiwen> So ignoring error with EOPNOTSUPP only in discard situation.

If an underlying device fails a WRITE SAME request, SCSI will not return
EOPNOTSUPP. Instead we will mark 0 bytes of the request as complete and
disable WRITE SAME support on the device. So any EOPNOTSUPP error in
this case would be coming from MD/DM.

That said, I agree with the patch.

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

-- 
Martin K. Petersen	Oracle Linux Engineering