Pages containing buffer_heads that are in one of the per-CPU
buffer_head LRU caches will be pinned and thus cannot be migrated.
This can prevent CMA allocations from succeeding, which are often used
on platforms with co-processors (such as a DSP) that can only use
physically contiguous memory. It can also prevent memory
hot-unplugging from succeeding, which involves migrating at least
MIN_MEMORY_BLOCK_SIZE bytes of memory, which ranges from 8 MiB to 1
GiB based on the architecture in use.
Correspondingly, invalidate the BH LRU caches before a migration
starts and stop any buffer_head from being cached in the LRU caches,
until migration has finished.
Tested-by: Oliver Sang <[email protected]>
Reported-by: kernel test robot <[email protected]>
Signed-off-by: Chris Goldsworthy <[email protected]>
Signed-off-by: Minchan Kim <[email protected]>
---
fs/buffer.c | 36 ++++++++++++++++++++++++++++++------
include/linux/buffer_head.h | 4 ++++
mm/swap.c | 5 ++++-
3 files changed, 38 insertions(+), 7 deletions(-)
diff --git a/fs/buffer.c b/fs/buffer.c
index 0cb7ffd4977c..e9872d0dcbf1 100644
--- a/fs/buffer.c
+++ b/fs/buffer.c
@@ -1264,6 +1264,15 @@ static void bh_lru_install(struct buffer_head *bh)
int i;
check_irqs_on();
+ /*
+ * the refcount of buffer_head in bh_lru prevents dropping the
+ * attached page(i.e., try_to_free_buffers) so it could cause
+ * failing page migration.
+ * Skip putting upcoming bh into bh_lru until migration is done.
+ */
+ if (lru_cache_disabled())
+ return;
+
bh_lru_lock();
b = this_cpu_ptr(&bh_lrus);
@@ -1404,6 +1413,15 @@ __bread_gfp(struct block_device *bdev, sector_t block,
}
EXPORT_SYMBOL(__bread_gfp);
+static void __invalidate_bh_lrus(struct bh_lru *b)
+{
+ int i;
+
+ for (i = 0; i < BH_LRU_SIZE; i++) {
+ brelse(b->bhs[i]);
+ b->bhs[i] = NULL;
+ }
+}
/*
* invalidate_bh_lrus() is called rarely - but not only at unmount.
* This doesn't race because it runs in each cpu either in irq
@@ -1412,16 +1430,12 @@ EXPORT_SYMBOL(__bread_gfp);
static void invalidate_bh_lru(void *arg)
{
struct bh_lru *b = &get_cpu_var(bh_lrus);
- int i;
- for (i = 0; i < BH_LRU_SIZE; i++) {
- brelse(b->bhs[i]);
- b->bhs[i] = NULL;
- }
+ __invalidate_bh_lrus(b);
put_cpu_var(bh_lrus);
}
-static bool has_bh_in_lru(int cpu, void *dummy)
+bool has_bh_in_lru(int cpu, void *dummy)
{
struct bh_lru *b = per_cpu_ptr(&bh_lrus, cpu);
int i;
@@ -1440,6 +1454,16 @@ void invalidate_bh_lrus(void)
}
EXPORT_SYMBOL_GPL(invalidate_bh_lrus);
+void invalidate_bh_lrus_cpu(int cpu)
+{
+ struct bh_lru *b;
+
+ bh_lru_lock();
+ b = per_cpu_ptr(&bh_lrus, cpu);
+ __invalidate_bh_lrus(b);
+ bh_lru_unlock();
+}
+
void set_bh_page(struct buffer_head *bh,
struct page *page, unsigned long offset)
{
diff --git a/include/linux/buffer_head.h b/include/linux/buffer_head.h
index 6b47f94378c5..e7e99da31349 100644
--- a/include/linux/buffer_head.h
+++ b/include/linux/buffer_head.h
@@ -194,6 +194,8 @@ void __breadahead_gfp(struct block_device *, sector_t block, unsigned int size,
struct buffer_head *__bread_gfp(struct block_device *,
sector_t block, unsigned size, gfp_t gfp);
void invalidate_bh_lrus(void);
+void invalidate_bh_lrus_cpu(int cpu);
+bool has_bh_in_lru(int cpu, void *dummy);
struct buffer_head *alloc_buffer_head(gfp_t gfp_flags);
void free_buffer_head(struct buffer_head * bh);
void unlock_buffer(struct buffer_head *bh);
@@ -406,6 +408,8 @@ static inline int inode_has_buffers(struct inode *inode) { return 0; }
static inline void invalidate_inode_buffers(struct inode *inode) {}
static inline int remove_inode_buffers(struct inode *inode) { return 1; }
static inline int sync_mapping_buffers(struct address_space *mapping) { return 0; }
+static inline void invalidate_bh_lrus_cpu(int cpu) {}
+static inline bool has_bh_in_lru(int cpu, void *dummy) { return 0; }
#define buffer_heads_over_limit 0
#endif /* CONFIG_BLOCK */
diff --git a/mm/swap.c b/mm/swap.c
index c94f55e7b649..a75a8265302b 100644
--- 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"
@@ -641,6 +642,7 @@ void lru_add_drain_cpu(int cpu)
pagevec_lru_move_fn(pvec, lru_lazyfree_fn);
activate_page_drain(cpu);
+ invalidate_bh_lrus_cpu(cpu);
}
/**
@@ -828,7 +830,8 @@ inline void __lru_add_drain_all(bool force_all_cpus)
pagevec_count(&per_cpu(lru_pvecs.lru_deactivate_file, cpu)) ||
pagevec_count(&per_cpu(lru_pvecs.lru_deactivate, cpu)) ||
pagevec_count(&per_cpu(lru_pvecs.lru_lazyfree, cpu)) ||
- need_activate_page_drain(cpu)) {
+ need_activate_page_drain(cpu) ||
+ has_bh_in_lru(cpu, NULL)) {
INIT_WORK(work, lru_add_drain_per_cpu);
queue_work_on(cpu, mm_percpu_wq, work);
__cpumask_set_cpu(cpu, &has_work);
--
2.31.0.rc2.261.g7f71774620-goog
On Fri, 19 Mar 2021 10:51:27 -0700 Minchan Kim <[email protected]> wrote:
> Pages containing buffer_heads that are in one of the per-CPU
> buffer_head LRU caches will be pinned and thus cannot be migrated.
> This can prevent CMA allocations from succeeding, which are often used
> on platforms with co-processors (such as a DSP) that can only use
> physically contiguous memory. It can also prevent memory
> hot-unplugging from succeeding, which involves migrating at least
> MIN_MEMORY_BLOCK_SIZE bytes of memory, which ranges from 8 MiB to 1
> GiB based on the architecture in use.
>
> Correspondingly, invalidate the BH LRU caches before a migration
> starts and stop any buffer_head from being cached in the LRU caches,
> until migration has finished.
>
> Tested-by: Oliver Sang <[email protected]>
> Reported-by: kernel test robot <[email protected]>
> Signed-off-by: Chris Goldsworthy <[email protected]>
> Signed-off-by: Minchan Kim <[email protected]>
The signoff chain ordering might mean that Chris was the primary author, but
there is no From:him. Please clarify?
On Sat, Mar 20, 2021 at 09:32:49AM -0700, Andrew Morton wrote:
> On Fri, 19 Mar 2021 10:51:27 -0700 Minchan Kim <[email protected]> wrote:
>
> > Pages containing buffer_heads that are in one of the per-CPU
> > buffer_head LRU caches will be pinned and thus cannot be migrated.
> > This can prevent CMA allocations from succeeding, which are often used
> > on platforms with co-processors (such as a DSP) that can only use
> > physically contiguous memory. It can also prevent memory
> > hot-unplugging from succeeding, which involves migrating at least
> > MIN_MEMORY_BLOCK_SIZE bytes of memory, which ranges from 8 MiB to 1
> > GiB based on the architecture in use.
> >
> > Correspondingly, invalidate the BH LRU caches before a migration
> > starts and stop any buffer_head from being cached in the LRU caches,
> > until migration has finished.
> >
> > Tested-by: Oliver Sang <[email protected]>
> > Reported-by: kernel test robot <[email protected]>
> > Signed-off-by: Chris Goldsworthy <[email protected]>
> > Signed-off-by: Minchan Kim <[email protected]>
>
> The signoff chain ordering might mean that Chris was the primary author, but
> there is no From:him. Please clarify?
He tried first version but was diffrent implementation since I
changed a lot. That's why I added his SoB even though current
implementaion is much different. So, maybe I am primary author?
On Sat, Mar 20, 2021 at 10:20:09AM -0700, Minchan Kim wrote:
> > > Tested-by: Oliver Sang <[email protected]>
> > > Reported-by: kernel test robot <[email protected]>
> > > Signed-off-by: Chris Goldsworthy <[email protected]>
> > > Signed-off-by: Minchan Kim <[email protected]>
> >
> > The signoff chain ordering might mean that Chris was the primary author, but
> > there is no From:him. Please clarify?
>
> He tried first version but was diffrent implementation since I
> changed a lot. That's why I added his SoB even though current
> implementaion is much different. So, maybe I am primary author?
Maybe Chris is Reported-by: ? And don't forget Laura Abbott as original
author of the patch Chris submitted. I think she should be Reported-by:
as well, since there is no code from either of them in this version of
the patch.
On 2021-03-20 12:54, Matthew Wilcox wrote:
> On Sat, Mar 20, 2021 at 10:20:09AM -0700, Minchan Kim wrote:
>> > > Tested-by: Oliver Sang <[email protected]>
>> > > Reported-by: kernel test robot <[email protected]>
>> > > Signed-off-by: Chris Goldsworthy <[email protected]>
>> > > Signed-off-by: Minchan Kim <[email protected]>
>> >
>> > The signoff chain ordering might mean that Chris was the primary author, but
>> > there is no From:him. Please clarify?
>>
>> He tried first version but was diffrent implementation since I
>> changed a lot. That's why I added his SoB even though current
>> implementaion is much different. So, maybe I am primary author?
Hey Minchan, let's have you as the primary author.
> Maybe Chris is Reported-by: ? And don't forget Laura Abbott as
> original
> author of the patch Chris submitted. I think she should be
> Reported-by:
> as well, since there is no code from either of them in this version of
> the patch.
Yes, let's have a Reported-by: from Laura. We can change my
Signed-off-by to Reported-by: as well.
--
The Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora
Forum,
a Linux Foundation Collaborative Project
On Sat, Mar 20, 2021 at 03:51:40PM -0700, Chris Goldsworthy wrote:
> On 2021-03-20 12:54, Matthew Wilcox wrote:
> > On Sat, Mar 20, 2021 at 10:20:09AM -0700, Minchan Kim wrote:
> > > > > Tested-by: Oliver Sang <[email protected]>
> > > > > Reported-by: kernel test robot <[email protected]>
> > > > > Signed-off-by: Chris Goldsworthy <[email protected]>
> > > > > Signed-off-by: Minchan Kim <[email protected]>
> > > >
> > > > The signoff chain ordering might mean that Chris was the primary author, but
> > > > there is no From:him. Please clarify?
> > >
> > > He tried first version but was diffrent implementation since I
> > > changed a lot. That's why I added his SoB even though current
> > > implementaion is much different. So, maybe I am primary author?
>
> Hey Minchan, let's have you as the primary author.
>
> > Maybe Chris is Reported-by: ? And don't forget Laura Abbott as original
> > author of the patch Chris submitted. I think she should be Reported-by:
> > as well, since there is no code from either of them in this version of
> > the patch.
>
> Yes, let's have a Reported-by: from Laura. We can change my Signed-off-by to
> Reported-by: as well.
Thanks Matthew and Chris.
I just wanted to give more credit(something like Co-authored-by if we have
such kinds of tag) to Chris since I felt he had more than. :)
Okay, let's have something like this.
From: Minchan Kim <[email protected]>
..
Tested-by: Oliver Sang <[email protected]>
Reported-by: kernel test robot <[email protected]>
Reported-by: Laura Abbott <[email protected]>
Reported-by: Chris Goldsworthy <[email protected]>
Signed-off-by: Minchan Kim <[email protected]>
Andrew, if you want me resend the patch, let me know.
Thank you.