| Message ID | 20240408194232.118537-12-willy@infradead.org (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | Some cleanups for memory-failure | expand |
On 4/8/2024 12:42 PM, Matthew Wilcox (Oracle) wrote: > We've already calculated it, so pass it in instead of recalculating it > in collect_procs_ksm(). > > Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org> > --- > include/linux/ksm.h | 14 +++----------- > mm/ksm.c | 5 ++--- > mm/memory-failure.c | 2 +- > 3 files changed, 6 insertions(+), 15 deletions(-) > > diff --git a/include/linux/ksm.h b/include/linux/ksm.h > index 358803cfd4d5..52c63a9c5a9c 100644 > --- a/include/linux/ksm.h > +++ b/include/linux/ksm.h > @@ -81,15 +81,9 @@ struct folio *ksm_might_need_to_copy(struct folio *folio, > > void rmap_walk_ksm(struct folio *folio, struct rmap_walk_control *rwc); > void folio_migrate_ksm(struct folio *newfolio, struct folio *folio); > - > -#ifdef CONFIG_MEMORY_FAILURE > -void collect_procs_ksm(struct page *page, struct list_head *to_kill, > - int force_early); > -#endif > - > -#ifdef CONFIG_PROC_FS > +void collect_procs_ksm(struct folio *folio, struct page *page, > + struct list_head *to_kill, int force_early); > long ksm_process_profit(struct mm_struct *); > -#endif /* CONFIG_PROC_FS */ Why is the #ifdef-#endif CONFIG_PROC_FS removed? In ksm.c, ksm_process_profit() is defined within the config switch. > > #else /* !CONFIG_KSM */ > > @@ -120,12 +114,10 @@ static inline void ksm_might_unmap_zero_page(struct mm_struct *mm, pte_t pte) > { > } > > -#ifdef CONFIG_MEMORY_FAILURE > -static inline void collect_procs_ksm(struct page *page, > +static inline void collect_procs_ksm(struct folio *folio, struct page *page, > struct list_head *to_kill, int force_early) > { > } > -#endif > > #ifdef CONFIG_MMU > static inline int ksm_madvise(struct vm_area_struct *vma, unsigned long start, > diff --git a/mm/ksm.c b/mm/ksm.c > index 8c001819cf10..7d032b52002f 100644 > --- a/mm/ksm.c > +++ b/mm/ksm.c > @@ -3172,12 +3172,11 @@ void rmap_walk_ksm(struct folio *folio, struct rmap_walk_control *rwc) > /* > * Collect processes when the error hit an ksm page. > */ > -void collect_procs_ksm(struct page *page, struct list_head *to_kill, > - int force_early) > +void collect_procs_ksm(struct folio *folio, struct page *page, > + struct list_head *to_kill, int force_early) > { > struct ksm_stable_node *stable_node; > struct ksm_rmap_item *rmap_item; > - struct folio *folio = page_folio(page); > struct vm_area_struct *vma; > struct task_struct *tsk; > > diff --git a/mm/memory-failure.c b/mm/memory-failure.c > index 1c3cf1fe4964..0630e34b583b 100644 > --- a/mm/memory-failure.c > +++ b/mm/memory-failure.c > @@ -713,7 +713,7 @@ static void collect_procs(struct folio *folio, struct page *page, > if (!folio->mapping) > return; > if (unlikely(folio_test_ksm(folio))) > - collect_procs_ksm(page, tokill, force_early); > + collect_procs_ksm(folio, page, tokill, force_early); > else if (folio_test_anon(folio)) > collect_procs_anon(folio, page, tokill, force_early); > else thanks, -jane
On Mon, Apr 08, 2024 at 11:27:02PM -0700, Jane Chu wrote: > On 4/8/2024 12:42 PM, Matthew Wilcox (Oracle) wrote: > > +++ b/include/linux/ksm.h > > @@ -81,15 +81,9 @@ struct folio *ksm_might_need_to_copy(struct folio *folio, > > void rmap_walk_ksm(struct folio *folio, struct rmap_walk_control *rwc); > > void folio_migrate_ksm(struct folio *newfolio, struct folio *folio); > > - > > -#ifdef CONFIG_MEMORY_FAILURE > > -void collect_procs_ksm(struct page *page, struct list_head *to_kill, > > - int force_early); > > -#endif > > - > > -#ifdef CONFIG_PROC_FS > > +void collect_procs_ksm(struct folio *folio, struct page *page, > > + struct list_head *to_kill, int force_early); > > long ksm_process_profit(struct mm_struct *); > > -#endif /* CONFIG_PROC_FS */ > Why is the #ifdef-#endif CONFIG_PROC_FS removed? In ksm.c, > ksm_process_profit() is defined within the config switch. Yes, but there's no need to put ifdefs around function declarations. All it does is add a rather bogus dependency on the CONFIG symbol. There's no harm in having a declaration for a function which doesn't exist.
On 4/9/2024 5:11 AM, Matthew Wilcox wrote: > On Mon, Apr 08, 2024 at 11:27:02PM -0700, Jane Chu wrote: >> On 4/8/2024 12:42 PM, Matthew Wilcox (Oracle) wrote: >>> +++ b/include/linux/ksm.h >>> @@ -81,15 +81,9 @@ struct folio *ksm_might_need_to_copy(struct folio *folio, >>> void rmap_walk_ksm(struct folio *folio, struct rmap_walk_control *rwc); >>> void folio_migrate_ksm(struct folio *newfolio, struct folio *folio); >>> - >>> -#ifdef CONFIG_MEMORY_FAILURE >>> -void collect_procs_ksm(struct page *page, struct list_head *to_kill, >>> - int force_early); >>> -#endif >>> - >>> -#ifdef CONFIG_PROC_FS >>> +void collect_procs_ksm(struct folio *folio, struct page *page, >>> + struct list_head *to_kill, int force_early); >>> long ksm_process_profit(struct mm_struct *); >>> -#endif /* CONFIG_PROC_FS */ >> Why is the #ifdef-#endif CONFIG_PROC_FS removed? In ksm.c, >> ksm_process_profit() is defined within the config switch. > Yes, but there's no need to put ifdefs around function declarations. > All it does is add a rather bogus dependency on the CONFIG symbol. > There's no harm in having a declaration for a function which doesn't > exist. I see, sounds good. Reviewed-by: Jane Chu <jane.chu@oracle.com> -jane
diff --git a/include/linux/ksm.h b/include/linux/ksm.h index 358803cfd4d5..52c63a9c5a9c 100644 --- a/include/linux/ksm.h +++ b/include/linux/ksm.h @@ -81,15 +81,9 @@ struct folio *ksm_might_need_to_copy(struct folio *folio, void rmap_walk_ksm(struct folio *folio, struct rmap_walk_control *rwc); void folio_migrate_ksm(struct folio *newfolio, struct folio *folio); - -#ifdef CONFIG_MEMORY_FAILURE -void collect_procs_ksm(struct page *page, struct list_head *to_kill, - int force_early); -#endif - -#ifdef CONFIG_PROC_FS +void collect_procs_ksm(struct folio *folio, struct page *page, + struct list_head *to_kill, int force_early); long ksm_process_profit(struct mm_struct *); -#endif /* CONFIG_PROC_FS */ #else /* !CONFIG_KSM */ @@ -120,12 +114,10 @@ static inline void ksm_might_unmap_zero_page(struct mm_struct *mm, pte_t pte) { } -#ifdef CONFIG_MEMORY_FAILURE -static inline void collect_procs_ksm(struct page *page, +static inline void collect_procs_ksm(struct folio *folio, struct page *page, struct list_head *to_kill, int force_early) { } -#endif #ifdef CONFIG_MMU static inline int ksm_madvise(struct vm_area_struct *vma, unsigned long start, diff --git a/mm/ksm.c b/mm/ksm.c index 8c001819cf10..7d032b52002f 100644 --- a/mm/ksm.c +++ b/mm/ksm.c @@ -3172,12 +3172,11 @@ void rmap_walk_ksm(struct folio *folio, struct rmap_walk_control *rwc) /* * Collect processes when the error hit an ksm page. */ -void collect_procs_ksm(struct page *page, struct list_head *to_kill, - int force_early) +void collect_procs_ksm(struct folio *folio, struct page *page, + struct list_head *to_kill, int force_early) { struct ksm_stable_node *stable_node; struct ksm_rmap_item *rmap_item; - struct folio *folio = page_folio(page); struct vm_area_struct *vma; struct task_struct *tsk; diff --git a/mm/memory-failure.c b/mm/memory-failure.c index 1c3cf1fe4964..0630e34b583b 100644 --- a/mm/memory-failure.c +++ b/mm/memory-failure.c @@ -713,7 +713,7 @@ static void collect_procs(struct folio *folio, struct page *page, if (!folio->mapping) return; if (unlikely(folio_test_ksm(folio))) - collect_procs_ksm(page, tokill, force_early); + collect_procs_ksm(folio, page, tokill, force_early); else if (folio_test_anon(folio)) collect_procs_anon(folio, page, tokill, force_early); else
We've already calculated it, so pass it in instead of recalculating it in collect_procs_ksm(). Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org> --- include/linux/ksm.h | 14 +++----------- mm/ksm.c | 5 ++--- mm/memory-failure.c | 2 +- 3 files changed, 6 insertions(+), 15 deletions(-)