| Message ID | 20210319175127.886124-3-minchan@kernel.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v4,1/3] mm: disable LRU pagevec during the migration temporarily | expand |
On Fri, 19 Mar 2021 10:51:27 -0700 Minchan Kim <minchan@kernel.org> 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 <oliver.sang@intel.com> > Reported-by: kernel test robot <oliver.sang@intel.com> > Signed-off-by: Chris Goldsworthy <cgoldswo@codeaurora.org> > Signed-off-by: Minchan Kim <minchan@kernel.org> 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 <minchan@kernel.org> 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 <oliver.sang@intel.com> > > Reported-by: kernel test robot <oliver.sang@intel.com> > > Signed-off-by: Chris Goldsworthy <cgoldswo@codeaurora.org> > > Signed-off-by: Minchan Kim <minchan@kernel.org> > > 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 <oliver.sang@intel.com> > > > Reported-by: kernel test robot <oliver.sang@intel.com> > > > Signed-off-by: Chris Goldsworthy <cgoldswo@codeaurora.org> > > > Signed-off-by: Minchan Kim <minchan@kernel.org> > > > > 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 <oliver.sang@intel.com> >> > > Reported-by: kernel test robot <oliver.sang@intel.com> >> > > Signed-off-by: Chris Goldsworthy <cgoldswo@codeaurora.org> >> > > Signed-off-by: Minchan Kim <minchan@kernel.org> >> > >> > 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.
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 <oliver.sang@intel.com> > > > > > Reported-by: kernel test robot <oliver.sang@intel.com> > > > > > Signed-off-by: Chris Goldsworthy <cgoldswo@codeaurora.org> > > > > > Signed-off-by: Minchan Kim <minchan@kernel.org> > > > > > > > > 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 <minchan@kernel.org> .. Tested-by: Oliver Sang <oliver.sang@intel.com> Reported-by: kernel test robot <oliver.sang@intel.com> Reported-by: Laura Abbott <lauraa@codeaurora.org> Reported-by: Chris Goldsworthy <cgoldswo@codeaurora.org> Signed-off-by: Minchan Kim <minchan@kernel.org> Andrew, if you want me resend the patch, let me know. Thank you.
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);