Re: [PATCH 01/14] xattr: add rcu_head and rhash_head to struct simple_xattr

[Jan Kara]

On Mon 16-02-26 14:31:57, Christian Brauner wrote:
> In preparation for converting simple_xattrs from rbtree to rhashtable,
> add rhash_head and rcu_head members to struct simple_xattr. The
> rhashtable implementation will use rhash_head for hash table linkage
> and RCU-based lockless reads, requiring that replaced or removed xattr
> entries be freed via call_rcu() rather than immediately.
> 
> Add simple_xattr_free_rcu() which schedules RCU-deferred freeing of an
> xattr entry.  This will be used by callers of simple_xattr_set() once
> they switch to the rhashtable-based xattr store.
> 
> No functional changes.
> 
> Signed-off-by: Christian Brauner <brauner@xxxxxxxxxx>

Looks good. Feel free to add:

Reviewed-by: Jan Kara <jack@xxxxxxx>

								Honza

> ---
>  fs/xattr.c            | 23 +++++++++++++++++++++++
>  include/linux/xattr.h |  4 ++++
>  2 files changed, 27 insertions(+)
> 
> diff --git a/fs/xattr.c b/fs/xattr.c
> index 3e49e612e1ba..9cbb1917bcb2 100644
> --- a/fs/xattr.c
> +++ b/fs/xattr.c
> @@ -1197,6 +1197,29 @@ void simple_xattr_free(struct simple_xattr *xattr)
>  	kvfree(xattr);
>  }
>  
> +static void simple_xattr_rcu_free(struct rcu_head *head)
> +{
> +	struct simple_xattr *xattr;
> +
> +	xattr = container_of(head, struct simple_xattr, rcu);
> +	simple_xattr_free(xattr);
> +}
> +
> +/**
> + * simple_xattr_free_rcu - free an xattr object after an RCU grace period
> + * @xattr: the xattr object
> + *
> + * Schedule RCU-deferred freeing of an xattr entry. This is used by
> + * rhashtable-based callers of simple_xattr_set() that replace or remove
> + * an existing entry while concurrent RCU readers may still be accessing
> + * it.
> + */
> +void simple_xattr_free_rcu(struct simple_xattr *xattr)
> +{
> +	if (xattr)
> +		call_rcu(&xattr->rcu, simple_xattr_rcu_free);
> +}
> +
>  /**
>   * simple_xattr_alloc - allocate new xattr object
>   * @value: value of the xattr object
> diff --git a/include/linux/xattr.h b/include/linux/xattr.h
> index 64e9afe7d647..1328f2bfd2ce 100644
> --- a/include/linux/xattr.h
> +++ b/include/linux/xattr.h
> @@ -16,6 +16,7 @@
>  #include <linux/types.h>
>  #include <linux/spinlock.h>
>  #include <linux/mm.h>
> +#include <linux/rhashtable-types.h>
>  #include <linux/user_namespace.h>
>  #include <uapi/linux/xattr.h>
>  
> @@ -112,6 +113,8 @@ struct simple_xattrs {
>  
>  struct simple_xattr {
>  	struct rb_node rb_node;
> +	struct rhash_head hash_node;
> +	struct rcu_head rcu;
>  	char *name;
>  	size_t size;
>  	char value[];
> @@ -122,6 +125,7 @@ void simple_xattrs_free(struct simple_xattrs *xattrs, size_t *freed_space);
>  size_t simple_xattr_space(const char *name, size_t size);
>  struct simple_xattr *simple_xattr_alloc(const void *value, size_t size);
>  void simple_xattr_free(struct simple_xattr *xattr);
> +void simple_xattr_free_rcu(struct simple_xattr *xattr);
>  int simple_xattr_get(struct simple_xattrs *xattrs, const char *name,
>  		     void *buffer, size_t size);
>  struct simple_xattr *simple_xattr_set(struct simple_xattrs *xattrs,
> 
> -- 
> 2.47.3
> 
-- 
Jan Kara <jack@xxxxxxxx>
SUSE Labs, CR