[RFC,0/4] iomap: zero range folio batch processing prototype

Message

Brian Foster Nov. 19, 2024, 3:46 p.m. UTC

Hi all,

Here's a first pass prototype of dirty folio batch processing for zero
range. The idea here is that the fs can avoid pagecache flushing or
wonky ordering/racing issues by passing in a batch of dirty folios that
might exist over unwritten mappings at lookup time.

This is only lightly tested so far, but works well enough to demonstrate
the idea. The biggest open question in my mind is how to better define
the interface. For now I just stuffed the folio_batch in the struct
iomap, but I don't really like it there due to confusion between iomap
and srcmap and because it bloats the structure.

I thought about using ->private along with a custom ->get_folio(), but I
don't think that really fits the idea of a built-in mechanism. It might
be more appropriate to attach to the iter, but that currently isn't
accessible to ->iomap_begin(). I suppose we could define an
iomap_to_iter() or some such helper that the fill helper could use to
populate the batch, but maybe there are other thoughts/ideas?

Brian

Brian Foster (4):
  iomap: allow passing a folio into write begin path
  iomap: optional zero range dirty folio processing
  xfs: always trim mapping to requested range for zero range
  xfs: fill dirty folios on zero range of unwritten mappings

 fs/iomap/buffered-io.c | 93 ++++++++++++++++++++++++++++++++++++++----
 fs/iomap/iter.c        |  2 +
 fs/xfs/xfs_iomap.c     | 28 ++++++++-----
 include/linux/iomap.h  |  5 +++
 4 files changed, 109 insertions(+), 19 deletions(-)

Comments

Christoph Hellwig Nov. 20, 2024, 8:37 a.m. UTC | #1

On Tue, Nov 19, 2024 at 10:46:52AM -0500, Brian Foster wrote:
> I thought about using ->private along with a custom ->get_folio(), but I
> don't think that really fits the idea of a built-in mechanism. It might
> be more appropriate to attach to the iter, but that currently isn't
> accessible to ->iomap_begin(). I suppose we could define an
> iomap_to_iter() or some such helper that the fill helper could use to
> populate the batch, but maybe there are other thoughts/ideas?

The iter is the right place, and you can get at it using
container_of as already done by btrfs (and osme of my upcoming code):

	struct iomap_iter *iter = container_of(iomap, struct iomap_iter, iomap);

Brian Foster Nov. 20, 2024, 2:29 p.m. UTC | #2

On Wed, Nov 20, 2024 at 12:37:41AM -0800, Christoph Hellwig wrote:
> On Tue, Nov 19, 2024 at 10:46:52AM -0500, Brian Foster wrote:
> > I thought about using ->private along with a custom ->get_folio(), but I
> > don't think that really fits the idea of a built-in mechanism. It might
> > be more appropriate to attach to the iter, but that currently isn't
> > accessible to ->iomap_begin(). I suppose we could define an
> > iomap_to_iter() or some such helper that the fill helper could use to
> > populate the batch, but maybe there are other thoughts/ideas?
> 
> The iter is the right place, and you can get at it using
> container_of as already done by btrfs (and osme of my upcoming code):
> 
> 	struct iomap_iter *iter = container_of(iomap, struct iomap_iter, iomap);
> 
> 

Ok, yeah.. that's pretty much what I meant by having an iomap_to_iter()
helper, I just wasn't aware that some things were already doing that to
poke at the iter. Thanks.

I'm assuming we'd want this to be a dynamic allocation as well, since
folio_batch is fairly large in comparison (256b compared to 208b
iomap_iter).

Brian

Christoph Hellwig Nov. 21, 2024, 5:50 a.m. UTC | #3

On Wed, Nov 20, 2024 at 09:29:17AM -0500, Brian Foster wrote:
> helper, I just wasn't aware that some things were already doing that to
> poke at the iter. Thanks.
> 
> I'm assuming we'd want this to be a dynamic allocation as well, since
> folio_batch is fairly large in comparison (256b compared to 208b
> iomap_iter).

Or at least only add a pointer to it in the iter and then point to a
separate stack allocation for it.