On Fri, Sep 12, 2025 at 3:56 PM Joanne Koong [off-list ref] wrote:

On Thu, Sep 11, 2025 at 9:11 PM Gao Xiang [off-list ref] wrote:

quoted

On 2025/9/12 09:09, Gao Xiang wrote:

quoted

On 2025/9/12 08:06, Gao Xiang wrote:

quoted

On 2025/9/12 03:45, Joanne Koong wrote:

quoted

On Thu, Sep 11, 2025 at 8:29 AM Gao Xiang [off-list ref] wrote:

quoted

quoted

But if FUSE or some other fs later needs to request L2P information
in their .iomap_begin() and need to send L2P requests to userspace
daemon to confirm where to get the physical data (maybe somewhat
like Darrick's work but I don't have extra time to dig into that
either) rather than just something totally bypass iomap-L2P logic
as above, then I'm not sure the current `iomap_iter->private` is
quite seperate to `struct iomap_read_folio_ctx->private`, it seems

If in the future this case arises, the L2P mapping info is accessible
by the read callback in the current design. `.read_folio_range()`
passes the iomap iter to the filesystem and they can access
iter->private to get the L2P mapping data they need.

The question is what exposes to `iter->private` then, take
an example:

struct file *file;

your .read_folio_range() needs `file->private_data` to get
`struct fuse_file` so `file` is kept into
`struct iomap_read_folio_ctx`.

If `file->private_data` will be used for `.iomap_begin()`
as well, what's your proposal then?

Duplicate the same `file` pointer in both
`struct iomap_read_folio_ctx` and `iter->private` context?

It's just an not-so-appropriate example because
`struct file *` and `struct fuse_file *` are widely used
in the (buffer/direct) read/write flow but Darrick's work
doesn't use `file` in .iomap_{begin/end}.

But you may find out `file` pointer is already used for
both FUSE buffer write and your proposal, e.g.

buffer write:
  /*
   * Use iomap so that we can do granular uptodate reads
   * and granular dirty tracking for large folios.
   */
  written = iomap_file_buffered_write(iocb, from,
                                      &fuse_iomap_ops,
                                      &fuse_iomap_write_ops,
                                      file);

And your buffer write per-fs context seems just use
`iter->private` entirely instead to keep `file`.

I don’t think the iomap buffered writes interface is good to use as a
model. I looked a bit at some of the other iomap file operations and I
think we should just pass operation-specific data through an
operation-specific context for those too, eg for buffered writes and
dio modifying the interface from

ssize_t iomap_file_buffered_write(struct kiocb *iocb, struct iov_iter
*from, const struct iomap_ops *ops, const struct iomap_write_ops
*write_ops, void *private);
ssize_t iomap_dio_rw(struct kiocb *iocb, struct iov_iter *iter, const
struct iomap_ops *ops, const struct iomap_dio_ops *dops, unsigned int
dio_flags, void *private, size_t done_before);

to something like

ssize_t iomap_file_buffered_write(const struct iomap_ops *ops, struct
iomap_write_folio_ctx *ctx);
ssize_t iomap_dio_rw(const struct iomap_ops *ops, struct iomap_dio_ctx *ctx);

There’s one filesystem besides fuse that uses “iter->private” and
that’s for xfs zoned inodes (xfs_zoned_buffered_write_iomap_begin()),
which passes the  struct xfs_zone_alloc_ctx*  through iter->private,
and it's used afaict for tracking block reservations. imo that's what
iter->private should be used for, to track the more high level
metadata stuff and then the lower-level details that are
operation-specific go through the ctx->data fields. That seems the
cleanest design to me. I think we should rename the iter->private
field to something like "iter->metadata" to make that delineation more
clear.  I'm not sure what the iomap maintainers think, but that is my
opinion.

I think if in the future there is a case/feature which needs something
previously in one of the operation-specific ctxes, it seems fine to me
to have both iter->private and ctx->data point to the same thing.


Thanks,
Joanne

quoted

quoted

I just try to say if there is a case/feature which needs
something previously in `struct iomap_read_folio_ctx` to
be available in .iomap_{begin,end} too, you have to either:
  - duplicate this in `iter->private` as well;
  - move this to `iter->private` entirely.

The problem is that both `iter->private` and
`struct iomap_read_folio_ctx` are filesystem-specific,
I can only see there is no clear boundary to leave something
in which one.  It seems just like an artificial choice.

Thanks,
Gao Xiang

quoted

quoted

quoted

both needs fs-specific extra contexts for the same I/O flow.

I think the reason why `struct iomap_read_folio_ctx->private` is
introduced is basically previous iomap filesystems are all
bio-based, and they shares `bio` concept in common but
`iter->private` was not designed for this usage.

But fuse `struct iomap_read_folio_ctx` and
`struct fuse_fill_read_data` are too FUSE-specific, I cannot
see it could be shared by other filesystems in the near future,
which is much like a single-filesystem specific concept, and
unlike to `bio` at all.

Currently fuse is the only non-block-based filesystem using iomap but
I don't see why there wouldn't be more in the future. For example,
while looking at some of the netfs code, a lot of the core
functionality looks the same between that and iomap and I think it
might be a good idea to have netfs in the future use iomap's interface
so that it can get the large folio dirty/uptodate tracking stuff and
any other large folio stuff like more granular writeback stats
accounting for free.

I think you need to ask David on this idea, I've told him to
switch fscache to use iomap in 2022 before netfs is fully out [1],
but I don't see it will happen.

[1] https://lore.kernel.org/linux-fsdevel/YfivxC9S52FlyKoL@B-P7TQMD6M-0146/ (local)

(sorry, just saw this part of the email otherwise I would have
included this in the previous message)

Thanks for the link to the thread. My understanding is that the large
folio optimizations stuff was added to iomap in July 2023 (afaict from
the git history) and iomap is entangled with the block layer but it's
becoming more of a generic interface now. Maybe now it makes sense to
go through iomap's interface than it did in 2022, but of course David
has the most context on this.


Thanks,
Joanne

quoted

quoted

quoted

Thanks,
Gao Xiang