Re: [RFC PATCH 2/5] mm: add a_ops->dirty_folio_range() and use the mkclean dirty harvest

[Kiryl Shutsemau]

On Wed, Sep 09, 2026 at 03:51:56AM -0700, Usama Arif wrote:
> On Thu,  3 Sep 2026 19:29:40 +0100 Kiryl Shutsemau <kirill@xxxxxxxxxxxxx> wrote:
> 
> > From: "Kiryl Shutsemau (Meta)" <kas@xxxxxxxxxx>
> > 
> > Every way of dirtying part of a folio through a mapping ends up at
> > folio_mark_dirty(), which has no way to say which part changed, so
> > a_ops->dirty_folio() dirties all of it. A filesystem that tracks dirty
> > state per block then writes back the whole folio for a single stored
> > byte.
> > 
> > Add a_ops->dirty_folio_range() and folio_mark_dirty_range() to pass the
> > range on. The new operation can express everything a_ops->dirty_folio()
> > can, so folio_mark_dirty() goes through it with a range covering the
> > folio and a filesystem needs only one of the two. Filesystems without it
> > dirty the whole folio.
> > 
> > Use it in folio_clear_dirty_for_io(), where the page table dirty bits
> > were being turned into a whole-folio dirty. It now collects them with
> > folio_mkclean_dirtymap() and hands the filesystem the runs that were
> > dirty. A folio that is not already dirty still dirties whole, because
> > the clean to dirty transition needs the accounting in folio_mark_dirty().
> > 
> > Assisted-by: Claude:claude-opus-5
> > Signed-off-by: Kiryl Shutsemau (Meta) <kas@xxxxxxxxxx>
> > ---
> >  include/linux/fs.h  |  3 ++
> >  include/linux/mm.h  |  1 +
> >  mm/page-writeback.c | 87 +++++++++++++++++++++++++++++++++++++++++----
> >  3 files changed, 85 insertions(+), 6 deletions(-)
> > 
> > diff --git a/include/linux/fs.h b/include/linux/fs.h
> > index 072d8cd09a0b..1d98b6c0b880 100644
> > --- a/include/linux/fs.h
> > +++ b/include/linux/fs.h
> > @@ -406,6 +406,9 @@ struct address_space_operations {
> >  
> >  	/* Mark a folio dirty.  Return true if this dirtied it */
> >  	bool (*dirty_folio)(struct address_space *, struct folio *);
> > +	/* Mark [off, off + len) of a folio dirty */
> > +	bool (*dirty_folio_range)(struct address_space *mapping,
> > +				  struct folio *folio, size_t off, size_t len);
> >  
> >  	void (*readahead)(struct readahead_control *);
> >  
> > diff --git a/include/linux/mm.h b/include/linux/mm.h
> > index 87feaa5a2b78..7628262c17e1 100644
> > --- a/include/linux/mm.h
> > +++ b/include/linux/mm.h
> > @@ -3337,6 +3337,7 @@ struct kvec;
> >  struct page *get_dump_page(unsigned long addr, int *locked);
> >  
> >  bool folio_mark_dirty(struct folio *folio);
> > +bool folio_mark_dirty_range(struct folio *folio, size_t off, size_t len);
> >  bool folio_mark_dirty_lock(struct folio *folio);
> >  bool set_page_dirty(struct page *page);
> >  int set_page_dirty_lock(struct page *page);
> > diff --git a/mm/page-writeback.c b/mm/page-writeback.c
> > index 6c9c7ba89b8a..39b54c25a9fa 100644
> > --- a/mm/page-writeback.c
> > +++ b/mm/page-writeback.c
> > @@ -2751,6 +2751,21 @@ bool folio_redirty_for_writepage(struct writeback_control *wbc,
> >  }
> >  EXPORT_SYMBOL(folio_redirty_for_writepage);
> >  
> > +/*
> > + * Hand a dirtied range of @folio to the filesystem.  ->dirty_folio_range() can
> > + * express everything ->dirty_folio() can, so a filesystem that implements it
> > + * does not need both, and a whole-folio dirty comes through here as a range
> > + * covering the folio.
> > + */
> > +static bool mapping_dirty_range(struct address_space *mapping,
> > +				struct folio *folio, size_t off, size_t len)
> > +{
> > +	if (!mapping->a_ops->dirty_folio_range)
> > +		return mapping->a_ops->dirty_folio(mapping, folio);
> > +
> > +	return mapping->a_ops->dirty_folio_range(mapping, folio, off, len);
> > +}
> > +
> >  /**
> >   * folio_mark_dirty - Mark a folio as being modified.
> >   * @folio: The folio.
> > @@ -2782,13 +2797,39 @@ bool folio_mark_dirty(struct folio *folio)
> >  		 */
> >  		if (folio_test_reclaim(folio))
> >  			folio_clear_reclaim(folio);
> > -		return mapping->a_ops->dirty_folio(mapping, folio);
> > +		return mapping_dirty_range(mapping, folio, 0,
> > +					   folio_size(folio));
> >  	}
> >  
> >  	return noop_dirty_folio(mapping, folio);
> >  }
> >  EXPORT_SYMBOL(folio_mark_dirty);
> >  
> > +/**
> > + * folio_mark_dirty_range - Mark part of a folio as being modified.
> > + * @folio: The folio.
> > + * @off: Offset of the modified range within the folio.
> > + * @len: Length of the modified range.
> > + *
> > + * Like folio_mark_dirty(), but tells a filesystem that tracks dirty state per
> > + * block that only [@off, @off + @len) changed, so writeback can skip the rest
> > + * of the folio.  Filesystems without that tracking dirty the whole folio.
> > + *
> > + * Return: True if the folio was newly dirtied, false if it was already dirty.
> > + */
> > +bool folio_mark_dirty_range(struct folio *folio, size_t off, size_t len)
> > +{
> > +	struct address_space *mapping = folio_mapping(folio);
> > +
> > +	if (likely(mapping)) {
> > +		if (folio_test_reclaim(folio))
> > +			folio_clear_reclaim(folio);
> > +		return mapping_dirty_range(mapping, folio, off, len);
> > +	}
> > +
> > +	return noop_dirty_folio(mapping, folio);
> > +}
> > +
> >  /*
> >   * folio_mark_dirty() is racy if the caller has no reference against
> >   * folio->mapping->host, and if the folio is unlocked.  This is because another
> > @@ -2844,6 +2885,41 @@ void __folio_cancel_dirty(struct folio *folio)
> >  }
> >  EXPORT_SYMBOL(__folio_cancel_dirty);
> >  
> > +/*
> > + * Write-protect every mapping of @folio and hand the filesystem the parts that
> > + * were dirty in a page table.
> > + *
> > + * Without ->dirty_folio_range() there is nowhere to put per-block state, so
> > + * any PTE dirty bit dirties the whole folio.  Same when the folio is not
> > + * already dirty, because then the dirty transition needs the full accounting
> > + * in folio_mark_dirty(), and for a folio too large for the bitmap, which the
> > + * page cache does not make.
> > + */
> > +static void folio_mkclean_for_io(struct folio *folio,
> > +				 struct address_space *mapping)
> > +{
> > +	DECLARE_BITMAP(map, 1UL << MAX_PAGECACHE_ORDER);
> > +	unsigned int nr = folio_nr_pages(folio);
> > +	unsigned int start, end;
> > +
> > +	if (!mapping->a_ops->dirty_folio_range || !folio_test_dirty(folio) ||
> > +	    WARN_ON_ONCE(nr > (1UL << MAX_PAGECACHE_ORDER))) {
> > +		if (folio_mkclean(folio))
> > +			folio_mark_dirty(folio);
> > +		return;
> > +	}
> > +
> > +	bitmap_zero(map, nr);
> > +	if (!folio_mkclean_dirtymap(folio, map))
> > +		return;
> > +
> > +	for_each_set_bitrange(start, end, map, nr) {
> > +		mapping_dirty_range(mapping, folio,
> > +				    (size_t)start << PAGE_SHIFT,
> > +				    (size_t)(end - start) << PAGE_SHIFT);
> 
> folio_mark_dirty() clears PG_reclaim, but mapping_dirty_range() doesnt.
> Do you need to clear PG_reclaim here?

Yes, good catch.

Fixup below moves the clearing into mapping_dirty_range(), which is the
one place that reaches the aops, and drops the copies in
folio_mark_dirty() and folio_mark_dirty_range().

Will be folded into v2.

diff --git a/mm/page-writeback.c b/mm/page-writeback.c
index 39b54c25a9fa..4995219eb8be 100644
--- a/mm/page-writeback.c
+++ b/mm/page-writeback.c
@@ -2760,6 +2760,20 @@ EXPORT_SYMBOL(folio_redirty_for_writepage);
 static bool mapping_dirty_range(struct address_space *mapping,
 				struct folio *folio, size_t off, size_t len)
 {
+	/*
+	 * readahead/folio_deactivate could remain
+	 * PG_readahead/PG_reclaim due to race with folio_end_writeback
+	 * About readahead, if the folio is written, the flags would be
+	 * reset. So no problem.
+	 * About folio_deactivate, if the folio is redirtied,
+	 * the flag will be reset. So no problem. but if the
+	 * folio is used by readahead it will confuse readahead
+	 * and make it restart the size rampup process. But it's
+	 * a trivial problem.
+	 */
+	if (folio_test_reclaim(folio))
+		folio_clear_reclaim(folio);
+
 	if (!mapping->a_ops->dirty_folio_range)
 		return mapping->a_ops->dirty_folio(mapping, folio);
 
@@ -2784,19 +2798,6 @@ bool folio_mark_dirty(struct folio *folio)
 	struct address_space *mapping = folio_mapping(folio);
 
 	if (likely(mapping)) {
-		/*
-		 * readahead/folio_deactivate could remain
-		 * PG_readahead/PG_reclaim due to race with folio_end_writeback
-		 * About readahead, if the folio is written, the flags would be
-		 * reset. So no problem.
-		 * About folio_deactivate, if the folio is redirtied,
-		 * the flag will be reset. So no problem. but if the
-		 * folio is used by readahead it will confuse readahead
-		 * and make it restart the size rampup process. But it's
-		 * a trivial problem.
-		 */
-		if (folio_test_reclaim(folio))
-			folio_clear_reclaim(folio);
 		return mapping_dirty_range(mapping, folio, 0,
 					   folio_size(folio));
 	}
@@ -2821,11 +2822,8 @@ bool folio_mark_dirty_range(struct folio *folio, size_t off, size_t len)
 {
 	struct address_space *mapping = folio_mapping(folio);
 
-	if (likely(mapping)) {
-		if (folio_test_reclaim(folio))
-			folio_clear_reclaim(folio);
+	if (likely(mapping))
 		return mapping_dirty_range(mapping, folio, off, len);
-	}
 
 	return noop_dirty_folio(mapping, folio);
 }
-- 
  Kiryl Shutsemau / Kirill A. Shutemov