On Mon, 1 Feb 2021 22:55:47 -0800 Chris Goldsworthy <cgoldswo@xxxxxxxxxxxxxx> 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.
> --- 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.
>
> ..
>
> --- 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.
> --- 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.
