Re: Removing ->dirty_folio

[Pedro Falcato <pfalcato@suse.de>]

On Mon, Aug 24, 2026 at 08:43:39PM +0100, Matthew Wilcox wrote:
> On Mon, Aug 24, 2026 at 12:25:42PM -0700, John Hubbard wrote:
> > > My proposal is this:
> > > 
> > >  - Fileystems take note of folio_maybe_dma_pinned() during writeback.
> > >    If it's true, do the writeback, but retain/recreate whatever data
> > >    structures you need in order to write the folio again; behave as if
> > >    ->page_mkdirty() had been called again for each page in the folio is
> > >    marked as dirty.
> > 
> > Yes, that would work nicely.
> > 
> > >  - The MM behaves similarly; we do not clear the writeback flag for
> > >    folio_maybe_dma_pinned().
> > > 
> > > This will have the effect of writing pinned folios back every time the
> > > inode is scheduled for writeback.  But since we have no idea whether
> > > the folio is actually dirty (because the GUP user won't tell us),
> > > this is the correct behaviour.
> > > 
> > > I'm probably missing some stuff here.  Let me know.
> > 
> > OK, so working through the end of the pinning, I think it still is
> > correct: device finishes writing to pinned memory, device driver
> > unpins the memory but the page has been left marked dirty the whole
> > time and still is, so the next writeback still does the writeback,
> > but this time sees no pins and so it marks the page clean.
> 
> Excellent!  By the way, what would you think to this?
> 
> @@ -2717,7 +2719,8 @@ static inline bool folio_maybe_dma_pinned(struct folio *folio)
>          * Here, for that overflow case, use the sign bit to count a little
>          * bit higher via unsigned math, and thus still get an accurate result.
>          */
> -       return ((unsigned int)folio_ref_count(folio)) >=
> +       mapcount = folio_mapcount(folio);
> +       return (folio_ref_count(folio) - mapcount) >=
>                 GUP_PIN_COUNTING_BIAS;
>  }
> 
> 
> It should improve the accuracy of folio_maybe_dma_pinned() for folios
> which are mapped many, many times (eg a page of libc).  I'm a little
> concerned about races turning that number negative since we don't
> necessarily have the folio locked at that point.

It's perhaps a radical idea, but how about just not incrementing
refcount for mappings? Essentially, for the main state transitions
(mapped to unmapped, and vice-versa):

0 mapcount -> 1 mapcount: refcount++
1 mapcount -> 0 mapcount: refcount--

and anything else doesn't touch refcount.

I think it's worth considering standalone (one less atomic inc :) and
reduces any ambiguity in folio_maybe_dma_pinned() to essentially 0 (unless
you have 2048 short-term pins for the folio, rather unlikely I would say).

-- 
Pedro