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

Ankit Kapoor <[email protected]>
Newsgroups org.kernel.vger.linux-bcache,org.kernel.vger.linux-kernel
Message-ID <[email protected]>
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 <[email protected]>
Signed-off-by: Ankit Kapoor <[email protected]>
---
 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
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.