Re: [PATCH vfs.tmpfs] tmpfs,xattr: GFP_KERNEL_ACCOUNT for simple xattrs

[Jan Kara <jack@suse.cz>]

On Mon 21-08-23 10:39:20, Hugh Dickins wrote:
> It is particularly important for the userns mount case (when a sensible
> nr_inodes maximum may not be enforced) that tmpfs user xattrs be subject
> to memory cgroup limiting.  Leave temporary buffer allocations as is,
> but change the persistent simple xattr allocations from GFP_KERNEL to
> GFP_KERNEL_ACCOUNT.  This limits kernfs's cgroupfs too, but that's good.
> 
> (I had intended to send this change earlier, but had been confused by
> shmem_alloc_inode() using GFP_KERNEL, and thought a discussion would be
> needed to change that too: no, I was forgetting the SLAB_ACCOUNT on that
> kmem_cache, which implicitly adds __GFP_ACCOUNT to all its allocations.)
> 
> Signed-off-by: Hugh Dickins <hughd@google.com>

So I've checked and also kernfs is affected by these changes. I guess that
makes sense as well. So feel free to add:

Reviewed-by: Jan Kara <jack@suse.cz>

								Honza

> ---
>  fs/xattr.c | 4 ++--
>  mm/shmem.c | 2 +-
>  2 files changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/fs/xattr.c b/fs/xattr.c
> index 2d607542281b..efd4736bc94b 100644
> --- a/fs/xattr.c
> +++ b/fs/xattr.c
> @@ -1093,7 +1093,7 @@ struct simple_xattr *simple_xattr_alloc(const void *value, size_t size)
>  	if (len < sizeof(*new_xattr))
>  		return NULL;
>  
> -	new_xattr = kvmalloc(len, GFP_KERNEL);
> +	new_xattr = kvmalloc(len, GFP_KERNEL_ACCOUNT);
>  	if (!new_xattr)
>  		return NULL;
>  
> @@ -1217,7 +1217,7 @@ struct simple_xattr *simple_xattr_set(struct simple_xattrs *xattrs,
>  		if (!new_xattr)
>  			return ERR_PTR(-ENOMEM);
>  
> -		new_xattr->name = kstrdup(name, GFP_KERNEL);
> +		new_xattr->name = kstrdup(name, GFP_KERNEL_ACCOUNT);
>  		if (!new_xattr->name) {
>  			simple_xattr_free(new_xattr);
>  			return ERR_PTR(-ENOMEM);
> diff --git a/mm/shmem.c b/mm/shmem.c
> index b782edeb69aa..11298c797cdc 100644
> --- a/mm/shmem.c
> +++ b/mm/shmem.c
> @@ -3616,7 +3616,7 @@ static int shmem_initxattrs(struct inode *inode,
>  
>  		len = strlen(xattr->name) + 1;
>  		new_xattr->name = kmalloc(XATTR_SECURITY_PREFIX_LEN + len,
> -					  GFP_KERNEL);
> +					  GFP_KERNEL_ACCOUNT);
>  		if (!new_xattr->name) {
>  			kvfree(new_xattr);
>  			break;
> -- 
> 2.35.3
> 
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR