On 25/02/24 03:40PM, Joanne Koong wrote:
> On Mon, Feb 24, 2025 at 7:25 AM John Groves <John@xxxxxxxxxx> wrote:
> >
> > Miklos et. al.:
> >
> > Here are some specific questions related to the famfs port into fuse [1][2]
> > that I hope Miklos (and others) can give me feedback on soonish.
> >
> > This work is active and serious, although you haven't heard much from me
> > recently. I'm showing a famfs poster at Usenix FAST '25 this week [3].
> >
> > I'm generally following the approach in [1] - in a famfs file system,
> > LOOKUP is followed by GET_FMAP to retrieve the famfs file/dax metadata.
> > It's tempting to merge the fmap into the LOOKUP reply, but this seems like
> > an optimization to consider once basic function is established.
> >
> > Q: Do you think it makes sense to make the famfs fmap an optional,
> > variable sized addition to the LOOKUP response?
> >
> > Whenever an fmap references a dax device that isn't already known to the
> > famfs/fuse kernel code, a GET_DAXDEV message is sent, with the reply
> > providing the info required to open teh daxdev. A file becomes available
> > when the fmap is complete and all referenced daxdevs are "opened".
> >
> > Q: Any heartburn here?
> >
> > When GET_FMAP is separate from LOOKUP, READDIRPLUS won't add value unless it
> > receives fmaps as part of the attributes (i.e. lookups) that come back in
> > its response - since a READDIRPLUS that gets 12 files will still need 12
> > GET_FMAP messages/responses to be complete. Merging fmaps as optional,
> > variable-length components of the READDIRPLUS response buffers could
> > eventualy make sense, but a cleaner solution intially would seem to be
> > to disable READDIRPLUS in famfs. But...
> >
>
> Hi John,
>
> > * The libfuse/kernel ABI appears to allow low-level fuse servers that don't
> > support READDIRPLUS...
> > * But libfuse doesn't look at conn->want for the READDIRPLUS related
> > capabilities
> > * I have overridden that, but the kernel still sends the READDIRPLUS
> > messages. It's possible I'm doing something hinky, and I'll keep looking
> > for it.
>
> On the kernel side, FUSE_READDIR / FUSE_READDIRPLUS requests are sent
> in fuse_readdir_uncached(). I don't see anything there that skips
> sending readdir / readdirplus requests if the server doesn't have
> .readdir / .readdirplus implemented. For some request types (eg
> FUSE_RENAME2, FUSE_LINK, FUSE_FSYNCDIR, FUSE_CREATE, ...), we do track
> if a request type isn't implemented by the server and then skip
> sending that request in the future (for example, see fuse_tmpfile()).
> If we wanted to do this skipping for readdir as well, it'd probably
> look something like
>
> --- a/fs/fuse/fuse_i.h
> +++ b/fs/fuse/fuse_i.h
> @@ -870,6 +870,9 @@ struct fuse_conn {
> /* Is link not implemented by fs? */
> unsigned int no_link:1;
>
> + /* Is readdir/readdirplus not implemented by fs? */
> + unsigned int no_readdir:1;
> +
> /* Use io_uring for communication */
> unsigned int io_uring;
>
> diff --git a/fs/fuse/readdir.c b/fs/fuse/readdir.c
> index 17ce9636a2b1..176d6ce953e5 100644
> --- a/fs/fuse/readdir.c
> +++ b/fs/fuse/readdir.c
> @@ -341,6 +341,9 @@ static int fuse_readdir_uncached(struct file
> *file, struct dir_context *ctx)
> u64 attr_version = 0, evict_ctr = 0;
> bool locked;
>
> + if (fm->fc->no_readdir)
> + return -ENOSYS;
> +
> folio = folio_alloc(GFP_KERNEL, 0);
> if (!folio)
> return -ENOMEM;
> @@ -376,6 +379,8 @@ static int fuse_readdir_uncached(struct file
> *file, struct dir_context *ctx)
> res = parse_dirfile(folio_address(folio), res, file,
> ctx);
> }
> + } else if (res == -ENOSYS) {
> + fm->fc->no_readdir = 1;
> }
>
> folio_put(folio);
>
> > * When I just return -ENOSYS to READDIRPLUS, things don't work well. Still
> > looking into this.
> >
> > Q: Do you know whether the current fuse kernel mod can handle a low-level
> > fuse server that doesn't support READDIRPLUS? This may be broken.
>
> From what I see, the fuse kernel code can handle servers that don't
> support readdir/readdirplus. The fuse readdir path gets invoked from
> file_operations->iterate_shared callback, which from what i see, the
> only ramification of this always returning an error is that the
> syscalls calling into this (eg getdents(), readdir()) fail.
Thanks for doing some of the digging Joanne. I'm not sure what was going wrong
a few weeks ago when I initially disabled readdirplus, but I have it working
now - in fact I now have famfs "fully" working under fuse (for some definition
of fully ;).
I have a gnarly rebase plus some documentation to write, but I may go ahead
and share branches this week, in case anybody wants to take a look. That will
be a fuse kernel branch, and a famfs user space branch...
Also: any plans for a fuse BOF at LSFMM?
<snip>
Thanks,
John
