Re: [PATCH] [RFC] mm: fs: Invalidate BH LRU during page migration
From: Andrew Morton <akpm@linux-foundation.org>
Date: 2021-02-02 20:07:40
Also in:
linux-fsdevel, lkml
On Mon, 1 Feb 2021 22:55:47 -0800 Chris Goldsworthy [off-list ref] wrote:
Pages containing buffer_heads that are in the buffer_head LRU cache will be pinned and thus cannot be migrated. Correspondingly, invalidate the BH LRU before a migration starts and stop any buffer_head from being cached in the LRU, until migration has finished.
It's 16 pages max, system-wide. That isn't much. Please include here a full description of why this is a problem and how serious it is and of the user-visible impact.
quoted hunk ↗ jump to hunk
--- a/fs/buffer.c +++ b/fs/buffer.c@@ -1289,6 +1289,8 @@ static inline void check_irqs_on(void) #endif } +bool bh_migration_done = true; + /* * Install a buffer_head into this cpu's LRU. If not already in the LRU, it is * inserted at the front, and the buffer_head at the back if any is evicted.@@ -1303,6 +1305,9 @@ static void bh_lru_install(struct buffer_head *bh) check_irqs_on(); bh_lru_lock(); + if (!bh_migration_done) + goto out; + b = this_cpu_ptr(&bh_lrus); for (i = 0; i < BH_LRU_SIZE; i++) { swap(evictee, b->bhs[i]);
Seems a bit hacky, but I guess it'll work. I expect the code won't compile with CONFIG_BLOCK=n && CONFIG_MIGRATION=y. Due to bh_migration_done being unimplemented. I suggest you add an interface function (buffer_block_lrus()?) and arrange for an empty inlined stub version when CONFIG_BLOCK=n.
quoted hunk ↗ jump to hunk
..--- a/mm/migrate.c +++ b/mm/migrate.c@@ -64,6 +64,19 @@ */ void migrate_prep(void) { + bh_migration_done = false; + + /* + * This barrier ensures that callers of bh_lru_install() between + * the barrier and the call to invalidate_bh_lrus() read + * bh_migration_done() as false. + */ + /* + * TODO: Remove me? lru_add_drain_all() already has an smp_mb(), + * but it would be good to ensure that the barrier isn't forgotten. + */ + smp_mb();
This stuff can be taken care of over in buffer_block_lrus() in fs/buffer.c.
quoted hunk ↗ jump to hunk
--- a/mm/swap.c +++ b/mm/swap.c@@ -36,6 +36,7 @@ #include <linux/hugetlb.h> #include <linux/page_idle.h> #include <linux/local_lock.h> +#include <linux/buffer_head.h> #include "internal.h"@@ -759,6 +760,8 @@ void lru_add_drain_all(void) if (WARN_ON(!mm_percpu_wq)) return; + invalidate_bh_lrus(); +
Add a comment explaining why we're doing this? Mention that bn_lru buffers can pin pages, preventing migration.