[Devel] [PATCH RHEL7 COMMIT] fuse kio: Read file attributes every first open

Konstantin Khorenko khorenko at virtuozzo.com
Fri Jul 6 12:41:48 MSK 2018


The commit is pushed to "branch-rh7-3.10.0-862.3.2.vz7.61.x-ovz" and will appear at https://src.openvz.org/scm/ovz/vzkernel.git
after rh7-3.10.0-862.3.2.vz7.61.15
------>
commit 80b98e3954a755753cbb88c4d69b342feaf61029
Author: Kirill Tkhai <ktkhai at virtuozzo.com>
Date:   Fri Jul 6 12:41:48 2018 +0300

    fuse kio: Read file attributes every first open
    
    Currently we do not reread file attributes in case of
    this is the existing inode:
    
    [root at s143 ~]# echo 1 >> /vzt/a.txt
    [root at s134 ~]# cat /vzt/a.txt
    1
    [root at s143 ~]# echo 1 >> /vzt/a.txt
    [root at s134 ~]# cat /vzt/a.txt
    1
    [root at s143 ~]# echo 1 >> /vzt/a.txt
    [root at s134 ~]# cat /vzt/a.txt
    1
    [root at s143 ~]# cat /vzt/a.txt
    1
    1
    1
    
    In this example the file was changed on s143, but s134
    used it's cached metadata.
    
    The patch reworks open sequence and makes dentry info
    to be populated every time we reopen the file again.
    Also, it fixed races during open of new file, since
    a new file may be obtained by another thread before
    the creator executes ->file_open(). So, we create
    every inode with num_users == 0, and then the first
    of them makes file_open().
    
    Note, that openers are synchronized by inode->mutex.
    Also, keep in mind that num_users are modified under
    fc->lock (except one unlikely path), and this exists
    long ago. So in further I'm going to make it like
    plain int, since this atomic does not do anything
    and just confuser a reader.
    
    https://jira.sw.ru/browse/PSBM-86234
    
    Signed-off-by: Kirill Tkhai <ktkhai at virtuozzo.com>
    Acked-by: Alexey Kuznetsov <kuznet at virtuozzo.com>
---
 fs/fuse/dir.c                      | 27 +++++++++++++++++----------
 fs/fuse/file.c                     |  9 ++++-----
 fs/fuse/inode.c                    |  3 +--
 fs/fuse/kio/pcs/pcs_fuse_kdirect.c | 26 +++++++++++++-------------
 4 files changed, 35 insertions(+), 30 deletions(-)

diff --git a/fs/fuse/dir.c b/fs/fuse/dir.c
index b04023bf230a..8dcad8b8c20e 100644
--- a/fs/fuse/dir.c
+++ b/fs/fuse/dir.c
@@ -490,24 +490,31 @@ static int fuse_create_open(struct inode *dir, struct dentry *entry,
 	fuse_invalidate_attr(dir);
 	err = finish_open(file, entry, generic_file_open, opened);
 	if (err) {
-		if (fc->writeback_cache) {
-			struct fuse_inode *fi = get_fuse_inode(inode);
-			atomic_dec(&fi->num_openers);
-		}
 		fuse_sync_release(ff, flags);
 		return err;
 	}
 
 	file->private_data = fuse_file_get(ff);
 	fuse_finish_open(inode, file);
-	if (fc->kio.op && fc->kio.op->file_open &&
-	    fc->kio.op->file_open(fc, file, inode)) {
-		if (err) {
-			fput(file);
-			return  err;
+
+	if (fc->writeback_cache) {
+		struct fuse_inode *fi = get_fuse_inode(inode);
+		bool need_open;
+
+		mutex_lock(&inode->i_mutex);
+		spin_lock(&fc->lock);
+		need_open = (atomic_inc_return(&fi->num_openers) == 1);
+		spin_unlock(&fc->lock);
+
+		if (need_open && fc->kio.op && fc->kio.op->file_open) {
+			err = fc->kio.op->file_open(fc, file, inode);
+			if (err)
+				fput(file);
 		}
+		mutex_unlock(&inode->i_mutex);
 	}
-	return 0;
+
+	return err;
 
 out_free_ff:
 	fuse_file_free(ff);
diff --git a/fs/fuse/file.c b/fs/fuse/file.c
index 313f81191ada..f96667cdabc8 100644
--- a/fs/fuse/file.c
+++ b/fs/fuse/file.c
@@ -291,11 +291,6 @@ int fuse_open_common(struct inode *inode, struct file *file, bool isdir)
 	err = fuse_do_open(fc, get_node_id(inode), file, isdir);
 	if (err)
 		return err;
-	if (fc->kio.op && fc->kio.op->file_open &&
-	    fc->kio.op->file_open(fc, file, inode)) {
-		fuse_release_common(file, FUSE_RELEASE);
-		return -EINVAL;
-	}
 
 	if (fc->writeback_cache && !isdir) {
 		struct fuse_inode *fi = get_fuse_inode(inode);
@@ -308,6 +303,10 @@ int fuse_open_common(struct inode *inode, struct file *file, bool isdir)
 			fi->i_size_unstable = 1;
 			spin_unlock(&fc->lock);
 			err = fuse_getattr_size(inode, file, &size);
+
+			if (!err && fc->kio.op && fc->kio.op->file_open)
+				err = fc->kio.op->file_open(fc, file, inode);
+
 			spin_lock(&fc->lock);
 			fi->i_size_unstable = 0;
 			if (err)
diff --git a/fs/fuse/inode.c b/fs/fuse/inode.c
index 6fc09495357a..13571c9bce93 100644
--- a/fs/fuse/inode.c
+++ b/fs/fuse/inode.c
@@ -327,8 +327,7 @@ struct inode *fuse_iget(struct super_block *sb, u64 nodeid,
 			inode->i_flags |= S_NOCMTIME;
 		inode->i_generation = generation;
 		inode->i_data.backing_dev_info = &fc->bdi;
-		fuse_init_inode(inode, attr,
-				fc->writeback_cache ? creat : 0);
+		fuse_init_inode(inode, attr, 0);
 		unlock_new_inode(inode);
 	} else if ((inode->i_mode ^ attr->mode) & S_IFMT) {
 		/* Inode has changed type, any I/O on the old should fail */
diff --git a/fs/fuse/kio/pcs/pcs_fuse_kdirect.c b/fs/fuse/kio/pcs/pcs_fuse_kdirect.c
index c9a291391405..118b75bf920c 100644
--- a/fs/fuse/kio/pcs/pcs_fuse_kdirect.c
+++ b/fs/fuse/kio/pcs/pcs_fuse_kdirect.c
@@ -296,18 +296,8 @@ static int kpcs_do_file_open(struct fuse_conn *fc, struct file *file, struct ino
 	      fi->nodeid, di->fileinfo.sys.chunk_size,
 	      di->fileinfo.sys.stripe_depth, di->fileinfo.sys.strip_width);
 
-	mutex_lock(&inode->i_mutex);
-	/* Some one already initialized it under us ? */
-	if (fi->private) {
-		mutex_unlock(&inode->i_mutex);
-		pcs_mapping_invalidate(&di->mapping);
-		pcs_mapping_deinit(&di->mapping);
-		kfree(di);
-		return 0;
-	}
 	ret = fuse_pcs_kdirect_claim_op(fc, file, true);
 	if (ret) {
-		mutex_unlock(&inode->i_mutex);
 		pcs_mapping_invalidate(&di->mapping);
 		pcs_mapping_deinit(&di->mapping);
 		kfree(di);
@@ -315,21 +305,31 @@ static int kpcs_do_file_open(struct fuse_conn *fc, struct file *file, struct ino
 	}
 	/* TODO: Propper initialization of dentry should be here!!! */
 	fi->private = di;
-	mutex_unlock(&inode->i_mutex);
 	return 0;
 }
 
 int kpcs_file_open(struct fuse_conn *fc, struct file *file, struct inode *inode)
 {
 	struct fuse_inode *fi = get_fuse_inode(inode);
+	struct pcs_dentry_info *di = fi->private;
+	struct pcs_mds_fileinfo info;
+	int ret;
 
 	if (!S_ISREG(inode->i_mode))
 		return 0;
 	if (fi->nodeid - FUSE_ROOT_ID >= PCS_FUSE_INO_SPECIAL_)
 		return 0;
-	/* Already initialized */
-	if (fi->private) {
+
+	lockdep_assert_held(&inode->i_mutex);
+	/* Already initialized. Update file size etc */
+	if (di) {
 		/*TODO: propper refcount for claim_cnt should be here */
+		ret = fuse_pcs_getfileinfo(fc, file, &info);
+		if (ret)
+			return ret;
+		spin_lock(&di->lock);
+		pcs_set_fileinfo(di, &info);
+		spin_unlock(&di->lock);
 		return 0;
 	}
 	return kpcs_do_file_open(fc, file, inode);


More information about the Devel mailing list