Re: [PATCH] ipc/shm: fix RSS accounting for hugetlb shared memory

[Weilin Tong]

Hi Muchun,

Thanks for the review and suggestion. The simplification makes sense. 
I'll update the patch accordingly and then send v2 after testing.

Thanks,
Weilin


在 2026/10/14 15:25, Muchun Song 写道:
> > On Oct 14, 2026, at 14:27, Weilin Tong <tongweilin@xxxxxxxxxxxxxxxxx> wrote:
> >
> > Commit a08c7193e4f1 ("mm/filemap: remove hugetlb special casing in
> > filemap.c") changed mapping->nrpages to count base pages for hugetlb
> > folios. However, shm_add_rss_swap() still multiplies it by
> > pages_per_huge_page(), overcounting RSS in both SHM_INFO and
> > /proc/sysvipc/shm.
> >
> > Use mapping->nrpages directly and remove the unused hstate variable.
> >
> > Fixes: a08c7193e4f1 ("mm/filemap: remove hugetlb special casing in filemap.c")
> > Signed-off-by: Weilin Tong <tongweilin@xxxxxxxxxxxxxxxxx>
> >
> > ---
> > ipc/shm.c | 3 +--
> > 1 file changed, 1 insertion(+), 2 deletions(-)
> >
> > diff --git a/ipc/shm.c b/ipc/shm.c
> > index 25190965c9a50..5ed2beef0f1d2 100644
> > --- a/ipc/shm.c
> > +++ b/ipc/shm.c
> > @@ -943,8 +943,7 @@ static void shm_add_rss_swap(struct shmid_kernel *shp,
> >
> > 	if (is_file_hugepages(shp->shm_file)) {
> > 		struct address_space *mapping = inode->i_mapping;
> > - 		struct hstate *h = hstate_file(shp->shm_file);
> > - 		*rss_add += pages_per_huge_page(h) * mapping->nrpages;
> > + 		*rss_add += mapping->nrpages;
> Hi Weilin,
>
> Thanks for fixing this. Using mapping->nrpages directly for hugetlb
> folios is correct.
>
> Could you please simplify shm_add_rss_swap() further as shown below?
>
> The info->lock taken here only serializes the read of info->swapped. It
> does not protect mapping->nrpages, which is updated under the page-cache
> XArray lock. Therefore, taking info->lock cannot provide a consistent
> snapshot of nrpages and swapped.
>
> These statistics are inherently best-effort, so READ_ONCE() should be
> sufficient for both fields. This is also consistent with
> shmem_swap_usage(), which reads info->swapped using READ_ONCE() without
> taking info->lock.
>
> With that, the nrpages accounting can be shared by all backing types,
> and only the swapped accounting needs to remain conditional on
> CONFIG_SHMEM and non-hugetlb files.
>
> Please also mention the lockless snapshot change in the commit message.
>
> Thanks,
> Muchun
>
> diff --git a/ipc/shm.c b/ipc/shm.c
> index bb1a721a3e74..6862f681373f 100644
> --- a/ipc/shm.c
> +++ b/ipc/shm.c
> @@ -936,26 +936,11 @@ static inline unsigned long copy_shminfo_to_user(void __user
> *buf, struct shminf
> static void shm_add_rss_swap(struct shmid_kernel *shp,
>       unsigned long *rss_add, unsigned long *swp_add)
> {
> -     struct inode *inode;
> +     struct inode *inode = file_inode(shp->shm_file);
>
> -     inode = file_inode(shp->shm_file);
> -
> -     if (is_file_hugepages(shp->shm_file)) {
> -             struct address_space *mapping = inode->i_mapping;
> -             struct hstate *h = hstate_file(shp->shm_file);
> -             *rss_add += pages_per_huge_page(h) * mapping->nrpages;
> -     } else {
> -#ifdef CONFIG_SHMEM
> -             struct shmem_inode_info *info = SHMEM_I(inode);
> -
> -             spin_lock_irq(&info->lock);
> -             *rss_add += inode->i_mapping->nrpages;
> -             *swp_add += info->swapped;
> -             spin_unlock_irq(&info->lock);
> -#else
> -             *rss_add += inode->i_mapping->nrpages;
> -#endif
> -     }
> +     *rss_add += READ_ONCE(inode->i_mapping->nrpages);
> +     if (IS_ENABLED(CONFIG_SHMEM) && !is_file_hugepages(shp->shm_file))
> +             *swp_add += READ_ONCE(SHMEM_I(inode)->swapped);
> }
>
> > 	} else {
> > #ifdef CONFIG_SHMEM
> > 		struct shmem_inode_info *info = SHMEM_I(inode);
> > --
> > 2.43.5