On Sun, Feb 15, 2026 at 07:45:04PM +0200, Mike Rapoport wrote:
> On Wed, Feb 11, 2026 at 03:00:58PM -0500, Peter Xu wrote:
> > On Sun, Feb 08, 2026 at 12:35:43PM +0200, Mike Rapoport wrote:
> > > > > +static void shmem_mfill_filemap_remove(struct folio *folio,
> > > > > +                                    struct vm_area_struct *vma)
> > > > > +{
> > > > > +     struct inode *inode = file_inode(vma->vm_file);
> > > > > +
> > > > > +     filemap_remove_folio(folio);
> > > > > +     shmem_recalc_inode(inode, 0, 0);
> > > > >       folio_unlock(folio);
> > > > > -     folio_put(folio);
> > > > > -out_unacct_blocks:
> > > > > -     shmem_inode_unacct_blocks(inode, 1);
> > > > 
> > > > This looks wrong, or maybe I miss somewhere we did the unacct_blocks()?
> > > 
> > > This is handled by shmem_recalc_inode(inode, 0, 0).
> > 
> > IIUC shmem_recalc_inode() only does the fixup of shmem_inode_info over
> > possiblly changing inode->i_mapping->nrpages.  It's not for reverting the
> > accounting in the failure paths here.
> > 
> > OTOH, we still need to maintain accounting for the rest things with
> > correctly invoke shmem_inode_unacct_blocks().  One thing we can try is
> > testing this series against either shmem quota support (since 2023, IIUC
> > it's relevant to "quota" mount option), or max_blocks accountings (IIUC,
> > "size" mount option), etc.  Any of those should reflect a difference if my
> > understanding is correct.
> > 
> > So IIUC we still need the unacct_blocks(), please kindly help double check.
> 
> I followed shmem_get_folio_gfp() error handling, and unless I missed
> something we should have the same sequence with uffd.
> 
> In shmem_mfill_filemap_add() we increment both i_mapping->nrpages and
> info->alloced in shmem_add_to_page_cache() and 
> shmem_recalc_inode(inode, 1, 0) respectively.
> 
> Then in shmem_filemap_remove() the call to filemap_remove_folio()
> decrements i_mapping->nrpages and shmem_recalc_inode(inode, 0, 0) will see
> freed=1 and will call shmem_inode_unacct_blocks().

You're correct.  I guess I was misleaded by the comments above
shmem_recalc_inode() when reading this part assuming it's only for the
cases where nrpages changed behind the hood.. :)

I believe we need shmem_recalc_inode(inode, 0, 0) to make sure
info->alloced is properly decremented, so shmem_inode_unacct_blocks()
explicit calls will miss that otherwise due to the reordering of shmem
accounting in this patch.

It's slightly tricky on using these functions, I wonder if we want to
mention them in the commit log, but I'm OK either way.

Thanks for double checking!

-- 
Peter Xu


Reply via email to