RE: [PATCH] block: remove redundant empty check of mq_list

From: Tianxianting
Date: Thu Sep 10 2020 - 05:45:46 EST


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@xxxxxxxxx]
Sent: Wednesday, September 09, 2020 10:21 PM
To: tianxianting (RD) <tian.xianting@xxxxxxx>; ast@xxxxxxxxxx; daniel@xxxxxxxxxxxxx; kafai@xxxxxx; songliubraving@xxxxxx; yhs@xxxxxx; andriin@xxxxxx; john.fastabend@xxxxxxxxx; kpsingh@xxxxxxxxxxxx
Cc: linux-block@xxxxxxxxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx; netdev@xxxxxxxxxxxxxxx; bpf@xxxxxxxxxxxxxxx
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