linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v3 0/4] Support user xattrs in cgroupfs
@ 2020-03-12 20:03 Daniel Xu
  2020-03-12 20:03 ` [PATCH v3 1/4] kernfs: kvmalloc xattr value instead of kmalloc Daniel Xu
                   ` (7 more replies)
  0 siblings, 8 replies; 13+ messages in thread
From: Daniel Xu @ 2020-03-12 20:03 UTC (permalink / raw)
  To: cgroups, tj, lizefan, hannes, viro
  Cc: Daniel Xu, shakeelb, linux-fsdevel, linux-kernel, gregkh, kernel-team

User extended attributes are useful as metadata storage for kernfs
consumers like cgroups. Especially in the case of cgroups, it is useful
to have a central metadata store that multiple processes/services can
use to coordinate actions.

A concrete example is for userspace out of memory killers. We want to
let delegated cgroup subtree owners (running as non-root) to be able to
say "please avoid killing this cgroup". This is especially important for
desktop linux as delegated subtrees owners are less likely to run as
root.

The first two commits set up some stuff for the third commit which
intro introduce a new flag, KERNFS_ROOT_SUPPORT_USER_XATTR,
that lets kernfs consumers enable user xattr support. The final commit
turns on user xattr support for cgroupfs.

Changes from v2:
- Rephrased commit message for "kernfs: kvmalloc xattr value instead of
  kmalloc"

Changes from v1:
- use kvmalloc for xattr values
- modify simple_xattr_set to return removed size
- add accounting for total user xattr size per cgroup

Daniel Xu (4):
  kernfs: kvmalloc xattr value instead of kmalloc
  kernfs: Add removed_size out param for simple_xattr_set
  kernfs: Add option to enable user xattrs
  cgroupfs: Support user xattrs

Daniel Xu (4):
  kernfs: kvmalloc xattr value instead of kmalloc
  kernfs: Add removed_size out param for simple_xattr_set
  kernfs: Add option to enable user xattrs
  cgroupfs: Support user xattrs

 fs/kernfs/inode.c           | 91 ++++++++++++++++++++++++++++++++++++-
 fs/kernfs/kernfs-internal.h |  2 +
 fs/xattr.c                  | 17 +++++--
 include/linux/kernfs.h      | 11 ++++-
 include/linux/xattr.h       |  3 +-
 kernel/cgroup/cgroup.c      |  3 +-
 mm/shmem.c                  |  2 +-
 7 files changed, 119 insertions(+), 10 deletions(-)

-- 
2.21.1


^ permalink raw reply	[flat|nested] 13+ messages in thread

* [PATCH v3 1/4] kernfs: kvmalloc xattr value instead of kmalloc
  2020-03-12 20:03 [PATCH v3 0/4] Support user xattrs in cgroupfs Daniel Xu
@ 2020-03-12 20:03 ` Daniel Xu
  2020-03-12 21:03   ` Shakeel Butt
  2020-03-12 21:09   ` Andreas Dilger
  2020-03-12 20:03 ` [PATCH v3 2/4] kernfs: Add removed_size out param for simple_xattr_set Daniel Xu
                   ` (6 subsequent siblings)
  7 siblings, 2 replies; 13+ messages in thread
From: Daniel Xu @ 2020-03-12 20:03 UTC (permalink / raw)
  To: cgroups, tj, lizefan, hannes, viro
  Cc: Daniel Xu, shakeelb, linux-fsdevel, linux-kernel, gregkh, kernel-team

xattr values have a 64k maximum size. This can result in an order 4
kmalloc request which can be difficult to fulfill. Since xattrs do not
need physically contiguous memory, we can switch to kvmalloc and not
have to worry about higher order allocations failing.

Signed-off-by: Daniel Xu <dxu@dxuuu.xyz>
---
 fs/xattr.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/fs/xattr.c b/fs/xattr.c
index 90dd78f0eb27..0d3c9b4d1914 100644
--- a/fs/xattr.c
+++ b/fs/xattr.c
@@ -817,7 +817,7 @@ struct simple_xattr *simple_xattr_alloc(const void *value, size_t size)
 	if (len < sizeof(*new_xattr))
 		return NULL;
 
-	new_xattr = kmalloc(len, GFP_KERNEL);
+	new_xattr = kvmalloc(len, GFP_KERNEL);
 	if (!new_xattr)
 		return NULL;
 
@@ -882,7 +882,7 @@ int simple_xattr_set(struct simple_xattrs *xattrs, const char *name,
 
 		new_xattr->name = kstrdup(name, GFP_KERNEL);
 		if (!new_xattr->name) {
-			kfree(new_xattr);
+			kvfree(new_xattr);
 			return -ENOMEM;
 		}
 	}
@@ -912,7 +912,7 @@ int simple_xattr_set(struct simple_xattrs *xattrs, const char *name,
 	spin_unlock(&xattrs->lock);
 	if (xattr) {
 		kfree(xattr->name);
-		kfree(xattr);
+		kvfree(xattr);
 	}
 	return err;
 
-- 
2.21.1


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH v3 2/4] kernfs: Add removed_size out param for simple_xattr_set
  2020-03-12 20:03 [PATCH v3 0/4] Support user xattrs in cgroupfs Daniel Xu
  2020-03-12 20:03 ` [PATCH v3 1/4] kernfs: kvmalloc xattr value instead of kmalloc Daniel Xu
@ 2020-03-12 20:03 ` Daniel Xu
  2020-03-12 20:03 ` [PATCH v3 3/4] kernfs: Add option to enable user xattrs Daniel Xu
                   ` (5 subsequent siblings)
  7 siblings, 0 replies; 13+ messages in thread
From: Daniel Xu @ 2020-03-12 20:03 UTC (permalink / raw)
  To: cgroups, tj, lizefan, hannes, viro
  Cc: Daniel Xu, shakeelb, linux-fsdevel, linux-kernel, gregkh, kernel-team

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@dxuuu.xyz>
---
 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


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH v3 3/4] kernfs: Add option to enable user xattrs
  2020-03-12 20:03 [PATCH v3 0/4] Support user xattrs in cgroupfs Daniel Xu
  2020-03-12 20:03 ` [PATCH v3 1/4] kernfs: kvmalloc xattr value instead of kmalloc Daniel Xu
  2020-03-12 20:03 ` [PATCH v3 2/4] kernfs: Add removed_size out param for simple_xattr_set Daniel Xu
@ 2020-03-12 20:03 ` Daniel Xu
  2020-03-12 20:03 ` [PATCH v3 4/4] cgroupfs: Support " Daniel Xu
                   ` (4 subsequent siblings)
  7 siblings, 0 replies; 13+ messages in thread
From: Daniel Xu @ 2020-03-12 20:03 UTC (permalink / raw)
  To: cgroups, tj, lizefan, hannes, viro
  Cc: Daniel Xu, shakeelb, linux-fsdevel, linux-kernel, gregkh, kernel-team

User extended attributes are useful as metadata storage for kernfs
consumers like cgroups. Especially in the case of cgroups, it is useful
to have a central metadata store that multiple processes/services can
use to coordinate actions.

A concrete example is for userspace out of memory killers. We want to
let delegated cgroup subtree owners (running as non-root) to be able to
say "please avoid killing this cgroup". This is especially important for
desktop linux as delegated subtrees owners are less likely to run as
root.

This patch introduces a new flag, KERNFS_ROOT_SUPPORT_USER_XATTR, that
lets kernfs consumers enable user xattr support. An initial limit of 128
entries or 128KB -- whichever is hit first -- is placed per cgroup
because xattrs come from kernel memory and we don't want to let
unprivileged users accidentally eat up too much kernel memory.

Signed-off-by: Daniel Xu <dxu@dxuuu.xyz>
---
 fs/kernfs/inode.c           | 89 +++++++++++++++++++++++++++++++++++++
 fs/kernfs/kernfs-internal.h |  2 +
 include/linux/kernfs.h      | 11 ++++-
 3 files changed, 100 insertions(+), 2 deletions(-)

diff --git a/fs/kernfs/inode.c b/fs/kernfs/inode.c
index 5f10ae95fbfa..fc2469a20fed 100644
--- a/fs/kernfs/inode.c
+++ b/fs/kernfs/inode.c
@@ -53,6 +53,8 @@ static struct kernfs_iattrs *__kernfs_iattrs(struct kernfs_node *kn, int alloc)
 	kn->iattr->ia_ctime = kn->iattr->ia_atime;
 
 	simple_xattrs_init(&kn->iattr->xattrs);
+	atomic_set(&kn->iattr->nr_user_xattrs, 0);
+	atomic_set(&kn->iattr->user_xattr_size, 0);
 out_unlock:
 	ret = kn->iattr;
 	mutex_unlock(&iattr_mutex);
@@ -327,6 +329,86 @@ static int kernfs_vfs_xattr_set(const struct xattr_handler *handler,
 	return kernfs_xattr_set(kn, name, value, size, flags);
 }
 
+static int kernfs_vfs_user_xattr_add(struct kernfs_node *kn,
+				     const char *full_name,
+				     struct simple_xattrs *xattrs,
+				     const void *value, size_t size, int flags)
+{
+	atomic_t *sz = &kn->iattr->user_xattr_size;
+	atomic_t *nr = &kn->iattr->nr_user_xattrs;
+	ssize_t removed_size;
+	int ret;
+
+	if (atomic_inc_return(nr) > KERNFS_MAX_USER_XATTRS) {
+		ret = -ENOSPC;
+		goto dec_count_out;
+	}
+
+	if (atomic_add_return(size, sz) > KERNFS_USER_XATTR_SIZE_LIMIT) {
+		ret = -ENOSPC;
+		goto dec_size_out;
+	}
+
+	ret = simple_xattr_set(xattrs, full_name, value, size, flags,
+			       &removed_size);
+
+	if (!ret && removed_size >= 0)
+		size = removed_size;
+	else if (!ret)
+		return 0;
+dec_size_out:
+	atomic_sub(size, sz);
+dec_count_out:
+	atomic_dec(nr);
+	return ret;
+}
+
+static int kernfs_vfs_user_xattr_rm(struct kernfs_node *kn,
+				    const char *full_name,
+				    struct simple_xattrs *xattrs,
+				    const void *value, size_t size, int flags)
+{
+	atomic_t *sz = &kn->iattr->user_xattr_size;
+	atomic_t *nr = &kn->iattr->nr_user_xattrs;
+	ssize_t removed_size;
+	int ret;
+
+	ret = simple_xattr_set(xattrs, full_name, value, size, flags,
+			       &removed_size);
+
+	if (removed_size >= 0) {
+		atomic_sub(removed_size, sz);
+		atomic_dec(nr);
+	}
+
+	return ret;
+}
+
+static int kernfs_vfs_user_xattr_set(const struct xattr_handler *handler,
+				     struct dentry *unused, struct inode *inode,
+				     const char *suffix, const void *value,
+				     size_t size, int flags)
+{
+	const char *full_name = xattr_full_name(handler, suffix);
+	struct kernfs_node *kn = inode->i_private;
+	struct kernfs_iattrs *attrs;
+
+	if (!(kernfs_root(kn)->flags & KERNFS_ROOT_SUPPORT_USER_XATTR))
+		return -EOPNOTSUPP;
+
+	attrs = kernfs_iattrs(kn);
+	if (!attrs)
+		return -ENOMEM;
+
+	if (value)
+		return kernfs_vfs_user_xattr_add(kn, full_name, &attrs->xattrs,
+						 value, size, flags);
+	else
+		return kernfs_vfs_user_xattr_rm(kn, full_name, &attrs->xattrs,
+						value, size, flags);
+
+}
+
 static const struct xattr_handler kernfs_trusted_xattr_handler = {
 	.prefix = XATTR_TRUSTED_PREFIX,
 	.get = kernfs_vfs_xattr_get,
@@ -339,8 +421,15 @@ static const struct xattr_handler kernfs_security_xattr_handler = {
 	.set = kernfs_vfs_xattr_set,
 };
 
+static const struct xattr_handler kernfs_user_xattr_handler = {
+	.prefix = XATTR_USER_PREFIX,
+	.get = kernfs_vfs_xattr_get,
+	.set = kernfs_vfs_user_xattr_set,
+};
+
 const struct xattr_handler *kernfs_xattr_handlers[] = {
 	&kernfs_trusted_xattr_handler,
 	&kernfs_security_xattr_handler,
+	&kernfs_user_xattr_handler,
 	NULL
 };
diff --git a/fs/kernfs/kernfs-internal.h b/fs/kernfs/kernfs-internal.h
index 2f3c51d55261..7ee97ef59184 100644
--- a/fs/kernfs/kernfs-internal.h
+++ b/fs/kernfs/kernfs-internal.h
@@ -26,6 +26,8 @@ struct kernfs_iattrs {
 	struct timespec64	ia_ctime;
 
 	struct simple_xattrs	xattrs;
+	atomic_t		nr_user_xattrs;
+	atomic_t		user_xattr_size;
 };
 
 /* +1 to avoid triggering overflow warning when negating it */
diff --git a/include/linux/kernfs.h b/include/linux/kernfs.h
index dded2e5a9f42..89f6a4214a70 100644
--- a/include/linux/kernfs.h
+++ b/include/linux/kernfs.h
@@ -37,8 +37,10 @@ enum kernfs_node_type {
 	KERNFS_LINK		= 0x0004,
 };
 
-#define KERNFS_TYPE_MASK	0x000f
-#define KERNFS_FLAG_MASK	~KERNFS_TYPE_MASK
+#define KERNFS_TYPE_MASK		0x000f
+#define KERNFS_FLAG_MASK		~KERNFS_TYPE_MASK
+#define KERNFS_MAX_USER_XATTRS		128
+#define KERNFS_USER_XATTR_SIZE_LIMIT	(128 << 10)
 
 enum kernfs_node_flag {
 	KERNFS_ACTIVATED	= 0x0010,
@@ -78,6 +80,11 @@ enum kernfs_root_flag {
 	 * fhandle to access nodes of the fs.
 	 */
 	KERNFS_ROOT_SUPPORT_EXPORTOP		= 0x0004,
+
+	/*
+	 * Support user xattrs to be written to nodes rooted at this root.
+	 */
+	KERNFS_ROOT_SUPPORT_USER_XATTR		= 0x0008,
 };
 
 /* type-specific structures for kernfs_node union members */
-- 
2.21.1


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH v3 4/4] cgroupfs: Support user xattrs
  2020-03-12 20:03 [PATCH v3 0/4] Support user xattrs in cgroupfs Daniel Xu
                   ` (2 preceding siblings ...)
  2020-03-12 20:03 ` [PATCH v3 3/4] kernfs: Add option to enable user xattrs Daniel Xu
@ 2020-03-12 20:03 ` Daniel Xu
  2020-03-12 20:09 ` [PATCH v3 0/4] Support user xattrs in cgroupfs Daniel Xu
                   ` (3 subsequent siblings)
  7 siblings, 0 replies; 13+ messages in thread
From: Daniel Xu @ 2020-03-12 20:03 UTC (permalink / raw)
  To: cgroups, tj, lizefan, hannes, viro
  Cc: Daniel Xu, shakeelb, linux-fsdevel, linux-kernel, gregkh, kernel-team

This patch turns on xattr support for cgroupfs. This is useful for
letting non-root owners of delegated subtrees attach metadata to
cgroups.

One use case is for subtree owners to tell a userspace out of memory
killer to bias away from killing specific subtrees.

Tests:

    [/sys/fs/cgroup]# for i in $(seq 0 130); \
        do setfattr workload.slice -n user.name$i -v wow; done
    setfattr: workload.slice: No space left on device
    setfattr: workload.slice: No space left on device
    setfattr: workload.slice: No space left on device

    [/sys/fs/cgroup]# for i in $(seq 0 130); \
        do setfattr workload.slice --remove user.name$i; done
    setfattr: workload.slice: No such attribute
    setfattr: workload.slice: No such attribute
    setfattr: workload.slice: No such attribute

    [/sys/fs/cgroup]# for i in $(seq 0 130); \
        do setfattr workload.slice -n user.name$i -v wow; done
    setfattr: workload.slice: No space left on device
    setfattr: workload.slice: No space left on device
    setfattr: workload.slice: No space left on device

`seq 0 130` is inclusive, and 131 - 128 = 3, which is the number of
errors we expect to see.

    [/data]# cat testxattr.c
    #include <sys/types.h>
    #include <sys/xattr.h>
    #include <stdio.h>
    #include <stdlib.h>

    int main() {
      char name[256];
      char *buf = malloc(64 << 10);
      if (!buf) {
        perror("malloc");
        return 1;
      }

      for (int i = 0; i < 4; ++i) {
        snprintf(name, 256, "user.bigone%d", i);
        if (setxattr("/sys/fs/cgroup/system.slice", name, buf,
                     64 << 10, 0)) {
          printf("setxattr failed on iteration=%d\n", i);
          return 1;
        }
      }

      return 0;
    }

    [/data]# ./a.out
    setxattr failed on iteration=2

    [/data]# ./a.out
    setxattr failed on iteration=0

    [/sys/fs/cgroup]# setfattr -x user.bigone0 system.slice/
    [/sys/fs/cgroup]# setfattr -x user.bigone1 system.slice/

    [/data]# ./a.out
    setxattr failed on iteration=2

Signed-off-by: Daniel Xu <dxu@dxuuu.xyz>
---
 kernel/cgroup/cgroup.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/kernel/cgroup/cgroup.c b/kernel/cgroup/cgroup.c
index 7a39dc882095..ae1d808c0b9b 100644
--- a/kernel/cgroup/cgroup.c
+++ b/kernel/cgroup/cgroup.c
@@ -1954,7 +1954,8 @@ int cgroup_setup_root(struct cgroup_root *root, u16 ss_mask)
 
 	root->kf_root = kernfs_create_root(kf_sops,
 					   KERNFS_ROOT_CREATE_DEACTIVATED |
-					   KERNFS_ROOT_SUPPORT_EXPORTOP,
+					   KERNFS_ROOT_SUPPORT_EXPORTOP |
+					   KERNFS_ROOT_SUPPORT_USER_XATTR,
 					   root_cgrp);
 	if (IS_ERR(root->kf_root)) {
 		ret = PTR_ERR(root->kf_root);
-- 
2.21.1


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* Re: [PATCH v3 0/4] Support user xattrs in cgroupfs
  2020-03-12 20:03 [PATCH v3 0/4] Support user xattrs in cgroupfs Daniel Xu
                   ` (3 preceding siblings ...)
  2020-03-12 20:03 ` [PATCH v3 4/4] cgroupfs: Support " Daniel Xu
@ 2020-03-12 20:09 ` Daniel Xu
  2020-03-12 21:17 ` Tejun Heo
                   ` (2 subsequent siblings)
  7 siblings, 0 replies; 13+ messages in thread
From: Daniel Xu @ 2020-03-12 20:09 UTC (permalink / raw)
  To: Daniel Xu, cgroups, tj, lizefan, hannes, viro
  Cc: Daniel Xu, shakeelb, linux-fsdevel, linux-kernel, gregkh, kernel-team

On Thu Mar 12, 2020 at 6:03 AM PST, Daniel Xu wrote:
> User extended attributes are useful as metadata storage for kernfs
> consumers like cgroups. Especially in the case of cgroups, it is useful
> to have a central metadata store that multiple processes/services can
> use to coordinate actions.
>
> A concrete example is for userspace out of memory killers. We want to
> let delegated cgroup subtree owners (running as non-root) to be able to
> say "please avoid killing this cgroup". This is especially important for
> desktop linux as delegated subtrees owners are less likely to run as
> root.
>
> The first two commits set up some stuff for the third commit which
> intro introduce a new flag, KERNFS_ROOT_SUPPORT_USER_XATTR,
> that lets kernfs consumers enable user xattr support. The final commit
> turns on user xattr support for cgroupfs.
>
> Changes from v2:
> - Rephrased commit message for "kernfs: kvmalloc xattr value instead of
> kmalloc"
>
> Changes from v1:
> - use kvmalloc for xattr values
> - modify simple_xattr_set to return removed size
> - add accounting for total user xattr size per cgroup
>
> Daniel Xu (4):
> kernfs: kvmalloc xattr value instead of kmalloc
> kernfs: Add removed_size out param for simple_xattr_set
> kernfs: Add option to enable user xattrs
> cgroupfs: Support user xattrs
>
> Daniel Xu (4):
> kernfs: kvmalloc xattr value instead of kmalloc
> kernfs: Add removed_size out param for simple_xattr_set
> kernfs: Add option to enable user xattrs
> cgroupfs: Support user xattrs
>
> fs/kernfs/inode.c | 91 ++++++++++++++++++++++++++++++++++++-
> fs/kernfs/kernfs-internal.h | 2 +
> fs/xattr.c | 17 +++++--
> include/linux/kernfs.h | 11 ++++-
> include/linux/xattr.h | 3 +-
> kernel/cgroup/cgroup.c | 3 +-
> mm/shmem.c | 2 +-
> 7 files changed, 119 insertions(+), 10 deletions(-)

Gah, messed up the copy paste. Let me know if you want a resend.

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v3 1/4] kernfs: kvmalloc xattr value instead of kmalloc
  2020-03-12 20:03 ` [PATCH v3 1/4] kernfs: kvmalloc xattr value instead of kmalloc Daniel Xu
@ 2020-03-12 21:03   ` Shakeel Butt
  2020-03-12 21:09   ` Andreas Dilger
  1 sibling, 0 replies; 13+ messages in thread
From: Shakeel Butt @ 2020-03-12 21:03 UTC (permalink / raw)
  To: Daniel Xu
  Cc: Cgroups, Tejun Heo, Li Zefan, Johannes Weiner, Alexander Viro,
	linux-fsdevel, LKML, Greg Kroah-Hartman, Kernel Team

On Thu, Mar 12, 2020 at 1:03 PM Daniel Xu <dxu@dxuuu.xyz> wrote:
>
> xattr values have a 64k maximum size. This can result in an order 4
> kmalloc request which can be difficult to fulfill. Since xattrs do not
> need physically contiguous memory, we can switch to kvmalloc and not
> have to worry about higher order allocations failing.
>
> Signed-off-by: Daniel Xu <dxu@dxuuu.xyz>

Reviewed-by: Shakeel Butt <shakeelb@google.com>

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v3 1/4] kernfs: kvmalloc xattr value instead of kmalloc
  2020-03-12 20:03 ` [PATCH v3 1/4] kernfs: kvmalloc xattr value instead of kmalloc Daniel Xu
  2020-03-12 21:03   ` Shakeel Butt
@ 2020-03-12 21:09   ` Andreas Dilger
  1 sibling, 0 replies; 13+ messages in thread
From: Andreas Dilger @ 2020-03-12 21:09 UTC (permalink / raw)
  To: Daniel Xu
  Cc: cgroups, tj, lizefan, hannes, viro, shakeelb, linux-fsdevel,
	linux-kernel, gregkh, kernel-team

[-- Attachment #1: Type: text/plain, Size: 1477 bytes --]

On Mar 12, 2020, at 2:03 PM, Daniel Xu <dxu@dxuuu.xyz> wrote:
> 
> xattr values have a 64k maximum size. This can result in an order 4
> kmalloc request which can be difficult to fulfill. Since xattrs do not
> need physically contiguous memory, we can switch to kvmalloc and not
> have to worry about higher order allocations failing.
> 
> Signed-off-by: Daniel Xu <dxu@dxuuu.xyz>

Reviewed-by: Andreas Dilger <adilger@dilger.ca>

> ---
> fs/xattr.c | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/fs/xattr.c b/fs/xattr.c
> index 90dd78f0eb27..0d3c9b4d1914 100644
> --- a/fs/xattr.c
> +++ b/fs/xattr.c
> @@ -817,7 +817,7 @@ struct simple_xattr *simple_xattr_alloc(const void *value, size_t size)
> 	if (len < sizeof(*new_xattr))
> 		return NULL;
> 
> -	new_xattr = kmalloc(len, GFP_KERNEL);
> +	new_xattr = kvmalloc(len, GFP_KERNEL);
> 	if (!new_xattr)
> 		return NULL;
> 
> @@ -882,7 +882,7 @@ int simple_xattr_set(struct simple_xattrs *xattrs, const char *name,
> 
> 		new_xattr->name = kstrdup(name, GFP_KERNEL);
> 		if (!new_xattr->name) {
> -			kfree(new_xattr);
> +			kvfree(new_xattr);
> 			return -ENOMEM;
> 		}
> 	}
> @@ -912,7 +912,7 @@ int simple_xattr_set(struct simple_xattrs *xattrs, const char *name,
> 	spin_unlock(&xattrs->lock);
> 	if (xattr) {
> 		kfree(xattr->name);
> -		kfree(xattr);
> +		kvfree(xattr);
> 	}
> 	return err;
> 
> --
> 2.21.1
> 


Cheers, Andreas






[-- Attachment #2: Message signed with OpenPGP --]
[-- Type: application/pgp-signature, Size: 873 bytes --]

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v3 0/4] Support user xattrs in cgroupfs
  2020-03-12 20:03 [PATCH v3 0/4] Support user xattrs in cgroupfs Daniel Xu
                   ` (4 preceding siblings ...)
  2020-03-12 20:09 ` [PATCH v3 0/4] Support user xattrs in cgroupfs Daniel Xu
@ 2020-03-12 21:17 ` Tejun Heo
  2020-03-12 21:19   ` Tejun Heo
  2020-03-12 22:05   ` Greg KH
  2020-03-13  1:00 ` Chris Down
  2020-03-16 19:55 ` Tejun Heo
  7 siblings, 2 replies; 13+ messages in thread
From: Tejun Heo @ 2020-03-12 21:17 UTC (permalink / raw)
  To: gregkh, Daniel Xu
  Cc: cgroups, lizefan, hannes, viro, shakeelb, linux-fsdevel,
	linux-kernel, kernel-team

Hello,

Daniel, the patchset looks good to me. Thanks a lot for working on
this.

Greg, provided that there aren't further objections, how do you wanna
route the patches? I'd be happy to take them through cgroup tree but
any tree is fine by me.

Thanks.

-- 
tejun

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v3 0/4] Support user xattrs in cgroupfs
  2020-03-12 21:17 ` Tejun Heo
@ 2020-03-12 21:19   ` Tejun Heo
  2020-03-12 22:05   ` Greg KH
  1 sibling, 0 replies; 13+ messages in thread
From: Tejun Heo @ 2020-03-12 21:19 UTC (permalink / raw)
  To: gregkh, Daniel Xu
  Cc: cgroups, lizefan, hannes, viro, shakeelb, linux-fsdevel,
	linux-kernel, kernel-team

On Thu, Mar 12, 2020 at 05:17:35PM -0400, Tejun Heo wrote:
> Greg, provided that there aren't further objections, how do you wanna
> route the patches? I'd be happy to take them through cgroup tree but
> any tree is fine by me.

Ooh, in case they get routed thorugh another tree, for the whole
series:

  Acked-by: Tejun Heo <tj@kernel.org>

Thanks.

-- 
tejun

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v3 0/4] Support user xattrs in cgroupfs
  2020-03-12 21:17 ` Tejun Heo
  2020-03-12 21:19   ` Tejun Heo
@ 2020-03-12 22:05   ` Greg KH
  1 sibling, 0 replies; 13+ messages in thread
From: Greg KH @ 2020-03-12 22:05 UTC (permalink / raw)
  To: Tejun Heo
  Cc: Daniel Xu, cgroups, lizefan, hannes, viro, shakeelb,
	linux-fsdevel, linux-kernel, kernel-team

On Thu, Mar 12, 2020 at 05:17:35PM -0400, Tejun Heo wrote:
> Hello,
> 
> Daniel, the patchset looks good to me. Thanks a lot for working on
> this.
> 
> Greg, provided that there aren't further objections, how do you wanna
> route the patches? I'd be happy to take them through cgroup tree but
> any tree is fine by me.

Sure, feel free to take them through your tree:

Reviewed-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v3 0/4] Support user xattrs in cgroupfs
  2020-03-12 20:03 [PATCH v3 0/4] Support user xattrs in cgroupfs Daniel Xu
                   ` (5 preceding siblings ...)
  2020-03-12 21:17 ` Tejun Heo
@ 2020-03-13  1:00 ` Chris Down
  2020-03-16 19:55 ` Tejun Heo
  7 siblings, 0 replies; 13+ messages in thread
From: Chris Down @ 2020-03-13  1:00 UTC (permalink / raw)
  To: Daniel Xu
  Cc: cgroups, tj, lizefan, hannes, viro, shakeelb, linux-fsdevel,
	linux-kernel, gregkh, kernel-team

Daniel Xu writes:
>User extended attributes are useful as metadata storage for kernfs
>consumers like cgroups. Especially in the case of cgroups, it is useful
>to have a central metadata store that multiple processes/services can
>use to coordinate actions.
>
>A concrete example is for userspace out of memory killers. We want to
>let delegated cgroup subtree owners (running as non-root) to be able to
>say "please avoid killing this cgroup". This is especially important for
>desktop linux as delegated subtrees owners are less likely to run as
>root.
>
>The first two commits set up some stuff for the third commit which
>intro introduce a new flag, KERNFS_ROOT_SUPPORT_USER_XATTR,
>that lets kernfs consumers enable user xattr support. The final commit
>turns on user xattr support for cgroupfs.

The whole series looks good to me, thanks.

For the whole series:

Acked-by: Chris Down <chris@chrisdown.name>

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v3 0/4] Support user xattrs in cgroupfs
  2020-03-12 20:03 [PATCH v3 0/4] Support user xattrs in cgroupfs Daniel Xu
                   ` (6 preceding siblings ...)
  2020-03-13  1:00 ` Chris Down
@ 2020-03-16 19:55 ` Tejun Heo
  7 siblings, 0 replies; 13+ messages in thread
From: Tejun Heo @ 2020-03-16 19:55 UTC (permalink / raw)
  To: Daniel Xu
  Cc: cgroups, lizefan, hannes, viro, shakeelb, linux-fsdevel,
	linux-kernel, gregkh, kernel-team

Applied to cgroup/for-5.7.

Thanks.

-- 
tejun

^ permalink raw reply	[flat|nested] 13+ messages in thread

end of thread, other threads:[~2020-03-16 19:55 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-03-12 20:03 [PATCH v3 0/4] Support user xattrs in cgroupfs Daniel Xu
2020-03-12 20:03 ` [PATCH v3 1/4] kernfs: kvmalloc xattr value instead of kmalloc Daniel Xu
2020-03-12 21:03   ` Shakeel Butt
2020-03-12 21:09   ` Andreas Dilger
2020-03-12 20:03 ` [PATCH v3 2/4] kernfs: Add removed_size out param for simple_xattr_set Daniel Xu
2020-03-12 20:03 ` [PATCH v3 3/4] kernfs: Add option to enable user xattrs Daniel Xu
2020-03-12 20:03 ` [PATCH v3 4/4] cgroupfs: Support " Daniel Xu
2020-03-12 20:09 ` [PATCH v3 0/4] Support user xattrs in cgroupfs Daniel Xu
2020-03-12 21:17 ` Tejun Heo
2020-03-12 21:19   ` Tejun Heo
2020-03-12 22:05   ` Greg KH
2020-03-13  1:00 ` Chris Down
2020-03-16 19:55 ` Tejun Heo

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).