| Message ID | 20200901204045.1250822-2-vgoyal@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | fuse: Enable SB_NOSEC if filesystem is not shared | expand |
On Tue, Sep 1, 2020 at 10:41 PM Vivek Goyal <vgoyal@redhat.com> wrote: > > FUSE_NONSHARED_FS will signify that filesystem is not shared. That is > all fuse modifications are going thourgh this single fuse connection. It > should not happen that file's data/metadata changed without the knowledge > of fuse. Automatic file time stamp changes will probably be an exception > to this rule. > > If filesystem is shared between different clients, then this flag should > not be set. I'm thinking maybe this flag should force some other parameters as well: ^FUSE_POSIX_LOCK ^FUSE_FLOCK_LOCKS ^FUSE_AUTO_INVAL_DATA FUSE_EXPLICIT_INVAL_DATA FUSE_CACHE_SYMLINKS attr_valid=inf entry_valid=inf FOPEN_KEEP_CACHE FOPEN_CACHE_DIR This would make sure that it's really only used in the non-shared case. Thanks, Miklos
On Wed, Sep 02, 2020 at 08:57:07AM +0200, Miklos Szeredi wrote: > On Tue, Sep 1, 2020 at 10:41 PM Vivek Goyal <vgoyal@redhat.com> wrote: > > > > FUSE_NONSHARED_FS will signify that filesystem is not shared. That is > > all fuse modifications are going thourgh this single fuse connection. It > > should not happen that file's data/metadata changed without the knowledge > > of fuse. Automatic file time stamp changes will probably be an exception > > to this rule. > > > > If filesystem is shared between different clients, then this flag should > > not be set. > > I'm thinking maybe this flag should force some other parameters as well: So you are thinking of fuse enforcing these parameters automatically without server asking for it. IOW, override what server says about following parameters and fuse sets their values if FUSE_NONSHARED_FS is set? Or should we recommend these for FUSE_NONSHARED_FS and let file server specify all this. > > ^FUSE_POSIX_LOCK No remote posix locks. Makes sense. If filesystem is not shared then there is no need of remote posix locks. > ^FUSE_FLOCK_LOCKS No remote BSD style file locks. If filesystem is not shared, then local locks should work just fine. > ^FUSE_AUTO_INVAL_DATA This controls if page cache data should be invalidated upon mtime change. For non shared fs, mtime should not change outside fuse, so makes sense to enforce ^FUSE_AUTO_INVAL_DATA. > FUSE_EXPLICIT_INVAL_DATA This tells fuse to not invalidate page cache and not truncate page cache if attr->size or mtime change is detected. For non-shared fs we don't expect attr->size or mtime to change, so it does not harm to enfroce this. > FUSE_CACHE_SYMLINKS This caches readlink responses. Makes sense to enable it for non shared fs. > attr_valid=inf > entry_valid=inf dentry and attr timeout being infinite should be good for performance if filesystem is not shared. > FOPEN_KEEP_CACHE If this is not set, by default fuse invalidates page cache on open. Makes sense to not flush page cache on open with FUSE_NONSHARED_FS. > FOPEN_CACHE_DIR Caching directory contents for FUSE_NONSHARED_FS makes sense too. > > This would make sure that it's really only used in the non-shared case. I am little afraid of enforcing this in fuse core because tomorrow somebody will say hey I need hybrid mode where FUSE_NONSHARED_FS is set but for some reason I want attrs to expire after some time. I don't have a good use case in my mind though. If I were to choose, I will probably document it and suggest that file servers sets all the above for the case of FUSE_NONSHARED_FS. But I am also open to enforcing this in fuse core if you prefer that option. Thanks Vivek
diff --git a/fs/fuse/fuse_i.h b/fs/fuse/fuse_i.h index 740a8a7d7ae6..3ace15488eb6 100644 --- a/fs/fuse/fuse_i.h +++ b/fs/fuse/fuse_i.h @@ -720,6 +720,9 @@ struct fuse_conn { /* Do not show mount options */ unsigned int no_mount_options:1; + /** This is not a shared filesystem */ + unsigned int nonshared_fs:1; + /** The number of requests waiting for completion */ atomic_t num_waiting; diff --git a/fs/fuse/inode.c b/fs/fuse/inode.c index bba747520e9b..088faa3e352c 100644 --- a/fs/fuse/inode.c +++ b/fs/fuse/inode.c @@ -965,6 +965,9 @@ static void process_init_reply(struct fuse_conn *fc, struct fuse_args *args, min_t(unsigned int, FUSE_MAX_MAX_PAGES, max_t(unsigned int, arg->max_pages, 1)); } + if (arg->flags & FUSE_NONSHARED_FS) { + fc->nonshared_fs = 1; + } } else { ra_pages = fc->max_read / PAGE_SIZE; fc->no_lock = 1; @@ -1002,7 +1005,8 @@ void fuse_send_init(struct fuse_conn *fc) FUSE_WRITEBACK_CACHE | FUSE_NO_OPEN_SUPPORT | FUSE_PARALLEL_DIROPS | FUSE_HANDLE_KILLPRIV | FUSE_POSIX_ACL | FUSE_ABORT_ERROR | FUSE_MAX_PAGES | FUSE_CACHE_SYMLINKS | - FUSE_NO_OPENDIR_SUPPORT | FUSE_EXPLICIT_INVAL_DATA; + FUSE_NO_OPENDIR_SUPPORT | FUSE_EXPLICIT_INVAL_DATA | + FUSE_NONSHARED_FS; ia->args.opcode = FUSE_INIT; ia->args.in_numargs = 1; ia->args.in_args[0].size = sizeof(ia->in); diff --git a/include/uapi/linux/fuse.h b/include/uapi/linux/fuse.h index 373cada89815..bdb106d9f10b 100644 --- a/include/uapi/linux/fuse.h +++ b/include/uapi/linux/fuse.h @@ -172,6 +172,8 @@ * - add FUSE_WRITE_KILL_PRIV flag * - add FUSE_SETUPMAPPING and FUSE_REMOVEMAPPING * - add map_alignment to fuse_init_out, add FUSE_MAP_ALIGNMENT flag + * 7.32 + * - add FUSE_NONSHARED_FS flag */ #ifndef _LINUX_FUSE_H @@ -314,6 +316,7 @@ struct fuse_file_lock { * FUSE_NO_OPENDIR_SUPPORT: kernel supports zero-message opendir * FUSE_EXPLICIT_INVAL_DATA: only invalidate cached pages on explicit request * FUSE_MAP_ALIGNMENT: map_alignment field is valid + * FUSE_NONSHARED_FS: Filesystem is not shared. */ #define FUSE_ASYNC_READ (1 << 0) #define FUSE_POSIX_LOCKS (1 << 1) @@ -342,6 +345,7 @@ struct fuse_file_lock { #define FUSE_NO_OPENDIR_SUPPORT (1 << 24) #define FUSE_EXPLICIT_INVAL_DATA (1 << 25) #define FUSE_MAP_ALIGNMENT (1 << 26) +#define FUSE_NONSHARED_FS (1 << 27) /** * CUSE INIT request/reply flags
FUSE_NONSHARED_FS will signify that filesystem is not shared. That is all fuse modifications are going thourgh this single fuse connection. It should not happen that file's data/metadata changed without the knowledge of fuse. Automatic file time stamp changes will probably be an exception to this rule. If filesystem is shared between different clients, then this flag should not be set. Signed-off-by: Vivek Goyal <vgoyal@redhat.com> --- fs/fuse/fuse_i.h | 3 +++ fs/fuse/inode.c | 6 +++++- include/uapi/linux/fuse.h | 4 ++++ 3 files changed, 12 insertions(+), 1 deletion(-)