Re: [PATCH v2 2/4] kernfs: Add removed_size out param for simple_xattr_set

[Daniel Xu]

On Thu Mar 5, 2020 at 1:16 PM, Daniel Xu wrote:
> This helps set up size accounting in the next commit. Without this out
> param, it's difficult to find out the removed xattr size without taking
> a lock for longer and walking the xattr linked list twice.
>
> 
> Signed-off-by: Daniel Xu <dxu@xxxxxxxxx>
> ---
> fs/kernfs/inode.c | 2 +-
> fs/xattr.c | 11 ++++++++++-
> include/linux/xattr.h | 3 ++-
> mm/shmem.c | 2 +-
> 4 files changed, 14 insertions(+), 4 deletions(-)
>
> 
> diff --git a/fs/kernfs/inode.c b/fs/kernfs/inode.c
> index d0f7a5abd9a9..5f10ae95fbfa 100644
> --- a/fs/kernfs/inode.c
> +++ b/fs/kernfs/inode.c
> @@ -303,7 +303,7 @@ int kernfs_xattr_set(struct kernfs_node *kn, const
> char *name,
> if (!attrs)
> return -ENOMEM;
>  
> - return simple_xattr_set(&attrs->xattrs, name, value, size, flags);
> + return simple_xattr_set(&attrs->xattrs, name, value, size, flags,
> NULL);
> }
>  
> static int kernfs_vfs_xattr_get(const struct xattr_handler *handler,
> diff --git a/fs/xattr.c b/fs/xattr.c
> index 0d3c9b4d1914..e13265e65871 100644
> --- a/fs/xattr.c
> +++ b/fs/xattr.c
> @@ -860,6 +860,7 @@ int simple_xattr_get(struct simple_xattrs *xattrs,
> const char *name,
> * @value: value of the xattr. If %NULL, will remove the attribute.
> * @size: size of the new xattr
> * @flags: %XATTR_{CREATE|REPLACE}
> + * @removed_size: returns size of the removed xattr, -1 if none removed
> *
> * %XATTR_CREATE is set, the xattr shouldn't exist already; otherwise
> fails
> * with -EEXIST. If %XATTR_REPLACE is set, the xattr should exist;
> @@ -868,7 +869,8 @@ int simple_xattr_get(struct simple_xattrs *xattrs,
> const char *name,
> * Returns 0 on success, -errno on failure.
> */
> int simple_xattr_set(struct simple_xattrs *xattrs, const char *name,
> - const void *value, size_t size, int flags)
> + const void *value, size_t size, int flags,
> + ssize_t *removed_size)
> {
> struct simple_xattr *xattr;
> struct simple_xattr *new_xattr = NULL;
> @@ -895,8 +897,12 @@ int simple_xattr_set(struct simple_xattrs *xattrs,
> const char *name,
> err = -EEXIST;
> } else if (new_xattr) {
> list_replace(&xattr->list, &new_xattr->list);
> + if (removed_size)
> + *removed_size = xattr->size;
> } else {
> list_del(&xattr->list);
> + if (removed_size)
> + *removed_size = xattr->size;
> }
> goto out;
> }
> @@ -908,6 +914,9 @@ int simple_xattr_set(struct simple_xattrs *xattrs,
> const char *name,
> list_add(&new_xattr->list, &xattrs->head);
> xattr = NULL;
> }
> +
> + if (removed_size)
> + *removed_size = -1;
> out:
> spin_unlock(&xattrs->lock);
> if (xattr) {
> diff --git a/include/linux/xattr.h b/include/linux/xattr.h
> index 6dad031be3c2..4cf6e11f4a3c 100644
> --- a/include/linux/xattr.h
> +++ b/include/linux/xattr.h
> @@ -102,7 +102,8 @@ struct simple_xattr *simple_xattr_alloc(const void
> *value, size_t size);
> int simple_xattr_get(struct simple_xattrs *xattrs, const char *name,
> void *buffer, size_t size);
> int simple_xattr_set(struct simple_xattrs *xattrs, const char *name,
> - const void *value, size_t size, int flags);
> + const void *value, size_t size, int flags,
> + ssize_t *removed_size);
> ssize_t simple_xattr_list(struct inode *inode, struct simple_xattrs
> *xattrs, char *buffer,
> size_t size);
> void simple_xattr_list_add(struct simple_xattrs *xattrs,
> diff --git a/mm/shmem.c b/mm/shmem.c
> index aad3ba74b0e9..f47347cb30f6 100644
> --- a/mm/shmem.c
> +++ b/mm/shmem.c
> @@ -3243,7 +3243,7 @@ static int shmem_xattr_handler_set(const struct
> xattr_handler *handler,
> struct shmem_inode_info *info = SHMEM_I(inode);
>  
> name = xattr_full_name(handler, name);
> - return simple_xattr_set(&info->xattrs, name, value, size, flags);
> + return simple_xattr_set(&info->xattrs, name, value, size, flags,
> NULL);
> }
>  
> static const struct xattr_handler shmem_security_xattr_handler = {
> --
> 2.21.1
>
> 

Adding Al Viro, who I forgot to add in the initial send. Will remember
on future sends.

Daniel