[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