Messages in this thread | | | Date | Wed, 8 May 2024 14:36:26 +0200 | Subject | Re: [PATCH 2/4] mm/ksm: fix ksm_zero_pages accounting | From | David Hildenbrand <> |
| |
On 08.05.24 11:55, Chengming Zhou wrote: > We normally ksm_zero_pages++ in ksmd when page is merged with zero page, > but ksm_zero_pages-- is done from page tables side, which can't protected > by the ksmd mutex. > > So we can read very exceptional value of ksm_zero_pages in rare cases, > such as -1, which is very confusing to users. > > Fix it by changing to use atomic_long_t, and the same case with the > mm->ksm_zero_pages. > > Signed-off-by: Chengming Zhou <chengming.zhou@linux.dev> > --- > fs/proc/base.c | 2 +- > include/linux/ksm.h | 22 +++++++++++++++++++--- > include/linux/mm_types.h | 2 +- > mm/ksm.c | 11 +++++------ > 4 files changed, 26 insertions(+), 11 deletions(-) > > diff --git a/fs/proc/base.c b/fs/proc/base.c > index 18550c071d71..72a1acd03675 100644 > --- a/fs/proc/base.c > +++ b/fs/proc/base.c > @@ -3214,7 +3214,7 @@ static int proc_pid_ksm_stat(struct seq_file *m, struct pid_namespace *ns, > mm = get_task_mm(task); > if (mm) { > seq_printf(m, "ksm_rmap_items %lu\n", mm->ksm_rmap_items); > - seq_printf(m, "ksm_zero_pages %lu\n", mm->ksm_zero_pages); > + seq_printf(m, "ksm_zero_pages %ld\n", mm_ksm_zero_pages(mm)); > seq_printf(m, "ksm_merging_pages %lu\n", mm->ksm_merging_pages); > seq_printf(m, "ksm_process_profit %ld\n", ksm_process_profit(mm)); > mmput(mm); > diff --git a/include/linux/ksm.h b/include/linux/ksm.h > index 52c63a9c5a9c..bfc2cf756b0d 100644 > --- a/include/linux/ksm.h > +++ b/include/linux/ksm.h > @@ -33,16 +33,32 @@ void __ksm_exit(struct mm_struct *mm); > */ > #define is_ksm_zero_pte(pte) (is_zero_pfn(pte_pfn(pte)) && pte_dirty(pte)) > > -extern unsigned long ksm_zero_pages; > +extern atomic_long_t ksm_zero_pages; > + > +static inline void ksm_map_zero_page(struct mm_struct *mm) > +{ > + atomic_long_inc(&ksm_zero_pages); > + atomic_long_inc(&mm->ksm_zero_pages); > +} > > static inline void ksm_might_unmap_zero_page(struct mm_struct *mm, pte_t pte) > { > if (is_ksm_zero_pte(pte)) { > - ksm_zero_pages--; > - mm->ksm_zero_pages--; > + atomic_long_dec(&ksm_zero_pages); > + atomic_long_dec(&mm->ksm_zero_pages); > } > } > > +static inline long get_ksm_zero_pages(void) > +{ > + return atomic_long_read(&ksm_zero_pages); > +}
I suggest inlining that one. The naming of the function also is a bit inconsistent staring at the others.
> + > +static inline long mm_ksm_zero_pages(struct mm_struct *mm) > +{ > + return atomic_long_read(&mm->ksm_zero_pages); > +} > +
Apart from that LGTM
Acked-by: David Hildenbrand <david@redhat.com>
-- Cheers,
David / dhildenb
| |