[PATCH 0/6] fuse: Further reducing contention of fc->lock

[PATCH 0/6] fuse: Further reducing contention of fc->lock

[Kirill Tkhai]

There was introduced fc->bg_lock to reduce the contention,
and this patchset continues this efforts.

This patchset introduces per fuse_inode lock to protect
inode metadata, synchronization with background writes, etc.
All of the above is related to a single inode, and there
is no a reason, that inodes are concurrents of each other
to make some of actions exclusive.

So, here we introduce fuse_inode::lock spinlock and get rid
of fc->lock in many places.

---

Kirill Tkhai (6):
      fuse: Change argument of fuse_flush_writepages()
      fuse: Add fuse_inode argument to fuse_prepare_release()
      fuse: Introduce fuse_inode::lock to protect write related fields and statistics
      fuse: Implement fuse_attr_version_inc()
      fuse: Protect fuse_inode::nlookup with fuse_inode::lock
      fuse: Protect fuse_file::reserved_req via corresponding fuse_inode::lock


 fs/fuse/cuse.c    |    3 +
 fs/fuse/dev.c     |   10 +++-
 fs/fuse/dir.c     |   41 +++++++++---------
 fs/fuse/file.c    |  119 ++++++++++++++++++++++++++++-------------------------
 fs/fuse/fuse_i.h  |   27 ++++++++++--
 fs/fuse/inode.c   |   16 ++++---
 fs/fuse/readdir.c |    4 +-
 7 files changed, 126 insertions(+), 94 deletions(-)

--
Signed-off-by: Kirill Tkhai <ktkhai@virtuozzo.com>

[PATCH 1/6] fuse: Change argument of fuse_flush_writepages()

[Kirill Tkhai]

Next patches introduce fuse_inode::lock, which will be used
in __releases() and __acquires() instead of fc->lock.
This patch makes this function to use argument fuse_inode
instead of inode as preparation for that.

Signed-off-by: Kirill Tkhai <ktkhai@virtuozzo.com>
---
 fs/fuse/dir.c    |    2 +-
 fs/fuse/file.c   |    8 ++++----
 fs/fuse/fuse_i.h |    2 +-
 3 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/fs/fuse/dir.c b/fs/fuse/dir.c
index 47395b0c3b35..c71f7e9ee0f7 100644
--- a/fs/fuse/dir.c
+++ b/fs/fuse/dir.c
@@ -1360,7 +1360,7 @@ static void __fuse_release_nowrite(struct inode *inode)
 
 	BUG_ON(fi->writectr != FUSE_NOWRITE);
 	fi->writectr = 0;
-	fuse_flush_writepages(inode);
+	fuse_flush_writepages(fi);
 }
 
 void fuse_release_nowrite(struct inode *inode)
diff --git a/fs/fuse/file.c b/fs/fuse/file.c
index cc2121b37bf5..d5bd29610875 100644
--- a/fs/fuse/file.c
+++ b/fs/fuse/file.c
@@ -1522,12 +1522,12 @@ __acquires(fc->lock)
  *
  * Called with fc->lock
  */
-void fuse_flush_writepages(struct inode *inode)
+void fuse_flush_writepages(struct fuse_inode *fi)
 __releases(fc->lock)
 __acquires(fc->lock)
 {
+	struct inode *inode = &fi->inode;
 	struct fuse_conn *fc = get_fuse_conn(inode);
-	struct fuse_inode *fi = get_fuse_inode(inode);
 	size_t crop = i_size_read(inode);
 	struct fuse_req *req;
 
@@ -1670,7 +1670,7 @@ static int fuse_writepage_locked(struct page *page)
 	spin_lock(&fc->lock);
 	list_add(&req->writepages_entry, &fi->writepages);
 	list_add_tail(&req->list, &fi->queued_writes);
-	fuse_flush_writepages(inode);
+	fuse_flush_writepages(fi);
 	spin_unlock(&fc->lock);
 
 	end_page_writeback(page);
@@ -1728,7 +1728,7 @@ static void fuse_writepages_send(struct fuse_fill_wb_data *data)
 	req->ff = fuse_file_get(data->ff);
 	spin_lock(&fc->lock);
 	list_add_tail(&req->list, &fi->queued_writes);
-	fuse_flush_writepages(inode);
+	fuse_flush_writepages(fi);
 	spin_unlock(&fc->lock);
 
 	for (i = 0; i < num_pages; i++)
diff --git a/fs/fuse/fuse_i.h b/fs/fuse/fuse_i.h
index e9f712e81c7d..38bd7ca1908a 100644
--- a/fs/fuse/fuse_i.h
+++ b/fs/fuse/fuse_i.h
@@ -995,7 +995,7 @@ void fuse_update_ctime(struct inode *inode);
 
 int fuse_update_attributes(struct inode *inode, struct file *file);
 
-void fuse_flush_writepages(struct inode *inode);
+void fuse_flush_writepages(struct fuse_inode *fi);
 
 void fuse_set_nowrite(struct inode *inode);
 void fuse_release_nowrite(struct inode *inode);

[PATCH 2/6] fuse: Add fuse_inode argument to fuse_prepare_release()

[Kirill Tkhai]

Here is preparation for next patches, which introduce
new fuse_inode::lock for protection fuse_file::write_entry
linked into fuse_inode::write_files.

This patch just passes new argument to the function.

Signed-off-by: Kirill Tkhai <ktkhai@virtuozzo.com>
---
 fs/fuse/cuse.c   |    3 ++-
 fs/fuse/dir.c    |    6 ++++--
 fs/fuse/file.c   |   10 ++++++----
 fs/fuse/fuse_i.h |    2 +-
 4 files changed, 13 insertions(+), 8 deletions(-)

diff --git a/fs/fuse/cuse.c b/fs/fuse/cuse.c
index 8f68181256c0..d73eba592ba1 100644
--- a/fs/fuse/cuse.c
+++ b/fs/fuse/cuse.c
@@ -141,10 +141,11 @@ static int cuse_open(struct inode *inode, struct file *file)
 
 static int cuse_release(struct inode *inode, struct file *file)
 {
+	struct fuse_inode *fi = get_fuse_inode(inode);
 	struct fuse_file *ff = file->private_data;
 	struct fuse_conn *fc = ff->fc;
 
-	fuse_sync_release(ff, file->f_flags);
+	fuse_sync_release(fi, ff, file->f_flags);
 	fuse_conn_put(fc);
 
 	return 0;
diff --git a/fs/fuse/dir.c b/fs/fuse/dir.c
index c71f7e9ee0f7..7058bbf69c3c 100644
--- a/fs/fuse/dir.c
+++ b/fs/fuse/dir.c
@@ -400,6 +400,7 @@ static int fuse_create_open(struct inode *dir, struct dentry *entry,
 	struct fuse_create_in inarg;
 	struct fuse_open_out outopen;
 	struct fuse_entry_out outentry;
+	struct fuse_inode *fi;
 	struct fuse_file *ff;
 
 	/* Userspace expects S_IFREG in create mode */
@@ -451,7 +452,7 @@ static int fuse_create_open(struct inode *dir, struct dentry *entry,
 			  &outentry.attr, entry_attr_timeout(&outentry), 0);
 	if (!inode) {
 		flags &= ~(O_CREAT | O_EXCL | O_TRUNC);
-		fuse_sync_release(ff, flags);
+		fuse_sync_release(NULL, ff, flags);
 		fuse_queue_forget(fc, forget, outentry.nodeid, 1);
 		err = -ENOMEM;
 		goto out_err;
@@ -462,7 +463,8 @@ static int fuse_create_open(struct inode *dir, struct dentry *entry,
 	fuse_dir_changed(dir);
 	err = finish_open(file, entry, generic_file_open);
 	if (err) {
-		fuse_sync_release(ff, flags);
+		fi = get_fuse_inode(inode);
+		fuse_sync_release(fi, ff, flags);
 	} else {
 		file->private_data = ff;
 		fuse_finish_open(inode, file);
diff --git a/fs/fuse/file.c b/fs/fuse/file.c
index d5bd29610875..fa7580d47d0c 100644
--- a/fs/fuse/file.c
+++ b/fs/fuse/file.c
@@ -224,7 +224,8 @@ int fuse_open_common(struct inode *inode, struct file *file, bool isdir)
 	return err;
 }
 
-static void fuse_prepare_release(struct fuse_file *ff, int flags, int opcode)
+static void fuse_prepare_release(struct fuse_inode *fi, struct fuse_file *ff,
+				 int flags, int opcode)
 {
 	struct fuse_conn *fc = ff->fc;
 	struct fuse_req *req = ff->reserved_req;
@@ -249,10 +250,11 @@ static void fuse_prepare_release(struct fuse_file *ff, int flags, int opcode)
 
 void fuse_release_common(struct file *file, int opcode)
 {
+	struct fuse_inode *fi = get_fuse_inode(file_inode(file));
 	struct fuse_file *ff = file->private_data;
 	struct fuse_req *req = ff->reserved_req;
 
-	fuse_prepare_release(ff, file->f_flags, opcode);
+	fuse_prepare_release(fi, ff, file->f_flags, opcode);
 
 	if (ff->flock) {
 		struct fuse_release_in *inarg = &req->misc.release.in;
@@ -294,10 +296,10 @@ static int fuse_release(struct inode *inode, struct file *file)
 	return 0;
 }
 
-void fuse_sync_release(struct fuse_file *ff, int flags)
+void fuse_sync_release(struct fuse_inode *fi, struct fuse_file *ff, int flags)
 {
 	WARN_ON(refcount_read(&ff->count) > 1);
-	fuse_prepare_release(ff, flags, FUSE_RELEASE);
+	fuse_prepare_release(fi, ff, flags, FUSE_RELEASE);
 	/*
 	 * iput(NULL) is a no-op and since the refcount is 1 and everything's
 	 * synchronous, we are fine with not doing igrab() here"
diff --git a/fs/fuse/fuse_i.h b/fs/fuse/fuse_i.h
index 38bd7ca1908a..9e23e871873b 100644
--- a/fs/fuse/fuse_i.h
+++ b/fs/fuse/fuse_i.h
@@ -817,7 +817,7 @@ struct fuse_file *fuse_file_alloc(struct fuse_conn *fc);
 void fuse_file_free(struct fuse_file *ff);
 void fuse_finish_open(struct inode *inode, struct file *file);
 
-void fuse_sync_release(struct fuse_file *ff, int flags);
+void fuse_sync_release(struct fuse_inode *fi, struct fuse_file *ff, int flags);
 
 /**
  * Send RELEASE or RELEASEDIR request

[PATCH 3/6] fuse: Introduce fuse_inode::lock to protect write related fields and statistics

[Kirill Tkhai]

To minimize contention of fuse_conn::lock, this patch
introduces a new spinlock for protection fuse_inode
metadata:
		fuse_inode::
			writectr
			writepages
			write_files
			queued_writes
			attr_version

		inode::
			i_size
			i_nlink
			i_mtime
			i_ctime

Also, it protects the fields changed in fuse_change_attributes_common()
(too many to list).

Signed-off-by: Kirill Tkhai <ktkhai@virtuozzo.com>
---
 fs/fuse/dir.c    |   25 ++++++++-------
 fs/fuse/file.c   |   93 ++++++++++++++++++++++++++++++------------------------
 fs/fuse/fuse_i.h |    7 +++-
 fs/fuse/inode.c  |   12 +++++--
 4 files changed, 81 insertions(+), 56 deletions(-)

diff --git a/fs/fuse/dir.c b/fs/fuse/dir.c
index 7058bbf69c3c..794b9f48fb8e 100644
--- a/fs/fuse/dir.c
+++ b/fs/fuse/dir.c
@@ -673,8 +673,10 @@ static int fuse_unlink(struct inode *dir, struct dentry *entry)
 		struct inode *inode = d_inode(entry);
 		struct fuse_inode *fi = get_fuse_inode(inode);
 
+		spin_lock(&fi->lock);
 		spin_lock(&fc->lock);
 		fi->attr_version = ++fc->attr_version;
+		spin_unlock(&fc->lock);
 		/*
 		 * If i_nlink == 0 then unlink doesn't make sense, yet this can
 		 * happen if userspace filesystem is careless.  It would be
@@ -683,7 +685,7 @@ static int fuse_unlink(struct inode *dir, struct dentry *entry)
 		 */
 		if (inode->i_nlink > 0)
 			drop_nlink(inode);
-		spin_unlock(&fc->lock);
+		spin_unlock(&fi->lock);
 		fuse_invalidate_attr(inode);
 		fuse_dir_changed(dir);
 		fuse_invalidate_entry_cache(entry);
@@ -827,10 +829,12 @@ static int fuse_link(struct dentry *entry, struct inode *newdir,
 	if (!err) {
 		struct fuse_inode *fi = get_fuse_inode(inode);
 
+		spin_lock(&fi->lock);
 		spin_lock(&fc->lock);
 		fi->attr_version = ++fc->attr_version;
-		inc_nlink(inode);
 		spin_unlock(&fc->lock);
+		inc_nlink(inode);
+		spin_unlock(&fi->lock);
 		fuse_invalidate_attr(inode);
 		fuse_update_ctime(inode);
 	} else if (err == -EINTR) {
@@ -1338,15 +1342,14 @@ static void iattr_to_fattr(struct fuse_conn *fc, struct iattr *iattr,
  */
 void fuse_set_nowrite(struct inode *inode)
 {
-	struct fuse_conn *fc = get_fuse_conn(inode);
 	struct fuse_inode *fi = get_fuse_inode(inode);
 
 	BUG_ON(!inode_is_locked(inode));
 
-	spin_lock(&fc->lock);
+	spin_lock(&fi->lock);
 	BUG_ON(fi->writectr < 0);
 	fi->writectr += FUSE_NOWRITE;
-	spin_unlock(&fc->lock);
+	spin_unlock(&fi->lock);
 	wait_event(fi->page_waitq, fi->writectr == FUSE_NOWRITE);
 }
 
@@ -1367,11 +1370,11 @@ static void __fuse_release_nowrite(struct inode *inode)
 
 void fuse_release_nowrite(struct inode *inode)
 {
-	struct fuse_conn *fc = get_fuse_conn(inode);
+	struct fuse_inode *fi = get_fuse_inode(inode);
 
-	spin_lock(&fc->lock);
+	spin_lock(&fi->lock);
 	__fuse_release_nowrite(inode);
-	spin_unlock(&fc->lock);
+	spin_unlock(&fi->lock);
 }
 
 static void fuse_setattr_fill(struct fuse_conn *fc, struct fuse_args *args,
@@ -1506,7 +1509,7 @@ int fuse_do_setattr(struct dentry *dentry, struct iattr *attr,
 		goto error;
 	}
 
-	spin_lock(&fc->lock);
+	spin_lock(&fi->lock);
 	/* the kernel maintains i_mtime locally */
 	if (trust_local_cmtime) {
 		if (attr->ia_valid & ATTR_MTIME)
@@ -1524,10 +1527,10 @@ int fuse_do_setattr(struct dentry *dentry, struct iattr *attr,
 		i_size_write(inode, outarg.attr.size);
 
 	if (is_truncate) {
-		/* NOTE: this may release/reacquire fc->lock */
+		/* NOTE: this may release/reacquire fi->lock */
 		__fuse_release_nowrite(inode);
 	}
-	spin_unlock(&fc->lock);
+	spin_unlock(&fi->lock);
 
 	/*
 	 * Only call invalidate_inode_pages2() after removing
diff --git a/fs/fuse/file.c b/fs/fuse/file.c
index fa7580d47d0c..48e2889ba6a6 100644
--- a/fs/fuse/file.c
+++ b/fs/fuse/file.c
@@ -159,17 +159,16 @@ EXPORT_SYMBOL_GPL(fuse_do_open);
 static void fuse_link_write_file(struct file *file)
 {
 	struct inode *inode = file_inode(file);
-	struct fuse_conn *fc = get_fuse_conn(inode);
 	struct fuse_inode *fi = get_fuse_inode(inode);
 	struct fuse_file *ff = file->private_data;
 	/*
 	 * file may be written through mmap, so chain it onto the
 	 * inodes's write_file list
 	 */
-	spin_lock(&fc->lock);
+	spin_lock(&fi->lock);
 	if (list_empty(&ff->write_entry))
 		list_add(&ff->write_entry, &fi->write_files);
-	spin_unlock(&fc->lock);
+	spin_unlock(&fi->lock);
 }
 
 void fuse_finish_open(struct inode *inode, struct file *file)
@@ -186,10 +185,12 @@ void fuse_finish_open(struct inode *inode, struct file *file)
 	if (fc->atomic_o_trunc && (file->f_flags & O_TRUNC)) {
 		struct fuse_inode *fi = get_fuse_inode(inode);
 
+		spin_lock(&fi->lock);
 		spin_lock(&fc->lock);
 		fi->attr_version = ++fc->attr_version;
-		i_size_write(inode, 0);
 		spin_unlock(&fc->lock);
+		i_size_write(inode, 0);
+		spin_unlock(&fi->lock);
 		fuse_invalidate_attr(inode);
 		if (fc->writeback_cache)
 			file_update_time(file);
@@ -231,8 +232,13 @@ static void fuse_prepare_release(struct fuse_inode *fi, struct fuse_file *ff,
 	struct fuse_req *req = ff->reserved_req;
 	struct fuse_release_in *inarg = &req->misc.release.in;
 
+	/* Inode is NULL on error path of fuse_create_open() */
+	if (likely(fi)) {
+		spin_lock(&fi->lock);
+		list_del(&ff->write_entry);
+		spin_unlock(&fi->lock);
+	}
 	spin_lock(&fc->lock);
-	list_del(&ff->write_entry);
 	if (!RB_EMPTY_NODE(&ff->polled_node))
 		rb_erase(&ff->polled_node, &fc->polled_files);
 	spin_unlock(&fc->lock);
@@ -339,12 +345,11 @@ u64 fuse_lock_owner_id(struct fuse_conn *fc, fl_owner_t id)
 static bool fuse_range_is_writeback(struct inode *inode, pgoff_t idx_from,
 				   pgoff_t idx_to)
 {
-	struct fuse_conn *fc = get_fuse_conn(inode);
 	struct fuse_inode *fi = get_fuse_inode(inode);
 	struct fuse_req *req;
 	bool found = false;
 
-	spin_lock(&fc->lock);
+	spin_lock(&fi->lock);
 	list_for_each_entry(req, &fi->writepages, writepages_entry) {
 		pgoff_t curr_index;
 
@@ -356,7 +361,7 @@ static bool fuse_range_is_writeback(struct inode *inode, pgoff_t idx_from,
 			break;
 		}
 	}
-	spin_unlock(&fc->lock);
+	spin_unlock(&fi->lock);
 
 	return found;
 }
@@ -598,9 +603,11 @@ static void fuse_aio_complete(struct fuse_io_priv *io, int err, ssize_t pos)
 			struct fuse_conn *fc = get_fuse_conn(inode);
 			struct fuse_inode *fi = get_fuse_inode(inode);
 
+			spin_lock(&fi->lock);
 			spin_lock(&fc->lock);
 			fi->attr_version = ++fc->attr_version;
 			spin_unlock(&fc->lock);
+			spin_unlock(&fi->lock);
 		}
 
 		io->iocb->ki_complete(io->iocb, res, 0);
@@ -675,13 +682,15 @@ static void fuse_read_update_size(struct inode *inode, loff_t size,
 	struct fuse_conn *fc = get_fuse_conn(inode);
 	struct fuse_inode *fi = get_fuse_inode(inode);
 
-	spin_lock(&fc->lock);
+	spin_lock(&fi->lock);
 	if (attr_ver == fi->attr_version && size < inode->i_size &&
 	    !test_bit(FUSE_I_SIZE_UNSTABLE, &fi->state)) {
+		spin_lock(&fc->lock);
 		fi->attr_version = ++fc->attr_version;
+		spin_unlock(&fc->lock);
 		i_size_write(inode, size);
 	}
-	spin_unlock(&fc->lock);
+	spin_unlock(&fi->lock);
 }
 
 static void fuse_short_read(struct fuse_req *req, struct inode *inode,
@@ -996,13 +1005,15 @@ bool fuse_write_update_size(struct inode *inode, loff_t pos)
 	struct fuse_inode *fi = get_fuse_inode(inode);
 	bool ret = false;
 
+	spin_lock(&fi->lock);
 	spin_lock(&fc->lock);
 	fi->attr_version = ++fc->attr_version;
+	spin_unlock(&fc->lock);
 	if (pos > inode->i_size) {
 		i_size_write(inode, pos);
 		ret = true;
 	}
-	spin_unlock(&fc->lock);
+	spin_unlock(&fi->lock);
 
 	return ret;
 }
@@ -1481,20 +1492,17 @@ static void fuse_writepage_finish(struct fuse_conn *fc, struct fuse_req *req)
 	wake_up(&fi->page_waitq);
 }
 
-/* Called under fc->lock, may release and reacquire it */
+/* Called under fi->lock, may release and reacquire it */
 static void fuse_send_writepage(struct fuse_conn *fc, struct fuse_req *req,
 				loff_t size)
-__releases(fc->lock)
-__acquires(fc->lock)
+__releases(fi->lock)
+__acquires(fi->lock)
 {
 	struct fuse_inode *fi = get_fuse_inode(req->inode);
 	struct fuse_write_in *inarg = &req->misc.write.in;
 	__u64 data_size = req->num_pages * PAGE_SIZE;
 	bool queued;
 
-	if (!fc->connected)
-		goto out_free;
-
 	if (inarg->offset + data_size <= size) {
 		inarg->size = data_size;
 	} else if (inarg->offset < size) {
@@ -1505,28 +1513,31 @@ __acquires(fc->lock)
 	}
 
 	req->in.args[1].size = inarg->size;
-	fi->writectr++;
 	queued = fuse_request_queue_background(fc, req);
-	WARN_ON(!queued);
+	/* Fails on broken connection only */
+	if (unlikely(!queued))
+		goto out_free;
+
+	fi->writectr++;
 	return;
 
  out_free:
 	fuse_writepage_finish(fc, req);
-	spin_unlock(&fc->lock);
+	spin_unlock(&fi->lock);
 	fuse_writepage_free(fc, req);
 	fuse_put_request(fc, req);
-	spin_lock(&fc->lock);
+	spin_lock(&fi->lock);
 }
 
 /*
  * If fi->writectr is positive (no truncate or fsync going on) send
  * all queued writepage requests.
  *
- * Called with fc->lock
+ * Called with fi->lock
  */
 void fuse_flush_writepages(struct fuse_inode *fi)
-__releases(fc->lock)
-__acquires(fc->lock)
+__releases(fi->lock)
+__acquires(fi->lock)
 {
 	struct inode *inode = &fi->inode;
 	struct fuse_conn *fc = get_fuse_conn(inode);
@@ -1546,7 +1557,7 @@ static void fuse_writepage_end(struct fuse_conn *fc, struct fuse_req *req)
 	struct fuse_inode *fi = get_fuse_inode(inode);
 
 	mapping_set_error(inode->i_mapping, req->out.h.error);
-	spin_lock(&fc->lock);
+	spin_lock(&fi->lock);
 	while (req->misc.write.next) {
 		struct fuse_conn *fc = get_fuse_conn(inode);
 		struct fuse_write_in *inarg = &req->misc.write.in;
@@ -1583,7 +1594,7 @@ static void fuse_writepage_end(struct fuse_conn *fc, struct fuse_req *req)
 	}
 	fi->writectr--;
 	fuse_writepage_finish(fc, req);
-	spin_unlock(&fc->lock);
+	spin_unlock(&fi->lock);
 	fuse_writepage_free(fc, req);
 }
 
@@ -1592,13 +1603,13 @@ static struct fuse_file *__fuse_write_file_get(struct fuse_conn *fc,
 {
 	struct fuse_file *ff = NULL;
 
-	spin_lock(&fc->lock);
+	spin_lock(&fi->lock);
 	if (!list_empty(&fi->write_files)) {
 		ff = list_entry(fi->write_files.next, struct fuse_file,
 				write_entry);
 		fuse_file_get(ff);
 	}
-	spin_unlock(&fc->lock);
+	spin_unlock(&fi->lock);
 
 	return ff;
 }
@@ -1669,11 +1680,11 @@ static int fuse_writepage_locked(struct page *page)
 	inc_wb_stat(&inode_to_bdi(inode)->wb, WB_WRITEBACK);
 	inc_node_page_state(tmp_page, NR_WRITEBACK_TEMP);
 
-	spin_lock(&fc->lock);
+	spin_lock(&fi->lock);
 	list_add(&req->writepages_entry, &fi->writepages);
 	list_add_tail(&req->list, &fi->queued_writes);
 	fuse_flush_writepages(fi);
-	spin_unlock(&fc->lock);
+	spin_unlock(&fi->lock);
 
 	end_page_writeback(page);
 
@@ -1722,16 +1733,15 @@ static void fuse_writepages_send(struct fuse_fill_wb_data *data)
 {
 	struct fuse_req *req = data->req;
 	struct inode *inode = data->inode;
-	struct fuse_conn *fc = get_fuse_conn(inode);
 	struct fuse_inode *fi = get_fuse_inode(inode);
 	int num_pages = req->num_pages;
 	int i;
 
 	req->ff = fuse_file_get(data->ff);
-	spin_lock(&fc->lock);
+	spin_lock(&fi->lock);
 	list_add_tail(&req->list, &fi->queued_writes);
 	fuse_flush_writepages(fi);
-	spin_unlock(&fc->lock);
+	spin_unlock(&fi->lock);
 
 	for (i = 0; i < num_pages; i++)
 		end_page_writeback(data->orig_pages[i]);
@@ -1749,7 +1759,7 @@ static bool fuse_writepage_in_flight(struct fuse_req *new_req,
 
 	BUG_ON(new_req->num_pages != 0);
 
-	spin_lock(&fc->lock);
+	spin_lock(&fi->lock);
 	list_del(&new_req->writepages_entry);
 	list_for_each_entry(old_req, &fi->writepages, writepages_entry) {
 		BUG_ON(old_req->inode != new_req->inode);
@@ -1779,7 +1789,7 @@ static bool fuse_writepage_in_flight(struct fuse_req *new_req,
 		struct backing_dev_info *bdi = inode_to_bdi(page->mapping->host);
 
 		copy_highpage(old_req->pages[0], page);
-		spin_unlock(&fc->lock);
+		spin_unlock(&fi->lock);
 
 		dec_wb_stat(&bdi->wb, WB_WRITEBACK);
 		dec_node_page_state(page, NR_WRITEBACK_TEMP);
@@ -1792,7 +1802,7 @@ static bool fuse_writepage_in_flight(struct fuse_req *new_req,
 		old_req->misc.write.next = new_req;
 	}
 out_unlock:
-	spin_unlock(&fc->lock);
+	spin_unlock(&fi->lock);
 out:
 	return found;
 }
@@ -1803,6 +1813,7 @@ static int fuse_writepages_fill(struct page *page,
 	struct fuse_fill_wb_data *data = _data;
 	struct fuse_req *req = data->req;
 	struct inode *inode = data->inode;
+	struct fuse_inode *fi = get_fuse_inode(inode);
 	struct fuse_conn *fc = get_fuse_conn(inode);
 	struct page *tmp_page;
 	bool is_writeback;
@@ -1873,9 +1884,9 @@ static int fuse_writepages_fill(struct page *page,
 		req->end = fuse_writepage_end;
 		req->inode = inode;
 
-		spin_lock(&fc->lock);
+		spin_lock(&fi->lock);
 		list_add(&req->writepages_entry, &fi->writepages);
-		spin_unlock(&fc->lock);
+		spin_unlock(&fi->lock);
 
 		data->req = req;
 	}
@@ -1898,12 +1909,12 @@ static int fuse_writepages_fill(struct page *page,
 	data->orig_pages[req->num_pages] = page;
 
 	/*
-	 * Protected by fc->lock against concurrent access by
+	 * Protected by fi->lock against concurrent access by
 	 * fuse_page_is_writeback().
 	 */
-	spin_lock(&fc->lock);
+	spin_lock(&fi->lock);
 	req->num_pages++;
-	spin_unlock(&fc->lock);
+	spin_unlock(&fi->lock);
 
 out_unlock:
 	unlock_page(page);
diff --git a/fs/fuse/fuse_i.h b/fs/fuse/fuse_i.h
index 9e23e871873b..075da40499b8 100644
--- a/fs/fuse/fuse_i.h
+++ b/fs/fuse/fuse_i.h
@@ -96,7 +96,7 @@ struct fuse_inode {
 	union {
 		/* Write related fields (regular file only) */
 		struct {
-			/* Files usable in writepage.  Protected by fc->lock */
+			/* Files usable in writepage.  Protected by fi->lock */
 			struct list_head write_files;
 
 			/* Writepages pending on truncate or fsync */
@@ -144,6 +144,9 @@ struct fuse_inode {
 
 	/** Lock for serializing lookup and readdir for back compatibility*/
 	struct mutex mutex;
+
+	/** Lock to protect write related fields */
+	spinlock_t lock;
 };
 
 /** FUSE inode state bits */
@@ -500,6 +503,8 @@ struct fuse_dev {
  * This structure is created, when the filesystem is mounted, and is
  * destroyed, when the client device is closed and the filesystem is
  * unmounted.
+ *
+ * Locking order: fuse_inode::lock -> fuse_conn::lock -> fuse_conn::bg_lock
  */
 struct fuse_conn {
 	/** Lock protecting accessess to  members of this structure */
diff --git a/fs/fuse/inode.c b/fs/fuse/inode.c
index 4727ef612019..f62d45686b42 100644
--- a/fs/fuse/inode.c
+++ b/fs/fuse/inode.c
@@ -97,6 +97,7 @@ static struct inode *fuse_alloc_inode(struct super_block *sb)
 	fi->orig_ino = 0;
 	fi->state = 0;
 	mutex_init(&fi->mutex);
+	spin_lock_init(&fi->lock);
 	fi->forget = fuse_alloc_forget();
 	if (!fi->forget) {
 		kmem_cache_free(fuse_inode_cachep, inode);
@@ -164,7 +165,12 @@ void fuse_change_attributes_common(struct inode *inode, struct fuse_attr *attr,
 	struct fuse_conn *fc = get_fuse_conn(inode);
 	struct fuse_inode *fi = get_fuse_inode(inode);
 
+	lockdep_assert_held(&fi->lock);
+
+	spin_lock(&fc->lock);
 	fi->attr_version = ++fc->attr_version;
+	spin_unlock(&fc->lock);
+
 	fi->i_time = attr_valid;
 	WRITE_ONCE(fi->inval_mask, 0);
 
@@ -210,10 +216,10 @@ void fuse_change_attributes(struct inode *inode, struct fuse_attr *attr,
 	loff_t oldsize;
 	struct timespec64 old_mtime;
 
-	spin_lock(&fc->lock);
+	spin_lock(&fi->lock);
 	if ((attr_version != 0 && fi->attr_version > attr_version) ||
 	    test_bit(FUSE_I_SIZE_UNSTABLE, &fi->state)) {
-		spin_unlock(&fc->lock);
+		spin_unlock(&fi->lock);
 		return;
 	}
 
@@ -228,7 +234,7 @@ void fuse_change_attributes(struct inode *inode, struct fuse_attr *attr,
 	 */
 	if (!is_wb || !S_ISREG(inode->i_mode))
 		i_size_write(inode, attr->size);
-	spin_unlock(&fc->lock);
+	spin_unlock(&fi->lock);
 
 	if (!is_wb && S_ISREG(inode->i_mode)) {
 		bool inval = false;

[PATCH 4/6] fuse: Implement fuse_attr_version_inc()

[Kirill Tkhai]

For cleanup purpose archive repeating pattern:

	spin_lock(&fc->lock);
	fi->attr_version = ++fc->attr_version;
	spin_unlock(&fc->lock);

into separate function.

Signed-off-by: Kirill Tkhai <ktkhai@virtuozzo.com>
---
 fs/fuse/dir.c    |    8 ++------
 fs/fuse/file.c   |   16 ++++------------
 fs/fuse/fuse_i.h |   11 +++++++++++
 fs/fuse/inode.c  |    4 +---
 4 files changed, 18 insertions(+), 21 deletions(-)

diff --git a/fs/fuse/dir.c b/fs/fuse/dir.c
index 794b9f48fb8e..35f3b3d1e044 100644
--- a/fs/fuse/dir.c
+++ b/fs/fuse/dir.c
@@ -674,9 +674,7 @@ static int fuse_unlink(struct inode *dir, struct dentry *entry)
 		struct fuse_inode *fi = get_fuse_inode(inode);
 
 		spin_lock(&fi->lock);
-		spin_lock(&fc->lock);
-		fi->attr_version = ++fc->attr_version;
-		spin_unlock(&fc->lock);
+		fi->attr_version = fuse_attr_version_inc_return(fc);
 		/*
 		 * If i_nlink == 0 then unlink doesn't make sense, yet this can
 		 * happen if userspace filesystem is careless.  It would be
@@ -830,9 +828,7 @@ static int fuse_link(struct dentry *entry, struct inode *newdir,
 		struct fuse_inode *fi = get_fuse_inode(inode);
 
 		spin_lock(&fi->lock);
-		spin_lock(&fc->lock);
-		fi->attr_version = ++fc->attr_version;
-		spin_unlock(&fc->lock);
+		fi->attr_version = fuse_attr_version_inc_return(fc);
 		inc_nlink(inode);
 		spin_unlock(&fi->lock);
 		fuse_invalidate_attr(inode);
diff --git a/fs/fuse/file.c b/fs/fuse/file.c
index 48e2889ba6a6..1164d3580c62 100644
--- a/fs/fuse/file.c
+++ b/fs/fuse/file.c
@@ -186,9 +186,7 @@ void fuse_finish_open(struct inode *inode, struct file *file)
 		struct fuse_inode *fi = get_fuse_inode(inode);
 
 		spin_lock(&fi->lock);
-		spin_lock(&fc->lock);
-		fi->attr_version = ++fc->attr_version;
-		spin_unlock(&fc->lock);
+		fi->attr_version = fuse_attr_version_inc_return(fc);
 		i_size_write(inode, 0);
 		spin_unlock(&fi->lock);
 		fuse_invalidate_attr(inode);
@@ -604,9 +602,7 @@ static void fuse_aio_complete(struct fuse_io_priv *io, int err, ssize_t pos)
 			struct fuse_inode *fi = get_fuse_inode(inode);
 
 			spin_lock(&fi->lock);
-			spin_lock(&fc->lock);
-			fi->attr_version = ++fc->attr_version;
-			spin_unlock(&fc->lock);
+			fi->attr_version = fuse_attr_version_inc_return(fc);
 			spin_unlock(&fi->lock);
 		}
 
@@ -685,9 +681,7 @@ static void fuse_read_update_size(struct inode *inode, loff_t size,
 	spin_lock(&fi->lock);
 	if (attr_ver == fi->attr_version && size < inode->i_size &&
 	    !test_bit(FUSE_I_SIZE_UNSTABLE, &fi->state)) {
-		spin_lock(&fc->lock);
-		fi->attr_version = ++fc->attr_version;
-		spin_unlock(&fc->lock);
+		fi->attr_version = fuse_attr_version_inc_return(fc);
 		i_size_write(inode, size);
 	}
 	spin_unlock(&fi->lock);
@@ -1006,9 +1000,7 @@ bool fuse_write_update_size(struct inode *inode, loff_t pos)
 	bool ret = false;
 
 	spin_lock(&fi->lock);
-	spin_lock(&fc->lock);
-	fi->attr_version = ++fc->attr_version;
-	spin_unlock(&fc->lock);
+	fi->attr_version = fuse_attr_version_inc_return(fc);
 	if (pos > inode->i_size) {
 		i_size_write(inode, pos);
 		ret = true;
diff --git a/fs/fuse/fuse_i.h b/fs/fuse/fuse_i.h
index 075da40499b8..f7442bcecbb0 100644
--- a/fs/fuse/fuse_i.h
+++ b/fs/fuse/fuse_i.h
@@ -775,6 +775,17 @@ static inline int invalid_nodeid(u64 nodeid)
 	return !nodeid || nodeid == FUSE_ROOT_ID;
 }
 
+static inline u64 fuse_attr_version_inc_return(struct fuse_conn *fc)
+{
+	u64 attr_version;
+
+	spin_lock(&fc->lock);
+	attr_version = ++fc->attr_version;
+	spin_unlock(&fc->lock);
+
+	return attr_version;
+}
+
 /** Device operations */
 extern const struct file_operations fuse_dev_operations;
 
diff --git a/fs/fuse/inode.c b/fs/fuse/inode.c
index f62d45686b42..5f488b019cd9 100644
--- a/fs/fuse/inode.c
+++ b/fs/fuse/inode.c
@@ -167,9 +167,7 @@ void fuse_change_attributes_common(struct inode *inode, struct fuse_attr *attr,
 
 	lockdep_assert_held(&fi->lock);
 
-	spin_lock(&fc->lock);
-	fi->attr_version = ++fc->attr_version;
-	spin_unlock(&fc->lock);
+	fi->attr_version = fuse_attr_version_inc_return(fc);
 
 	fi->i_time = attr_valid;
 	WRITE_ONCE(fi->inval_mask, 0);

[PATCH 5/6] fuse: Protect fuse_inode::nlookup with fuse_inode::lock

[Kirill Tkhai]

This continues previous patch and introduces the same
protection for nlookup field. It goes as separate patch
since it's separate logic change (sadly, but it looks
impossible to split previous patch more then in this way).

Signed-off-by: Kirill Tkhai <ktkhai@virtuozzo.com>
---
 fs/fuse/dir.c     |    4 ++--
 fs/fuse/inode.c   |    4 ++--
 fs/fuse/readdir.c |    4 ++--
 3 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/fs/fuse/dir.c b/fs/fuse/dir.c
index 35f3b3d1e044..ac8519285327 100644
--- a/fs/fuse/dir.c
+++ b/fs/fuse/dir.c
@@ -222,9 +222,9 @@ static int fuse_dentry_revalidate(struct dentry *entry, unsigned int flags)
 				fuse_queue_forget(fc, forget, outarg.nodeid, 1);
 				goto invalid;
 			}
-			spin_lock(&fc->lock);
+			spin_lock(&fi->lock);
 			fi->nlookup++;
-			spin_unlock(&fc->lock);
+			spin_unlock(&fi->lock);
 		}
 		kfree(forget);
 		if (ret == -ENOMEM)
diff --git a/fs/fuse/inode.c b/fs/fuse/inode.c
index 5f488b019cd9..b8092d49a4b2 100644
--- a/fs/fuse/inode.c
+++ b/fs/fuse/inode.c
@@ -327,9 +327,9 @@ struct inode *fuse_iget(struct super_block *sb, u64 nodeid,
 	}
 
 	fi = get_fuse_inode(inode);
-	spin_lock(&fc->lock);
+	spin_lock(&fi->lock);
 	fi->nlookup++;
-	spin_unlock(&fc->lock);
+	spin_unlock(&fi->lock);
 	fuse_change_attributes(inode, attr, attr_valid, attr_version);
 
 	return inode;
diff --git a/fs/fuse/readdir.c b/fs/fuse/readdir.c
index ab18b78f4755..574d03f8a573 100644
--- a/fs/fuse/readdir.c
+++ b/fs/fuse/readdir.c
@@ -213,9 +213,9 @@ static int fuse_direntplus_link(struct file *file,
 		}
 
 		fi = get_fuse_inode(inode);
-		spin_lock(&fc->lock);
+		spin_lock(&fi->lock);
 		fi->nlookup++;
-		spin_unlock(&fc->lock);
+		spin_unlock(&fi->lock);
 
 		forget_all_cached_acls(inode);
 		fuse_change_attributes(inode, &o->attr,

[PATCH 6/6] fuse: Protect fuse_file::reserved_req via corresponding fuse_inode::lock

[Kirill Tkhai]

This is rather natural action after previous patches,
and it just decreases load of fc->lock.

Signed-off-by: Kirill Tkhai <ktkhai@virtuozzo.com>
---
 fs/fuse/dev.c    |   10 ++++++----
 fs/fuse/fuse_i.h |    5 ++++-
 2 files changed, 10 insertions(+), 5 deletions(-)

diff --git a/fs/fuse/dev.c b/fs/fuse/dev.c
index 3bfc5ed61c9a..cc8b95ad5b16 100644
--- a/fs/fuse/dev.c
+++ b/fs/fuse/dev.c
@@ -247,17 +247,18 @@ static struct fuse_req *get_reserved_req(struct fuse_conn *fc,
 					 struct file *file)
 {
 	struct fuse_req *req = NULL;
+	struct fuse_inode *fi = get_fuse_inode(file_inode(file));
 	struct fuse_file *ff = file->private_data;
 
 	do {
 		wait_event(fc->reserved_req_waitq, ff->reserved_req);
-		spin_lock(&fc->lock);
+		spin_lock(&fi->lock);
 		if (ff->reserved_req) {
 			req = ff->reserved_req;
 			ff->reserved_req = NULL;
 			req->stolen_file = get_file(file);
 		}
-		spin_unlock(&fc->lock);
+		spin_unlock(&fi->lock);
 	} while (!req);
 
 	return req;
@@ -269,16 +270,17 @@ static struct fuse_req *get_reserved_req(struct fuse_conn *fc,
 static void put_reserved_req(struct fuse_conn *fc, struct fuse_req *req)
 {
 	struct file *file = req->stolen_file;
+	struct fuse_inode *fi = get_fuse_inode(file_inode(file));
 	struct fuse_file *ff = file->private_data;
 
 	WARN_ON(req->max_pages);
-	spin_lock(&fc->lock);
+	spin_lock(&fi->lock);
 	memset(req, 0, sizeof(*req));
 	fuse_request_init(req, NULL, NULL, 0);
 	BUG_ON(ff->reserved_req);
 	ff->reserved_req = req;
 	wake_up_all(&fc->reserved_req_waitq);
-	spin_unlock(&fc->lock);
+	spin_unlock(&fi->lock);
 	fput(file);
 }
 
diff --git a/fs/fuse/fuse_i.h b/fs/fuse/fuse_i.h
index f7442bcecbb0..10b20a24f693 100644
--- a/fs/fuse/fuse_i.h
+++ b/fs/fuse/fuse_i.h
@@ -166,7 +166,10 @@ struct fuse_file {
 	/** Fuse connection for this file */
 	struct fuse_conn *fc;
 
-	/** Request reserved for flush and release */
+	/*
+	 * Request reserved for flush and release.
+	 * Modified under relative fuse_inode::lock.
+	 */
 	struct fuse_req *reserved_req;
 
 	/** Kernel file handle guaranteed to be unique */

Re: [PATCH 1/6] fuse: Change argument of fuse_flush_writepages()

[Miklos Szeredi]

On Tue, Nov 6, 2018 at 10:43 AM, Kirill Tkhai <ktkhai@virtuozzo.com> wrote:
> Next patches introduce fuse_inode::lock, which will be used
> in __releases() and __acquires() instead of fc->lock.
> This patch makes this function to use argument fuse_inode
> instead of inode as preparation for that.

This patch seems like perfectly useless, just change fc->lock to
fi->lock in the next patch and be done with that.


>
> Signed-off-by: Kirill Tkhai <ktkhai@virtuozzo.com>
> ---
>  fs/fuse/dir.c    |    2 +-
>  fs/fuse/file.c   |    8 ++++----
>  fs/fuse/fuse_i.h |    2 +-
>  3 files changed, 6 insertions(+), 6 deletions(-)
>
> diff --git a/fs/fuse/dir.c b/fs/fuse/dir.c
> index 47395b0c3b35..c71f7e9ee0f7 100644
> --- a/fs/fuse/dir.c
> +++ b/fs/fuse/dir.c
> @@ -1360,7 +1360,7 @@ static void __fuse_release_nowrite(struct inode *inode)
>
>         BUG_ON(fi->writectr != FUSE_NOWRITE);
>         fi->writectr = 0;
> -       fuse_flush_writepages(inode);
> +       fuse_flush_writepages(fi);
>  }
>
>  void fuse_release_nowrite(struct inode *inode)
> diff --git a/fs/fuse/file.c b/fs/fuse/file.c
> index cc2121b37bf5..d5bd29610875 100644
> --- a/fs/fuse/file.c
> +++ b/fs/fuse/file.c
> @@ -1522,12 +1522,12 @@ __acquires(fc->lock)
>   *
>   * Called with fc->lock
>   */
> -void fuse_flush_writepages(struct inode *inode)
> +void fuse_flush_writepages(struct fuse_inode *fi)
>  __releases(fc->lock)
>  __acquires(fc->lock)
>  {
> +       struct inode *inode = &fi->inode;
>         struct fuse_conn *fc = get_fuse_conn(inode);
> -       struct fuse_inode *fi = get_fuse_inode(inode);
>         size_t crop = i_size_read(inode);
>         struct fuse_req *req;
>
> @@ -1670,7 +1670,7 @@ static int fuse_writepage_locked(struct page *page)
>         spin_lock(&fc->lock);
>         list_add(&req->writepages_entry, &fi->writepages);
>         list_add_tail(&req->list, &fi->queued_writes);
> -       fuse_flush_writepages(inode);
> +       fuse_flush_writepages(fi);
>         spin_unlock(&fc->lock);
>
>         end_page_writeback(page);
> @@ -1728,7 +1728,7 @@ static void fuse_writepages_send(struct fuse_fill_wb_data *data)
>         req->ff = fuse_file_get(data->ff);
>         spin_lock(&fc->lock);
>         list_add_tail(&req->list, &fi->queued_writes);
> -       fuse_flush_writepages(inode);
> +       fuse_flush_writepages(fi);
>         spin_unlock(&fc->lock);
>
>         for (i = 0; i < num_pages; i++)
> diff --git a/fs/fuse/fuse_i.h b/fs/fuse/fuse_i.h
> index e9f712e81c7d..38bd7ca1908a 100644
> --- a/fs/fuse/fuse_i.h
> +++ b/fs/fuse/fuse_i.h
> @@ -995,7 +995,7 @@ void fuse_update_ctime(struct inode *inode);
>
>  int fuse_update_attributes(struct inode *inode, struct file *file);
>
> -void fuse_flush_writepages(struct inode *inode);
> +void fuse_flush_writepages(struct fuse_inode *fi);
>
>  void fuse_set_nowrite(struct inode *inode);
>  void fuse_release_nowrite(struct inode *inode);
>

Re: [PATCH 4/6] fuse: Implement fuse_attr_version_inc()

[Miklos Szeredi]

On Tue, Nov 6, 2018 at 10:43 AM, Kirill Tkhai <ktkhai@virtuozzo.com> wrote:
> For cleanup purpose archive repeating pattern:
>
>         spin_lock(&fc->lock);
>         fi->attr_version = ++fc->attr_version;
>         spin_unlock(&fc->lock);
>
> into separate function.
>
> Signed-off-by: Kirill Tkhai <ktkhai@virtuozzo.com>
> ---
>  fs/fuse/dir.c    |    8 ++------
>  fs/fuse/file.c   |   16 ++++------------
>  fs/fuse/fuse_i.h |   11 +++++++++++
>  fs/fuse/inode.c  |    4 +---
>  4 files changed, 18 insertions(+), 21 deletions(-)
>
> diff --git a/fs/fuse/dir.c b/fs/fuse/dir.c
> index 794b9f48fb8e..35f3b3d1e044 100644
> --- a/fs/fuse/dir.c
> +++ b/fs/fuse/dir.c
> @@ -674,9 +674,7 @@ static int fuse_unlink(struct inode *dir, struct dentry *entry)
>                 struct fuse_inode *fi = get_fuse_inode(inode);
>
>                 spin_lock(&fi->lock);
> -               spin_lock(&fc->lock);
> -               fi->attr_version = ++fc->attr_version;
> -               spin_unlock(&fc->lock);
> +               fi->attr_version = fuse_attr_version_inc_return(fc);
>                 /*
>                  * If i_nlink == 0 then unlink doesn't make sense, yet this can
>                  * happen if userspace filesystem is careless.  It would be
> @@ -830,9 +828,7 @@ static int fuse_link(struct dentry *entry, struct inode *newdir,
>                 struct fuse_inode *fi = get_fuse_inode(inode);
>
>                 spin_lock(&fi->lock);
> -               spin_lock(&fc->lock);
> -               fi->attr_version = ++fc->attr_version;
> -               spin_unlock(&fc->lock);
> +               fi->attr_version = fuse_attr_version_inc_return(fc);
>                 inc_nlink(inode);
>                 spin_unlock(&fi->lock);
>                 fuse_invalidate_attr(inode);
> diff --git a/fs/fuse/file.c b/fs/fuse/file.c
> index 48e2889ba6a6..1164d3580c62 100644
> --- a/fs/fuse/file.c
> +++ b/fs/fuse/file.c
> @@ -186,9 +186,7 @@ void fuse_finish_open(struct inode *inode, struct file *file)
>                 struct fuse_inode *fi = get_fuse_inode(inode);
>
>                 spin_lock(&fi->lock);
> -               spin_lock(&fc->lock);
> -               fi->attr_version = ++fc->attr_version;
> -               spin_unlock(&fc->lock);
> +               fi->attr_version = fuse_attr_version_inc_return(fc);
>                 i_size_write(inode, 0);
>                 spin_unlock(&fi->lock);
>                 fuse_invalidate_attr(inode);
> @@ -604,9 +602,7 @@ static void fuse_aio_complete(struct fuse_io_priv *io, int err, ssize_t pos)
>                         struct fuse_inode *fi = get_fuse_inode(inode);
>
>                         spin_lock(&fi->lock);
> -                       spin_lock(&fc->lock);
> -                       fi->attr_version = ++fc->attr_version;
> -                       spin_unlock(&fc->lock);
> +                       fi->attr_version = fuse_attr_version_inc_return(fc);
>                         spin_unlock(&fi->lock);
>                 }
>
> @@ -685,9 +681,7 @@ static void fuse_read_update_size(struct inode *inode, loff_t size,
>         spin_lock(&fi->lock);
>         if (attr_ver == fi->attr_version && size < inode->i_size &&
>             !test_bit(FUSE_I_SIZE_UNSTABLE, &fi->state)) {
> -               spin_lock(&fc->lock);
> -               fi->attr_version = ++fc->attr_version;
> -               spin_unlock(&fc->lock);
> +               fi->attr_version = fuse_attr_version_inc_return(fc);
>                 i_size_write(inode, size);
>         }
>         spin_unlock(&fi->lock);
> @@ -1006,9 +1000,7 @@ bool fuse_write_update_size(struct inode *inode, loff_t pos)
>         bool ret = false;
>
>         spin_lock(&fi->lock);
> -       spin_lock(&fc->lock);
> -       fi->attr_version = ++fc->attr_version;
> -       spin_unlock(&fc->lock);
> +       fi->attr_version = fuse_attr_version_inc_return(fc);
>         if (pos > inode->i_size) {
>                 i_size_write(inode, pos);
>                 ret = true;
> diff --git a/fs/fuse/fuse_i.h b/fs/fuse/fuse_i.h
> index 075da40499b8..f7442bcecbb0 100644
> --- a/fs/fuse/fuse_i.h
> +++ b/fs/fuse/fuse_i.h
> @@ -775,6 +775,17 @@ static inline int invalid_nodeid(u64 nodeid)
>         return !nodeid || nodeid == FUSE_ROOT_ID;
>  }
>
> +static inline u64 fuse_attr_version_inc_return(struct fuse_conn *fc)
> +{
> +       u64 attr_version;
> +
> +       spin_lock(&fc->lock);
> +       attr_version = ++fc->attr_version;
> +       spin_unlock(&fc->lock);
> +
> +       return attr_version;
> +}

atomic64_t?

And move this patch one up, 3/6 is complicated as is without having to
add extra locking around fc->attr_version.



> +
>  /** Device operations */
>  extern const struct file_operations fuse_dev_operations;
>
> diff --git a/fs/fuse/inode.c b/fs/fuse/inode.c
> index f62d45686b42..5f488b019cd9 100644
> --- a/fs/fuse/inode.c
> +++ b/fs/fuse/inode.c
> @@ -167,9 +167,7 @@ void fuse_change_attributes_common(struct inode *inode, struct fuse_attr *attr,
>
>         lockdep_assert_held(&fi->lock);
>
> -       spin_lock(&fc->lock);
> -       fi->attr_version = ++fc->attr_version;
> -       spin_unlock(&fc->lock);
> +       fi->attr_version = fuse_attr_version_inc_return(fc);
>
>         fi->i_time = attr_valid;
>         WRITE_ONCE(fi->inval_mask, 0);
>

Re: [PATCH 5/6] fuse: Protect fuse_inode::nlookup with fuse_inode::lock

[Miklos Szeredi]

On Tue, Nov 6, 2018 at 10:44 AM, Kirill Tkhai <ktkhai@virtuozzo.com> wrote:
> This continues previous patch and introduces the same
> protection for nlookup field. It goes as separate patch
> since it's separate logic change (sadly, but it looks
> impossible to split previous patch more then in this way).

Well, the way you can split things up better is to just add the fine
grained lock first, which you can do in multiple patches, since all of
it is going to be a no-op.  And when that is done, remove the coarse
grained lock in yet another patch, which hopefully will result in
better performance and no other change.

>
> Signed-off-by: Kirill Tkhai <ktkhai@virtuozzo.com>
> ---
>  fs/fuse/dir.c     |    4 ++--
>  fs/fuse/inode.c   |    4 ++--
>  fs/fuse/readdir.c |    4 ++--
>  3 files changed, 6 insertions(+), 6 deletions(-)
>
> diff --git a/fs/fuse/dir.c b/fs/fuse/dir.c
> index 35f3b3d1e044..ac8519285327 100644
> --- a/fs/fuse/dir.c
> +++ b/fs/fuse/dir.c
> @@ -222,9 +222,9 @@ static int fuse_dentry_revalidate(struct dentry *entry, unsigned int flags)
>                                 fuse_queue_forget(fc, forget, outarg.nodeid, 1);
>                                 goto invalid;
>                         }
> -                       spin_lock(&fc->lock);
> +                       spin_lock(&fi->lock);
>                         fi->nlookup++;
> -                       spin_unlock(&fc->lock);
> +                       spin_unlock(&fi->lock);
>                 }
>                 kfree(forget);
>                 if (ret == -ENOMEM)
> diff --git a/fs/fuse/inode.c b/fs/fuse/inode.c
> index 5f488b019cd9..b8092d49a4b2 100644
> --- a/fs/fuse/inode.c
> +++ b/fs/fuse/inode.c
> @@ -327,9 +327,9 @@ struct inode *fuse_iget(struct super_block *sb, u64 nodeid,
>         }
>
>         fi = get_fuse_inode(inode);
> -       spin_lock(&fc->lock);
> +       spin_lock(&fi->lock);
>         fi->nlookup++;
> -       spin_unlock(&fc->lock);
> +       spin_unlock(&fi->lock);
>         fuse_change_attributes(inode, attr, attr_valid, attr_version);
>
>         return inode;
> diff --git a/fs/fuse/readdir.c b/fs/fuse/readdir.c
> index ab18b78f4755..574d03f8a573 100644
> --- a/fs/fuse/readdir.c
> +++ b/fs/fuse/readdir.c
> @@ -213,9 +213,9 @@ static int fuse_direntplus_link(struct file *file,
>                 }
>
>                 fi = get_fuse_inode(inode);
> -               spin_lock(&fc->lock);
> +               spin_lock(&fi->lock);
>                 fi->nlookup++;
> -               spin_unlock(&fc->lock);
> +               spin_unlock(&fi->lock);
>
>                 forget_all_cached_acls(inode);
>                 fuse_change_attributes(inode, &o->attr,
>