* [PATCH 1/1] block: optimise in irq bio put caching
@ 2024-01-19 1:23 Pavel Begunkov
2024-01-29 14:36 ` Pavel Begunkov
0 siblings, 1 reply; 5+ messages in thread
From: Pavel Begunkov @ 2024-01-19 1:23 UTC (permalink / raw
To: linux-block; +Cc: Jens Axboe, asml.silence
The put side of the percpu bio caching is mainly targeting completions
in the hard irq context, but the context is not guaranteed so we guard
against those cases by switching interrupts off.
Disabling interrupts while they're already disabled is supposed to be
fast, but profiling shows it's far from perfect. Instead, we can infer
the interrupt state from in_hardirq(), which is just a fast var read,
and fall back to the normal bio_free() otherwise. With that, the caching
doesn't cover in softirq/task completions anymore, but that should be
just fine, we have never measured if caching brings anything in those
scenarios.
Profiling indicates that the bio_put() cost is reduced by ~3.5 times
(1.76% -> 0.49%), and and throughput of CPU bound benchmarks improve
by around 1% (t/io_uring with high QD and several drives).
Signed-off-by: Pavel Begunkov <asml.silence@gmail.com>
---
block/bio.c | 17 ++++++++++-------
1 file changed, 10 insertions(+), 7 deletions(-)
diff --git a/block/bio.c b/block/bio.c
index 816d412c06e9..a8a4f3211893 100644
--- a/block/bio.c
+++ b/block/bio.c
@@ -763,26 +763,29 @@ static inline void bio_put_percpu_cache(struct bio *bio)
cache = per_cpu_ptr(bio->bi_pool->cache, get_cpu());
if (READ_ONCE(cache->nr_irq) + cache->nr > ALLOC_CACHE_MAX) {
+free:
put_cpu();
bio_free(bio);
return;
}
- bio_uninit(bio);
-
- if ((bio->bi_opf & REQ_POLLED) && !WARN_ON_ONCE(in_interrupt())) {
+ if (bio->bi_opf & REQ_POLLED) {
+ if (WARN_ON_ONCE(!in_task()))
+ goto free;
+ bio_uninit(bio);
bio->bi_next = cache->free_list;
bio->bi_bdev = NULL;
cache->free_list = bio;
cache->nr++;
- } else {
- unsigned long flags;
+ } else if (in_hardirq()) {
+ lockdep_assert_irqs_disabled();
- local_irq_save(flags);
+ bio_uninit(bio);
bio->bi_next = cache->free_list_irq;
cache->free_list_irq = bio;
cache->nr_irq++;
- local_irq_restore(flags);
+ } else {
+ goto free;
}
put_cpu();
}
--
2.43.0
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH 1/1] block: optimise in irq bio put caching
2024-01-19 1:23 [PATCH 1/1] block: optimise in irq bio put caching Pavel Begunkov
@ 2024-01-29 14:36 ` Pavel Begunkov
2024-01-29 17:24 ` Christoph Hellwig
0 siblings, 1 reply; 5+ messages in thread
From: Pavel Begunkov @ 2024-01-29 14:36 UTC (permalink / raw
To: linux-block; +Cc: Jens Axboe
On 1/19/24 01:23, Pavel Begunkov wrote:
> The put side of the percpu bio caching is mainly targeting completions
> in the hard irq context, but the context is not guaranteed so we guard
> against those cases by switching interrupts off.
>
> Disabling interrupts while they're already disabled is supposed to be
> fast, but profiling shows it's far from perfect. Instead, we can infer
> the interrupt state from in_hardirq(), which is just a fast var read,
> and fall back to the normal bio_free() otherwise. With that, the caching
> doesn't cover in softirq/task completions anymore, but that should be
> just fine, we have never measured if caching brings anything in those
> scenarios.
>
> Profiling indicates that the bio_put() cost is reduced by ~3.5 times
> (1.76% -> 0.49%), and and throughput of CPU bound benchmarks improve
> by around 1% (t/io_uring with high QD and several drives).
Let me know if there are any concerns with the patch
--
Pavel Begunkov
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH 1/1] block: optimise in irq bio put caching
2024-01-29 14:36 ` Pavel Begunkov
@ 2024-01-29 17:24 ` Christoph Hellwig
2024-01-29 18:00 ` Pavel Begunkov
0 siblings, 1 reply; 5+ messages in thread
From: Christoph Hellwig @ 2024-01-29 17:24 UTC (permalink / raw
To: Pavel Begunkov; +Cc: linux-block, Jens Axboe
On Mon, Jan 29, 2024 at 02:36:57PM +0000, Pavel Begunkov wrote:
> Let me know if there are any concerns with the patch
This seems to lose the case where non-polled bios are freed
form process context, which can be true with threadead interrupts
or various block remappers that defer I/O completions to workqueues,
and also a lot of file systems (but currentl the alloc cache isn't
used by file systems).
Also jumping backward for non-loop code flow is a nasty pattern.
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH 1/1] block: optimise in irq bio put caching
2024-01-29 17:24 ` Christoph Hellwig
@ 2024-01-29 18:00 ` Pavel Begunkov
2024-01-30 8:25 ` Christoph Hellwig
0 siblings, 1 reply; 5+ messages in thread
From: Pavel Begunkov @ 2024-01-29 18:00 UTC (permalink / raw
To: Christoph Hellwig; +Cc: linux-block, Jens Axboe
On 1/29/24 17:24, Christoph Hellwig wrote:
> On Mon, Jan 29, 2024 at 02:36:57PM +0000, Pavel Begunkov wrote:
>> Let me know if there are any concerns with the patch
>
> This seems to lose the case where non-polled bios are freed
> form process context, which can be true with threadead interrupts
> or various block remappers that defer I/O completions to workqueues,
> and also a lot of file systems (but currentl the alloc cache isn't
> used by file systems).
For the task context I can generalise the poll branch
if (in_task()) { // previously if (REQ_POLLED)
// ->free_list;
} else if (in_hardirq()) {
// ->free_list_irq;
} else {
bio_free();
}
> Also jumping backward for non-loop code flow is a nasty pattern.
How come considering it's jumping to a return? I can switch
the bio_free() and goto blocks so it's a jump forward, if
that's a preference
--
Pavel Begunkov
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH 1/1] block: optimise in irq bio put caching
2024-01-29 18:00 ` Pavel Begunkov
@ 2024-01-30 8:25 ` Christoph Hellwig
0 siblings, 0 replies; 5+ messages in thread
From: Christoph Hellwig @ 2024-01-30 8:25 UTC (permalink / raw
To: Pavel Begunkov; +Cc: Christoph Hellwig, linux-block, Jens Axboe
On Mon, Jan 29, 2024 at 06:00:28PM +0000, Pavel Begunkov wrote:
> How come considering it's jumping to a return? I can switch
> the bio_free() and goto blocks so it's a jump forward, if
> that's a preference
We generally jump to the end unless it is a loop implemented using
goto as that is the natural reading flow.
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2024-01-30 8:25 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-01-19 1:23 [PATCH 1/1] block: optimise in irq bio put caching Pavel Begunkov
2024-01-29 14:36 ` Pavel Begunkov
2024-01-29 17:24 ` Christoph Hellwig
2024-01-29 18:00 ` Pavel Begunkov
2024-01-30 8:25 ` Christoph Hellwig
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 read-only IMAP folder(s) and NNTP newsgroup(s).