Hi Christoph,
On 2025/9/11 19:37, Christoph Hellwig wrote:
On Wed, Sep 10, 2025 at 12:59:41PM +0800, Gao Xiang wrote:
At least it sounds better on my side, but anyway it's just
my own overall thought. If other folks have different idea,
I don't have strong opinion, I just need something for my own
as previous said.
I already dropped my two suggestions on the earlier patch. Not totally
happy about either my suggestions or data, but in full agreement that
it should be something else than private.
To just quote your previous comment and try to discuss here:
```
On Wed, Sep 10, 2025 at 01:41:25PM -0400, Joanne Koong wrote:
In my mind, the big question is whether or not the data the
filesystems pass in is logically shared by both iomap_begin/end and
buffered reads/writes/dio callbacks, or whether the data needed by
both are basically separate entities
They are separate entities.
```
I try to push this again because I'm still not quite sure it's
a good idea, let's take this FUSE iomap-read proposal (but sorry
honestly I not fully look into the whole series.)
```
struct fuse_fill_read_data {
struct file *file;
+
+ /*
+ * Fields below are used if sending the read request
+ * asynchronously.
+ */
+ struct fuse_conn *fc;
+ struct fuse_io_args *ia;
+ unsigned int nr_bytes;
};
```
which is just a new FUSE-only-specific context for
`struct iomap_read_folio_ctx`, it's not used by .iomap_{begin,end}
is that basically FUSE _currently_ doesn't have logical-to-physical
mapping requirement (except for another fuse_iomap_begin in dax.c):
```
static int fuse_iomap_begin(struct inode *inode, loff_t offset, loff_t length,
unsigned int flags, struct iomap *iomap,
struct iomap *srcmap)
{
iomap->type = IOMAP_MAPPED;
iomap->length = length;
iomap->offset = offset;
return 0;
}
```
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
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.
I've already racked my brains on this but I have no better
idea on the current callback-hook model (so I don't want to argue
more). Anyway, I really think it should be carefully designed
(because the current FUSE .iomap_{begin,end} are just like no-op
but that is just fuse-specific). If folks really think Joanne's
work is already best or we can live with that, I'm totally fine.
Thanks,
Gao Xiang
