RE: [EXTERNAL] Re: Git clone fails in p9 file system marked with FANOTIFY

[Krishna Vivek Vitta <kvitta@xxxxxxxxxxxxx>]

Hi Amir

Thanks for the experiment. Though reproducer program is succeeding but fanotify listener is terminating since its failing to read the event. Right ?

Can you elaborate on: " We should consider adding it as is or maybe ratelimited." ?

Does this mean there should be a fix at fanotify side ?

Can you summarize the problem statement for once in larger interest of group.

I assume the whole of these experiments succeed in ext, xfs.


Thank you,
Krishna Vivek

-----Original Message-----
From: Amir Goldstein <amir73il@xxxxxxxxx> 
Sent: Friday, September 27, 2024 3:40 AM
To: Krishna Vivek Vitta <kvitta@xxxxxxxxxxxxx>
Cc: Jan Kara <jack@xxxxxxx>; linux-fsdevel@xxxxxxxxxxxxxxx; Eric Van Hensbergen <ericvh@xxxxxxxxxx>; Latchesar Ionkov <lucho@xxxxxxxxxx>; Dominique Martinet <asmadeus@xxxxxxxxxxxxx>; v9fs@xxxxxxxxxxxxxxx
Subject: Re: [EXTERNAL] Re: Git clone fails in p9 file system marked with FANOTIFY

On Thu, Sep 26, 2024 at 10:28 PM Amir Goldstein <amir73il@xxxxxxxxx> wrote:
>
> > > What would be the next steps for this investigation ?
> > >
> >
> > I need to find some time and to debug the reason for 9p open failure 
> > so we can make sure the problem is in 9p code and report more 
> > details of the bug to 9p maintainers, but since a simple reproducer 
> > exists, they can also try to reproduce the issue right now.
>
> FWIW, the attached reproducer mimics git clone rename_over pattern closer.
> It reproduces fanotify_read() errors sometimes, not always, with 
> either CLOSE_WRITE or OPEN_PERM | CLOSE_WRITE.
> maybe CLOSE_WRITE alone has better odds - I'm not sure.
>

scratch that.
I think the renames were just a destruction git clone events do not always fail on a close+rename pattern, they always fail on the close+unlink that follows the renames:

1776  openat(AT_FDCWD, "/vtmp/filebench/.git/tjEzMUw", O_RDWR|O_CREAT|O_EXCL, 0600) = 3
1776  close(3)                          = 0
1776  unlink("/vtmp/filebench/.git/tjEzMUw") = 0
1776  symlink("testing", "/vtmp/filebench/.git/tjEzMUw") = 0
1776  lstat("/vtmp/filebench/.git/tjEzMUw", {st_mode=S_IFLNK|0777, st_size=7, ...}) = 0
1776  unlink("/vtmp/filebench/.git/tjEzMUw") = 0

I know because I added this print:

--- a/fs/notify/fanotify/fanotify_user.c
+++ b/fs/notify/fanotify/fanotify_user.c
@@ -275,6 +275,7 @@ static int create_fd(struct fsnotify_group *group, const struct path *path,
                 */
                put_unused_fd(client_fd);
                client_fd = PTR_ERR(new_file);
+               pr_warn("%s(%pd2): ret=%d\n", __func__, path->dentry,
client_fd);
        } else {

We should consider adding it as is or maybe ratelimited.

The trivial reproducer below fails fanotify_read() always with one try.

Thanks,
Amir.

int main() {
    const char *filename = "config.lock";
    int fd;

    // Create a new file
    fd = open(filename, O_WRONLY | O_CREAT | O_TRUNC, 0644);
    if (fd == -1) {
        perror("Failed to create file");
        return EXIT_FAILURE;
    }
    close(fd);

    // Remove the file
    if (unlink(filename) != 0) {
        perror("Failed to unlink file");
        return EXIT_FAILURE;
    }

    return EXIT_SUCCESS;
}