bcache: don't count buckets twice

If owned_by_allocator is set, we don't want to count the bucket
as metadata, dirty or cached since it is already counted as alloc.

Similarly, if dirty_sectors is set, don't count the bucket as
cached.

This was causing buckets_free_cache() to return a negative value,
causing btree node allocations to fail while splitting interior
nodes, which is not supposed to happen.

This fixes a regression from "refactor bucket stats code".

Also, fix a misleading variable name in alloc.c, and fix argument
order for btree_check_reserve_fail tracepoint.

Signed-off-by: Kent Overstreet <kent.overstreet@gmail.com>

5 files changed, 43 insertions, 24 deletions

diff --git a/drivers/md/bcache/alloc.c b/drivers/md/bcache/alloc.c
index 23e60b19508c..a294251a9fe7 100644
--- a/drivers/md/bcache/alloc.c
+++ b/drivers/md/bcache/alloc.c
@@ -647,7 +647,7 @@ static struct cache *bch_next_cache(struct cache_set *c,
 				    struct closure *cl)
 {
 	struct cache *device, **devices;
-	size_t sectors_count = 0, rand;
+	size_t bucket_count = 0, rand;
 	int i, nr_devices;
 
 	/* first ptr allocation will always go to the specified tier,
@@ -671,10 +671,10 @@ static struct cache *bch_next_cache(struct cache_set *c,
 		if (test_bit(devices[i]->sb.nr_this_dev, cache_used))
 			continue;
 
-		sectors_count += buckets_free_cache(devices[i], reserve);
+		bucket_count += buckets_free_cache(devices[i], reserve);
 	}
 
-	if (!sectors_count) {
+	if (!bucket_count) {
 		trace_bcache_bucket_alloc_set_fail(c, reserve, cl);
 		return ERR_PTR(bch_bucket_wait(c, reserve, cl));
 	}
@@ -688,7 +688,7 @@ static struct cache *bch_next_cache(struct cache_set *c,
 	 */
 
 	get_random_bytes(&rand, sizeof(rand));
-	rand %= sectors_count;
+	rand %= bucket_count;
 
 	device = NULL;
 
@@ -700,9 +700,9 @@ static struct cache *bch_next_cache(struct cache_set *c,
 			continue;
 
 		device = devices[i];
-		sectors_count -= buckets_free_cache(device, reserve);
+		bucket_count -= buckets_free_cache(device, reserve);
 
-		if (rand >= sectors_count) {
+		if (rand >= bucket_count) {
 			__set_bit(device->sb.nr_this_dev, cache_used);
 			return device;
 		}
diff --git a/drivers/md/bcache/btree.c b/drivers/md/bcache/btree.c
index 3bd698c9b5a9..c16958c79c66 100644
--- a/drivers/md/bcache/btree.c
+++ b/drivers/md/bcache/btree.c
@@ -1354,9 +1354,9 @@ static int __btree_check_reserve(struct cache_set *c,
 
 	for_each_cache_rcu(ca, c, i) {
 		if (fifo_used(&ca->free[reserve]) < required) {
-			trace_bcache_btree_check_reserve_fail(ca,
+			trace_bcache_btree_check_reserve_fail(ca, reserve,
 					fifo_used(&ca->free[reserve]),
-					reserve, cl);
+					required, cl);
 
 			ret = bch_bucket_wait(c, reserve, cl);
 			rcu_read_unlock();
diff --git a/drivers/md/bcache/buckets.c b/drivers/md/bcache/buckets.c
index 3dd629647acb..caef62419ff4 100644
--- a/drivers/md/bcache/buckets.c
+++ b/drivers/md/bcache/buckets.c
@@ -58,6 +58,21 @@
 
 #include <trace/events/bcache.h>
 
+static inline int is_meta_bucket(struct bucket_mark m)
+{
+	return !m.owned_by_allocator && m.is_metadata;
+}
+
+static inline int is_dirty_bucket(struct bucket_mark m)
+{
+	return !m.owned_by_allocator && !!m.dirty_sectors;
+}
+
+static inline int is_cached_bucket(struct bucket_mark m)
+{
+	return !m.owned_by_allocator && !m.dirty_sectors && !!m.cached_sectors;
+}
+
 static void bucket_stats_update(struct cache *ca,
 				struct bucket_mark old,
 				struct bucket_mark new)
@@ -65,22 +80,22 @@ static void bucket_stats_update(struct cache *ca,
 	struct bucket_stats *stats = &ca->bucket_stats[0];
 	int v;
 
+	if ((v = ((int) new.cached_sectors - (int) old.cached_sectors)))
+		atomic64_add_bug(v, &stats->sectors_cached);
+
+	if ((v = ((int) new.dirty_sectors - (int) old.dirty_sectors)))
+		atomic64_add_bug(v, &stats->sectors_dirty);
+
 	if ((v = ((int) new.owned_by_allocator - (int) old.owned_by_allocator)))
 		atomic_add_bug(v, &stats->buckets_alloc);
 
-	if ((v = ((int) new.is_metadata - (int) old.is_metadata)))
+	if ((v = (is_meta_bucket(new) - is_meta_bucket(old))))
 		atomic_add_bug(v, &stats->buckets_meta);
 
-	if ((v = ((int) new.cached_sectors - (int) old.cached_sectors)))
-		atomic64_add_bug(v, &stats->sectors_cached);
-
-	if ((v = ((int) !!new.cached_sectors - (int) !!old.cached_sectors)))
+	if ((v = (is_cached_bucket(old) - is_cached_bucket(new))))
 		atomic_add_bug(v, &stats->buckets_cached);
 
-	if ((v = ((int) new.dirty_sectors - (int) old.dirty_sectors)))
-		atomic64_add_bug(v, &stats->sectors_dirty);
-
-	if ((v = ((int) !!new.dirty_sectors - (int) !!old.dirty_sectors)))
+	if ((v = (is_dirty_bucket(old) - is_dirty_bucket(new))))
 		atomic_add_bug(v, &stats->buckets_dirty);
 }
 
diff --git a/drivers/md/bcache/buckets.h b/drivers/md/bcache/buckets.h
index fcac0946e9d6..e2d6bdb43e1e 100644
--- a/drivers/md/bcache/buckets.h
+++ b/drivers/md/bcache/buckets.h
@@ -40,14 +40,15 @@ static inline unsigned bucket_sectors_used(struct bucket *b)
 
 static inline size_t buckets_available_cache(struct cache *ca)
 {
-	size_t buckets = ca->sb.nbuckets - ca->sb.first_bucket;
 	struct bucket_stats stats = bucket_stats_read(ca);
-
 	/* XXX: awkward? */
-	return buckets -
+	ssize_t buckets = ca->sb.nbuckets -
+		ca->sb.first_bucket -
 		atomic_read(&stats.buckets_dirty) -
 		atomic_read(&stats.buckets_alloc) -
 		atomic_read(&stats.buckets_meta);
+
+	return max_t(ssize_t, buckets, 0);
 }
 
 static inline size_t buckets_available(struct cache_set *c)
diff --git a/include/trace/events/bcache.h b/include/trace/events/bcache.h
index ff1396dd9b09..41075e821c91 100644
--- a/include/trace/events/bcache.h
+++ b/include/trace/events/bcache.h
@@ -547,13 +547,14 @@ TRACE_EVENT(bcache_alloc_batch,
 
 TRACE_EVENT(bcache_btree_check_reserve_fail,
 	TP_PROTO(struct cache *ca, enum btree_id id, size_t free,
-		 struct closure *cl),
-	TP_ARGS(ca, id, free, cl),
+		 size_t required, struct closure *cl),
+	TP_ARGS(ca, id, free, required, cl),
 
 	TP_STRUCT__entry(
 		__array(char,			uuid,	16	)
 		__field(enum btree_id,		id		)
 		__field(size_t,			free		)
+		__field(size_t,			required	)
 		__field(struct closure *,	cl		)
 	),
 
@@ -561,11 +562,13 @@ TRACE_EVENT(bcache_btree_check_reserve_fail,
 		memcpy(__entry->uuid, ca->sb.uuid.b, 16);
 		__entry->id = id;
 		__entry->free = free;
+		__entry->required = required;
 		__entry->cl = cl;
 	),
 
-	TP_printk("%pU id %u free %zu by %p",
-		__entry->uuid, __entry->id, __entry->free, __entry->cl)
+	TP_printk("%pU id %u free %zu required %zu by %p",
+		__entry->uuid, __entry->id, __entry->free, __entry->required,
+		__entry->cl)
 );
 
 DEFINE_EVENT(cache, bcache_moving_gc_start,