All of lore.kernel.org
 help / color / mirror / Atom feed
* [dm-devel] [PATCH] dm: add support for DM_TARGET_NOWAIT for various targets
@ 2020-11-10  6:55 Jeffle Xu
  2020-11-11 15:38 ` [dm-devel] " Mike Snitzer
  2020-11-13  2:05 ` [dm-devel] [PATCH v2] " Jeffle Xu
  0 siblings, 2 replies; 12+ messages in thread
From: Jeffle Xu @ 2020-11-10  6:55 UTC (permalink / raw)
  To: snitzer; +Cc: joseph.qi, dm-devel, koct9i

This is one prep patch for supporting iopoll for dm device.

The direct IO routine will set REQ_NOWAIT flag for REQ_HIPRI IO (that
is, IO will do iopoll) in bio_set_polled(). Then in the IO submission
routine, the ability of handling REQ_NOWAIT of the block device will
be checked for REQ_HIPRI IO in submit_bio_checks(). -EOPNOTSUPP will
be returned if the block device doesn't support REQ_NOWAIT.

DM lacks support for REQ_NOWAIT until commit 6abc49468eea ("dm: add
support for REQ_NOWAIT and enable it for linear target"). Since then,
dm targets that support REQ_NOWAIT should advertise DM_TARGET_NOWAIT
feature.

This patch adds support for DM_TARGET_NOWAIT for those dm targets, the
.map() algorithm of which just involves sector recalculation.

Signed-off-by: Jeffle Xu <jefflexu@linux.alibaba.com>
---
Hi Mike,

I could split these boilerplate code that each dm target have one
seperate patch if you think that would be better.

---
 drivers/md/dm-stripe.c   | 2 +-
 drivers/md/dm-switch.c   | 1 +
 drivers/md/dm-unstripe.c | 1 +
 drivers/md/dm-zero.c     | 1 +
 4 files changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/md/dm-stripe.c b/drivers/md/dm-stripe.c
index 151d022b032d..df359d33cda8 100644
--- a/drivers/md/dm-stripe.c
+++ b/drivers/md/dm-stripe.c
@@ -496,7 +496,7 @@ static void stripe_io_hints(struct dm_target *ti,
 static struct target_type stripe_target = {
 	.name   = "striped",
 	.version = {1, 6, 0},
-	.features = DM_TARGET_PASSES_INTEGRITY,
+	.features = DM_TARGET_PASSES_INTEGRITY | DM_TARGET_NOWAIT,
 	.module = THIS_MODULE,
 	.ctr    = stripe_ctr,
 	.dtr    = stripe_dtr,
diff --git a/drivers/md/dm-switch.c b/drivers/md/dm-switch.c
index bff4c7fa1cd2..262e2b0fd975 100644
--- a/drivers/md/dm-switch.c
+++ b/drivers/md/dm-switch.c
@@ -550,6 +550,7 @@ static int switch_iterate_devices(struct dm_target *ti,
 static struct target_type switch_target = {
 	.name = "switch",
 	.version = {1, 1, 0},
+	.features = DM_TARGET_NOWAIT,
 	.module = THIS_MODULE,
 	.ctr = switch_ctr,
 	.dtr = switch_dtr,
diff --git a/drivers/md/dm-unstripe.c b/drivers/md/dm-unstripe.c
index e673dacf6418..7357c1bd5863 100644
--- a/drivers/md/dm-unstripe.c
+++ b/drivers/md/dm-unstripe.c
@@ -178,6 +178,7 @@ static void unstripe_io_hints(struct dm_target *ti,
 static struct target_type unstripe_target = {
 	.name = "unstriped",
 	.version = {1, 1, 0},
+	.features = DM_TARGET_NOWAIT,
 	.module = THIS_MODULE,
 	.ctr = unstripe_ctr,
 	.dtr = unstripe_dtr,
diff --git a/drivers/md/dm-zero.c b/drivers/md/dm-zero.c
index b65ca8dcfbdc..faa1dbffc8b4 100644
--- a/drivers/md/dm-zero.c
+++ b/drivers/md/dm-zero.c
@@ -59,6 +59,7 @@ static int zero_map(struct dm_target *ti, struct bio *bio)
 static struct target_type zero_target = {
 	.name   = "zero",
 	.version = {1, 1, 0},
+	.features = DM_TARGET_NOWAIT,
 	.module = THIS_MODULE,
 	.ctr    = zero_ctr,
 	.map    = zero_map,
-- 
2.27.0

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


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [dm-devel] dm: add support for DM_TARGET_NOWAIT for various targets
  2020-11-10  6:55 [dm-devel] [PATCH] dm: add support for DM_TARGET_NOWAIT for various targets Jeffle Xu
@ 2020-11-11 15:38 ` Mike Snitzer
  2020-11-12  6:05     ` JeffleXu
  2020-11-13  2:05 ` [dm-devel] [PATCH v2] " Jeffle Xu
  1 sibling, 1 reply; 12+ messages in thread
From: Mike Snitzer @ 2020-11-11 15:38 UTC (permalink / raw)
  To: Jeffle Xu; +Cc: joseph.qi, dm-devel, koct9i

On Tue, Nov 10 2020 at  1:55am -0500,
Jeffle Xu <jefflexu@linux.alibaba.com> wrote:

> This is one prep patch for supporting iopoll for dm device.
> 
> The direct IO routine will set REQ_NOWAIT flag for REQ_HIPRI IO (that
> is, IO will do iopoll) in bio_set_polled(). Then in the IO submission
> routine, the ability of handling REQ_NOWAIT of the block device will
> be checked for REQ_HIPRI IO in submit_bio_checks(). -EOPNOTSUPP will
> be returned if the block device doesn't support REQ_NOWAIT.

submit_bio_checks() verifies the request_queue has QUEUE_FLAG_NOWAIT set
if the bio has REQ_NOWAIT.

> DM lacks support for REQ_NOWAIT until commit 6abc49468eea ("dm: add
> support for REQ_NOWAIT and enable it for linear target"). Since then,
> dm targets that support REQ_NOWAIT should advertise DM_TARGET_NOWAIT
> feature.

I'm not seeing why DM_TARGET_NOWAIT is needed (since you didn't add any
code that consumes the flag).

dm-table.c:dm_table_set_restrictions() has:

        if (dm_table_supports_nowait(t))
                blk_queue_flag_set(QUEUE_FLAG_NOWAIT, q);
        else
                blk_queue_flag_clear(QUEUE_FLAG_NOWAIT, q);

> This patch adds support for DM_TARGET_NOWAIT for those dm targets, the
> .map() algorithm of which just involves sector recalculation.

So you're looking to constrain which targets will properly support
REQ_NOWAIT, based on whether they do a simple remapping?


> Signed-off-by: Jeffle Xu <jefflexu@linux.alibaba.com>
> ---
> Hi Mike,
> 
> I could split these boilerplate code that each dm target have one
> seperate patch if you think that would be better.

One patch for all these is fine.  But it should include the code that I
assume you'll be adding to dm_table_supports_nowait() to further verify
that the targets in the table are all DM_TARGET_NOWAIT.

And why isn't dm-linear setting DM_TARGET_NOWAIT?

Also, other targets _could_ be made to support REQ_NOWAIT by
conditionally returning bio_wouldblock_error() if appropriate
(e.g. bio-based dm-multipath's case of queue_if_no_path).

Mike

> ---
>  drivers/md/dm-stripe.c   | 2 +-
>  drivers/md/dm-switch.c   | 1 +
>  drivers/md/dm-unstripe.c | 1 +
>  drivers/md/dm-zero.c     | 1 +
>  4 files changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/md/dm-stripe.c b/drivers/md/dm-stripe.c
> index 151d022b032d..df359d33cda8 100644
> --- a/drivers/md/dm-stripe.c
> +++ b/drivers/md/dm-stripe.c
> @@ -496,7 +496,7 @@ static void stripe_io_hints(struct dm_target *ti,
>  static struct target_type stripe_target = {
>  	.name   = "striped",
>  	.version = {1, 6, 0},
> -	.features = DM_TARGET_PASSES_INTEGRITY,
> +	.features = DM_TARGET_PASSES_INTEGRITY | DM_TARGET_NOWAIT,
>  	.module = THIS_MODULE,
>  	.ctr    = stripe_ctr,
>  	.dtr    = stripe_dtr,
> diff --git a/drivers/md/dm-switch.c b/drivers/md/dm-switch.c
> index bff4c7fa1cd2..262e2b0fd975 100644
> --- a/drivers/md/dm-switch.c
> +++ b/drivers/md/dm-switch.c
> @@ -550,6 +550,7 @@ static int switch_iterate_devices(struct dm_target *ti,
>  static struct target_type switch_target = {
>  	.name = "switch",
>  	.version = {1, 1, 0},
> +	.features = DM_TARGET_NOWAIT,
>  	.module = THIS_MODULE,
>  	.ctr = switch_ctr,
>  	.dtr = switch_dtr,
> diff --git a/drivers/md/dm-unstripe.c b/drivers/md/dm-unstripe.c
> index e673dacf6418..7357c1bd5863 100644
> --- a/drivers/md/dm-unstripe.c
> +++ b/drivers/md/dm-unstripe.c
> @@ -178,6 +178,7 @@ static void unstripe_io_hints(struct dm_target *ti,
>  static struct target_type unstripe_target = {
>  	.name = "unstriped",
>  	.version = {1, 1, 0},
> +	.features = DM_TARGET_NOWAIT,
>  	.module = THIS_MODULE,
>  	.ctr = unstripe_ctr,
>  	.dtr = unstripe_dtr,
> diff --git a/drivers/md/dm-zero.c b/drivers/md/dm-zero.c
> index b65ca8dcfbdc..faa1dbffc8b4 100644
> --- a/drivers/md/dm-zero.c
> +++ b/drivers/md/dm-zero.c
> @@ -59,6 +59,7 @@ static int zero_map(struct dm_target *ti, struct bio *bio)
>  static struct target_type zero_target = {
>  	.name   = "zero",
>  	.version = {1, 1, 0},
> +	.features = DM_TARGET_NOWAIT,
>  	.module = THIS_MODULE,
>  	.ctr    = zero_ctr,
>  	.map    = zero_map,
> -- 
> 2.27.0
> 

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


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [dm-devel] dm: add support for DM_TARGET_NOWAIT for various targets
  2020-11-11 15:38 ` [dm-devel] " Mike Snitzer
@ 2020-11-12  6:05     ` JeffleXu
  0 siblings, 0 replies; 12+ messages in thread
From: JeffleXu @ 2020-11-12  6:05 UTC (permalink / raw)
  To: Mike Snitzer; +Cc: joseph.qi, dm-devel, koct9i, axboe, io-uring, linux-block

Hi Jens and guys in block/io_uring mailing list, this mail contains some 
discussion abount

RWF_NOWAIT, please see the following contents.



On 11/11/20 11:38 PM, Mike Snitzer wrote:
> On Tue, Nov 10 2020 at  1:55am -0500,
> Jeffle Xu <jefflexu@linux.alibaba.com> wrote:
>
>> This is one prep patch for supporting iopoll for dm device.
>>
>> The direct IO routine will set REQ_NOWAIT flag for REQ_HIPRI IO (that
>> is, IO will do iopoll) in bio_set_polled(). Then in the IO submission
>> routine, the ability of handling REQ_NOWAIT of the block device will
>> be checked for REQ_HIPRI IO in submit_bio_checks(). -EOPNOTSUPP will
>> be returned if the block device doesn't support REQ_NOWAIT.
> submit_bio_checks() verifies the request_queue has QUEUE_FLAG_NOWAIT set
> if the bio has REQ_NOWAIT.
Yes that's the case.
>
>> DM lacks support for REQ_NOWAIT until commit 6abc49468eea ("dm: add
>> support for REQ_NOWAIT and enable it for linear target"). Since then,
>> dm targets that support REQ_NOWAIT should advertise DM_TARGET_NOWAIT
>> feature.
> I'm not seeing why DM_TARGET_NOWAIT is needed (since you didn't add any
> code that consumes the flag).

As I said, it's needed if we support iopoll for dm device.  Only if a 
block device is capable of

handling NOWAIT, then it can support iopoll.


IO submitted for iopoll (marked with IOCB_HIPRI) is usually also marked 
with REQ_NOWAIT.

There are two scenario when it could happen.


1. io_uring will set REQ_NOWAIT

The IO submission of io_uring can be divided into two phase. First, IO 
will be submitted

synchronously in user process context (when sqthread feature disabled), 
or sqthread

context (when sqthread feature enabled).


```sh
- current process context when sqthread disabled, or sqthread when it's 
enabled
     io_uring_enter
         io_submit_sqes
             io_submit_sqe
                 io_queue_sqe
                     __io_queue_sqe
                         io_issue_sqe // with @force_nonblock is true
                             io_read/io_write
```

In this case, IO should be handled in a NOWAIT way, since the user 
process or sqthread

can not be blocked for performance.

```

io_read/io_write

     /* Ensure we clear previously set non-block flag */
     if (!force_nonblock)
         kiocb->ki_flags &= ~IOCB_NOWAIT;
     else
         kiocb->ki_flags |= IOCB_NOWAIT;

```


2. The direct IO routine will set REQ_NOWAIT for polling IO

Both fs/block_dev.c: __blkdev_direct_IO and fs/iomap/direct-io.c: 
iomap_dio_submit_bio will

call bio_set_polled(), in which will set REQ_NOWAIT for polling IO.


```sh
__blkdev_direct_IO / iomap_dio_submit_bio:
     if (dio->iocb->ki_flags & IOCB_HIPRI)
         bio_set_polled
           bio->bi_opf |= REQ_NOWAIT
```


Thus to support iopoll for dm device, the dm target should be capable of 
handling NOWAIT,

or submit_bio_checks() will fail with -EOPNOTSUPP when submitting bio to 
dm device.


>
> dm-table.c:dm_table_set_restrictions() has:
>
>          if (dm_table_supports_nowait(t))
>                  blk_queue_flag_set(QUEUE_FLAG_NOWAIT, q);
>          else
>                  blk_queue_flag_clear(QUEUE_FLAG_NOWAIT, q);
>
>> This patch adds support for DM_TARGET_NOWAIT for those dm targets, the
>> .map() algorithm of which just involves sector recalculation.
> So you're looking to constrain which targets will properly support
> REQ_NOWAIT, based on whether they do a simple remapping?

To be honest, I'm a little confused about the semantics of REQ_NOWAIT. 
Jens may had ever

explained it in block or io_uring mailing list, but I can't find the 
specific mail.


The man page explains FMODE_NOWAIT as 'File is capable of returning 
-EAGAIN if I/O will

block'.


And RWF_NOWAIT as

```

               RWF_NOWAIT (since Linux 4.14)
                      Don't wait if the I/O will block for operations 
such as
                      file block allocations, dirty page flush, mutex locks,
                      or a congested block device inside the kernel.  If any
                      of these conditions are met, the control block is re‐
                      turned immediately with a return value of -EAGAIN in
                      the res field of the io_event structure (see
                      io_getevents(2)).

```


commit 6abc49468eea ("dm: add support for REQ_NOWAIT and enable it for 
linear

target") handles NOWAIT for DM core as


```

@@ -1802,7 +1802,9 @@ static blk_qc_t dm_submit_bio(struct bio *bio)
         if (unlikely(test_bit(DMF_BLOCK_IO_FOR_SUSPEND, &md->flags))) {
+               if (bio->bi_opf & REQ_NOWAIT)
+                       bio_wouldblock_error(bio);

+               else if (!(bio->bi_opf & REQ_RAHEAD))
                         queue_io(md, bio);

```


Theoretically the block device could advertise QUEUE_FLAG_NOWAIT as long 
as it could

'return -EAGAIN if I/O will block' as the man page said. However, 
considering when the

dm device detected as suspending, the submitted bios are deferred to 
workqueue in

drivers/dm/dm.c: dm_submit_bio. In this case, IO gets **deferred** while 
the user process

will not be **blocked**. Can we say IO gets **blocked** in this case?


Actually several dm targets handle submitted bio in this deferred way, 
such as dm-crypt/

dm-delay/dm-era/dm-ebs. Can we say these targets are not capable of 
handling NOWAIT?


Also when system is short of memory, bio allocation in 
bio_alloc_bioset() may trigger memory

direct reclaim, as the gfp_mask is usually GFP_NOIO. While in memory 
direct reclaim, the

process may be scheduled out, but I have never seen the proper handling 
for NOWAIT in this

situation. Maybe the block or io_uring guys have more insights?


So there's just too many possibilities that may get blocked, not to say 
mutex locks.


>
>
>> Signed-off-by: Jeffle Xu <jefflexu@linux.alibaba.com>
>> ---
>> Hi Mike,
>>
>> I could split these boilerplate code that each dm target have one
>> seperate patch if you think that would be better.
> One patch for all these is fine.  But it should include the code that I
> assume you'll be adding to dm_table_supports_nowait() to further verify
> that the targets in the table are all DM_TARGET_NOWAIT.
>
> And why isn't dm-linear setting DM_TARGET_NOWAIT?
These are all done in commit 6abc49468eea ("dm: add support for 
REQ_NOWAIT and enable it for
linear target").
>
> Also, other targets _could_ be made to support REQ_NOWAIT by
> conditionally returning bio_wouldblock_error() if appropriate
> (e.g. bio-based dm-multipath's case of queue_if_no_path).



-- 
Thanks,
Jeffle


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [dm-devel] dm: add support for DM_TARGET_NOWAIT for various targets
@ 2020-11-12  6:05     ` JeffleXu
  0 siblings, 0 replies; 12+ messages in thread
From: JeffleXu @ 2020-11-12  6:05 UTC (permalink / raw)
  To: Mike Snitzer; +Cc: axboe, linux-block, joseph.qi, dm-devel, io-uring, koct9i

Hi Jens and guys in block/io_uring mailing list, this mail contains some 
discussion abount

RWF_NOWAIT, please see the following contents.



On 11/11/20 11:38 PM, Mike Snitzer wrote:
> On Tue, Nov 10 2020 at  1:55am -0500,
> Jeffle Xu <jefflexu@linux.alibaba.com> wrote:
>
>> This is one prep patch for supporting iopoll for dm device.
>>
>> The direct IO routine will set REQ_NOWAIT flag for REQ_HIPRI IO (that
>> is, IO will do iopoll) in bio_set_polled(). Then in the IO submission
>> routine, the ability of handling REQ_NOWAIT of the block device will
>> be checked for REQ_HIPRI IO in submit_bio_checks(). -EOPNOTSUPP will
>> be returned if the block device doesn't support REQ_NOWAIT.
> submit_bio_checks() verifies the request_queue has QUEUE_FLAG_NOWAIT set
> if the bio has REQ_NOWAIT.
Yes that's the case.
>
>> DM lacks support for REQ_NOWAIT until commit 6abc49468eea ("dm: add
>> support for REQ_NOWAIT and enable it for linear target"). Since then,
>> dm targets that support REQ_NOWAIT should advertise DM_TARGET_NOWAIT
>> feature.
> I'm not seeing why DM_TARGET_NOWAIT is needed (since you didn't add any
> code that consumes the flag).

As I said, it's needed if we support iopoll for dm device.  Only if a 
block device is capable of

handling NOWAIT, then it can support iopoll.


IO submitted for iopoll (marked with IOCB_HIPRI) is usually also marked 
with REQ_NOWAIT.

There are two scenario when it could happen.


1. io_uring will set REQ_NOWAIT

The IO submission of io_uring can be divided into two phase. First, IO 
will be submitted

synchronously in user process context (when sqthread feature disabled), 
or sqthread

context (when sqthread feature enabled).


```sh
- current process context when sqthread disabled, or sqthread when it's 
enabled
     io_uring_enter
         io_submit_sqes
             io_submit_sqe
                 io_queue_sqe
                     __io_queue_sqe
                         io_issue_sqe // with @force_nonblock is true
                             io_read/io_write
```

In this case, IO should be handled in a NOWAIT way, since the user 
process or sqthread

can not be blocked for performance.

```

io_read/io_write

     /* Ensure we clear previously set non-block flag */
     if (!force_nonblock)
         kiocb->ki_flags &= ~IOCB_NOWAIT;
     else
         kiocb->ki_flags |= IOCB_NOWAIT;

```


2. The direct IO routine will set REQ_NOWAIT for polling IO

Both fs/block_dev.c: __blkdev_direct_IO and fs/iomap/direct-io.c: 
iomap_dio_submit_bio will

call bio_set_polled(), in which will set REQ_NOWAIT for polling IO.


```sh
__blkdev_direct_IO / iomap_dio_submit_bio:
     if (dio->iocb->ki_flags & IOCB_HIPRI)
         bio_set_polled
           bio->bi_opf |= REQ_NOWAIT
```


Thus to support iopoll for dm device, the dm target should be capable of 
handling NOWAIT,

or submit_bio_checks() will fail with -EOPNOTSUPP when submitting bio to 
dm device.


>
> dm-table.c:dm_table_set_restrictions() has:
>
>          if (dm_table_supports_nowait(t))
>                  blk_queue_flag_set(QUEUE_FLAG_NOWAIT, q);
>          else
>                  blk_queue_flag_clear(QUEUE_FLAG_NOWAIT, q);
>
>> This patch adds support for DM_TARGET_NOWAIT for those dm targets, the
>> .map() algorithm of which just involves sector recalculation.
> So you're looking to constrain which targets will properly support
> REQ_NOWAIT, based on whether they do a simple remapping?

To be honest, I'm a little confused about the semantics of REQ_NOWAIT. 
Jens may had ever

explained it in block or io_uring mailing list, but I can't find the 
specific mail.


The man page explains FMODE_NOWAIT as 'File is capable of returning 
-EAGAIN if I/O will

block'.


And RWF_NOWAIT as

```

               RWF_NOWAIT (since Linux 4.14)
                      Don't wait if the I/O will block for operations 
such as
                      file block allocations, dirty page flush, mutex locks,
                      or a congested block device inside the kernel.  If any
                      of these conditions are met, the control block is re‐
                      turned immediately with a return value of -EAGAIN in
                      the res field of the io_event structure (see
                      io_getevents(2)).

```


commit 6abc49468eea ("dm: add support for REQ_NOWAIT and enable it for 
linear

target") handles NOWAIT for DM core as


```

@@ -1802,7 +1802,9 @@ static blk_qc_t dm_submit_bio(struct bio *bio)
         if (unlikely(test_bit(DMF_BLOCK_IO_FOR_SUSPEND, &md->flags))) {
+               if (bio->bi_opf & REQ_NOWAIT)
+                       bio_wouldblock_error(bio);

+               else if (!(bio->bi_opf & REQ_RAHEAD))
                         queue_io(md, bio);

```


Theoretically the block device could advertise QUEUE_FLAG_NOWAIT as long 
as it could

'return -EAGAIN if I/O will block' as the man page said. However, 
considering when the

dm device detected as suspending, the submitted bios are deferred to 
workqueue in

drivers/dm/dm.c: dm_submit_bio. In this case, IO gets **deferred** while 
the user process

will not be **blocked**. Can we say IO gets **blocked** in this case?


Actually several dm targets handle submitted bio in this deferred way, 
such as dm-crypt/

dm-delay/dm-era/dm-ebs. Can we say these targets are not capable of 
handling NOWAIT?


Also when system is short of memory, bio allocation in 
bio_alloc_bioset() may trigger memory

direct reclaim, as the gfp_mask is usually GFP_NOIO. While in memory 
direct reclaim, the

process may be scheduled out, but I have never seen the proper handling 
for NOWAIT in this

situation. Maybe the block or io_uring guys have more insights?


So there's just too many possibilities that may get blocked, not to say 
mutex locks.


>
>
>> Signed-off-by: Jeffle Xu <jefflexu@linux.alibaba.com>
>> ---
>> Hi Mike,
>>
>> I could split these boilerplate code that each dm target have one
>> seperate patch if you think that would be better.
> One patch for all these is fine.  But it should include the code that I
> assume you'll be adding to dm_table_supports_nowait() to further verify
> that the targets in the table are all DM_TARGET_NOWAIT.
>
> And why isn't dm-linear setting DM_TARGET_NOWAIT?
These are all done in commit 6abc49468eea ("dm: add support for 
REQ_NOWAIT and enable it for
linear target").
>
> Also, other targets _could_ be made to support REQ_NOWAIT by
> conditionally returning bio_wouldblock_error() if appropriate
> (e.g. bio-based dm-multipath's case of queue_if_no_path).



-- 
Thanks,
Jeffle

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

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [dm-devel] dm: add support for DM_TARGET_NOWAIT for various targets
  2020-11-12  6:05     ` JeffleXu
@ 2020-11-12  7:58       ` JeffleXu
  -1 siblings, 0 replies; 12+ messages in thread
From: JeffleXu @ 2020-11-12  7:58 UTC (permalink / raw)
  To: Mike Snitzer; +Cc: joseph.qi, dm-devel, koct9i, axboe, io-uring, linux-block


On 11/12/20 2:05 PM, JeffleXu wrote:
>>
>> dm-table.c:dm_table_set_restrictions() has:
>>
>>          if (dm_table_supports_nowait(t))
>>                  blk_queue_flag_set(QUEUE_FLAG_NOWAIT, q);
>>          else
>>                  blk_queue_flag_clear(QUEUE_FLAG_NOWAIT, q);
>>
>>> This patch adds support for DM_TARGET_NOWAIT for those dm targets, the
>>> .map() algorithm of which just involves sector recalculation.
>> So you're looking to constrain which targets will properly support
>> REQ_NOWAIT, based on whether they do a simple remapping?
>
> To be honest, I'm a little confused about the semantics of REQ_NOWAIT. 
> Jens may had ever
>
> explained it in block or io_uring mailing list, but I can't find the 
> specific mail.
>
I find it here 
https://lore.kernel.org/linux-block/f1a6ae88-1436-e947-1124-41e10b3ea9bc@kernel.dk/


So if the IO is offloaded to workqueue and the current process context 
will not get blocked,

then is this device capable of handling REQ_NOWAIT or not?

-- 
Thanks,
Jeffle


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [dm-devel] dm: add support for DM_TARGET_NOWAIT for various targets
@ 2020-11-12  7:58       ` JeffleXu
  0 siblings, 0 replies; 12+ messages in thread
From: JeffleXu @ 2020-11-12  7:58 UTC (permalink / raw)
  To: Mike Snitzer; +Cc: axboe, linux-block, joseph.qi, dm-devel, io-uring, koct9i


On 11/12/20 2:05 PM, JeffleXu wrote:
>>
>> dm-table.c:dm_table_set_restrictions() has:
>>
>>          if (dm_table_supports_nowait(t))
>>                  blk_queue_flag_set(QUEUE_FLAG_NOWAIT, q);
>>          else
>>                  blk_queue_flag_clear(QUEUE_FLAG_NOWAIT, q);
>>
>>> This patch adds support for DM_TARGET_NOWAIT for those dm targets, the
>>> .map() algorithm of which just involves sector recalculation.
>> So you're looking to constrain which targets will properly support
>> REQ_NOWAIT, based on whether they do a simple remapping?
>
> To be honest, I'm a little confused about the semantics of REQ_NOWAIT. 
> Jens may had ever
>
> explained it in block or io_uring mailing list, but I can't find the 
> specific mail.
>
I find it here 
https://lore.kernel.org/linux-block/f1a6ae88-1436-e947-1124-41e10b3ea9bc@kernel.dk/


So if the IO is offloaded to workqueue and the current process context 
will not get blocked,

then is this device capable of handling REQ_NOWAIT or not?

-- 
Thanks,
Jeffle

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

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: dm: add support for DM_TARGET_NOWAIT for various targets
  2020-11-12  6:05     ` JeffleXu
@ 2020-11-12 16:11       ` Mike Snitzer
  -1 siblings, 0 replies; 12+ messages in thread
From: Mike Snitzer @ 2020-11-12 16:11 UTC (permalink / raw)
  To: JeffleXu; +Cc: joseph.qi, dm-devel, koct9i, axboe, io-uring, linux-block

On Thu, Nov 12 2020 at  1:05am -0500,
JeffleXu <jefflexu@linux.alibaba.com> wrote:

> Hi Jens and guys in block/io_uring mailing list, this mail contains
> some discussion abount
> 
> RWF_NOWAIT, please see the following contents.
> 
> 
> 
> On 11/11/20 11:38 PM, Mike Snitzer wrote:
> >On Tue, Nov 10 2020 at  1:55am -0500,
> >Jeffle Xu <jefflexu@linux.alibaba.com> wrote:
> >
> >>This is one prep patch for supporting iopoll for dm device.
> >>
> >>The direct IO routine will set REQ_NOWAIT flag for REQ_HIPRI IO (that
> >>is, IO will do iopoll) in bio_set_polled(). Then in the IO submission
> >>routine, the ability of handling REQ_NOWAIT of the block device will
> >>be checked for REQ_HIPRI IO in submit_bio_checks(). -EOPNOTSUPP will
> >>be returned if the block device doesn't support REQ_NOWAIT.
> >submit_bio_checks() verifies the request_queue has QUEUE_FLAG_NOWAIT set
> >if the bio has REQ_NOWAIT.
> Yes that's the case.
> >
> >>DM lacks support for REQ_NOWAIT until commit 6abc49468eea ("dm: add
> >>support for REQ_NOWAIT and enable it for linear target"). Since then,
> >>dm targets that support REQ_NOWAIT should advertise DM_TARGET_NOWAIT
> >>feature.
> >I'm not seeing why DM_TARGET_NOWAIT is needed (since you didn't add any
> >code that consumes the flag).
> 
> As I said, it's needed if we support iopoll for dm device.  Only if
> a block device is capable of
> 
> handling NOWAIT, then it can support iopoll.
> 
> 
> IO submitted for iopoll (marked with IOCB_HIPRI) is usually also
> marked with REQ_NOWAIT.
> 
> There are two scenario when it could happen.
> 
> 
> 1. io_uring will set REQ_NOWAIT
> 
> The IO submission of io_uring can be divided into two phase. First,
> IO will be submitted
> 
> synchronously in user process context (when sqthread feature
> disabled), or sqthread
> 
> context (when sqthread feature enabled).
> 
> 
> ```sh
> - current process context when sqthread disabled, or sqthread when
> it's enabled
>     io_uring_enter
>         io_submit_sqes
>             io_submit_sqe
>                 io_queue_sqe
>                     __io_queue_sqe
>                         io_issue_sqe // with @force_nonblock is true
>                             io_read/io_write
> ```
> 
> In this case, IO should be handled in a NOWAIT way, since the user
> process or sqthread
> 
> can not be blocked for performance.
> 
> ```
> 
> io_read/io_write
> 
>     /* Ensure we clear previously set non-block flag */
>     if (!force_nonblock)
>         kiocb->ki_flags &= ~IOCB_NOWAIT;
>     else
>         kiocb->ki_flags |= IOCB_NOWAIT;
> 
> ```
> 
> 
> 2. The direct IO routine will set REQ_NOWAIT for polling IO
> 
> Both fs/block_dev.c: __blkdev_direct_IO and fs/iomap/direct-io.c:
> iomap_dio_submit_bio will
> 
> call bio_set_polled(), in which will set REQ_NOWAIT for polling IO.
> 
> 
> ```sh
> __blkdev_direct_IO / iomap_dio_submit_bio:
>     if (dio->iocb->ki_flags & IOCB_HIPRI)
>         bio_set_polled
>           bio->bi_opf |= REQ_NOWAIT
> ```
> 
> 
> Thus to support iopoll for dm device, the dm target should be
> capable of handling NOWAIT,
> 
> or submit_bio_checks() will fail with -EOPNOTSUPP when submitting
> bio to dm device.
> 
> 
> >
> >dm-table.c:dm_table_set_restrictions() has:
> >
> >         if (dm_table_supports_nowait(t))
> >                 blk_queue_flag_set(QUEUE_FLAG_NOWAIT, q);
> >         else
> >                 blk_queue_flag_clear(QUEUE_FLAG_NOWAIT, q);
> >
> >>This patch adds support for DM_TARGET_NOWAIT for those dm targets, the
> >>.map() algorithm of which just involves sector recalculation.
> >So you're looking to constrain which targets will properly support
> >REQ_NOWAIT, based on whether they do a simple remapping?
> 
> To be honest, I'm a little confused about the semantics of
> REQ_NOWAIT. Jens may had ever
> 
> explained it in block or io_uring mailing list, but I can't find the
> specific mail.
> 
> 
> The man page explains FMODE_NOWAIT as 'File is capable of returning
> -EAGAIN if I/O will
> 
> block'.
> 
> 
> And RWF_NOWAIT as
> 
> ```
> 
>               RWF_NOWAIT (since Linux 4.14)
>                      Don't wait if the I/O will block for operations
> such as
>                      file block allocations, dirty page flush, mutex locks,
>                      or a congested block device inside the kernel.  If any
>                      of these conditions are met, the control block is re‐
>                      turned immediately with a return value of -EAGAIN in
>                      the res field of the io_event structure (see
>                      io_getevents(2)).
> 
> ```
> 
> 
> commit 6abc49468eea ("dm: add support for REQ_NOWAIT and enable it
> for linear
> 
> target") handles NOWAIT for DM core as
> 
> 
> ```
> 
> @@ -1802,7 +1802,9 @@ static blk_qc_t dm_submit_bio(struct bio *bio)
>         if (unlikely(test_bit(DMF_BLOCK_IO_FOR_SUSPEND, &md->flags))) {
> +               if (bio->bi_opf & REQ_NOWAIT)
> +                       bio_wouldblock_error(bio);
> 
> +               else if (!(bio->bi_opf & REQ_RAHEAD))
>                         queue_io(md, bio);
> 
> ```
> 
> 
> Theoretically the block device could advertise QUEUE_FLAG_NOWAIT as
> long as it could
> 
> 'return -EAGAIN if I/O will block' as the man page said. However,
> considering when the
> 
> dm device detected as suspending, the submitted bios are deferred to
> workqueue in
> 
> drivers/dm/dm.c: dm_submit_bio. In this case, IO gets **deferred**
> while the user process
> 
> will not be **blocked**. Can we say IO gets **blocked** in this case?
> 
> 
> Actually several dm targets handle submitted bio in this deferred
> way, such as dm-crypt/
> 
> dm-delay/dm-era/dm-ebs. Can we say these targets are not capable of
> handling NOWAIT?
> 
> 
> Also when system is short of memory, bio allocation in
> bio_alloc_bioset() may trigger memory
> 
> direct reclaim, as the gfp_mask is usually GFP_NOIO. While in memory
> direct reclaim, the
> 
> process may be scheduled out, but I have never seen the proper
> handling for NOWAIT in this
> 
> situation. Maybe the block or io_uring guys have more insights?
> 
> 
> So there's just too many possibilities that may get blocked, not to
> say mutex locks.
> 
> 
> >
> >
> >>Signed-off-by: Jeffle Xu <jefflexu@linux.alibaba.com>
> >>---
> >>Hi Mike,
> >>
> >>I could split these boilerplate code that each dm target have one
> >>seperate patch if you think that would be better.
> >One patch for all these is fine.  But it should include the code that I
> >assume you'll be adding to dm_table_supports_nowait() to further verify
> >that the targets in the table are all DM_TARGET_NOWAIT.
> >
> >And why isn't dm-linear setting DM_TARGET_NOWAIT?
> These are all done in commit 6abc49468eea ("dm: add support for
> REQ_NOWAIT and enable it for
> linear target").

Ha, oops.  You'd think I'd have remembered adding DM_TARGET_NOWAIT to
dm-linear and dm_target_supports_nowait, etc.

Thanks for clarifying.  So your patch was just about extending
the capability to other targets where you think it applicable.

We'll be able to make that clearer by tweaking the header a bit.

Mike


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [dm-devel] dm: add support for DM_TARGET_NOWAIT for various targets
@ 2020-11-12 16:11       ` Mike Snitzer
  0 siblings, 0 replies; 12+ messages in thread
From: Mike Snitzer @ 2020-11-12 16:11 UTC (permalink / raw)
  To: JeffleXu; +Cc: axboe, linux-block, joseph.qi, dm-devel, io-uring, koct9i

On Thu, Nov 12 2020 at  1:05am -0500,
JeffleXu <jefflexu@linux.alibaba.com> wrote:

> Hi Jens and guys in block/io_uring mailing list, this mail contains
> some discussion abount
> 
> RWF_NOWAIT, please see the following contents.
> 
> 
> 
> On 11/11/20 11:38 PM, Mike Snitzer wrote:
> >On Tue, Nov 10 2020 at  1:55am -0500,
> >Jeffle Xu <jefflexu@linux.alibaba.com> wrote:
> >
> >>This is one prep patch for supporting iopoll for dm device.
> >>
> >>The direct IO routine will set REQ_NOWAIT flag for REQ_HIPRI IO (that
> >>is, IO will do iopoll) in bio_set_polled(). Then in the IO submission
> >>routine, the ability of handling REQ_NOWAIT of the block device will
> >>be checked for REQ_HIPRI IO in submit_bio_checks(). -EOPNOTSUPP will
> >>be returned if the block device doesn't support REQ_NOWAIT.
> >submit_bio_checks() verifies the request_queue has QUEUE_FLAG_NOWAIT set
> >if the bio has REQ_NOWAIT.
> Yes that's the case.
> >
> >>DM lacks support for REQ_NOWAIT until commit 6abc49468eea ("dm: add
> >>support for REQ_NOWAIT and enable it for linear target"). Since then,
> >>dm targets that support REQ_NOWAIT should advertise DM_TARGET_NOWAIT
> >>feature.
> >I'm not seeing why DM_TARGET_NOWAIT is needed (since you didn't add any
> >code that consumes the flag).
> 
> As I said, it's needed if we support iopoll for dm device.  Only if
> a block device is capable of
> 
> handling NOWAIT, then it can support iopoll.
> 
> 
> IO submitted for iopoll (marked with IOCB_HIPRI) is usually also
> marked with REQ_NOWAIT.
> 
> There are two scenario when it could happen.
> 
> 
> 1. io_uring will set REQ_NOWAIT
> 
> The IO submission of io_uring can be divided into two phase. First,
> IO will be submitted
> 
> synchronously in user process context (when sqthread feature
> disabled), or sqthread
> 
> context (when sqthread feature enabled).
> 
> 
> ```sh
> - current process context when sqthread disabled, or sqthread when
> it's enabled
>     io_uring_enter
>         io_submit_sqes
>             io_submit_sqe
>                 io_queue_sqe
>                     __io_queue_sqe
>                         io_issue_sqe // with @force_nonblock is true
>                             io_read/io_write
> ```
> 
> In this case, IO should be handled in a NOWAIT way, since the user
> process or sqthread
> 
> can not be blocked for performance.
> 
> ```
> 
> io_read/io_write
> 
>     /* Ensure we clear previously set non-block flag */
>     if (!force_nonblock)
>         kiocb->ki_flags &= ~IOCB_NOWAIT;
>     else
>         kiocb->ki_flags |= IOCB_NOWAIT;
> 
> ```
> 
> 
> 2. The direct IO routine will set REQ_NOWAIT for polling IO
> 
> Both fs/block_dev.c: __blkdev_direct_IO and fs/iomap/direct-io.c:
> iomap_dio_submit_bio will
> 
> call bio_set_polled(), in which will set REQ_NOWAIT for polling IO.
> 
> 
> ```sh
> __blkdev_direct_IO / iomap_dio_submit_bio:
>     if (dio->iocb->ki_flags & IOCB_HIPRI)
>         bio_set_polled
>           bio->bi_opf |= REQ_NOWAIT
> ```
> 
> 
> Thus to support iopoll for dm device, the dm target should be
> capable of handling NOWAIT,
> 
> or submit_bio_checks() will fail with -EOPNOTSUPP when submitting
> bio to dm device.
> 
> 
> >
> >dm-table.c:dm_table_set_restrictions() has:
> >
> >         if (dm_table_supports_nowait(t))
> >                 blk_queue_flag_set(QUEUE_FLAG_NOWAIT, q);
> >         else
> >                 blk_queue_flag_clear(QUEUE_FLAG_NOWAIT, q);
> >
> >>This patch adds support for DM_TARGET_NOWAIT for those dm targets, the
> >>.map() algorithm of which just involves sector recalculation.
> >So you're looking to constrain which targets will properly support
> >REQ_NOWAIT, based on whether they do a simple remapping?
> 
> To be honest, I'm a little confused about the semantics of
> REQ_NOWAIT. Jens may had ever
> 
> explained it in block or io_uring mailing list, but I can't find the
> specific mail.
> 
> 
> The man page explains FMODE_NOWAIT as 'File is capable of returning
> -EAGAIN if I/O will
> 
> block'.
> 
> 
> And RWF_NOWAIT as
> 
> ```
> 
>               RWF_NOWAIT (since Linux 4.14)
>                      Don't wait if the I/O will block for operations
> such as
>                      file block allocations, dirty page flush, mutex locks,
>                      or a congested block device inside the kernel.  If any
>                      of these conditions are met, the control block is re‐
>                      turned immediately with a return value of -EAGAIN in
>                      the res field of the io_event structure (see
>                      io_getevents(2)).
> 
> ```
> 
> 
> commit 6abc49468eea ("dm: add support for REQ_NOWAIT and enable it
> for linear
> 
> target") handles NOWAIT for DM core as
> 
> 
> ```
> 
> @@ -1802,7 +1802,9 @@ static blk_qc_t dm_submit_bio(struct bio *bio)
>         if (unlikely(test_bit(DMF_BLOCK_IO_FOR_SUSPEND, &md->flags))) {
> +               if (bio->bi_opf & REQ_NOWAIT)
> +                       bio_wouldblock_error(bio);
> 
> +               else if (!(bio->bi_opf & REQ_RAHEAD))
>                         queue_io(md, bio);
> 
> ```
> 
> 
> Theoretically the block device could advertise QUEUE_FLAG_NOWAIT as
> long as it could
> 
> 'return -EAGAIN if I/O will block' as the man page said. However,
> considering when the
> 
> dm device detected as suspending, the submitted bios are deferred to
> workqueue in
> 
> drivers/dm/dm.c: dm_submit_bio. In this case, IO gets **deferred**
> while the user process
> 
> will not be **blocked**. Can we say IO gets **blocked** in this case?
> 
> 
> Actually several dm targets handle submitted bio in this deferred
> way, such as dm-crypt/
> 
> dm-delay/dm-era/dm-ebs. Can we say these targets are not capable of
> handling NOWAIT?
> 
> 
> Also when system is short of memory, bio allocation in
> bio_alloc_bioset() may trigger memory
> 
> direct reclaim, as the gfp_mask is usually GFP_NOIO. While in memory
> direct reclaim, the
> 
> process may be scheduled out, but I have never seen the proper
> handling for NOWAIT in this
> 
> situation. Maybe the block or io_uring guys have more insights?
> 
> 
> So there's just too many possibilities that may get blocked, not to
> say mutex locks.
> 
> 
> >
> >
> >>Signed-off-by: Jeffle Xu <jefflexu@linux.alibaba.com>
> >>---
> >>Hi Mike,
> >>
> >>I could split these boilerplate code that each dm target have one
> >>seperate patch if you think that would be better.
> >One patch for all these is fine.  But it should include the code that I
> >assume you'll be adding to dm_table_supports_nowait() to further verify
> >that the targets in the table are all DM_TARGET_NOWAIT.
> >
> >And why isn't dm-linear setting DM_TARGET_NOWAIT?
> These are all done in commit 6abc49468eea ("dm: add support for
> REQ_NOWAIT and enable it for
> linear target").

Ha, oops.  You'd think I'd have remembered adding DM_TARGET_NOWAIT to
dm-linear and dm_target_supports_nowait, etc.

Thanks for clarifying.  So your patch was just about extending
the capability to other targets where you think it applicable.

We'll be able to make that clearer by tweaking the header a bit.

Mike

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

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [dm-devel] [PATCH v2] dm: add support for DM_TARGET_NOWAIT for various targets
  2020-11-10  6:55 [dm-devel] [PATCH] dm: add support for DM_TARGET_NOWAIT for various targets Jeffle Xu
  2020-11-11 15:38 ` [dm-devel] " Mike Snitzer
@ 2020-11-13  2:05 ` Jeffle Xu
  2020-11-13  2:12   ` JeffleXu
  2020-11-18  2:01   ` JeffleXu
  1 sibling, 2 replies; 12+ messages in thread
From: Jeffle Xu @ 2020-11-13  2:05 UTC (permalink / raw)
  To: snitzer; +Cc: joseph.qi, dm-devel, koct9i

commit 021a24460dc2 ("block: add QUEUE_FLAG_NOWAIT") adds a new queue
flag QUEUE_FLAG_NOWAIT to advertise if the bdev supports handling of
REQ_NOWAIT or not. DM core supports this in commit 6abc49468eea ("dm:
add support for REQ_NOWAIT and enable it for linear target"), in which
only dm-linear is enabled.

This patch also enables several dm-linear likely dm targets, the mapping
algorithm of which is just simple remapping.

Signed-off-by: Jeffle Xu <jefflexu@linux.alibaba.com>
---
 drivers/md/dm-stripe.c   | 2 +-
 drivers/md/dm-switch.c   | 1 +
 drivers/md/dm-unstripe.c | 1 +
 drivers/md/dm-zero.c     | 1 +
 4 files changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/md/dm-stripe.c b/drivers/md/dm-stripe.c
index 151d022b032d..df359d33cda8 100644
--- a/drivers/md/dm-stripe.c
+++ b/drivers/md/dm-stripe.c
@@ -496,7 +496,7 @@ static void stripe_io_hints(struct dm_target *ti,
 static struct target_type stripe_target = {
 	.name   = "striped",
 	.version = {1, 6, 0},
-	.features = DM_TARGET_PASSES_INTEGRITY,
+	.features = DM_TARGET_PASSES_INTEGRITY | DM_TARGET_NOWAIT,
 	.module = THIS_MODULE,
 	.ctr    = stripe_ctr,
 	.dtr    = stripe_dtr,
diff --git a/drivers/md/dm-switch.c b/drivers/md/dm-switch.c
index bff4c7fa1cd2..262e2b0fd975 100644
--- a/drivers/md/dm-switch.c
+++ b/drivers/md/dm-switch.c
@@ -550,6 +550,7 @@ static int switch_iterate_devices(struct dm_target *ti,
 static struct target_type switch_target = {
 	.name = "switch",
 	.version = {1, 1, 0},
+	.features = DM_TARGET_NOWAIT,
 	.module = THIS_MODULE,
 	.ctr = switch_ctr,
 	.dtr = switch_dtr,
diff --git a/drivers/md/dm-unstripe.c b/drivers/md/dm-unstripe.c
index e673dacf6418..7357c1bd5863 100644
--- a/drivers/md/dm-unstripe.c
+++ b/drivers/md/dm-unstripe.c
@@ -178,6 +178,7 @@ static void unstripe_io_hints(struct dm_target *ti,
 static struct target_type unstripe_target = {
 	.name = "unstriped",
 	.version = {1, 1, 0},
+	.features = DM_TARGET_NOWAIT,
 	.module = THIS_MODULE,
 	.ctr = unstripe_ctr,
 	.dtr = unstripe_dtr,
diff --git a/drivers/md/dm-zero.c b/drivers/md/dm-zero.c
index b65ca8dcfbdc..faa1dbffc8b4 100644
--- a/drivers/md/dm-zero.c
+++ b/drivers/md/dm-zero.c
@@ -59,6 +59,7 @@ static int zero_map(struct dm_target *ti, struct bio *bio)
 static struct target_type zero_target = {
 	.name   = "zero",
 	.version = {1, 1, 0},
+	.features = DM_TARGET_NOWAIT,
 	.module = THIS_MODULE,
 	.ctr    = zero_ctr,
 	.map    = zero_map,
-- 
2.27.0

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


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [dm-devel] [PATCH v2] dm: add support for DM_TARGET_NOWAIT for various targets
  2020-11-13  2:05 ` [dm-devel] [PATCH v2] " Jeffle Xu
@ 2020-11-13  2:12   ` JeffleXu
  2020-11-18  2:01   ` JeffleXu
  1 sibling, 0 replies; 12+ messages in thread
From: JeffleXu @ 2020-11-13  2:12 UTC (permalink / raw)
  To: snitzer; +Cc: joseph.qi, dm-devel, koct9i

Hi Mike,

I just tweaks the commit message in this v2 patch to make the purpose 
clearer,

while keep the code unstained.


Thanks,

Jeffle


On 11/13/20 10:05 AM, Jeffle Xu wrote:
> commit 021a24460dc2 ("block: add QUEUE_FLAG_NOWAIT") adds a new queue
> flag QUEUE_FLAG_NOWAIT to advertise if the bdev supports handling of
> REQ_NOWAIT or not. DM core supports this in commit 6abc49468eea ("dm:
> add support for REQ_NOWAIT and enable it for linear target"), in which
> only dm-linear is enabled.
>
> This patch also enables several dm-linear likely dm targets, the mapping
> algorithm of which is just simple remapping.
>
> Signed-off-by: Jeffle Xu <jefflexu@linux.alibaba.com>
> ---
>   drivers/md/dm-stripe.c   | 2 +-
>   drivers/md/dm-switch.c   | 1 +
>   drivers/md/dm-unstripe.c | 1 +
>   drivers/md/dm-zero.c     | 1 +
>   4 files changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/md/dm-stripe.c b/drivers/md/dm-stripe.c
> index 151d022b032d..df359d33cda8 100644
> --- a/drivers/md/dm-stripe.c
> +++ b/drivers/md/dm-stripe.c
> @@ -496,7 +496,7 @@ static void stripe_io_hints(struct dm_target *ti,
>   static struct target_type stripe_target = {
>   	.name   = "striped",
>   	.version = {1, 6, 0},
> -	.features = DM_TARGET_PASSES_INTEGRITY,
> +	.features = DM_TARGET_PASSES_INTEGRITY | DM_TARGET_NOWAIT,
>   	.module = THIS_MODULE,
>   	.ctr    = stripe_ctr,
>   	.dtr    = stripe_dtr,
> diff --git a/drivers/md/dm-switch.c b/drivers/md/dm-switch.c
> index bff4c7fa1cd2..262e2b0fd975 100644
> --- a/drivers/md/dm-switch.c
> +++ b/drivers/md/dm-switch.c
> @@ -550,6 +550,7 @@ static int switch_iterate_devices(struct dm_target *ti,
>   static struct target_type switch_target = {
>   	.name = "switch",
>   	.version = {1, 1, 0},
> +	.features = DM_TARGET_NOWAIT,
>   	.module = THIS_MODULE,
>   	.ctr = switch_ctr,
>   	.dtr = switch_dtr,
> diff --git a/drivers/md/dm-unstripe.c b/drivers/md/dm-unstripe.c
> index e673dacf6418..7357c1bd5863 100644
> --- a/drivers/md/dm-unstripe.c
> +++ b/drivers/md/dm-unstripe.c
> @@ -178,6 +178,7 @@ static void unstripe_io_hints(struct dm_target *ti,
>   static struct target_type unstripe_target = {
>   	.name = "unstriped",
>   	.version = {1, 1, 0},
> +	.features = DM_TARGET_NOWAIT,
>   	.module = THIS_MODULE,
>   	.ctr = unstripe_ctr,
>   	.dtr = unstripe_dtr,
> diff --git a/drivers/md/dm-zero.c b/drivers/md/dm-zero.c
> index b65ca8dcfbdc..faa1dbffc8b4 100644
> --- a/drivers/md/dm-zero.c
> +++ b/drivers/md/dm-zero.c
> @@ -59,6 +59,7 @@ static int zero_map(struct dm_target *ti, struct bio *bio)
>   static struct target_type zero_target = {
>   	.name   = "zero",
>   	.version = {1, 1, 0},
> +	.features = DM_TARGET_NOWAIT,
>   	.module = THIS_MODULE,
>   	.ctr    = zero_ctr,
>   	.map    = zero_map,

-- 
Thanks,
Jeffle

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


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [dm-devel] [PATCH v2] dm: add support for DM_TARGET_NOWAIT for various targets
  2020-11-13  2:05 ` [dm-devel] [PATCH v2] " Jeffle Xu
  2020-11-13  2:12   ` JeffleXu
@ 2020-11-18  2:01   ` JeffleXu
  2020-11-18 15:38     ` Mike Snitzer
  1 sibling, 1 reply; 12+ messages in thread
From: JeffleXu @ 2020-11-18  2:01 UTC (permalink / raw)
  To: snitzer; +Cc: joseph.qi, dm-devel, koct9i

Hi Mike,

How about this patch? I just tweaks the commit message in this v2 patch 
to make

the purpose clearer, while keep the code unstained.


Thanks,

Jeffle


On 11/13/20 10:05 AM, Jeffle Xu wrote:
> commit 021a24460dc2 ("block: add QUEUE_FLAG_NOWAIT") adds a new queue
> flag QUEUE_FLAG_NOWAIT to advertise if the bdev supports handling of
> REQ_NOWAIT or not. DM core supports this in commit 6abc49468eea ("dm:
> add support for REQ_NOWAIT and enable it for linear target"), in which
> only dm-linear is enabled.
>
> This patch also enables several dm-linear likely dm targets, the mapping
> algorithm of which is just simple remapping.
>
> Signed-off-by: Jeffle Xu <jefflexu@linux.alibaba.com>
> ---
>   drivers/md/dm-stripe.c   | 2 +-
>   drivers/md/dm-switch.c   | 1 +
>   drivers/md/dm-unstripe.c | 1 +
>   drivers/md/dm-zero.c     | 1 +
>   4 files changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/md/dm-stripe.c b/drivers/md/dm-stripe.c
> index 151d022b032d..df359d33cda8 100644
> --- a/drivers/md/dm-stripe.c
> +++ b/drivers/md/dm-stripe.c
> @@ -496,7 +496,7 @@ static void stripe_io_hints(struct dm_target *ti,
>   static struct target_type stripe_target = {
>   	.name   = "striped",
>   	.version = {1, 6, 0},
> -	.features = DM_TARGET_PASSES_INTEGRITY,
> +	.features = DM_TARGET_PASSES_INTEGRITY | DM_TARGET_NOWAIT,
>   	.module = THIS_MODULE,
>   	.ctr    = stripe_ctr,
>   	.dtr    = stripe_dtr,
> diff --git a/drivers/md/dm-switch.c b/drivers/md/dm-switch.c
> index bff4c7fa1cd2..262e2b0fd975 100644
> --- a/drivers/md/dm-switch.c
> +++ b/drivers/md/dm-switch.c
> @@ -550,6 +550,7 @@ static int switch_iterate_devices(struct dm_target *ti,
>   static struct target_type switch_target = {
>   	.name = "switch",
>   	.version = {1, 1, 0},
> +	.features = DM_TARGET_NOWAIT,
>   	.module = THIS_MODULE,
>   	.ctr = switch_ctr,
>   	.dtr = switch_dtr,
> diff --git a/drivers/md/dm-unstripe.c b/drivers/md/dm-unstripe.c
> index e673dacf6418..7357c1bd5863 100644
> --- a/drivers/md/dm-unstripe.c
> +++ b/drivers/md/dm-unstripe.c
> @@ -178,6 +178,7 @@ static void unstripe_io_hints(struct dm_target *ti,
>   static struct target_type unstripe_target = {
>   	.name = "unstriped",
>   	.version = {1, 1, 0},
> +	.features = DM_TARGET_NOWAIT,
>   	.module = THIS_MODULE,
>   	.ctr = unstripe_ctr,
>   	.dtr = unstripe_dtr,
> diff --git a/drivers/md/dm-zero.c b/drivers/md/dm-zero.c
> index b65ca8dcfbdc..faa1dbffc8b4 100644
> --- a/drivers/md/dm-zero.c
> +++ b/drivers/md/dm-zero.c
> @@ -59,6 +59,7 @@ static int zero_map(struct dm_target *ti, struct bio *bio)
>   static struct target_type zero_target = {
>   	.name   = "zero",
>   	.version = {1, 1, 0},
> +	.features = DM_TARGET_NOWAIT,
>   	.module = THIS_MODULE,
>   	.ctr    = zero_ctr,
>   	.map    = zero_map,

-- 
Thanks,
Jeffle

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


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [dm-devel] [PATCH v2] dm: add support for DM_TARGET_NOWAIT for various targets
  2020-11-18  2:01   ` JeffleXu
@ 2020-11-18 15:38     ` Mike Snitzer
  0 siblings, 0 replies; 12+ messages in thread
From: Mike Snitzer @ 2020-11-18 15:38 UTC (permalink / raw)
  To: JeffleXu; +Cc: joseph.qi, dm-devel, koct9i

I just staged this for 5.11, see:
https://git.kernel.org/pub/scm/linux/kernel/git/device-mapper/linux-dm.git/commit/?h=dm-5.11&id=373ce365b756bf6fec237461a0bbe65f33f201f6

I tweaked your patch header just a bit.

Thanks,
Mike


On Tue, Nov 17 2020 at  9:01pm -0500,
JeffleXu <jefflexu@linux.alibaba.com> wrote:

> Hi Mike,
> 
> How about this patch? I just tweaks the commit message in this v2
> patch to make
> 
> the purpose clearer, while keep the code unstained.
> 
> 
> Thanks,
> 
> Jeffle
> 
> 
> On 11/13/20 10:05 AM, Jeffle Xu wrote:
> >commit 021a24460dc2 ("block: add QUEUE_FLAG_NOWAIT") adds a new queue
> >flag QUEUE_FLAG_NOWAIT to advertise if the bdev supports handling of
> >REQ_NOWAIT or not. DM core supports this in commit 6abc49468eea ("dm:
> >add support for REQ_NOWAIT and enable it for linear target"), in which
> >only dm-linear is enabled.
> >
> >This patch also enables several dm-linear likely dm targets, the mapping
> >algorithm of which is just simple remapping.
> >
> >Signed-off-by: Jeffle Xu <jefflexu@linux.alibaba.com>
> >---
> >  drivers/md/dm-stripe.c   | 2 +-
> >  drivers/md/dm-switch.c   | 1 +
> >  drivers/md/dm-unstripe.c | 1 +
> >  drivers/md/dm-zero.c     | 1 +
> >  4 files changed, 4 insertions(+), 1 deletion(-)
> >
> >diff --git a/drivers/md/dm-stripe.c b/drivers/md/dm-stripe.c
> >index 151d022b032d..df359d33cda8 100644
> >--- a/drivers/md/dm-stripe.c
> >+++ b/drivers/md/dm-stripe.c
> >@@ -496,7 +496,7 @@ static void stripe_io_hints(struct dm_target *ti,
> >  static struct target_type stripe_target = {
> >  	.name   = "striped",
> >  	.version = {1, 6, 0},
> >-	.features = DM_TARGET_PASSES_INTEGRITY,
> >+	.features = DM_TARGET_PASSES_INTEGRITY | DM_TARGET_NOWAIT,
> >  	.module = THIS_MODULE,
> >  	.ctr    = stripe_ctr,
> >  	.dtr    = stripe_dtr,
> >diff --git a/drivers/md/dm-switch.c b/drivers/md/dm-switch.c
> >index bff4c7fa1cd2..262e2b0fd975 100644
> >--- a/drivers/md/dm-switch.c
> >+++ b/drivers/md/dm-switch.c
> >@@ -550,6 +550,7 @@ static int switch_iterate_devices(struct dm_target *ti,
> >  static struct target_type switch_target = {
> >  	.name = "switch",
> >  	.version = {1, 1, 0},
> >+	.features = DM_TARGET_NOWAIT,
> >  	.module = THIS_MODULE,
> >  	.ctr = switch_ctr,
> >  	.dtr = switch_dtr,
> >diff --git a/drivers/md/dm-unstripe.c b/drivers/md/dm-unstripe.c
> >index e673dacf6418..7357c1bd5863 100644
> >--- a/drivers/md/dm-unstripe.c
> >+++ b/drivers/md/dm-unstripe.c
> >@@ -178,6 +178,7 @@ static void unstripe_io_hints(struct dm_target *ti,
> >  static struct target_type unstripe_target = {
> >  	.name = "unstriped",
> >  	.version = {1, 1, 0},
> >+	.features = DM_TARGET_NOWAIT,
> >  	.module = THIS_MODULE,
> >  	.ctr = unstripe_ctr,
> >  	.dtr = unstripe_dtr,
> >diff --git a/drivers/md/dm-zero.c b/drivers/md/dm-zero.c
> >index b65ca8dcfbdc..faa1dbffc8b4 100644
> >--- a/drivers/md/dm-zero.c
> >+++ b/drivers/md/dm-zero.c
> >@@ -59,6 +59,7 @@ static int zero_map(struct dm_target *ti, struct bio *bio)
> >  static struct target_type zero_target = {
> >  	.name   = "zero",
> >  	.version = {1, 1, 0},
> >+	.features = DM_TARGET_NOWAIT,
> >  	.module = THIS_MODULE,
> >  	.ctr    = zero_ctr,
> >  	.map    = zero_map,
> 
> -- 
> Thanks,
> Jeffle
> 

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


^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2020-11-18 15:38 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-11-10  6:55 [dm-devel] [PATCH] dm: add support for DM_TARGET_NOWAIT for various targets Jeffle Xu
2020-11-11 15:38 ` [dm-devel] " Mike Snitzer
2020-11-12  6:05   ` JeffleXu
2020-11-12  6:05     ` JeffleXu
2020-11-12  7:58     ` JeffleXu
2020-11-12  7:58       ` JeffleXu
2020-11-12 16:11     ` Mike Snitzer
2020-11-12 16:11       ` [dm-devel] " Mike Snitzer
2020-11-13  2:05 ` [dm-devel] [PATCH v2] " Jeffle Xu
2020-11-13  2:12   ` JeffleXu
2020-11-18  2:01   ` JeffleXu
2020-11-18 15:38     ` Mike Snitzer

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.