linux-mmc.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2 0/2] blk-mq: Allow to complete requests directly
@ 2021-10-18 13:55 Sebastian Andrzej Siewior
  2021-10-18 13:55 ` [PATCH v2 1/2] blk-mq: Add blk_mq_complete_request_direct() Sebastian Andrzej Siewior
  2021-10-18 13:55 ` [PATCH v2 2/2] mmc: core: Use blk_mq_complete_request_direct() Sebastian Andrzej Siewior
  0 siblings, 2 replies; 9+ messages in thread
From: Sebastian Andrzej Siewior @ 2021-10-18 13:55 UTC (permalink / raw)
  To: linux-block, linux-mmc; +Cc: tglx, Ulf Hansson, Jens Axboe, Christoph Hellwig

v1…v2:
 - Drop the SCSI patch for now.
 - Make blk_mq_complete_request_direct() call the completion handler
   directly instead going through struct chain (Jens and hch might had
   the same in mind).

This series converts a part from the MMC layer which completes the
requests from kworker/ preemptible context. Its intention is to avoid
going through the softirq stack in preemptible context which would
involve the ksoftirqd in this case.

Sebastian



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

* [PATCH v2 1/2] blk-mq: Add blk_mq_complete_request_direct()
  2021-10-18 13:55 [PATCH v2 0/2] blk-mq: Allow to complete requests directly Sebastian Andrzej Siewior
@ 2021-10-18 13:55 ` Sebastian Andrzej Siewior
  2021-10-18 15:20   ` Christoph Hellwig
  2021-10-18 13:55 ` [PATCH v2 2/2] mmc: core: Use blk_mq_complete_request_direct() Sebastian Andrzej Siewior
  1 sibling, 1 reply; 9+ messages in thread
From: Sebastian Andrzej Siewior @ 2021-10-18 13:55 UTC (permalink / raw)
  To: linux-block, linux-mmc
  Cc: tglx, Ulf Hansson, Jens Axboe, Christoph Hellwig,
	Sebastian Andrzej Siewior

Add blk_mq_complete_request_direct() which completes the block request
directly instead deferring it to softirq for single queue devices.

This is useful for devices which complete the requests in preemptible
context and raising softirq from means scheduling ksoftirqd.

Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
---
 include/linux/blk-mq.h | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/include/linux/blk-mq.h b/include/linux/blk-mq.h
index 13ba1861e688f..93780c890b479 100644
--- a/include/linux/blk-mq.h
+++ b/include/linux/blk-mq.h
@@ -521,6 +521,17 @@ static inline void blk_mq_set_request_complete(struct request *rq)
 	WRITE_ONCE(rq->state, MQ_RQ_COMPLETE);
 }
 
+/*
+ * Complete the request directly instead of deferring it to softirq or
+ * completing it another CPU. Useful in preemptible instead of an interrupt.
+ */
+static inline void blk_mq_complete_request_direct(struct request *rq,
+						  void (*complete)(struct request *rq))
+{
+	WRITE_ONCE(rq->state, MQ_RQ_COMPLETE);
+	complete(rq);
+}
+
 void blk_mq_start_request(struct request *rq);
 void blk_mq_end_request(struct request *rq, blk_status_t error);
 void __blk_mq_end_request(struct request *rq, blk_status_t error);
-- 
2.33.0


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

* [PATCH v2 2/2] mmc: core: Use blk_mq_complete_request_direct().
  2021-10-18 13:55 [PATCH v2 0/2] blk-mq: Allow to complete requests directly Sebastian Andrzej Siewior
  2021-10-18 13:55 ` [PATCH v2 1/2] blk-mq: Add blk_mq_complete_request_direct() Sebastian Andrzej Siewior
@ 2021-10-18 13:55 ` Sebastian Andrzej Siewior
  2021-10-19 11:32   ` Ulf Hansson
  2021-10-21 19:45   ` Ulf Hansson
  1 sibling, 2 replies; 9+ messages in thread
From: Sebastian Andrzej Siewior @ 2021-10-18 13:55 UTC (permalink / raw)
  To: linux-block, linux-mmc
  Cc: tglx, Ulf Hansson, Jens Axboe, Christoph Hellwig,
	Sebastian Andrzej Siewior

The completion callback for the sdhci-pci device is invoked from a
kworker.
I couldn't identify in which context is mmc_blk_mq_req_done() invoke but
the remaining caller are from invoked from preemptible context. Here it
would make sense to complete the request directly instead scheduling
ksoftirqd for its completion.

Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
---
 drivers/mmc/core/block.c | 22 ++++++++++++++--------
 1 file changed, 14 insertions(+), 8 deletions(-)

diff --git a/drivers/mmc/core/block.c b/drivers/mmc/core/block.c
index 431af5e8be2f8..7d6b43fe52e8a 100644
--- a/drivers/mmc/core/block.c
+++ b/drivers/mmc/core/block.c
@@ -2051,7 +2051,8 @@ static void mmc_blk_mq_dec_in_flight(struct mmc_queue *mq, struct request *req)
 		mmc_put_card(mq->card, &mq->ctx);
 }
 
-static void mmc_blk_mq_post_req(struct mmc_queue *mq, struct request *req)
+static void mmc_blk_mq_post_req(struct mmc_queue *mq, struct request *req,
+				bool can_sleep)
 {
 	struct mmc_queue_req *mqrq = req_to_mmc_queue_req(req);
 	struct mmc_request *mrq = &mqrq->brq.mrq;
@@ -2063,10 +2064,14 @@ static void mmc_blk_mq_post_req(struct mmc_queue *mq, struct request *req)
 	 * Block layer timeouts race with completions which means the normal
 	 * completion path cannot be used during recovery.
 	 */
-	if (mq->in_recovery)
+	if (mq->in_recovery) {
 		mmc_blk_mq_complete_rq(mq, req);
-	else if (likely(!blk_should_fake_timeout(req->q)))
-		blk_mq_complete_request(req);
+	} else if (likely(!blk_should_fake_timeout(req->q))) {
+		if (can_sleep)
+			blk_mq_complete_request_direct(req, mmc_blk_mq_complete);
+		else
+			blk_mq_complete_request(req);
+	}
 
 	mmc_blk_mq_dec_in_flight(mq, req);
 }
@@ -2087,7 +2092,7 @@ void mmc_blk_mq_recovery(struct mmc_queue *mq)
 
 	mmc_blk_urgent_bkops(mq, mqrq);
 
-	mmc_blk_mq_post_req(mq, req);
+	mmc_blk_mq_post_req(mq, req, true);
 }
 
 static void mmc_blk_mq_complete_prev_req(struct mmc_queue *mq,
@@ -2106,7 +2111,7 @@ static void mmc_blk_mq_complete_prev_req(struct mmc_queue *mq,
 	if (prev_req)
 		*prev_req = mq->complete_req;
 	else
-		mmc_blk_mq_post_req(mq, mq->complete_req);
+		mmc_blk_mq_post_req(mq, mq->complete_req, true);
 
 	mq->complete_req = NULL;
 
@@ -2178,7 +2183,8 @@ static void mmc_blk_mq_req_done(struct mmc_request *mrq)
 	mq->rw_wait = false;
 	wake_up(&mq->wait);
 
-	mmc_blk_mq_post_req(mq, req);
+	/* context unknown */
+	mmc_blk_mq_post_req(mq, req, false);
 }
 
 static bool mmc_blk_rw_wait_cond(struct mmc_queue *mq, int *err)
@@ -2238,7 +2244,7 @@ static int mmc_blk_mq_issue_rw_rq(struct mmc_queue *mq,
 	err = mmc_start_request(host, &mqrq->brq.mrq);
 
 	if (prev_req)
-		mmc_blk_mq_post_req(mq, prev_req);
+		mmc_blk_mq_post_req(mq, prev_req, true);
 
 	if (err)
 		mq->rw_wait = false;
-- 
2.33.0


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

* Re: [PATCH v2 1/2] blk-mq: Add blk_mq_complete_request_direct()
  2021-10-18 13:55 ` [PATCH v2 1/2] blk-mq: Add blk_mq_complete_request_direct() Sebastian Andrzej Siewior
@ 2021-10-18 15:20   ` Christoph Hellwig
  2021-10-25  6:45     ` Sebastian Andrzej Siewior
  0 siblings, 1 reply; 9+ messages in thread
From: Christoph Hellwig @ 2021-10-18 15:20 UTC (permalink / raw)
  To: Sebastian Andrzej Siewior
  Cc: linux-block, linux-mmc, tglx, Ulf Hansson, Jens Axboe, Christoph Hellwig

On Mon, Oct 18, 2021 at 03:55:58PM +0200, Sebastian Andrzej Siewior wrote:
> +static inline void blk_mq_complete_request_direct(struct request *rq,
> +						  void (*complete)(struct request *rq))

Pleae avoid the overly long line.

ote that by doing the normal two tab indent for the continuation that
would be super trivial and way more readable.

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

* Re: [PATCH v2 2/2] mmc: core: Use blk_mq_complete_request_direct().
  2021-10-18 13:55 ` [PATCH v2 2/2] mmc: core: Use blk_mq_complete_request_direct() Sebastian Andrzej Siewior
@ 2021-10-19 11:32   ` Ulf Hansson
  2021-10-20  6:39     ` Adrian Hunter
  2021-10-25  6:44     ` Sebastian Andrzej Siewior
  2021-10-21 19:45   ` Ulf Hansson
  1 sibling, 2 replies; 9+ messages in thread
From: Ulf Hansson @ 2021-10-19 11:32 UTC (permalink / raw)
  To: Sebastian Andrzej Siewior, Adrian Hunter
  Cc: linux-block, linux-mmc, Thomas Gleixner, Jens Axboe, Christoph Hellwig

+ Adrian

On Mon, 18 Oct 2021 at 15:56, Sebastian Andrzej Siewior
<bigeasy@linutronix.de> wrote:
>
> The completion callback for the sdhci-pci device is invoked from a
> kworker.
> I couldn't identify in which context is mmc_blk_mq_req_done() invoke but
> the remaining caller are from invoked from preemptible context. Here it
> would make sense to complete the request directly instead scheduling
> ksoftirqd for its completion.
>
> Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>

Thanks for working on this!

I have looped in Adrian, to allow him to provide us with his input too.

> ---
>  drivers/mmc/core/block.c | 22 ++++++++++++++--------
>  1 file changed, 14 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/mmc/core/block.c b/drivers/mmc/core/block.c
> index 431af5e8be2f8..7d6b43fe52e8a 100644
> --- a/drivers/mmc/core/block.c
> +++ b/drivers/mmc/core/block.c
> @@ -2051,7 +2051,8 @@ static void mmc_blk_mq_dec_in_flight(struct mmc_queue *mq, struct request *req)
>                 mmc_put_card(mq->card, &mq->ctx);
>  }
>
> -static void mmc_blk_mq_post_req(struct mmc_queue *mq, struct request *req)
> +static void mmc_blk_mq_post_req(struct mmc_queue *mq, struct request *req,
> +                               bool can_sleep)
>  {
>         struct mmc_queue_req *mqrq = req_to_mmc_queue_req(req);
>         struct mmc_request *mrq = &mqrq->brq.mrq;
> @@ -2063,10 +2064,14 @@ static void mmc_blk_mq_post_req(struct mmc_queue *mq, struct request *req)
>          * Block layer timeouts race with completions which means the normal
>          * completion path cannot be used during recovery.
>          */
> -       if (mq->in_recovery)
> +       if (mq->in_recovery) {
>                 mmc_blk_mq_complete_rq(mq, req);
> -       else if (likely(!blk_should_fake_timeout(req->q)))
> -               blk_mq_complete_request(req);
> +       } else if (likely(!blk_should_fake_timeout(req->q))) {
> +               if (can_sleep)
> +                       blk_mq_complete_request_direct(req, mmc_blk_mq_complete);
> +               else
> +                       blk_mq_complete_request(req);
> +       }
>
>         mmc_blk_mq_dec_in_flight(mq, req);
>  }
> @@ -2087,7 +2092,7 @@ void mmc_blk_mq_recovery(struct mmc_queue *mq)
>
>         mmc_blk_urgent_bkops(mq, mqrq);
>
> -       mmc_blk_mq_post_req(mq, req);
> +       mmc_blk_mq_post_req(mq, req, true);
>  }
>
>  static void mmc_blk_mq_complete_prev_req(struct mmc_queue *mq,
> @@ -2106,7 +2111,7 @@ static void mmc_blk_mq_complete_prev_req(struct mmc_queue *mq,
>         if (prev_req)
>                 *prev_req = mq->complete_req;
>         else
> -               mmc_blk_mq_post_req(mq, mq->complete_req);
> +               mmc_blk_mq_post_req(mq, mq->complete_req, true);
>
>         mq->complete_req = NULL;
>
> @@ -2178,7 +2183,8 @@ static void mmc_blk_mq_req_done(struct mmc_request *mrq)
>         mq->rw_wait = false;
>         wake_up(&mq->wait);
>
> -       mmc_blk_mq_post_req(mq, req);
> +       /* context unknown */
> +       mmc_blk_mq_post_req(mq, req, false);

So it seems we would benefit from knowing the context here, right?

At this point, what you suggest seems like a reasonable way forward
(assuming atomic context), but in a next step we could potentially add
a non-atomic helper function for mmc host drivers to call, when that
is suitable. Would that make sense you think?

>  }
>
>  static bool mmc_blk_rw_wait_cond(struct mmc_queue *mq, int *err)
> @@ -2238,7 +2244,7 @@ static int mmc_blk_mq_issue_rw_rq(struct mmc_queue *mq,
>         err = mmc_start_request(host, &mqrq->brq.mrq);
>
>         if (prev_req)
> -               mmc_blk_mq_post_req(mq, prev_req);
> +               mmc_blk_mq_post_req(mq, prev_req, true);
>
>         if (err)
>                 mq->rw_wait = false;

Kind regards
Uffe

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

* Re: [PATCH v2 2/2] mmc: core: Use blk_mq_complete_request_direct().
  2021-10-19 11:32   ` Ulf Hansson
@ 2021-10-20  6:39     ` Adrian Hunter
  2021-10-25  6:44     ` Sebastian Andrzej Siewior
  1 sibling, 0 replies; 9+ messages in thread
From: Adrian Hunter @ 2021-10-20  6:39 UTC (permalink / raw)
  To: Ulf Hansson, Sebastian Andrzej Siewior
  Cc: linux-block, linux-mmc, Thomas Gleixner, Jens Axboe, Christoph Hellwig

On 19/10/2021 14:32, Ulf Hansson wrote:
> + Adrian
> 
> On Mon, 18 Oct 2021 at 15:56, Sebastian Andrzej Siewior
> <bigeasy@linutronix.de> wrote:
>>
>> The completion callback for the sdhci-pci device is invoked from a
>> kworker.
>> I couldn't identify in which context is mmc_blk_mq_req_done() invoke but
>> the remaining caller are from invoked from preemptible context. Here it
>> would make sense to complete the request directly instead scheduling
>> ksoftirqd for its completion.
>>
>> Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
> 
> Thanks for working on this!
> 
> I have looped in Adrian, to allow him to provide us with his input too.

Thanks!

Looks good to me.

Acked-by: Adrian Hunter <adrian.hunter@intel.com>


> 
>> ---
>>  drivers/mmc/core/block.c | 22 ++++++++++++++--------
>>  1 file changed, 14 insertions(+), 8 deletions(-)
>>
>> diff --git a/drivers/mmc/core/block.c b/drivers/mmc/core/block.c
>> index 431af5e8be2f8..7d6b43fe52e8a 100644
>> --- a/drivers/mmc/core/block.c
>> +++ b/drivers/mmc/core/block.c
>> @@ -2051,7 +2051,8 @@ static void mmc_blk_mq_dec_in_flight(struct mmc_queue *mq, struct request *req)
>>                 mmc_put_card(mq->card, &mq->ctx);
>>  }
>>
>> -static void mmc_blk_mq_post_req(struct mmc_queue *mq, struct request *req)
>> +static void mmc_blk_mq_post_req(struct mmc_queue *mq, struct request *req,
>> +                               bool can_sleep)
>>  {
>>         struct mmc_queue_req *mqrq = req_to_mmc_queue_req(req);
>>         struct mmc_request *mrq = &mqrq->brq.mrq;
>> @@ -2063,10 +2064,14 @@ static void mmc_blk_mq_post_req(struct mmc_queue *mq, struct request *req)
>>          * Block layer timeouts race with completions which means the normal
>>          * completion path cannot be used during recovery.
>>          */
>> -       if (mq->in_recovery)
>> +       if (mq->in_recovery) {
>>                 mmc_blk_mq_complete_rq(mq, req);
>> -       else if (likely(!blk_should_fake_timeout(req->q)))
>> -               blk_mq_complete_request(req);
>> +       } else if (likely(!blk_should_fake_timeout(req->q))) {
>> +               if (can_sleep)
>> +                       blk_mq_complete_request_direct(req, mmc_blk_mq_complete);
>> +               else
>> +                       blk_mq_complete_request(req);
>> +       }
>>
>>         mmc_blk_mq_dec_in_flight(mq, req);
>>  }
>> @@ -2087,7 +2092,7 @@ void mmc_blk_mq_recovery(struct mmc_queue *mq)
>>
>>         mmc_blk_urgent_bkops(mq, mqrq);
>>
>> -       mmc_blk_mq_post_req(mq, req);
>> +       mmc_blk_mq_post_req(mq, req, true);
>>  }
>>
>>  static void mmc_blk_mq_complete_prev_req(struct mmc_queue *mq,
>> @@ -2106,7 +2111,7 @@ static void mmc_blk_mq_complete_prev_req(struct mmc_queue *mq,
>>         if (prev_req)
>>                 *prev_req = mq->complete_req;
>>         else
>> -               mmc_blk_mq_post_req(mq, mq->complete_req);
>> +               mmc_blk_mq_post_req(mq, mq->complete_req, true);
>>
>>         mq->complete_req = NULL;
>>
>> @@ -2178,7 +2183,8 @@ static void mmc_blk_mq_req_done(struct mmc_request *mrq)
>>         mq->rw_wait = false;
>>         wake_up(&mq->wait);
>>
>> -       mmc_blk_mq_post_req(mq, req);
>> +       /* context unknown */
>> +       mmc_blk_mq_post_req(mq, req, false);
> 
> So it seems we would benefit from knowing the context here, right?
> 
> At this point, what you suggest seems like a reasonable way forward
> (assuming atomic context), but in a next step we could potentially add
> a non-atomic helper function for mmc host drivers to call, when that
> is suitable. Would that make sense you think?
> 
>>  }
>>
>>  static bool mmc_blk_rw_wait_cond(struct mmc_queue *mq, int *err)
>> @@ -2238,7 +2244,7 @@ static int mmc_blk_mq_issue_rw_rq(struct mmc_queue *mq,
>>         err = mmc_start_request(host, &mqrq->brq.mrq);
>>
>>         if (prev_req)
>> -               mmc_blk_mq_post_req(mq, prev_req);
>> +               mmc_blk_mq_post_req(mq, prev_req, true);
>>
>>         if (err)
>>                 mq->rw_wait = false;
> 
> Kind regards
> Uffe
> 


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

* Re: [PATCH v2 2/2] mmc: core: Use blk_mq_complete_request_direct().
  2021-10-18 13:55 ` [PATCH v2 2/2] mmc: core: Use blk_mq_complete_request_direct() Sebastian Andrzej Siewior
  2021-10-19 11:32   ` Ulf Hansson
@ 2021-10-21 19:45   ` Ulf Hansson
  1 sibling, 0 replies; 9+ messages in thread
From: Ulf Hansson @ 2021-10-21 19:45 UTC (permalink / raw)
  To: Sebastian Andrzej Siewior, Jens Axboe
  Cc: linux-block, linux-mmc, Thomas Gleixner, Christoph Hellwig

On Mon, 18 Oct 2021 at 15:56, Sebastian Andrzej Siewior
<bigeasy@linutronix.de> wrote:
>
> The completion callback for the sdhci-pci device is invoked from a
> kworker.
> I couldn't identify in which context is mmc_blk_mq_req_done() invoke but
> the remaining caller are from invoked from preemptible context. Here it
> would make sense to complete the request directly instead scheduling
> ksoftirqd for its completion.
>
> Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>

Reviewed-by: Ulf Hansson <ulf.hansson@linaro.org>

Jens, will you funnel this via your tree?

Kind regards
Uffe

> ---
>  drivers/mmc/core/block.c | 22 ++++++++++++++--------
>  1 file changed, 14 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/mmc/core/block.c b/drivers/mmc/core/block.c
> index 431af5e8be2f8..7d6b43fe52e8a 100644
> --- a/drivers/mmc/core/block.c
> +++ b/drivers/mmc/core/block.c
> @@ -2051,7 +2051,8 @@ static void mmc_blk_mq_dec_in_flight(struct mmc_queue *mq, struct request *req)
>                 mmc_put_card(mq->card, &mq->ctx);
>  }
>
> -static void mmc_blk_mq_post_req(struct mmc_queue *mq, struct request *req)
> +static void mmc_blk_mq_post_req(struct mmc_queue *mq, struct request *req,
> +                               bool can_sleep)
>  {
>         struct mmc_queue_req *mqrq = req_to_mmc_queue_req(req);
>         struct mmc_request *mrq = &mqrq->brq.mrq;
> @@ -2063,10 +2064,14 @@ static void mmc_blk_mq_post_req(struct mmc_queue *mq, struct request *req)
>          * Block layer timeouts race with completions which means the normal
>          * completion path cannot be used during recovery.
>          */
> -       if (mq->in_recovery)
> +       if (mq->in_recovery) {
>                 mmc_blk_mq_complete_rq(mq, req);
> -       else if (likely(!blk_should_fake_timeout(req->q)))
> -               blk_mq_complete_request(req);
> +       } else if (likely(!blk_should_fake_timeout(req->q))) {
> +               if (can_sleep)
> +                       blk_mq_complete_request_direct(req, mmc_blk_mq_complete);
> +               else
> +                       blk_mq_complete_request(req);
> +       }
>
>         mmc_blk_mq_dec_in_flight(mq, req);
>  }
> @@ -2087,7 +2092,7 @@ void mmc_blk_mq_recovery(struct mmc_queue *mq)
>
>         mmc_blk_urgent_bkops(mq, mqrq);
>
> -       mmc_blk_mq_post_req(mq, req);
> +       mmc_blk_mq_post_req(mq, req, true);
>  }
>
>  static void mmc_blk_mq_complete_prev_req(struct mmc_queue *mq,
> @@ -2106,7 +2111,7 @@ static void mmc_blk_mq_complete_prev_req(struct mmc_queue *mq,
>         if (prev_req)
>                 *prev_req = mq->complete_req;
>         else
> -               mmc_blk_mq_post_req(mq, mq->complete_req);
> +               mmc_blk_mq_post_req(mq, mq->complete_req, true);
>
>         mq->complete_req = NULL;
>
> @@ -2178,7 +2183,8 @@ static void mmc_blk_mq_req_done(struct mmc_request *mrq)
>         mq->rw_wait = false;
>         wake_up(&mq->wait);
>
> -       mmc_blk_mq_post_req(mq, req);
> +       /* context unknown */
> +       mmc_blk_mq_post_req(mq, req, false);
>  }
>
>  static bool mmc_blk_rw_wait_cond(struct mmc_queue *mq, int *err)
> @@ -2238,7 +2244,7 @@ static int mmc_blk_mq_issue_rw_rq(struct mmc_queue *mq,
>         err = mmc_start_request(host, &mqrq->brq.mrq);
>
>         if (prev_req)
> -               mmc_blk_mq_post_req(mq, prev_req);
> +               mmc_blk_mq_post_req(mq, prev_req, true);
>
>         if (err)
>                 mq->rw_wait = false;
> --
> 2.33.0
>

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

* Re: [PATCH v2 2/2] mmc: core: Use blk_mq_complete_request_direct().
  2021-10-19 11:32   ` Ulf Hansson
  2021-10-20  6:39     ` Adrian Hunter
@ 2021-10-25  6:44     ` Sebastian Andrzej Siewior
  1 sibling, 0 replies; 9+ messages in thread
From: Sebastian Andrzej Siewior @ 2021-10-25  6:44 UTC (permalink / raw)
  To: Ulf Hansson
  Cc: Adrian Hunter, linux-block, linux-mmc, Thomas Gleixner,
	Jens Axboe, Christoph Hellwig

On 2021-10-19 13:32:42 [+0200], Ulf Hansson wrote:
> > diff --git a/drivers/mmc/core/block.c b/drivers/mmc/core/block.c
> > index 431af5e8be2f8..7d6b43fe52e8a 100644
> > --- a/drivers/mmc/core/block.c
> > +++ b/drivers/mmc/core/block.c
> > @@ -2178,7 +2183,8 @@ static void mmc_blk_mq_req_done(struct mmc_request *mrq)
> >         mq->rw_wait = false;
> >         wake_up(&mq->wait);
> >
> > -       mmc_blk_mq_post_req(mq, req);
> > +       /* context unknown */
> > +       mmc_blk_mq_post_req(mq, req, false);
> 
> So it seems we would benefit from knowing the context here, right?

Yes.

> At this point, what you suggest seems like a reasonable way forward
> (assuming atomic context), but in a next step we could potentially add
> a non-atomic helper function for mmc host drivers to call, when that
> is suitable. Would that make sense you think?

Sure. I didn't mange to figure where this can be called from so I
assumed atomic so it behaves the way it did. If you can provide
additional information here then the additional scheduling of ksoftirqd
could be avoided. Also, if there are drivers which complete their
requests in a threaded-irq handler, then the softirq could be also
avoided (there should be no irq-disabling then).

I *think* there were other completion paths, I just touched the one I
managed to reproduce.

> 
> Kind regards
> Uffe

Sebastian

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

* Re: [PATCH v2 1/2] blk-mq: Add blk_mq_complete_request_direct()
  2021-10-18 15:20   ` Christoph Hellwig
@ 2021-10-25  6:45     ` Sebastian Andrzej Siewior
  0 siblings, 0 replies; 9+ messages in thread
From: Sebastian Andrzej Siewior @ 2021-10-25  6:45 UTC (permalink / raw)
  To: Christoph Hellwig; +Cc: linux-block, linux-mmc, tglx, Ulf Hansson, Jens Axboe

On 2021-10-18 08:20:50 [-0700], Christoph Hellwig wrote:
> On Mon, Oct 18, 2021 at 03:55:58PM +0200, Sebastian Andrzej Siewior wrote:
> > +static inline void blk_mq_complete_request_direct(struct request *rq,
> > +						  void (*complete)(struct request *rq))
> 
> Pleae avoid the overly long line.
> 
> ote that by doing the normal two tab indent for the continuation that
> would be super trivial and way more readable.

Oki.

Sebastian

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

end of thread, other threads:[~2021-10-25  6:45 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-10-18 13:55 [PATCH v2 0/2] blk-mq: Allow to complete requests directly Sebastian Andrzej Siewior
2021-10-18 13:55 ` [PATCH v2 1/2] blk-mq: Add blk_mq_complete_request_direct() Sebastian Andrzej Siewior
2021-10-18 15:20   ` Christoph Hellwig
2021-10-25  6:45     ` Sebastian Andrzej Siewior
2021-10-18 13:55 ` [PATCH v2 2/2] mmc: core: Use blk_mq_complete_request_direct() Sebastian Andrzej Siewior
2021-10-19 11:32   ` Ulf Hansson
2021-10-20  6:39     ` Adrian Hunter
2021-10-25  6:44     ` Sebastian Andrzej Siewior
2021-10-21 19:45   ` Ulf Hansson

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).