Skip to contentSkip to conversation

bio put in-IRQ caching optimisation

PATCH Applied Pavel Begunkov 2 participants 4 messages Feb 7, 2024 – Feb 8, 2024 Block layer

lore
Share & tools
lore
#0 lore raw

v2 cover bio put in-IRQ caching optimisation

Patch 1 is a preparation patch, which enables caching of !IOPOLL bios for the task context execution.

Patch 2 optimise out local_irq_{save,restore}() from bio_put_percpu_cache() for in-IRQ completions.

v2: Extend caching to the task context

Move error path to the end of bio_put_percpu_cache(). It looks uglier, but I'm happy to make the change as long as it aligns with the community standards and helps folks around.

Pavel Begunkov (2):
block: extend bio caching to task context
block: optimise in irq bio put caching

 block/bio.c | 26 ++++++++++++++------------
 1 file changed, 14 insertions(+), 12 deletions(-)
Signature · 1 line
2.43.0
#1 lore raw

PATCH v2 1/2 block: extend bio caching to task context

bio_put_percpu_cache() puts all non-iopoll bios into the irq-safe list, which entails disabling irqs. The overhead of that is not that bad when interrupts are already off but getting worse otherwise. We can optimise it when we're in the task context by using ->free_list directly just as the IOPOLL path does.

---
 block/bio.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)
block/bio.c +2 -1
index b9642a41f286..8da941974f88 100644
@@ -770,8 +770,9 @@ static inline void bio_put_percpu_cache(struct bio *bio)
 
 	bio_uninit(bio);
 
-	if ((bio->bi_opf & REQ_POLLED) && !WARN_ON_ONCE(in_interrupt())) {
+	if (in_task()) {
 		bio->bi_next = cache->free_list;
+		/* Not necessary but helps not to iopoll already freed bios */
 		bio->bi_bdev = NULL;
 		cache->free_list = bio;
 		cache->nr++;
Signature · 1 line
2.43.0
#2 lore raw

PATCH v2 2/2 block: optimise in irq bio put caching

In reply to Pavel Begunkov

When enlisting a bio into ->free_list_irq we protect the list by disabling irqs. It's likely they're already disabled and performance of local_irq_{save,restore}() is decent, but it's not zero cost.

Let's only use the irq cache when when we're serving a hard irq, which allows to remove local_irq_{save,restore}(), and fall back to bio_free() in all left cases.

Profiles indicate that the bio_put() cost is reduced by ~3.5 times (1.76% -> 0.49%), and total throughput of a CPU bound benchmark improve by around 1% (t/io_uring with high QD and several drives).

---
 block/bio.c | 23 ++++++++++++-----------
 1 file changed, 12 insertions(+), 11 deletions(-)
block/bio.c +12 -11
index 8da941974f88..00847ff1415c 100644
@@ -762,30 +762,31 @@ static inline void bio_put_percpu_cache(struct bio *bio)
 	struct bio_alloc_cache *cache;
 
 	cache = per_cpu_ptr(bio->bi_pool->cache, get_cpu());
-	if (READ_ONCE(cache->nr_irq) + cache->nr > ALLOC_CACHE_MAX) {
-		put_cpu();
-		bio_free(bio);
-		return;
-	}
-
-	bio_uninit(bio);
+	if (READ_ONCE(cache->nr_irq) + cache->nr > ALLOC_CACHE_MAX)
+		goto out_free;
 
 	if (in_task()) {
+		bio_uninit(bio);
 		bio->bi_next = cache->free_list;
 		/* Not necessary but helps not to iopoll already freed bios */
 		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 out_free;
 	}
 	put_cpu();
+	return;
+out_free:
+	put_cpu();
+	bio_free(bio);
 }
 
 /**
Signature · 1 line
2.43.0
maintainer Applied
#3 lore raw
In reply to Pavel Begunkov
Quoted 9 lines · Pavel Begunkov

Patch 1 is a preparation patch, which enables caching of !IOPOLL bios for the task context execution.

Patch 2 optimise out local_irq_{save,restore}() from bio_put_percpu_cache() for in-IRQ completions.

v2: Extend caching to the task context

[...]

Applied, thanks!

[1/2] block: extend bio caching to task context

      commit: c9f5f3aa19c617fe85085b19abbf7a9a077336d0
[2/2] block: optimise in irq bio put caching
      commit: e516c3fc6c182736aec5418a73f15199640491e2

Best regards,

Signature · 1 line
Jens Axboe