dm-bufio: remove maximum age based eviction

Every 30 seconds, dm-bufio evicts all buffers that were not accessed
within the last max_age_seconds, except those pinned in memory via
retain_bytes.  By default max_age_seconds is 300 (i.e. 5 minutes), and
retain_bytes is 262144 (i.e. 256 KiB) per dm-bufio client.

This eviction algorithm is much too eager and is also redundant with the
shinker based eviction.

Testing on an Android phone shows that about 30 MB of dm-bufio buffers
(from dm-verity Merkle tree blocks) are loaded at boot time, and then
about 90% of them are suddenly thrown away 5 minutes after boot.  This
results in unnecessary Merkle tree I/O later.

Meanwhile, if the system actually encounters memory pressure, testing
also shows that the shrinker is effective at evicting the buffers.

Other major Linux kernel caches, such as the page cache, do not enforce
a maximum age, instead relying on the shrinker.

For these reasons, Android is now setting max_age_seconds to 86400
(i.e. 1 day), which mostly disables it; see
https://android.googlesource.com/platform/system/core/+/cadad290a79d5b0a30add935aaadab7c1b1ef5e9%5E%21/

That is a much better default, but really the maximum age based eviction
should not exist at all.  Let's remove it.

Note that this also eliminates the need to run work every 30 seconds,
which is beneficial too.

Signed-off-by: Eric Biggers <[email protected]>
Signed-off-by: Mikulas Patocka <[email protected]>

Author: ebiggers

drivers/md/dm-bufio.c

@@ -40,16 +40,6 @@
 #define DM_BUFIO_WRITEBACK_RATIO	3
 #define DM_BUFIO_LOW_WATERMARK_RATIO	16
 
-/*
- * Check buffer ages in this interval (seconds)
- */
-#define DM_BUFIO_WORK_TIMER_SECS	30
-
-/*
- * Free buffers when they are older than this (seconds)
- */
-#define DM_BUFIO_DEFAULT_AGE_SECS	300
-
 /*
  * The nr of bytes of cached data to keep around.
  */
@@ -1055,10 +1045,8 @@ static unsigned long dm_bufio_cache_size_latch;
 
 static DEFINE_SPINLOCK(global_spinlock);
 
-/*
- * Buffers are freed after this timeout
- */
-static unsigned int dm_bufio_max_age = DM_BUFIO_DEFAULT_AGE_SECS;
+static unsigned int dm_bufio_max_age; /* No longer does anything */
+
 static unsigned long dm_bufio_retain_bytes = DM_BUFIO_DEFAULT_RETAIN_BYTES;
 
 static unsigned long dm_bufio_peak_allocated;
@@ -1086,7 +1074,6 @@ static LIST_HEAD(dm_bufio_all_clients);
 static DEFINE_MUTEX(dm_bufio_clients_lock);
 
 static struct workqueue_struct *dm_bufio_wq;
-static struct delayed_work dm_bufio_cleanup_old_work;
 static struct work_struct dm_bufio_replacement_work;
 
 
@@ -2673,130 +2660,6 @@ EXPORT_SYMBOL_GPL(dm_bufio_set_sector_offset);
 
 /*--------------------------------------------------------------*/
 
-static unsigned int get_max_age_hz(void)
-{
-	unsigned int max_age = READ_ONCE(dm_bufio_max_age);
-
-	if (max_age > UINT_MAX / HZ)
-		max_age = UINT_MAX / HZ;
-
-	return max_age * HZ;
-}
-
-static bool older_than(struct dm_buffer *b, unsigned long age_hz)
-{
-	return time_after_eq(jiffies, READ_ONCE(b->last_accessed) + age_hz);
-}
-
-struct evict_params {
-	gfp_t gfp;
-	unsigned long age_hz;
-
-	/*
-	 * This gets updated with the largest last_accessed (ie. most
-	 * recently used) of the evicted buffers.  It will not be reinitialised
-	 * by __evict_many(), so you can use it across multiple invocations.
-	 */
-	unsigned long last_accessed;
-};
-
-/*
- * We may not be able to evict this buffer if IO pending or the client
- * is still using it.
- *
- * And if GFP_NOFS is used, we must not do any I/O because we hold
- * dm_bufio_clients_lock and we would risk deadlock if the I/O gets
- * rerouted to different bufio client.
- */
-static enum evict_result select_for_evict(struct dm_buffer *b, void *context)
-{
-	struct evict_params *params = context;
-
-	if (!(params->gfp & __GFP_FS) ||
-	    (static_branch_unlikely(&no_sleep_enabled) && b->c->no_sleep)) {
-		if (test_bit_acquire(B_READING, &b->state) ||
-		    test_bit(B_WRITING, &b->state) ||
-		    test_bit(B_DIRTY, &b->state))
-			return ER_DONT_EVICT;
-	}
-
-	return older_than(b, params->age_hz) ? ER_EVICT : ER_STOP;
-}
-
-static unsigned long __evict_many(struct dm_bufio_client *c,
-				  struct evict_params *params,
-				  int list_mode, unsigned long max_count)
-{
-	unsigned long count;
-	unsigned long last_accessed;
-	struct dm_buffer *b;
-
-	for (count = 0; count < max_count; count++) {
-		b = cache_evict(&c->cache, list_mode, select_for_evict, params);
-		if (!b)
-			break;
-
-		last_accessed = READ_ONCE(b->last_accessed);
-		if (time_after_eq(params->last_accessed, last_accessed))
-			params->last_accessed = last_accessed;
-
-		__make_buffer_clean(b);
-		__free_buffer_wake(b);
-
-		cond_resched();
-	}
-
-	return count;
-}
-
-static void evict_old_buffers(struct dm_bufio_client *c, unsigned long age_hz)
-{
-	struct evict_params params = {.gfp = 0, .age_hz = age_hz, .last_accessed = 0};
-	unsigned long retain = get_retain_buffers(c);
-	unsigned long count;
-	LIST_HEAD(write_list);
-
-	dm_bufio_lock(c);
-
-	__check_watermark(c, &write_list);
-	if (unlikely(!list_empty(&write_list))) {
-		dm_bufio_unlock(c);
-		__flush_write_list(&write_list);
-		dm_bufio_lock(c);
-	}
-
-	count = cache_total(&c->cache);
-	if (count > retain)
-		__evict_many(c, &params, LIST_CLEAN, count - retain);
-
-	dm_bufio_unlock(c);
-}
-
-static void cleanup_old_buffers(void)
-{
-	unsigned long max_age_hz = get_max_age_hz();
-	struct dm_bufio_client *c;
-
-	mutex_lock(&dm_bufio_clients_lock);
-
-	__cache_size_refresh();
-
-	list_for_each_entry(c, &dm_bufio_all_clients, client_list)
-		evict_old_buffers(c, max_age_hz);
-
-	mutex_unlock(&dm_bufio_clients_lock);
-}
-
-static void work_fn(struct work_struct *w)
-{
-	cleanup_old_buffers();
-
-	queue_delayed_work(dm_bufio_wq, &dm_bufio_cleanup_old_work,
-			   DM_BUFIO_WORK_TIMER_SECS * HZ);
-}
-
-/*--------------------------------------------------------------*/
-
 /*
  * Global cleanup tries to evict the oldest buffers from across _all_
  * the clients.  It does this by repeatedly evicting a few buffers from
@@ -2834,27 +2697,51 @@ static void __insert_client(struct dm_bufio_client *new_client)
 	list_add_tail(&new_client->client_list, h);
 }
 
+static enum evict_result select_for_evict(struct dm_buffer *b, void *context)
+{
+	/* In no-sleep mode, we cannot wait on IO. */
+	if (static_branch_unlikely(&no_sleep_enabled) && b->c->no_sleep) {
+		if (test_bit_acquire(B_READING, &b->state) ||
+		    test_bit(B_WRITING, &b->state) ||
+		    test_bit(B_DIRTY, &b->state))
+			return ER_DONT_EVICT;
+	}
+	return ER_EVICT;
+}
+
 static unsigned long __evict_a_few(unsigned long nr_buffers)
 {
-	unsigned long count;
 	struct dm_bufio_client *c;
-	struct evict_params params = {
-		.gfp = GFP_KERNEL,
-		.age_hz = 0,
-		/* set to jiffies in case there are no buffers in this client */
-		.last_accessed = jiffies
-	};
+	unsigned long oldest_buffer = jiffies;
+	unsigned long last_accessed;
+	unsigned long count;
+	struct dm_buffer *b;
 
 	c = __pop_client();
 	if (!c)
 		return 0;
 
 	dm_bufio_lock(c);
-	count = __evict_many(c, &params, LIST_CLEAN, nr_buffers);
+
+	for (count = 0; count < nr_buffers; count++) {
+		b = cache_evict(&c->cache, LIST_CLEAN, select_for_evict, NULL);
+		if (!b)
+			break;
+
+		last_accessed = READ_ONCE(b->last_accessed);
+		if (time_after_eq(oldest_buffer, last_accessed))
+			oldest_buffer = last_accessed;
+
+		__make_buffer_clean(b);
+		__free_buffer_wake(b);
+
+		cond_resched();
+	}
+
 	dm_bufio_unlock(c);
 
 	if (count)
-		c->oldest_buffer = params.last_accessed;
+		c->oldest_buffer = oldest_buffer;
 	__insert_client(c);
 
 	return count;
@@ -2937,10 +2824,7 @@ static int __init dm_bufio_init(void)
 	if (!dm_bufio_wq)
 		return -ENOMEM;
 
-	INIT_DELAYED_WORK(&dm_bufio_cleanup_old_work, work_fn);
 	INIT_WORK(&dm_bufio_replacement_work, do_global_cleanup);
-	queue_delayed_work(dm_bufio_wq, &dm_bufio_cleanup_old_work,
-			   DM_BUFIO_WORK_TIMER_SECS * HZ);
 
 	return 0;
 }
@@ -2952,7 +2836,6 @@ static void __exit dm_bufio_exit(void)
 {
 	int bug = 0;
 
-	cancel_delayed_work_sync(&dm_bufio_cleanup_old_work);
 	destroy_workqueue(dm_bufio_wq);
 
 	if (dm_bufio_client_count) {
@@ -2989,7 +2872,7 @@ module_param_named(max_cache_size_bytes, dm_bufio_cache_size, ulong, 0644);
 MODULE_PARM_DESC(max_cache_size_bytes, "Size of metadata cache");
 
 module_param_named(max_age_seconds, dm_bufio_max_age, uint, 0644);
-MODULE_PARM_DESC(max_age_seconds, "Max age of a buffer in seconds");
+MODULE_PARM_DESC(max_age_seconds, "No longer does anything");
 
 module_param_named(retain_bytes, dm_bufio_retain_bytes, ulong, 0644);
 MODULE_PARM_DESC(retain_bytes, "Try to keep at least this many bytes cached in memory");