[PATCH v3 2/2] bcache: inspect active bypass writes lock-free via RCU

[Ankit Kapoor <ankitkap@google.com>]

Checking for active bypass writes on the cache miss read path currently
requires acquiring page-level spinlocks across the target sector range.
While contention is low under normal operation, acquiring a lock in the
latency-sensitive read path introduces unnecessary overhead.

Optimize the read path by using RCU to inspect active bypass write
counters lock-free. Writers continue to use page-level spinlocks to
synchronize counter updates and allocations, while readers inspect the
tracking array under rcu_read_lock().

Suggested-by: Coly Li <colyli@fygo.io>
Signed-off-by: Ankit Kapoor <ankitkap@google.com>
---
 drivers/md/bcache/bcache.h  | 14 +++++++--
 drivers/md/bcache/request.c | 58 +++++++++++++++++++++++++------------
 drivers/md/bcache/super.c   |  9 ++++--
 3 files changed, 56 insertions(+), 25 deletions(-)

diff --git a/drivers/md/bcache/bcache.h b/drivers/md/bcache/bcache.h
index 2e50526a52fc..2ecff48d8902 100644
--- a/drivers/md/bcache/bcache.h
+++ b/drivers/md/bcache/bcache.h
@@ -299,10 +299,18 @@ enum stop_on_failure {
 	BCH_CACHED_DEV_STOP_MODE_MAX,
 };
 
+extern struct kmem_cache *bch_bypass_cache;
+
+struct bch_bypass_counts {
+	struct rcu_head		rcu;
+	struct cached_dev	*dc;
+	u32			counts[PAGE_SIZE / sizeof(u32)];
+};
+
 struct bch_bypass_page {
-	u32		*counts;
-	unsigned int	active;
-	spinlock_t	lock;
+	struct bch_bypass_counts __rcu *counts;
+	unsigned int		active;
+	spinlock_t		lock;
 };
 
 struct cached_dev {
diff --git a/drivers/md/bcache/request.c b/drivers/md/bcache/request.c
index bfd28b68f499..9221db669050 100644
--- a/drivers/md/bcache/request.c
+++ b/drivers/md/bcache/request.c
@@ -24,6 +24,7 @@
 #define CUTOFF_CACHE_READA	90
 
 struct kmem_cache *bch_search_cache;
+struct kmem_cache *bch_bypass_cache;
 
 static CLOSURE_CALLBACK(bch_data_insert_start);
 
@@ -852,22 +853,28 @@ static void bch_bypass_write_start(struct cached_dev *dc, sector_t sector, unsig
 		unsigned long pg_idx = bypass_chunk_to_page(chunk);
 		unsigned long pg_off = bypass_chunk_to_offset(chunk);
 		struct bch_bypass_page *pg = &dc->bypass_pages[pg_idx];
-		u32 *new_counts;
-		u32 *dup_counts = NULL;
+		struct bch_bypass_counts *new_counts;
+		struct bch_bypass_counts *dup_counts = NULL;
+		struct bch_bypass_counts *counts;
 		unsigned long flags;
 
 		spin_lock_irqsave(&pg->lock, flags);
-		if (!pg->counts) {
+		counts = rcu_dereference_protected(pg->counts, lockdep_is_held(&pg->lock));
+		if (!counts) {
 			spin_unlock_irqrestore(&pg->lock, flags);
 			new_counts = mempool_alloc(&dc->bypass_mempool, GFP_NOIO);
-			memset(new_counts, 0, PAGE_SIZE);
+			memset(new_counts->counts, 0, PAGE_SIZE);
+			new_counts->dc = dc;
 			spin_lock_irqsave(&pg->lock, flags);
-			if (pg->counts)
+			counts = rcu_dereference_protected(pg->counts, lockdep_is_held(&pg->lock));
+			if (counts) {
 				dup_counts = new_counts;
-			else
-				pg->counts = new_counts;
+			} else {
+				counts = new_counts;
+				rcu_assign_pointer(pg->counts, counts);
+			}
 		}
-		pg->counts[pg_off]++;
+		WRITE_ONCE(counts->counts[pg_off], counts->counts[pg_off] + 1);
 		pg->active++;
 		spin_unlock_irqrestore(&pg->lock, flags);
 
@@ -876,6 +883,13 @@ static void bch_bypass_write_start(struct cached_dev *dc, sector_t sector, unsig
 	}
 }
 
+static void bch_bypass_counts_free_rcu(struct rcu_head *rcu)
+{
+	struct bch_bypass_counts *counts = container_of(rcu, struct bch_bypass_counts, rcu);
+
+	mempool_free(counts, &counts->dc->bypass_mempool);
+}
+
 static void bch_bypass_write_end(struct cached_dev *dc, sector_t sector, unsigned int sectors)
 {
 	unsigned long start_chunk = sector_to_bypass_chunk(sector);
@@ -890,25 +904,27 @@ static void bch_bypass_write_end(struct cached_dev *dc, sector_t sector, unsigne
 		unsigned long pg_idx = bypass_chunk_to_page(chunk);
 		unsigned long pg_off = bypass_chunk_to_offset(chunk);
 		struct bch_bypass_page *pg = &dc->bypass_pages[pg_idx];
-		u32 *counts = NULL;
+		struct bch_bypass_counts *counts = NULL;
+		struct bch_bypass_counts *current_counts;
 		unsigned long flags;
 
 		spin_lock_irqsave(&pg->lock, flags);
-		if (WARN_ON_ONCE(!pg->counts || !pg->counts[pg_off])) {
+		current_counts = rcu_dereference_protected(pg->counts, lockdep_is_held(&pg->lock));
+		if (WARN_ON_ONCE(!current_counts || !current_counts->counts[pg_off])) {
 			spin_unlock_irqrestore(&pg->lock, flags);
 			continue;
 		}
 
-		pg->counts[pg_off]--;
+		WRITE_ONCE(current_counts->counts[pg_off], current_counts->counts[pg_off] - 1);
 		pg->active--;
 		if (!pg->active) {
-			counts = pg->counts;
-			pg->counts = NULL;
+			counts = current_counts;
+			rcu_assign_pointer(pg->counts, NULL);
 		}
 		spin_unlock_irqrestore(&pg->lock, flags);
 
 		if (counts)
-			mempool_free(counts, &dc->bypass_mempool);
+			call_rcu(&counts->rcu, bch_bypass_counts_free_rcu);
 	}
 }
 
@@ -924,20 +940,19 @@ static bool bch_has_active_bypass_writes(struct cached_dev *dc, sector_t sector,
 	if (WARN_ON_ONCE(end_pg_idx >= dc->bypass_num_pages))
 		return false;
 
+	rcu_read_lock();
 	for (chunk = start_chunk; chunk <= end_chunk; chunk++) {
 		unsigned long pg_idx = bypass_chunk_to_page(chunk);
 		unsigned long pg_off = bypass_chunk_to_offset(chunk);
 		struct bch_bypass_page *pg = &dc->bypass_pages[pg_idx];
-		unsigned long flags;
+		struct bch_bypass_counts *current_counts = rcu_dereference(pg->counts);
 
-		spin_lock_irqsave(&pg->lock, flags);
-		if (pg->counts && pg->counts[pg_off] > 0) {
+		if (current_counts && READ_ONCE(current_counts->counts[pg_off]) > 0) {
 			has_active = true;
-			spin_unlock_irqrestore(&pg->lock, flags);
 			break;
 		}
-		spin_unlock_irqrestore(&pg->lock, flags);
 	}
+	rcu_read_unlock();
 
 	return has_active;
 }
@@ -1458,6 +1473,7 @@ void bch_flash_dev_request_init(struct bcache_device *d)
 
 void bch_request_exit(void)
 {
+	kmem_cache_destroy(bch_bypass_cache);
 	kmem_cache_destroy(bch_search_cache);
 }
 
@@ -1467,5 +1483,9 @@ int __init bch_request_init(void)
 	if (!bch_search_cache)
 		return -ENOMEM;
 
+	bch_bypass_cache = KMEM_CACHE(bch_bypass_counts, 0);
+	if (!bch_bypass_cache)
+		return -ENOMEM;
+
 	return 0;
 }
diff --git a/drivers/md/bcache/super.c b/drivers/md/bcache/super.c
index b994483511de..75aa6bb2e00b 100644
--- a/drivers/md/bcache/super.c
+++ b/drivers/md/bcache/super.c
@@ -1350,9 +1350,12 @@ void bch_cached_dev_release(struct kobject *kobj)
 		unsigned long i;
 
 		for (i = 0; i < dc->bypass_num_pages; i++) {
-			if (dc->bypass_pages[i].counts)
-				mempool_free(dc->bypass_pages[i].counts, &dc->bypass_mempool);
+			struct bch_bypass_counts *counts =
+				rcu_dereference_protected(dc->bypass_pages[i].counts, 1);
+			if (counts)
+				mempool_free(counts, &dc->bypass_mempool);
 		}
+		rcu_barrier();
 		kvfree(dc->bypass_pages);
 		mempool_exit(&dc->bypass_mempool);
 	}
@@ -1433,7 +1436,7 @@ static int bch_cached_dev_bypass_init(struct cached_dev *dc, sector_t sectors)
 	for (i = 0; i < dc->bypass_num_pages; i++)
 		spin_lock_init(&dc->bypass_pages[i].lock);
 
-	if (mempool_init_kmalloc_pool(&dc->bypass_mempool, 16, PAGE_SIZE)) {
+	if (mempool_init_slab_pool(&dc->bypass_mempool, 16, bch_bypass_cache)) {
 		kvfree(dc->bypass_pages);
 		dc->bypass_pages = NULL;
 		return -ENOMEM;
-- 
2.55.0.679.g6767b8d81c-goog