[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CAJfpegspUywVumOs-HcJVdm7CiGAHeNGfn4Kqg0dLup_5ufi8Q@mail.gmail.com>
Date: Wed, 7 Nov 2018 15:12:15 +0100
From: Miklos Szeredi <miklos@...redi.hu>
To: Kirill Tkhai <ktkhai@...tuozzo.com>
Cc: linux-fsdevel@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH 4/6] fuse: Implement fuse_attr_version_inc()
On Tue, Nov 6, 2018 at 10:43 AM, Kirill Tkhai <ktkhai@...tuozzo.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@...tuozzo.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);
>
Powered by blists - more mailing lists