* cleanup and optimize block plug handling
@ 2021-10-20 14:41 Christoph Hellwig
2021-10-20 14:41 ` [PATCH 1/4] blk-mq: only flush requests from the plug in blk_mq_submit_bio Christoph Hellwig
` (4 more replies)
0 siblings, 5 replies; 8+ messages in thread
From: Christoph Hellwig @ 2021-10-20 14:41 UTC (permalink / raw)
To: Jens Axboe; +Cc: Pavel Begunkov, linux-block
Hi Jens,
this is a spinoff from Pavel's misc optimizations. It shuld take care
of his two painpoints and also cleans up the interface for flushing
plugs a bit.
Diffstat:
block/blk-core.c | 17 ++++++++---------
block/blk-mq.c | 2 +-
block/blk-mq.h | 1 +
fs/fs-writeback.c | 5 +++--
include/linux/blk-mq.h | 2 --
include/linux/blkdev.h | 29 ++++-------------------------
kernel/sched/core.c | 5 +++--
7 files changed, 20 insertions(+), 41 deletions(-)
^ permalink raw reply [flat|nested] 8+ messages in thread* [PATCH 1/4] blk-mq: only flush requests from the plug in blk_mq_submit_bio 2021-10-20 14:41 cleanup and optimize block plug handling Christoph Hellwig @ 2021-10-20 14:41 ` Christoph Hellwig 2021-10-20 14:41 ` [PATCH 2/4] blk-mq: move blk_mq_flush_plug_list to block/blk-mq.h Christoph Hellwig ` (3 subsequent siblings) 4 siblings, 0 replies; 8+ messages in thread From: Christoph Hellwig @ 2021-10-20 14:41 UTC (permalink / raw) To: Jens Axboe; +Cc: Pavel Begunkov, linux-block Replace the call to blk_flush_plug_list in blk_mq_submit_bio with a direct call to blk_mq_flush_plug_list. This means we do not flush plug callback from stackable devices, which doesn't really help with the accumulated requests anyway, and it also means the cached requests aren't freed here as they can still be used later on. Signed-off-by: Christoph Hellwig <hch@lst.de> --- block/blk-mq.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/block/blk-mq.c b/block/blk-mq.c index 31d9e612d2360..9bb6ece419a5b 100644 --- a/block/blk-mq.c +++ b/block/blk-mq.c @@ -2571,7 +2571,7 @@ void blk_mq_submit_bio(struct bio *bio) } if (request_count >= blk_plug_max_rq_count(plug) || last) { - blk_flush_plug_list(plug, false); + blk_mq_flush_plug_list(plug, false); trace_block_plug(q); } -- 2.30.2 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH 2/4] blk-mq: move blk_mq_flush_plug_list to block/blk-mq.h 2021-10-20 14:41 cleanup and optimize block plug handling Christoph Hellwig 2021-10-20 14:41 ` [PATCH 1/4] blk-mq: only flush requests from the plug in blk_mq_submit_bio Christoph Hellwig @ 2021-10-20 14:41 ` Christoph Hellwig 2021-10-20 14:41 ` [PATCH 3/4] block: optimise blk_flush_plug_list Christoph Hellwig ` (2 subsequent siblings) 4 siblings, 0 replies; 8+ messages in thread From: Christoph Hellwig @ 2021-10-20 14:41 UTC (permalink / raw) To: Jens Axboe; +Cc: Pavel Begunkov, linux-block This helper is internal to the block layer. Signed-off-by: Christoph Hellwig <hch@lst.de> --- block/blk-mq.h | 1 + include/linux/blk-mq.h | 2 -- 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/block/blk-mq.h b/block/blk-mq.h index d8ccb341e82e9..08fb5922e611b 100644 --- a/block/blk-mq.h +++ b/block/blk-mq.h @@ -122,6 +122,7 @@ extern int blk_mq_sysfs_register(struct request_queue *q); extern void blk_mq_sysfs_unregister(struct request_queue *q); extern void blk_mq_hctx_kobj_init(struct blk_mq_hw_ctx *hctx); void blk_mq_free_plug_rqs(struct blk_plug *plug); +void blk_mq_flush_plug_list(struct blk_plug *plug, bool from_schedule); void blk_mq_release(struct request_queue *q); diff --git a/include/linux/blk-mq.h b/include/linux/blk-mq.h index 6cf35de151a95..e137802365509 100644 --- a/include/linux/blk-mq.h +++ b/include/linux/blk-mq.h @@ -656,8 +656,6 @@ int blk_mq_alloc_sq_tag_set(struct blk_mq_tag_set *set, unsigned int set_flags); void blk_mq_free_tag_set(struct blk_mq_tag_set *set); -void blk_mq_flush_plug_list(struct blk_plug *plug, bool from_schedule); - void blk_mq_free_request(struct request *rq); bool blk_mq_queue_inflight(struct request_queue *q); -- 2.30.2 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH 3/4] block: optimise blk_flush_plug_list 2021-10-20 14:41 cleanup and optimize block plug handling Christoph Hellwig 2021-10-20 14:41 ` [PATCH 1/4] blk-mq: only flush requests from the plug in blk_mq_submit_bio Christoph Hellwig 2021-10-20 14:41 ` [PATCH 2/4] blk-mq: move blk_mq_flush_plug_list to block/blk-mq.h Christoph Hellwig @ 2021-10-20 14:41 ` Christoph Hellwig 2021-10-20 14:41 ` [PATCH 4/4] block: cleanup the flush plug helpers Christoph Hellwig 2021-10-20 16:10 ` cleanup and optimize block plug handling Jens Axboe 4 siblings, 0 replies; 8+ messages in thread From: Christoph Hellwig @ 2021-10-20 14:41 UTC (permalink / raw) To: Jens Axboe; +Cc: Pavel Begunkov, linux-block From: Pavel Begunkov <asml.silence@gmail.com> Don't call flush_plug_callbacks if there are no plug callbacks. Signed-off-by: Pavel Begunkov <asml.silence@gmail.com> [hch: split from a larger patch] Signed-off-by: Christoph Hellwig <hch@lst.de> --- block/blk-core.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/block/blk-core.c b/block/blk-core.c index e6ad5b51d0c3d..63fee1f82bd7d 100644 --- a/block/blk-core.c +++ b/block/blk-core.c @@ -1639,8 +1639,8 @@ EXPORT_SYMBOL(blk_check_plugged); void blk_flush_plug_list(struct blk_plug *plug, bool from_schedule) { - flush_plug_callbacks(plug, from_schedule); - + if (!list_empty(&plug->cb_list)) + flush_plug_callbacks(plug, from_schedule); if (!rq_list_empty(plug->mq_list)) blk_mq_flush_plug_list(plug, from_schedule); if (unlikely(!from_schedule && plug->cached_rq)) -- 2.30.2 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH 4/4] block: cleanup the flush plug helpers 2021-10-20 14:41 cleanup and optimize block plug handling Christoph Hellwig ` (2 preceding siblings ...) 2021-10-20 14:41 ` [PATCH 3/4] block: optimise blk_flush_plug_list Christoph Hellwig @ 2021-10-20 14:41 ` Christoph Hellwig 2021-10-22 20:09 ` Nathan Chancellor 2021-10-20 16:10 ` cleanup and optimize block plug handling Jens Axboe 4 siblings, 1 reply; 8+ messages in thread From: Christoph Hellwig @ 2021-10-20 14:41 UTC (permalink / raw) To: Jens Axboe; +Cc: Pavel Begunkov, linux-block Consolidate the various helpers into a single blk_flush_plug helper that takes a plk_plug and the from_scheduler bool and switch all callsites to call it directly. Checks that the plug is non-NULL must be performed by the caller, something that most already do anyway. Signed-off-by: Christoph Hellwig <hch@lst.de> --- block/blk-core.c | 13 ++++++------- fs/fs-writeback.c | 5 +++-- include/linux/blkdev.h | 29 ++++------------------------- kernel/sched/core.c | 5 +++-- 4 files changed, 16 insertions(+), 36 deletions(-) diff --git a/block/blk-core.c b/block/blk-core.c index 63fee1f82bd7d..bb25934c9408a 100644 --- a/block/blk-core.c +++ b/block/blk-core.c @@ -1089,7 +1089,7 @@ int bio_poll(struct bio *bio, struct io_comp_batch *iob, unsigned int flags) return 0; if (current->plug) - blk_flush_plug_list(current->plug, false); + blk_flush_plug(current->plug, false); if (blk_queue_enter(q, BLK_MQ_REQ_NOWAIT)) return 0; @@ -1637,7 +1637,7 @@ struct blk_plug_cb *blk_check_plugged(blk_plug_cb_fn unplug, void *data, } EXPORT_SYMBOL(blk_check_plugged); -void blk_flush_plug_list(struct blk_plug *plug, bool from_schedule) +void blk_flush_plug(struct blk_plug *plug, bool from_schedule) { if (!list_empty(&plug->cb_list)) flush_plug_callbacks(plug, from_schedule); @@ -1659,11 +1659,10 @@ void blk_flush_plug_list(struct blk_plug *plug, bool from_schedule) */ void blk_finish_plug(struct blk_plug *plug) { - if (plug != current->plug) - return; - blk_flush_plug_list(plug, false); - - current->plug = NULL; + if (plug == current->plug) { + blk_flush_plug(plug, false); + current->plug = NULL; + } } EXPORT_SYMBOL(blk_finish_plug); diff --git a/fs/fs-writeback.c b/fs/fs-writeback.c index 81ec192ce0673..4124a89a1a5df 100644 --- a/fs/fs-writeback.c +++ b/fs/fs-writeback.c @@ -1893,7 +1893,8 @@ static long writeback_sb_inodes(struct super_block *sb, * unplug, so get our IOs out the door before we * give up the CPU. */ - blk_flush_plug(current); + if (current->plug) + blk_flush_plug(current->plug, false); cond_resched(); } @@ -2291,7 +2292,7 @@ void wakeup_flusher_threads(enum wb_reason reason) * If we are expecting writeback progress we must submit plugged IO. */ if (blk_needs_flush_plug(current)) - blk_schedule_flush_plug(current); + blk_flush_plug(current->plug, true); rcu_read_lock(); list_for_each_entry_rcu(bdi, &bdi_list, bdi_list) diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h index c95aee920bbb4..16647fbd1c2c4 100644 --- a/include/linux/blkdev.h +++ b/include/linux/blkdev.h @@ -708,9 +708,8 @@ extern void blk_set_queue_dying(struct request_queue *); * as the lock contention for request_queue lock is reduced. * * It is ok not to disable preemption when adding the request to the plug list - * or when attempting a merge, because blk_schedule_flush_list() will only flush - * the plug list when the task sleeps by itself. For details, please see - * schedule() where blk_schedule_flush_plug() is called. + * or when attempting a merge. For details, please see schedule() where + * blk_flush_plug() is called. */ struct blk_plug { struct request *mq_list; /* blk-mq requests */ @@ -740,23 +739,8 @@ extern struct blk_plug_cb *blk_check_plugged(blk_plug_cb_fn unplug, extern void blk_start_plug(struct blk_plug *); extern void blk_start_plug_nr_ios(struct blk_plug *, unsigned short); extern void blk_finish_plug(struct blk_plug *); -extern void blk_flush_plug_list(struct blk_plug *, bool); -static inline void blk_flush_plug(struct task_struct *tsk) -{ - struct blk_plug *plug = tsk->plug; - - if (plug) - blk_flush_plug_list(plug, false); -} - -static inline void blk_schedule_flush_plug(struct task_struct *tsk) -{ - struct blk_plug *plug = tsk->plug; - - if (plug) - blk_flush_plug_list(plug, true); -} +void blk_flush_plug(struct blk_plug *plug, bool from_schedule); static inline bool blk_needs_flush_plug(struct task_struct *tsk) { @@ -785,15 +769,10 @@ static inline void blk_finish_plug(struct blk_plug *plug) { } -static inline void blk_flush_plug(struct task_struct *task) -{ -} - -static inline void blk_schedule_flush_plug(struct task_struct *task) +static inline void blk_flush_plug(struct blk_plug *plug, bool async) { } - static inline bool blk_needs_flush_plug(struct task_struct *tsk) { return false; diff --git a/kernel/sched/core.c b/kernel/sched/core.c index 92ef7b68198c4..34f37502c27e1 100644 --- a/kernel/sched/core.c +++ b/kernel/sched/core.c @@ -6343,7 +6343,7 @@ static inline void sched_submit_work(struct task_struct *tsk) * make sure to submit it to avoid deadlocks. */ if (blk_needs_flush_plug(tsk)) - blk_schedule_flush_plug(tsk); + blk_flush_plug(tsk->plug, true); } static void sched_update_worker(struct task_struct *tsk) @@ -8354,7 +8354,8 @@ int io_schedule_prepare(void) int old_iowait = current->in_iowait; current->in_iowait = 1; - blk_schedule_flush_plug(current); + if (current->plug) + blk_flush_plug(current->plug, true); return old_iowait; } -- 2.30.2 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH 4/4] block: cleanup the flush plug helpers 2021-10-20 14:41 ` [PATCH 4/4] block: cleanup the flush plug helpers Christoph Hellwig @ 2021-10-22 20:09 ` Nathan Chancellor 2021-10-23 1:38 ` Jens Axboe 0 siblings, 1 reply; 8+ messages in thread From: Nathan Chancellor @ 2021-10-22 20:09 UTC (permalink / raw) To: Christoph Hellwig; +Cc: Jens Axboe, Pavel Begunkov, linux-block On Wed, Oct 20, 2021 at 04:41:19PM +0200, Christoph Hellwig wrote: > Consolidate the various helpers into a single blk_flush_plug helper that > takes a plk_plug and the from_scheduler bool and switch all callsites to > call it directly. Checks that the plug is non-NULL must be performed by > the caller, something that most already do anyway. > > Signed-off-by: Christoph Hellwig <hch@lst.de> This patch as commit 008f75a20e70 ("block: cleanup the flush plug helpers") in -next causes the following errors with CONFIG_BLOCK=n (tinyconfig): kernel/sched/core.c: In function ‘sched_submit_work’: kernel/sched/core.c:6346:35: error: ‘struct task_struct’ has no member named ‘plug’ 6346 | blk_flush_plug(tsk->plug, true); | ^~ kernel/sched/core.c: In function ‘io_schedule_prepare’: kernel/sched/core.c:8357:20: error: ‘struct task_struct’ has no member named ‘plug’ 8357 | if (current->plug) | ^~ kernel/sched/core.c:8358:39: error: ‘struct task_struct’ has no member named ‘plug’ 8358 | blk_flush_plug(current->plug, true); | ^~ I tested the latest block tree and did not see it fixed nor did I see it reported or fixed elsewhere. Cheers, Nathan ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 4/4] block: cleanup the flush plug helpers 2021-10-22 20:09 ` Nathan Chancellor @ 2021-10-23 1:38 ` Jens Axboe 0 siblings, 0 replies; 8+ messages in thread From: Jens Axboe @ 2021-10-23 1:38 UTC (permalink / raw) To: Nathan Chancellor, Christoph Hellwig; +Cc: Pavel Begunkov, linux-block On 10/22/21 2:09 PM, Nathan Chancellor wrote: > On Wed, Oct 20, 2021 at 04:41:19PM +0200, Christoph Hellwig wrote: >> Consolidate the various helpers into a single blk_flush_plug helper that >> takes a plk_plug and the from_scheduler bool and switch all callsites to >> call it directly. Checks that the plug is non-NULL must be performed by >> the caller, something that most already do anyway. >> >> Signed-off-by: Christoph Hellwig <hch@lst.de> > > This patch as commit 008f75a20e70 ("block: cleanup the flush plug > helpers") in -next causes the following errors with CONFIG_BLOCK=n > (tinyconfig): > > kernel/sched/core.c: In function ‘sched_submit_work’: > kernel/sched/core.c:6346:35: error: ‘struct task_struct’ has no member named ‘plug’ > 6346 | blk_flush_plug(tsk->plug, true); > | ^~ > kernel/sched/core.c: In function ‘io_schedule_prepare’: > kernel/sched/core.c:8357:20: error: ‘struct task_struct’ has no member named ‘plug’ > 8357 | if (current->plug) > | ^~ > kernel/sched/core.c:8358:39: error: ‘struct task_struct’ has no member named ‘plug’ > 8358 | blk_flush_plug(current->plug, true); > | ^~ > > I tested the latest block tree and did not see it fixed nor did I see it > reported or fixed elsewhere. This should fix it, thanks for reporting. commit 599593a82fc57f5e9453c8ef7420df3206934a0c Author: Jens Axboe <axboe@kernel.dk> Date: Fri Oct 22 19:35:45 2021 -0600 sched: make task_struct->plug always defined If CONFIG_BLOCK isn't set, then it's an empty struct anyway. Just make it generally available, so we don't break the compile: kernel/sched/core.c: In function ‘sched_submit_work’: kernel/sched/core.c:6346:35: error: ‘struct task_struct’ has no member named ‘plug’ 6346 | blk_flush_plug(tsk->plug, true); | ^~ kernel/sched/core.c: In function ‘io_schedule_prepare’: kernel/sched/core.c:8357:20: error: ‘struct task_struct’ has no member named ‘plug’ 8357 | if (current->plug) | ^~ kernel/sched/core.c:8358:39: error: ‘struct task_struct’ has no member named ‘plug’ 8358 | blk_flush_plug(current->plug, true); | ^~ Reported-by: Nathan Chancellor <nathan@kernel.org> Fixes: 008f75a20e70 ("block: cleanup the flush plug helpers") Signed-off-by: Jens Axboe <axboe@kernel.dk> diff --git a/include/linux/sched.h b/include/linux/sched.h index c1a927ddec64..e0454e60fe8f 100644 --- a/include/linux/sched.h +++ b/include/linux/sched.h @@ -1160,10 +1160,8 @@ struct task_struct { /* Stacked block device info: */ struct bio_list *bio_list; -#ifdef CONFIG_BLOCK /* Stack plugging: */ struct blk_plug *plug; -#endif /* VM state: */ struct reclaim_state *reclaim_state; -- Jens Axboe ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: cleanup and optimize block plug handling 2021-10-20 14:41 cleanup and optimize block plug handling Christoph Hellwig ` (3 preceding siblings ...) 2021-10-20 14:41 ` [PATCH 4/4] block: cleanup the flush plug helpers Christoph Hellwig @ 2021-10-20 16:10 ` Jens Axboe 4 siblings, 0 replies; 8+ messages in thread From: Jens Axboe @ 2021-10-20 16:10 UTC (permalink / raw) To: Christoph Hellwig; +Cc: linux-block, Pavel Begunkov On Wed, 20 Oct 2021 16:41:15 +0200, Christoph Hellwig wrote: > this is a spinoff from Pavel's misc optimizations. It shuld take care > of his two painpoints and also cleans up the interface for flushing > plugs a bit. > > Diffstat: > block/blk-core.c | 17 ++++++++--------- > block/blk-mq.c | 2 +- > block/blk-mq.h | 1 + > fs/fs-writeback.c | 5 +++-- > include/linux/blk-mq.h | 2 -- > include/linux/blkdev.h | 29 ++++------------------------- > kernel/sched/core.c | 5 +++-- > 7 files changed, 20 insertions(+), 41 deletions(-) > > [...] Applied, thanks! [1/4] blk-mq: only flush requests from the plug in blk_mq_submit_bio commit: a214b949d8e365583dd67441f6f608f0b20f7f52 [2/4] blk-mq: move blk_mq_flush_plug_list to block/blk-mq.h commit: dbb6f764a079d1dea883c6f2439d91db4f0fb2f2 [3/4] block: optimise blk_flush_plug_list commit: b600455d84307696b3cb7debdaf3081080748409 [4/4] block: cleanup the flush plug helpers commit: 008f75a20e7072d0840ec323c39b42206f3fa8a0 Best regards, -- Jens Axboe ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2021-10-23 1:38 UTC | newest] Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2021-10-20 14:41 cleanup and optimize block plug handling Christoph Hellwig 2021-10-20 14:41 ` [PATCH 1/4] blk-mq: only flush requests from the plug in blk_mq_submit_bio Christoph Hellwig 2021-10-20 14:41 ` [PATCH 2/4] blk-mq: move blk_mq_flush_plug_list to block/blk-mq.h Christoph Hellwig 2021-10-20 14:41 ` [PATCH 3/4] block: optimise blk_flush_plug_list Christoph Hellwig 2021-10-20 14:41 ` [PATCH 4/4] block: cleanup the flush plug helpers Christoph Hellwig 2021-10-22 20:09 ` Nathan Chancellor 2021-10-23 1:38 ` Jens Axboe 2021-10-20 16:10 ` cleanup and optimize block plug handling Jens Axboe
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).