Re: [PATCH 08/14] libfuse: connect high level fuse library to fuse_reply_attr_iflags

[Bernd Schubert <bschubert@ddn.com>]

On 7/18/25 17:55, Darrick J. Wong wrote:
> On Fri, Jul 18, 2025 at 04:27:50PM +0200, Amir Goldstein wrote:
>> On Fri, Jul 18, 2025 at 1:36 AM Darrick J. Wong <djwong@kernel.org> wrote:
>>>
>>> From: Darrick J. Wong <djwong@kernel.org>
>>>
>>> Create a new ->getattr_iflags function so that iomap filesystems can set
>>> the appropriate in-kernel inode flags on instantiation.
>>>
>>> Signed-off-by: "Darrick J. Wong" <djwong@kernel.org>
> 
> <snip for brevity>
> 
>>> diff --git a/lib/fuse.c b/lib/fuse.c
>>> index 8dbf88877dd37c..685d0181e569d0 100644
>>> --- a/lib/fuse.c
>>> +++ b/lib/fuse.c
>>> @@ -3710,14 +3832,19 @@ static int readdir_fill_from_list(fuse_req_t req, struct fuse_dh *dh,
>>>                         if (de->flags & FUSE_FILL_DIR_PLUS &&
>>>                             !is_dot_or_dotdot(de->name)) {
>>>                                 res = do_lookup(dh->fuse, dh->nodeid,
>>> -                                               de->name, &e);
>>> +                                               de->name, &e, &iflags);
>>>                                 if (res) {
>>>                                         dh->error = res;
>>>                                         return 1;
>>>                                 }
>>>                         }
>>>
>>> -                       thislen = fuse_add_direntry_plus(req, p, rem,
>>> +                       if (f->want_iflags)
>>> +                               thislen = fuse_add_direntry_plus_iflags(req, p,
>>> +                                                        rem, de->name, iflags,
>>> +                                                        &e, pos);
>>> +                       else
>>> +                               thislen = fuse_add_direntry_plus(req, p, rem,
>>>                                                          de->name, &e, pos);
>>
>>
>> All those conditional statements look pretty moot.
>> Can't we just force iflags to 0 if (!f->want_iflags)
>> and always call the *_iflags functions?
> 
> Heh, it already is zero, so yes, this could be a straight call to
> fuse_add_direntry_plus_iflags without the want_iflags check.  Will fix
> up this and the other thing you mentioned in the previous patch.
> 
> Thanks for the code review!
> 
> Having said that, the significant difficulties with iomap and the
> upper level fuse library still exist.  To summarize -- upper libfuse has
> its own nodeids which don't necssarily correspond to the filesystem's,
> and struct node/nodeid are duplicated for hardlinked files.  As a
> result, the kernel has multiple struct inodes for an ondisk ext4 inode,
> which completely breaks the locking for the iomap file IO model.
> 
> That forces me to port fuse2fs to the lowlevel library, so I might
> remove the lib/fuse.c patches entirely.  Are there plans to make the
> upper libfuse handle hardlinks better?

I don't have plans for high level improvements. To be honest, I didn't
know about the hard link issue at all. 
Also a bit surprising to see all your lowlevel work and then fuse high
level coming ;)

Btw, I will go on vacation on Wednesday and still other things queued,
going to try to review in the evenings (but not before next Saturday).



Cheers,
Bernd