Linux-rt-users Archive on lore.kernel.org
 help / color / Atom feed
From: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
To: Jens Axboe <axboe@kernel.dk>
Cc: Sagi Grimberg <sagi@grimberg.me>,
	Christoph Hellwig <hch@infradead.org>,
	linux-block@vger.kernel.org, Thomas Gleixner <tglx@linutronix.de>,
	David Runge <dave@sleepmap.de>,
	linux-rt-users@vger.kernel.org, linux-kernel@vger.kernel.org,
	Peter Zijlstra <peterz@infradead.org>,
	Daniel Wagner <dwagner@suse.de>, Mike Galbraith <efault@gmx.de>
Subject: Re: [PATCH 3/3] blk-mq: Use llist_head for blk_cpu_done
Date: Mon, 2 Nov 2020 10:55:33 +0100
Message-ID: <20201102095533.fxc2xpauzsoju7cm@linutronix.de> (raw)
In-Reply-To: <3bbfb5e1-c5d7-8f3b-4b96-6dc02be0550d@kernel.dk>

On 2020-10-31 09:00:49 [-0600], Jens Axboe wrote:
> There really aren't any rules for this, and it's perfectly legit to
> complete from process context. Maybe you're a kthread driven driver and
> that's how you handle completions. The block completion path has always
> been hard IRQ safe, but possible to call from anywhere.

I'm not trying to put restrictions and forbidding completions from a
kthread. I'm trying to avoid the pointless softirq dance for no added
value. We could:

diff --git a/block/blk-mq.c b/block/blk-mq.c
index 4f53de48e5038..c4693b3750878 100644
--- a/block/blk-mq.c
+++ b/block/blk-mq.c
@@ -644,9 +644,11 @@ bool blk_mq_complete_request_remote(struct request *rq)
 	} else {
 		if (rq->q->nr_hw_queues > 1)
 			return false;
+		preempt_disable();
 		cpu_list = this_cpu_ptr(&blk_cpu_done);
 		if (llist_add(&rq->ipi_list, cpu_list))
 			raise_softirq(BLOCK_SOFTIRQ);
+		preempt_enable();
 	}
 
 	return true;

to not break that assumption you just mentioned and provide 
|static inline void blk_mq_complete_request_local(struct request *rq)
|{
|                 rq->q->mq_ops->complete(rq);
|}

so that completion issued from from process context (like those from
usb-storage) don't end up waking `ksoftird' (running at SCHED_OTHER)
completing the requests but rather performing it right away. The softirq
dance makes no sense here.

As mentioned earlier, the alternative _could_ be to
	s/preempt_/local_bh_/

in the above patch. This would ensure that any invocation outside of
IRQ/Softirq context would invoke the softirq _directly_ at
local_bh_enable() time rather than waking the daemon for that purpose.
It would also avoid another completion function for the direct case
which could be abused if used from outside the thread context.
The last one is currently my favorite.

Sebastian

  parent reply index

Thread overview: 34+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-10-21 17:50 5.9.1-rt18: issues with Firewire card on AMD hardware David Runge
2020-10-23 11:04 ` [PATCH RFC] blk-mq: Don't IPI requests on PREEMPT_RT Sebastian Andrzej Siewior
2020-10-23 11:21   ` Christoph Hellwig
2020-10-23 13:52     ` Sebastian Andrzej Siewior
2020-10-27  9:26       ` Christoph Hellwig
2020-10-27 10:11         ` Sebastian Andrzej Siewior
2020-10-27 16:07           ` Christoph Hellwig
2020-10-27 17:05             ` Thomas Gleixner
2020-10-27 17:23               ` Christoph Hellwig
2020-10-27 17:59                 ` Sebastian Andrzej Siewior
2020-10-27 20:58                 ` Sebastian Andrzej Siewior
2020-10-28  6:56                   ` Christoph Hellwig
2020-10-28 14:12                     ` [PATCH 1/3] blk-mq: Don't complete on a remote CPU in force threaded mode Sebastian Andrzej Siewior
2020-10-28 14:12                       ` [PATCH 2/3] blk-mq: Always complete remote completions requests in softirq Sebastian Andrzej Siewior
2020-10-28 14:12                       ` [PATCH 3/3] blk-mq: Use llist_head for blk_cpu_done Sebastian Andrzej Siewior
2020-10-28 14:44                         ` Christoph Hellwig
2020-10-28 14:47                           ` Sebastian Andrzej Siewior
2020-10-29 13:12                         ` Sebastian Andrzej Siewior
2020-10-29 14:05                           ` Christoph Hellwig
2020-10-29 14:56                             ` Sebastian Andrzej Siewior
2020-10-29 14:57                               ` Christoph Hellwig
2020-10-29 20:03                                 ` Sagi Grimberg
2020-10-29 21:01                                   ` Sebastian Andrzej Siewior
2020-10-29 21:07                                     ` Sagi Grimberg
2020-10-31 10:41                                       ` Sebastian Andrzej Siewior
2020-10-31 15:00                                         ` Jens Axboe
2020-10-31 15:01                                           ` Jens Axboe
2020-10-31 18:09                                             ` Christoph Hellwig
2020-11-02  9:55                                           ` Sebastian Andrzej Siewior [this message]
2020-11-02 18:12                                             ` Christoph Hellwig
2020-11-04 19:15                                               ` Sagi Grimberg
2020-11-06 15:23                                               ` Sebastian Andrzej Siewior
2020-10-28 10:04                 ` [PATCH RFC] blk-mq: Don't IPI requests on PREEMPT_RT Peter Zijlstra
2020-10-26  0:37 ` 5.9.1-rt18: issues with Firewire card on AMD hardware David Runge

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20201102095533.fxc2xpauzsoju7cm@linutronix.de \
    --to=bigeasy@linutronix.de \
    --cc=axboe@kernel.dk \
    --cc=dave@sleepmap.de \
    --cc=dwagner@suse.de \
    --cc=efault@gmx.de \
    --cc=hch@infradead.org \
    --cc=linux-block@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-rt-users@vger.kernel.org \
    --cc=peterz@infradead.org \
    --cc=sagi@grimberg.me \
    --cc=tglx@linutronix.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

Linux-rt-users Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-rt-users/0 linux-rt-users/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-rt-users linux-rt-users/ https://lore.kernel.org/linux-rt-users \
		linux-rt-users@vger.kernel.org
	public-inbox-index linux-rt-users

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-rt-users


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git