linux-fsdevel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Amir Goldstein <amir73il@gmail.com>
To: Vivek Goyal <vgoyal@redhat.com>
Cc: Jan Kara <jack@suse.cz>,
	Ioannis Angelakopoulos <iangelak@redhat.com>,
	linux-fsdevel <linux-fsdevel@vger.kernel.org>,
	virtio-fs-list <virtio-fs@redhat.com>,
	linux-kernel <linux-kernel@vger.kernel.org>,
	Al Viro <viro@zeniv.linux.org.uk>,
	Miklos Szeredi <miklos@szeredi.hu>,
	Steve French <sfrench@samba.org>
Subject: Re: [RFC PATCH 0/7] Inotify support in FUSE and virtiofs
Date: Thu, 4 Nov 2021 07:19:37 +0200	[thread overview]
Message-ID: <CAOQ4uxjTw+4ReoxdMKN-EX0q1dwtLCZgZq4A9qdWhgnOiRb1vg@mail.gmail.com> (raw)
In-Reply-To: <YYMNPqVnOWD3gNsw@redhat.com>

> > > If event queue becomes too full, we might drop these events. But I guess
> > > in that case we will have to generate IN_Q_OVERFLOW and that can somehow
> > > be used to cleanup such S_DEAD inodes?
> >
> > That depends on the server implementation.
> > If the server is watching host fs using fanotify filesystem mark, then
> > an overflow
> > event does NOT mean that other new events on inode may be missed only
> > that old events could have been missed.
> > Server should know about all the watched inodes, so it can check on overflow
> > if any of the watched inodes were deleted and notify the client using a reliable
> > channel.
>
> Ok. We have only one channel for notifications. I guess we can program
> the channel in such a way so that it does not drop overflow events but
> can drop other kind of events if things get crazy. If too many overflow
> events and we allocate too much of memory, I guess at some point of
> time, oom killer will kick in a kill server.
>

The kernel implementation of fsnotify events queue pre-allocates
a single overflow event and never queues more than a single overflow
event. IN_Q_OVERFLOW must be delivered reliably, but delivering one
overflow event is enough (until it is consumed).

> >
> > Given the current server implementation with inotify, IN_Q_OVERFLOW
> > means server may have lost an IN_IGNORED event and may not get any
> > more events on inode, so server should check all the watched inodes after
> > overflow, notify the client of all deleted inodes and try to re-create
> > the watches
> > for all inodes with known path or use magic /prod/pid/fd path if that
> > works (??).
>
> Re-doing the watches sounds very painful.

Event overflow is a painful incident and systems usually pay a large
penalty when it happens (e.g. full recrawl of watched tree).
If virtiofsd is going to use inotify, it is no different than any other inotify
application that needs to bear the consequence of event overflow.

> That means we will need to
> keep track of aggregated mask in server side inode as well. As of
> now we just pass mask to kernel using inotify_add_watch() and forget
> about it.
>

It costs nothing to keep the aggregated mask in server side inode
and it makes sense to do that anyway.
This allows an implementation to notify about changes that the server
itself handles even if there is no backing filesystem behind it or
host OS has no fs notification support.

> /proc/pid/fd should work because I think that's how ioannis is putting
> current watches on inodes. We don't send path info to server.
>
> >
> > >
> > > nodeid is managed by server. So I am assuming that FORGET messages will
> > > not be sent to server for this inode till we have seen FS_IN_IGNORED
> > > and FS_DELETE_SELF events?
> > >
> >
> > Or until the application that requested the watch calls
> > inotify_rm_watch() or closes
> > the inotify fd.
> >
> > IOW, when fs implements remote fsnotify, the local watch keeps the local deleted
> > inode object in limbo until the local watch is removed.
> > When the remote fsnotify server informs that the remote watch (or remote inode)
> > is gone, the local watch is removed as well and then the inotify
> > application also gets
> > an FS_IN_IGNORED event.
>
> Hmm.., I guess remote server will simply send IN_DELETE event when it
> gets it and forward to client. And client will have to then cleanup
> this S_DEAD inode which is in limbo waiting for IN_DELETE_SELF event.
> And that should trigger cleanup of marks/local-watches on the inode, IIUC.
>

In very broad lines, but the server notification must be delivered reliably.

> >
> > Lifetime of local inode is complicated and lifetime of this "shared inode"
> > is much more complicated, so I am not pretending to claim that I have this all
> > figured out or that it could be reliably done at all.
>
> Yes this handling of IN_DELETE_SELF is turning out to be the most
> complicated piece of this proposal. I wish initial implementation
> could just be designed that it does not send IN_DELETE_SELF and
> IN_INGORED is generated locally. And later enhance it to support
> reliable delivery of IN_DELETE_SELF.
>

Not allowing DELETE_SELF in the mask sounds reasonable, but
as Ioannis explained, other events can be missed on local file delete.
If you want to preserve inotify semantics, you could queue an overflow
event if a fuse inode that gets evicted still has inotify marks.
That's a bit harsh though.
Alternatively, you could document in inotify man page that IN_INGORED
could mean that some events were dropped and hope for the best...

Thanks,
Amir.

  reply	other threads:[~2021-11-04  5:19 UTC|newest]

Thread overview: 66+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-10-25 20:46 [RFC PATCH 0/7] Inotify support in FUSE and virtiofs Ioannis Angelakopoulos
2021-10-25 20:46 ` [RFC PATCH 1/7] FUSE: Add the fsnotify opcode and in/out structs to FUSE Ioannis Angelakopoulos
2021-10-26 14:56   ` Amir Goldstein
2021-10-26 18:28     ` Amir Goldstein
     [not found]     ` <CAO17o20+jiij64y7b3eKoCjG5b_mLZj6o1LSnZ7+8exN3dFYEg@mail.gmail.com>
2021-10-27  5:46       ` Amir Goldstein
2021-10-27 21:46     ` Vivek Goyal
2021-10-28  4:13       ` Amir Goldstein
2021-10-28 14:20         ` Vivek Goyal
2021-10-25 20:46 ` [RFC PATCH 2/7] FUSE: Add the remote inotify support capability " Ioannis Angelakopoulos
2021-10-25 20:46 ` [RFC PATCH 3/7] FUSE,Inotify,Fsnotify,VFS: Add the fuse_fsnotify_update_mark inode operation Ioannis Angelakopoulos
2021-10-26 15:06   ` Amir Goldstein
2021-11-01 17:49     ` Vivek Goyal
2021-11-02  7:34       ` Amir Goldstein
2021-10-25 20:46 ` [RFC PATCH 4/7] FUSE: Add the fuse_fsnotify_send_request to FUSE Ioannis Angelakopoulos
2021-10-25 20:46 ` [RFC PATCH 5/7] Fsnotify: Add a wrapper around the fsnotify function Ioannis Angelakopoulos
2021-10-26 14:37   ` Amir Goldstein
2021-10-26 15:38     ` Vivek Goyal
2021-10-25 20:46 ` [RFC PATCH 6/7] FUSE,Fsnotify: Add the fuse_fsnotify_event inode operation Ioannis Angelakopoulos
2021-10-25 20:46 ` [RFC PATCH 7/7] virtiofs: Add support for handling the remote fsnotify notifications Ioannis Angelakopoulos
2021-10-26 15:23 ` [RFC PATCH 0/7] Inotify support in FUSE and virtiofs Amir Goldstein
2021-10-26 15:52   ` Vivek Goyal
2021-10-26 18:19     ` Amir Goldstein
2021-10-26 16:18   ` Vivek Goyal
2021-10-26 17:59     ` Amir Goldstein
2021-10-26 18:27       ` Vivek Goyal
2021-10-26 19:04         ` Amir Goldstein
     [not found]         ` <CAO17o20sdKAWQN6w7Oe0Ze06qcK+J=6rrmA_aWGnY__MRVDCKw@mail.gmail.com>
2021-10-27  5:59           ` Amir Goldstein
2021-10-27 13:23             ` Jan Kara
2021-10-27 20:29               ` Vivek Goyal
2021-10-27 20:37                 ` Vivek Goyal
2021-11-02 11:09                 ` Jan Kara
2021-11-02 12:54                   ` Amir Goldstein
2021-11-02 20:34                     ` Vivek Goyal
2021-11-03  7:31                       ` Amir Goldstein
2021-11-03 22:29                         ` Vivek Goyal
2021-11-04  5:19                           ` Amir Goldstein [this message]
2021-11-03 10:09                     ` Jan Kara
2021-11-03 11:17                       ` Amir Goldstein
2021-11-03 22:36                         ` Vivek Goyal
2021-11-04  5:29                           ` Amir Goldstein
2021-11-04 10:03                           ` Jan Kara
2021-11-05 14:30                             ` Vivek Goyal
2021-11-10  6:28                               ` Amir Goldstein
2021-11-11 17:30                                 ` Jan Kara
2021-11-11 20:52                                   ` Amir Goldstein
2021-11-16  5:09                                     ` Stef Bon
     [not found]                                       ` <CAO17o21YVczE2-BTAVg-0HJU6gjSUkzUSqJVs9k-_t7mYFNHaA@mail.gmail.com>
2021-11-17  6:40                                         ` Amir Goldstein
2021-11-30 15:27                                           ` Vivek Goyal
     [not found]                                             ` <CAO17o21uh3fJHd0gMu-SmZei5et6HJo91DiLk_YyfUqrtHy2pQ@mail.gmail.com>
2021-12-15  7:10                                               ` Amir Goldstein
2021-12-15 16:44                                                 ` Vivek Goyal
2021-12-15 17:29                                                   ` Amir Goldstein
2021-12-15 19:20                                                     ` Vivek Goyal
2021-12-15 19:54                                                       ` Amir Goldstein
2021-12-16 11:03                                                         ` Amir Goldstein
2021-12-16 16:24                                                           ` Vivek Goyal
2021-12-16 18:22                                                             ` Amir Goldstein
2021-12-16 22:24                                                               ` Vivek Goyal
2021-12-17  4:21                                                                 ` Amir Goldstein
2021-12-17 14:15                                                                   ` Vivek Goyal
2021-12-18  8:28                                                                     ` Amir Goldstein
2021-12-20 16:41                                                                       ` Vivek Goyal
2021-12-20 18:22                                                                         ` Amir Goldstein
2022-01-06 23:37                                             ` Steve French
2021-11-30 15:36                                       ` Vivek Goyal
2021-10-27 20:24             ` Vivek Goyal
2021-10-28  5:11               ` Amir Goldstein

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=CAOQ4uxjTw+4ReoxdMKN-EX0q1dwtLCZgZq4A9qdWhgnOiRb1vg@mail.gmail.com \
    --to=amir73il@gmail.com \
    --cc=iangelak@redhat.com \
    --cc=jack@suse.cz \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=miklos@szeredi.hu \
    --cc=sfrench@samba.org \
    --cc=vgoyal@redhat.com \
    --cc=viro@zeniv.linux.org.uk \
    --cc=virtio-fs@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).