From: Amir Goldstein <amir73il@gmail.com>
To: Ioannis Angelakopoulos <iangelak@redhat.com>
Cc: linux-fsdevel <linux-fsdevel@vger.kernel.org>,
virtio-fs-list <virtio-fs@redhat.com>,
linux-kernel <linux-kernel@vger.kernel.org>,
Jan Kara <jack@suse.cz>, Al Viro <viro@zeniv.linux.org.uk>,
Miklos Szeredi <miklos@szeredi.hu>,
Vivek Goyal <vgoyal@redhat.com>
Subject: Re: [RFC PATCH 3/7] FUSE,Inotify,Fsnotify,VFS: Add the fuse_fsnotify_update_mark inode operation
Date: Tue, 26 Oct 2021 18:06:15 +0300 [thread overview]
Message-ID: <CAOQ4uxjEHQLhB2oWuC4Tba2jpt5RgJvTi8CFiLvsd9C_ydqExA@mail.gmail.com> (raw)
In-Reply-To: <20211025204634.2517-4-iangelak@redhat.com>
On Mon, Oct 25, 2021 at 11:47 PM Ioannis Angelakopoulos
<iangelak@redhat.com> wrote:
>
> Every time a local watch is placed/modified/removed on/from an inode the
> same operation has to take place in the FUSE server.
>
> Thus add the inode operation "fuse_fsnotify_update_mark", which is
> specific to FUSE inodes. This operation is called from the
> "inotify_add_watch" system call in the inotify subsystem.
>
> Specifically, the operation is called when a process tries to add, modify
> or remove a watch from a FUSE inode and the remote fsnotify support is
> enabled both in the guest kernel and the FUSE server (virtiofsd).
>
> Essentially, when the kernel adds/modifies a watch locally, also send a
> fsnotify request to the FUSE server to do the same. We keep the local watch
> placement since it is essential for the functionality of the fsnotify
> notification subsystem. However, the local events generated by the guest
> kernel will be suppressed if they affect FUSE inodes and the remote
> fsnotify support is enabled.
>
> Also modify the "fsnotify_detach_mark" function in fs/notify/mark.c to add
> support for the remote deletion of watches for FUSE inodes. In contrast to
> the add/modify operation we do not modify the inotify subsystem, but the
> fsnotify subsystem. That is because there are two ways of deleting a watch
> from an inode. The first is by manually calling the "inotify_rm_watch"
> system call and the second is automatically by the kernel when the process
> that has created an inotify instance exits. In that case the kernel is
> responsible for deleting all the watches corresponding to said inotify
> instance.
>
> Thus we send the fsnotify request for the deletion of the remote watch at
> the lowest level within "fsnotify_detach_mark" to catch both watch removal
> cases.
>
> The "fuse_fsnotify_update_mark" function in turn calls the
> "fuse_fsnotify_send_request" function, to send an fsnotify request to the
> FUSE server related to an inode watch.
>
>
> Signed-off-by: Ioannis Angelakopoulos <iangelak@redhat.com>
> ---
> fs/fuse/dir.c | 29 +++++++++++++++++++++++++++++
> fs/notify/inotify/inotify_user.c | 11 +++++++++++
> fs/notify/mark.c | 10 ++++++++++
> include/linux/fs.h | 2 ++
> 4 files changed, 52 insertions(+)
>
> diff --git a/fs/fuse/dir.c b/fs/fuse/dir.c
> index d9b977c0f38d..f666aafc8d3f 100644
> --- a/fs/fuse/dir.c
> +++ b/fs/fuse/dir.c
> @@ -17,6 +17,8 @@
> #include <linux/xattr.h>
> #include <linux/iversion.h>
> #include <linux/posix_acl.h>
> +#include <linux/fsnotify_backend.h>
> +#include <linux/inotify.h>
>
> static void fuse_advise_use_readdirplus(struct inode *dir)
> {
> @@ -1805,6 +1807,30 @@ static int fuse_getattr(struct user_namespace *mnt_userns,
> return fuse_update_get_attr(inode, NULL, stat, request_mask, flags);
> }
>
> +static int fuse_fsnotify_update_mark(struct inode *inode, uint32_t action,
> + uint64_t group, uint32_t mask)
> +{
> + /*
> + * We have to remove the bits added to the mask before being attached
> + * or detached to the inode, since these bits are going to be
> + * added by the "remote" host kernel. If these bits were still enabled
> + * in the mask that was sent to the "remote" kernel then the watch would
> + * be rejected as an unsupported value. These bits are added by the
> + * fsnotify subsystem thus we use the corresponding fsnotify bits here.
> + */
> + mask = mask & ~(FS_IN_IGNORED | FS_UNMOUNT | FS_IN_ONESHOT |
> + FS_EXCL_UNLINK | FS_EVENT_ON_CHILD);
> +
> + if (!(mask & IN_ALL_EVENTS))
> + return -EINVAL;
> +
> + /*
> + * Action 0: Remove a watch
> + * Action 1: Add/Modify watch
> + */
> + return fuse_fsnotify_send_request(inode, mask, action, group);
> +}
> +
> static const struct inode_operations fuse_dir_inode_operations = {
> .lookup = fuse_lookup,
> .mkdir = fuse_mkdir,
> @@ -1824,6 +1850,7 @@ static const struct inode_operations fuse_dir_inode_operations = {
> .set_acl = fuse_set_acl,
> .fileattr_get = fuse_fileattr_get,
> .fileattr_set = fuse_fileattr_set,
> + .fsnotify_update = fuse_fsnotify_update_mark,
> };
>
> static const struct file_operations fuse_dir_operations = {
> @@ -1846,6 +1873,7 @@ static const struct inode_operations fuse_common_inode_operations = {
> .set_acl = fuse_set_acl,
> .fileattr_get = fuse_fileattr_get,
> .fileattr_set = fuse_fileattr_set,
> + .fsnotify_update = fuse_fsnotify_update_mark,
> };
>
> static const struct inode_operations fuse_symlink_inode_operations = {
> @@ -1853,6 +1881,7 @@ static const struct inode_operations fuse_symlink_inode_operations = {
> .get_link = fuse_get_link,
> .getattr = fuse_getattr,
> .listxattr = fuse_listxattr,
> + .fsnotify_update = fuse_fsnotify_update_mark,
> };
>
> void fuse_init_common(struct inode *inode)
> diff --git a/fs/notify/inotify/inotify_user.c b/fs/notify/inotify/inotify_user.c
> index 62051247f6d2..3a0fee09a7c3 100644
> --- a/fs/notify/inotify/inotify_user.c
> +++ b/fs/notify/inotify/inotify_user.c
> @@ -46,6 +46,8 @@
> #define INOTIFY_WATCH_COST (sizeof(struct inotify_inode_mark) + \
> 2 * sizeof(struct inode))
>
> +#define FSNOTIFY_ADD_MODIFY_MARK 1
> +
> /* configurable via /proc/sys/fs/inotify/ */
> static int inotify_max_queued_events __read_mostly;
>
> @@ -764,6 +766,15 @@ SYSCALL_DEFINE3(inotify_add_watch, int, fd, const char __user *, pathname,
>
> /* create/update an inode mark */
> ret = inotify_update_watch(group, inode, mask);
> + /*
> + * If the inode belongs to a remote filesystem/server that supports
> + * remote inotify events then send the mark to the remote server
> + */
> + if (ret >= 0 && inode->i_op->fsnotify_update) {
> + inode->i_op->fsnotify_update(inode,
> + FSNOTIFY_ADD_MODIFY_MARK,
> + (uint64_t)group, mask);
> + }
> path_put(&path);
> fput_and_out:
> fdput(f);
> diff --git a/fs/notify/mark.c b/fs/notify/mark.c
> index fa1d99101f89..f0d37276afcb 100644
> --- a/fs/notify/mark.c
> +++ b/fs/notify/mark.c
> @@ -77,6 +77,7 @@
> #include "fsnotify.h"
>
> #define FSNOTIFY_REAPER_DELAY (1) /* 1 jiffy */
> +#define FSNOTIFY_DELETE_MARK 0 /* Delete a mark in remote fsnotify */
This define is part of the vfs API it should be in an include file along side
FSNOTIFY_ADD_MODIFY_MARK (if we keep them in the API).
>
> struct srcu_struct fsnotify_mark_srcu;
> struct kmem_cache *fsnotify_mark_connector_cachep;
> @@ -399,6 +400,7 @@ void fsnotify_finish_user_wait(struct fsnotify_iter_info *iter_info)
> void fsnotify_detach_mark(struct fsnotify_mark *mark)
> {
> struct fsnotify_group *group = mark->group;
> + struct inode *inode = NULL;
>
> WARN_ON_ONCE(!mutex_is_locked(&group->mark_mutex));
> WARN_ON_ONCE(!srcu_read_lock_held(&fsnotify_mark_srcu) &&
> @@ -411,6 +413,14 @@ void fsnotify_detach_mark(struct fsnotify_mark *mark)
> spin_unlock(&mark->lock);
> return;
> }
> +
> + /* Only if the object is an inode send a request to FUSE server */
> + inode = fsnotify_conn_inode(mark->connector);
> + if (inode && inode->i_op->fsnotify_update) {
> + inode->i_op->fsnotify_update(inode, FSNOTIFY_DELETE_MARK,
> + (uint64_t)group, mark->mask);
> + }
> +
> mark->flags &= ~FSNOTIFY_MARK_FLAG_ATTACHED;
> list_del_init(&mark->g_list);
> spin_unlock(&mark->lock);
> diff --git a/include/linux/fs.h b/include/linux/fs.h
> index e7a633353fd2..86bcc44e3ab8 100644
> --- a/include/linux/fs.h
> +++ b/include/linux/fs.h
> @@ -2149,6 +2149,8 @@ struct inode_operations {
> int (*fileattr_set)(struct user_namespace *mnt_userns,
> struct dentry *dentry, struct fileattr *fa);
> int (*fileattr_get)(struct dentry *dentry, struct fileattr *fa);
> + int (*fsnotify_update)(struct inode *inode, uint32_t action,
> + uint64_t group, uint32_t mask);
> } ____cacheline_aligned;
>
Please split the patch that introduces the API from the FUSE implementation.
Regarding the API, group does not belong in this interface.
The inode object has an "aggregated mask" at i_fsnotify_mask
indicating an interest for an event from any group.
Remote servers should be notified when the aggregated mask changes.
Hence, Miklos has proposed a "remote fsnotify update" API which does
not carry the mask nor the action, only the watched object:
https://lore.kernel.org/linux-fsdevel/20190501205541.GC30899@veci.piliscsaba.redhat.com/
On that same thread, you will see that I also proposed the API to support
full filesystem watch (by passing sb).
I am not requiring that you implement sb watch for FUSE/virtiofs, but the
API should take this future extension into account.
Thanks,
Amir.
next prev parent reply other threads:[~2021-10-26 15:06 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 [this message]
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
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=CAOQ4uxjEHQLhB2oWuC4Tba2jpt5RgJvTi8CFiLvsd9C_ydqExA@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=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).