[RFC PATCH] network fs notification

[RFC PATCH] network fs notification

[Miklos Szeredi]

This is a really really trivial first iteration, but I think it's enough to
try out CIFS notification support.  Doesn't deal with mark deletion, but
that's best effort anyway: fsnotify() will filter out unneeded events.

Signed-off-by: Miklos Szeredi <mszeredi@redhat.com>
---
 fs/notify/fanotify/fanotify_user.c |    6 +++++-
 fs/notify/inotify/inotify_user.c   |    2 ++
 include/linux/fs.h                 |    1 +
 3 files changed, 8 insertions(+), 1 deletion(-)

--- a/fs/notify/fanotify/fanotify_user.c
+++ b/fs/notify/fanotify/fanotify_user.c
@@ -1041,9 +1041,13 @@ static int do_fanotify_mark(int fanotify
 		else if (mark_type == FAN_MARK_FILESYSTEM)
 			ret = fanotify_add_sb_mark(group, mnt->mnt_sb, mask,
 						   flags, fsid);
-		else
+		else {
 			ret = fanotify_add_inode_mark(group, inode, mask,
 						      flags, fsid);
+
+			if (!ret && inode->i_op->notify_update)
+				inode->i_op->notify_update(inode);
+		}
 		break;
 	case FAN_MARK_REMOVE:
 		if (mark_type == FAN_MARK_MOUNT)
--- a/fs/notify/inotify/inotify_user.c
+++ b/fs/notify/inotify/inotify_user.c
@@ -754,6 +754,8 @@ SYSCALL_DEFINE3(inotify_add_watch, int,
 
 	/* create/update an inode mark */
 	ret = inotify_update_watch(group, inode, mask);
+	if (!ret && inode->i_op->notify_update)
+		inode->i_op->notify_update(inode);
 	path_put(&path);
 fput_and_out:
 	fdput(f);
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -1852,6 +1852,7 @@ struct inode_operations {
 			   umode_t create_mode);
 	int (*tmpfile) (struct inode *, struct dentry *, umode_t);
 	int (*set_acl)(struct inode *, struct posix_acl *, int);
+	void (*notify_update)(struct inode *inode);
 } ____cacheline_aligned;
 
 static inline ssize_t call_read_iter(struct file *file, struct kiocb *kio,

Re: [RFC PATCH] network fs notification

[Jan Kara]

On Wed 01-05-19 16:55:41, Miklos Szeredi wrote:
> This is a really really trivial first iteration, but I think it's enough to
> try out CIFS notification support.  Doesn't deal with mark deletion, but
> that's best effort anyway: fsnotify() will filter out unneeded events.
> 
> Signed-off-by: Miklos Szeredi <mszeredi@redhat.com>
> ---
>  fs/notify/fanotify/fanotify_user.c |    6 +++++-
>  fs/notify/inotify/inotify_user.c   |    2 ++
>  include/linux/fs.h                 |    1 +
>  3 files changed, 8 insertions(+), 1 deletion(-)
> 
> --- a/fs/notify/fanotify/fanotify_user.c
> +++ b/fs/notify/fanotify/fanotify_user.c
> @@ -1041,9 +1041,13 @@ static int do_fanotify_mark(int fanotify
>  		else if (mark_type == FAN_MARK_FILESYSTEM)
>  			ret = fanotify_add_sb_mark(group, mnt->mnt_sb, mask,
>  						   flags, fsid);
> -		else
> +		else {
>  			ret = fanotify_add_inode_mark(group, inode, mask,
>  						      flags, fsid);
> +
> +			if (!ret && inode->i_op->notify_update)
> +				inode->i_op->notify_update(inode);
> +		}

Yeah, so I had something like this in mind but I wanted to inform the
filesystem about superblock and mountpoint marks as well. And I'd pass the
'mask' as well as presumably filesystem could behave differently depending
on whether we are looking for create vs unlink vs file change events etc...

								Honza
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR

Re: [RFC PATCH] network fs notification

[Amir Goldstein]

On Thu, May 2, 2019 at 10:39 AM Jan Kara <jack@suse.cz> wrote:
>
> On Wed 01-05-19 16:55:41, Miklos Szeredi wrote:
> > This is a really really trivial first iteration, but I think it's enough to
> > try out CIFS notification support.  Doesn't deal with mark deletion, but
> > that's best effort anyway: fsnotify() will filter out unneeded events.
> >
> > Signed-off-by: Miklos Szeredi <mszeredi@redhat.com>
> > ---
> >  fs/notify/fanotify/fanotify_user.c |    6 +++++-
> >  fs/notify/inotify/inotify_user.c   |    2 ++
> >  include/linux/fs.h                 |    1 +
> >  3 files changed, 8 insertions(+), 1 deletion(-)
> >
> > --- a/fs/notify/fanotify/fanotify_user.c
> > +++ b/fs/notify/fanotify/fanotify_user.c
> > @@ -1041,9 +1041,13 @@ static int do_fanotify_mark(int fanotify
> >               else if (mark_type == FAN_MARK_FILESYSTEM)
> >                       ret = fanotify_add_sb_mark(group, mnt->mnt_sb, mask,
> >                                                  flags, fsid);
> > -             else
> > +             else {
> >                       ret = fanotify_add_inode_mark(group, inode, mask,
> >                                                     flags, fsid);
> > +
> > +                     if (!ret && inode->i_op->notify_update)
> > +                             inode->i_op->notify_update(inode);
> > +             }
>
> Yeah, so I had something like this in mind but I wanted to inform the
> filesystem about superblock and mountpoint marks as well. And I'd pass the
> 'mask' as well as presumably filesystem could behave differently depending
> on whether we are looking for create vs unlink vs file change events etc...
>

It probably wouldn't hurt to update fs about mount marks,
but in the context of "remote" fs, the changes are most certainly
being done on a different mount, a different machine most likely...

Thanks,
Amir.

Re: [RFC PATCH] network fs notification

[Jan Kara]

On Thu 02-05-19 11:08:41, Amir Goldstein wrote:
> On Thu, May 2, 2019 at 10:39 AM Jan Kara <jack@suse.cz> wrote:
> >
> > On Wed 01-05-19 16:55:41, Miklos Szeredi wrote:
> > > This is a really really trivial first iteration, but I think it's enough to
> > > try out CIFS notification support.  Doesn't deal with mark deletion, but
> > > that's best effort anyway: fsnotify() will filter out unneeded events.
> > >
> > > Signed-off-by: Miklos Szeredi <mszeredi@redhat.com>
> > > ---
> > >  fs/notify/fanotify/fanotify_user.c |    6 +++++-
> > >  fs/notify/inotify/inotify_user.c   |    2 ++
> > >  include/linux/fs.h                 |    1 +
> > >  3 files changed, 8 insertions(+), 1 deletion(-)
> > >
> > > --- a/fs/notify/fanotify/fanotify_user.c
> > > +++ b/fs/notify/fanotify/fanotify_user.c
> > > @@ -1041,9 +1041,13 @@ static int do_fanotify_mark(int fanotify
> > >               else if (mark_type == FAN_MARK_FILESYSTEM)
> > >                       ret = fanotify_add_sb_mark(group, mnt->mnt_sb, mask,
> > >                                                  flags, fsid);
> > > -             else
> > > +             else {
> > >                       ret = fanotify_add_inode_mark(group, inode, mask,
> > >                                                     flags, fsid);
> > > +
> > > +                     if (!ret && inode->i_op->notify_update)
> > > +                             inode->i_op->notify_update(inode);
> > > +             }
> >
> > Yeah, so I had something like this in mind but I wanted to inform the
> > filesystem about superblock and mountpoint marks as well. And I'd pass the
> > 'mask' as well as presumably filesystem could behave differently depending
> > on whether we are looking for create vs unlink vs file change events etc...
> >
> 
> It probably wouldn't hurt to update fs about mount marks,
> but in the context of "remote" fs, the changes are most certainly
> being done on a different mount, a different machine most likely...

I agree. I guess I'm missing your point :) What I understood from Steve is
that e.g. cifs could ask the server to provide the notifications. E.g. FUSE
could propagate this information to userspace daemon which could place
appropriate fsnotify marks on underlying objects and then transform the
events to events on the FUSE filesystem? At least that's what I was
imagining, didn't think too much about it.

								Honza

-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR

Re: [RFC PATCH] network fs notification

[Miklos Szeredi]

On Thu, May 2, 2019 at 11:41 AM Jan Kara <jack@suse.cz> wrote:
>
> On Thu 02-05-19 11:08:41, Amir Goldstein wrote:
> > On Thu, May 2, 2019 at 10:39 AM Jan Kara <jack@suse.cz> wrote:
> > >
> > > On Wed 01-05-19 16:55:41, Miklos Szeredi wrote:
> > > > This is a really really trivial first iteration, but I think it's enough to
> > > > try out CIFS notification support.  Doesn't deal with mark deletion, but
> > > > that's best effort anyway: fsnotify() will filter out unneeded events.
> > > >
> > > > Signed-off-by: Miklos Szeredi <mszeredi@redhat.com>
> > > > ---
> > > >  fs/notify/fanotify/fanotify_user.c |    6 +++++-
> > > >  fs/notify/inotify/inotify_user.c   |    2 ++
> > > >  include/linux/fs.h                 |    1 +
> > > >  3 files changed, 8 insertions(+), 1 deletion(-)
> > > >
> > > > --- a/fs/notify/fanotify/fanotify_user.c
> > > > +++ b/fs/notify/fanotify/fanotify_user.c
> > > > @@ -1041,9 +1041,13 @@ static int do_fanotify_mark(int fanotify
> > > >               else if (mark_type == FAN_MARK_FILESYSTEM)
> > > >                       ret = fanotify_add_sb_mark(group, mnt->mnt_sb, mask,
> > > >                                                  flags, fsid);
> > > > -             else
> > > > +             else {
> > > >                       ret = fanotify_add_inode_mark(group, inode, mask,
> > > >                                                     flags, fsid);
> > > > +
> > > > +                     if (!ret && inode->i_op->notify_update)
> > > > +                             inode->i_op->notify_update(inode);
> > > > +             }
> > >
> > > Yeah, so I had something like this in mind but I wanted to inform the
> > > filesystem about superblock and mountpoint marks as well. And I'd pass the
> > > 'mask' as well as presumably filesystem could behave differently depending
> > > on whether we are looking for create vs unlink vs file change events etc...

Hmm.  I'm not sure we need to pass the mask, since it's in the inode
filesystem can read it.  But this code is completely racy in this
respect and doesn't even include hooks in the delete notification.  So
it's basically just to try it out.

> >
> > It probably wouldn't hurt to update fs about mount marks,
> > but in the context of "remote" fs, the changes are most certainly
> > being done on a different mount, a different machine most likely...
>
> I agree. I guess I'm missing your point :) What I understood from Steve is
> that e.g. cifs could ask the server to provide the notifications. E.g. FUSE
> could propagate this information to userspace daemon which could place
> appropriate fsnotify marks on underlying objects and then transform the
> events to events on the FUSE filesystem? At least that's what I was
> imagining, didn't think too much about it.

Exactly.  For inode and superblock notification that's clear (don't
know if CIFS can do superblock notifications or not).  However, mount
notification is something that possibly means: notify me of any
changes made through this mount only.  I.e. if there's a bind mount
and the filesystem is modified through that, then we don't get the
mount notification for the original mount.  So I guess it makes sense
to say: if you want remote notifications, just use the superblock
notification.

Thanks,
Miklos

Re: [RFC PATCH] network fs notification

[Miklos Szeredi]

On Wed, May 01, 2019 at 04:55:41PM -0400, Miklos Szeredi wrote:
> This is a really really trivial first iteration, but I think it's enough to
> try out CIFS notification support.  Doesn't deal with mark deletion, but
> that's best effort anyway: fsnotify() will filter out unneeded events.

And this one actually does something for inotify.  I haven't tested fanotify
yet, but that one looks okay.

Note: FAN_MARK_MOUNT doesn't work yet, and we are not sure if it should work or
not.  FAN_MARK_FILESYSTEM would be a better candidate for remote notification,
since remote accesses are not associated with any particular local mount of the
filesystem.  But perhaps we need to turn on whole file notification for remote
due to the fact that applications rely on FAN_MARK_MOUNT already...  Btw, does the smb protocol support whole filesystem (or subtree) notifications?

Thanks,
Miklos

---
 fs/notify/fanotify/fanotify_user.c |    6 +++++-
 fs/notify/inotify/inotify_user.c   |    2 ++
 include/linux/fs.h                 |    1 +
 3 files changed, 8 insertions(+), 1 deletion(-)

--- a/fs/notify/fanotify/fanotify_user.c
+++ b/fs/notify/fanotify/fanotify_user.c
@@ -1041,9 +1041,13 @@ static int do_fanotify_mark(int fanotify
 		else if (mark_type == FAN_MARK_FILESYSTEM)
 			ret = fanotify_add_sb_mark(group, mnt->mnt_sb, mask,
 						   flags, fsid);
-		else
+		else {
 			ret = fanotify_add_inode_mark(group, inode, mask,
 						      flags, fsid);
+
+			if (!ret && inode->i_op->notify_update)
+				inode->i_op->notify_update(inode);
+		}
 		break;
 	case FAN_MARK_REMOVE:
 		if (mark_type == FAN_MARK_MOUNT)
--- a/fs/notify/inotify/inotify_user.c
+++ b/fs/notify/inotify/inotify_user.c
@@ -754,6 +754,8 @@ SYSCALL_DEFINE3(inotify_add_watch, int,
 
 	/* create/update an inode mark */
 	ret = inotify_update_watch(group, inode, mask);
+	if (ret >= 0 && inode->i_op->notify_update)
+		inode->i_op->notify_update(inode);
 	path_put(&path);
 fput_and_out:
 	fdput(f);
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -1852,6 +1852,7 @@ struct inode_operations {
 			   umode_t create_mode);
 	int (*tmpfile) (struct inode *, struct dentry *, umode_t);
 	int (*set_acl)(struct inode *, struct posix_acl *, int);
+	void (*notify_update)(struct inode *inode);
 } ____cacheline_aligned;
 
 static inline ssize_t call_read_iter(struct file *file, struct kiocb *kio,