[Adding 9p guys to CC]
On Wed 25-09-24 12:51:38, Amir Goldstein wrote:
> On Wed, Sep 25, 2024 at 10:18 AM Jan Kara <jack@xxxxxxx> wrote:
> > On Wed 25-09-24 10:11:46, Jan Kara wrote:
> > > On Tue 24-09-24 12:07:51, Krishna Vivek Vitta wrote:
> > > > Please ignore the last line.
> > > > Git clone operation is failing with fanotify example code as well.
> > > >
> > > > root@MININT-S244RA7:/mnt/c/Users/kvitta/Desktop/MDE binaries/GitCloneIssue# ./fanotify_ex /mnt/c
> > > > Press enter key to terminate.
> > > > root@MININT-S244RA7:/mnt/c/Users/kvitta/Desktop/MDE binaries/GitCloneIssue# ./fanotify_ex /mnt/c
> > > > Press enter key to terminate.
> > > > Listening for events.
> > > > FAN_OPEN_PERM: File /mnt/c/Users/kvitta/gtest/.git/info/exclude
> > > > FAN_CLOSE_WRITE: File /mnt/c/Users/kvitta/gtest/.git/info/exclude
> > > > FAN_OPEN_PERM: File /mnt/c/Users/kvitta/gtest/.git/hooks/pre-applypatch.sample
> > > > FAN_CLOSE_WRITE: File /mnt/c/Users/kvitta/gtest/.git/hooks/pre-applypatch.sample
> > > > FAN_OPEN_PERM: File /mnt/c/Users/kvitta/gtest/.git/hooks/applypatch-msg.sample
> > > > FAN_CLOSE_WRITE: File /mnt/c/Users/kvitta/gtest/.git/hooks/applypatch-msg.sample
> > > > FAN_OPEN_PERM: File /mnt/c/Users/kvitta/gtest/.git/hooks/commit-msg.sample
> > > > FAN_CLOSE_WRITE: File /mnt/c/Users/kvitta/gtest/.git/hooks/commit-msg.sample
> > > > FAN_OPEN_PERM: File /mnt/c/Users/kvitta/gtest/.git/hooks/pre-push.sample
> > > > FAN_CLOSE_WRITE: File /mnt/c/Users/kvitta/gtest/.git/hooks/pre-push.sample
> > > > FAN_OPEN_PERM: File /mnt/c/Users/kvitta/gtest/.git/hooks/pre-merge-commit.sample
> > > > FAN_CLOSE_WRITE: File /mnt/c/Users/kvitta/gtest/.git/hooks/pre-merge-commit.sample
> > > > FAN_OPEN_PERM: File /mnt/c/Users/kvitta/gtest/.git/hooks/pre-commit.sample
> > > > FAN_CLOSE_WRITE: File /mnt/c/Users/kvitta/gtest/.git/hooks/pre-commit.sample
> > > > FAN_OPEN_PERM: File /mnt/c/Users/kvitta/gtest/.git/hooks/post-update.sample
> > > > FAN_CLOSE_WRITE: File /mnt/c/Users/kvitta/gtest/.git/hooks/post-update.sample
> > > > FAN_OPEN_PERM: File /mnt/c/Users/kvitta/gtest/.git/hooks/push-to-checkout.sample
> > > > FAN_CLOSE_WRITE: File /mnt/c/Users/kvitta/gtest/.git/hooks/push-to-checkout.sample
> > > > FAN_OPEN_PERM: File /mnt/c/Users/kvitta/gtest/.git/hooks/fsmonitor-watchman.sample
> > > > FAN_CLOSE_WRITE: File /mnt/c/Users/kvitta/gtest/.git/hooks/fsmonitor-watchman.sample
> > > > FAN_OPEN_PERM: File /mnt/c/Users/kvitta/gtest/.git/hooks/update.sample
> > > > FAN_CLOSE_WRITE: File /mnt/c/Users/kvitta/gtest/.git/hooks/update.sample
> > > > FAN_OPEN_PERM: File /mnt/c/Users/kvitta/gtest/.git/hooks/pre-rebase.sample
> > > > FAN_CLOSE_WRITE: File /mnt/c/Users/kvitta/gtest/.git/hooks/pre-rebase.sample
> > > > FAN_OPEN_PERM: File /mnt/c/Users/kvitta/gtest/.git/hooks/pre-receive.sample
> > > > FAN_CLOSE_WRITE: File /mnt/c/Users/kvitta/gtest/.git/hooks/pre-receive.sample
> > > > FAN_OPEN_PERM: File /mnt/c/Users/kvitta/gtest/.git/hooks/prepare-commit-msg.sample
> > > > FAN_CLOSE_WRITE: File /mnt/c/Users/kvitta/gtest/.git/hooks/prepare-commit-msg.sample
> > > > FAN_OPEN_PERM: File /mnt/c/Users/kvitta/gtest/.git/description
> > > > FAN_CLOSE_WRITE: File /mnt/c/Users/kvitta/gtest/.git/description
> > > > FAN_OPEN_PERM: File /mnt/c/Users/kvitta/gtest/.git/HEAD.lock
> > > > FAN_CLOSE_WRITE: File /mnt/c/Users/kvitta/gtest/.git/HEAD.lock
> > > > FAN_OPEN_PERM: File /mnt/c/Users/kvitta/gtest/.git/config.lock
> > > > FAN_CLOSE_WRITE: File /mnt/c/Users/kvitta/gtest/.git/config.lock
> > > > FAN_OPEN_PERM: File /mnt/c/Users/kvitta/gtest/.git/config.lock
> > > > FAN_OPEN_PERM: File /mnt/c/Users/kvitta/gtest/.git/config
> > > > FAN_OPEN_PERM: File /mnt/c/Users/kvitta/gtest/.git/config
> > > > FAN_CLOSE_WRITE: File /mnt/c/Users/kvitta/gtest/.git/config
> > > > FAN_OPEN_PERM: File /mnt/c/Users/kvitta/gtest/.git/config.lock
> > > > FAN_OPEN_PERM: File /mnt/c/Users/kvitta/gtest/.git/config
> > > > FAN_OPEN_PERM: File /mnt/c/Users/kvitta/gtest/.git/config
> > > > FAN_CLOSE_WRITE: File /mnt/c/Users/kvitta/gtest/.git/config
> > > > FAN_OPEN_PERM: File /mnt/c/Users/kvitta/gtest/.git/config.lock
> > > > FAN_OPEN_PERM: File /mnt/c/Users/kvitta/gtest/.git/config
> > > > FAN_OPEN_PERM: File /mnt/c/Users/kvitta/gtest/.git/config
> > > > FAN_CLOSE_WRITE: File /mnt/c/Users/kvitta/gtest/.git/config
> > > > FAN_OPEN_PERM: File /mnt/c/Users/kvitta/gtest/.git/tNbqjiA
> > > > read: No such file or directory
> > > > root@MININT-S244RA7:/mnt/c/Users/kvitta/Desktop/MDE binaries/GitCloneIssue#
> > >
> > > OK, so it appears that dentry_open() is failing with ENOENT when we try to
> > > open the file descriptor to return with the event. This is indeed
> > > unexpected from the filesystem.
>
> How did you conclude that is what is happening?
> Were you able to reproduce, because I did not.
No, I didn't reproduce. But checking the source of the reproducer the
message "read: No such file or directory" means fanotify_read() has
returned -ENOENT and within fanotify there's no way how that could happen.
So the only possible explanation is that dentry_open() returns it.
> > > On the other hand we already do silently
> > > fixup similar EOPENSTALE error that can come from NFS so perhaps we should
> > > be fixing ENOENT similarly? What do you thing Amir?
> >
>
> But we never return this error to the caller for a non-permission event,
> so what am I missing?
Umm. If dentry_open() fails, we return the error from copy_event_to_user()
without copying anything. Then fanotify_read() does:
ret = copy_event_to_user(group, event, buf, count);
...
if (!fanotify_is_perm_event(event->mask)) {
fsnotify_destroy_event(group, &event->fse);
// unused event destroyed
} else {
...
}
if (ret < 0)
break; // read loop aborted
...
}
...
if (start != buf && ret != -EFAULT)
ret = buf - start;
return ret;
So the error *is* IMO returned to userspace if this was the first event.
> > But what is still unclear to me is how this failure to generate fanotify
> > event relates to git clone failing. Perhaps the dentry references fanotify
> > holds in the notification queue confuse 9p and it returns those ENOENT
> > errors?
>
> My guess is that ENOENT for openat(2)/newfstatat(2) is from this code
> in fid_out label in:
> v9fs_vfs_getattr() => v9fs_fid_lookup() =>
> v9fs_fid_lookup_with_uid()
>
> if (d_unhashed(dentry)) {
> spin_unlock(&dentry->d_lock);
> p9_fid_put(fid);
> fid = ERR_PTR(-ENOENT);
> } else {
> __add_fid(dentry, fid);
>
> So fanotify contributes a deferred reference on the dentry,
> and that can somehow lead to operating on a stale unhashed dentry?
> Not exactly sure how to piece that all together.
For example d_delete() will only unhash the deleted dentry if it does not
hold the last reference to it (which it would not due to fanotify holding
reference). So presumably 9p could return this error if we delivered event
for an already deleted dentry. That would explain the fanotify side of
things. But not really the git clone failure.
> This seems like a problem that requires p9 developers to look at it.
> fanotify mark has an indirect effect on this use case IMO.
I agree, this looks more like a problem with 9p that gets exposed by
fanotify. Guys, Krishna is seeing git-clone failure on 9p filesystem when
fanotify events are generated for the filesystem. We suspect additional
dentry references from generated fanotify events somehow upset something
inside 9p so it returns -ENOENT errors sometimes (see previous email in
this thread for strace output).
Krishna, you've mentioned you are using the kernel from WSL
(https://github.com/microsoft/WSL2-Linux-Kernel/). Is it possible to
reproduce the failure with 9p from a standard Linux kernel?
Honza
--
Jan Kara <jack@xxxxxxxx>
SUSE Labs, CR
