linux-block.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] block: remove redundant empty check of mq_list
@ 2020-09-09  6:48 Xianting Tian
  2020-09-09 14:21 ` Jens Axboe
  0 siblings, 1 reply; 3+ messages in thread
From: Xianting Tian @ 2020-09-09  6:48 UTC (permalink / raw)
  To: axboe, ast, daniel, kafai, songliubraving, yhs, andriin,
	john.fastabend, kpsingh
  Cc: linux-block, linux-kernel, netdev, bpf, Xianting Tian

blk_mq_flush_plug_list() itself will do the empty check of mq_list,
so remove such check in blk_flush_plug_list().
Actually normally mq_list is not empty when blk_flush_plug_list is
called.

Signed-off-by: Xianting Tian <tian.xianting@h3c.com>
---
 block/blk-core.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/block/blk-core.c b/block/blk-core.c
index 10c08ac50..dda301610 100644
--- a/block/blk-core.c
+++ b/block/blk-core.c
@@ -1864,8 +1864,7 @@ void blk_flush_plug_list(struct blk_plug *plug, bool from_schedule)
 {
 	flush_plug_callbacks(plug, from_schedule);
 
-	if (!list_empty(&plug->mq_list))
-		blk_mq_flush_plug_list(plug, from_schedule);
+	blk_mq_flush_plug_list(plug, from_schedule);
 }
 
 /**
-- 
2.17.1


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

* Re: [PATCH] block: remove redundant empty check of mq_list
  2020-09-09  6:48 [PATCH] block: remove redundant empty check of mq_list Xianting Tian
@ 2020-09-09 14:21 ` Jens Axboe
  2020-09-10  9:44   ` Tianxianting
  0 siblings, 1 reply; 3+ messages in thread
From: Jens Axboe @ 2020-09-09 14:21 UTC (permalink / raw)
  To: Xianting Tian, ast, daniel, kafai, songliubraving, yhs, andriin,
	john.fastabend, kpsingh
  Cc: linux-block, linux-kernel, netdev, bpf

On 9/9/20 12:48 AM, Xianting Tian wrote:
> blk_mq_flush_plug_list() itself will do the empty check of mq_list,
> so remove such check in blk_flush_plug_list().
> Actually normally mq_list is not empty when blk_flush_plug_list is
> called.

It's cheaper to do in the caller, instead of doing the function call
and then aborting if it's empty. So I'd suggest just leaving it alone.
Right now this is the only caller, but it's nicer to assume we can
be called in any state vs not having the check.

-- 
Jens Axboe


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

* RE: [PATCH] block: remove redundant empty check of mq_list
  2020-09-09 14:21 ` Jens Axboe
@ 2020-09-10  9:44   ` Tianxianting
  0 siblings, 0 replies; 3+ messages in thread
From: Tianxianting @ 2020-09-10  9:44 UTC (permalink / raw)
  To: Jens Axboe, ast, daniel, kafai, songliubraving, yhs, andriin,
	john.fastabend, kpsingh
  Cc: linux-block, linux-kernel, netdev, bpf

Hi Jens,
Thanks for your feedback,
Yes, blk_flush_plug_list() is only caller of blk_mq_flush_plug_list().
So I checked the callers of blk_flush_plug_list(), found below code path will call blk_flush_plug_list():
	io_schedule_prepare/sched_submit_work->blk_schedule_flush_plug
	writeback_sb_inodes->blk_flush_plug
	blk_finish_plug
	dm_submit_bio/__submit_bio_noacct_mq/__submit_bio->blk_mq_submit_bio
	blk_poll

So I think there are still many chances to do the redundant judge?

-----Original Message-----
From: Jens Axboe [mailto:axboe@kernel.dk] 
Sent: Wednesday, September 09, 2020 10:21 PM
To: tianxianting (RD) <tian.xianting@h3c.com>; ast@kernel.org; daniel@iogearbox.net; kafai@fb.com; songliubraving@fb.com; yhs@fb.com; andriin@fb.com; john.fastabend@gmail.com; kpsingh@chromium.org
Cc: linux-block@vger.kernel.org; linux-kernel@vger.kernel.org; netdev@vger.kernel.org; bpf@vger.kernel.org
Subject: Re: [PATCH] block: remove redundant empty check of mq_list

On 9/9/20 12:48 AM, Xianting Tian wrote:
> blk_mq_flush_plug_list() itself will do the empty check of mq_list, so 
> remove such check in blk_flush_plug_list().
> Actually normally mq_list is not empty when blk_flush_plug_list is 
> called.

It's cheaper to do in the caller, instead of doing the function call and then aborting if it's empty. So I'd suggest just leaving it alone.
Right now this is the only caller, but it's nicer to assume we can be called in any state vs not having the check.

--
Jens Axboe


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

end of thread, other threads:[~2020-09-10  9:45 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-09-09  6:48 [PATCH] block: remove redundant empty check of mq_list Xianting Tian
2020-09-09 14:21 ` Jens Axboe
2020-09-10  9:44   ` Tianxianting

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