2020-03-12 20:05:31

by Daniel Xu

[permalink] [raw]
Subject: [PATCH v3 0/4] Support user xattrs in cgroupfs

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


2020-03-12 20:06:52

by Daniel Xu

[permalink] [raw]
Subject: [PATCH v3 2/4] kernfs: Add removed_size out param for simple_xattr_set

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 <[email protected]>
---
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

2020-03-12 20:10:44

by Daniel Xu

[permalink] [raw]
Subject: Re: [PATCH v3 0/4] Support user xattrs in cgroupfs

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.

2020-03-12 21:19:30

by Tejun Heo

[permalink] [raw]
Subject: Re: [PATCH v3 0/4] Support user xattrs in cgroupfs

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

2020-03-12 21:21:11

by Tejun Heo

[permalink] [raw]
Subject: Re: [PATCH v3 0/4] Support user xattrs in cgroupfs

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 <[email protected]>

Thanks.

--
tejun

2020-03-12 22:06:02

by Greg Kroah-Hartman

[permalink] [raw]
Subject: Re: [PATCH v3 0/4] Support user xattrs in cgroupfs

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 <[email protected]>

2020-03-13 01:01:58

by Chris Down

[permalink] [raw]
Subject: Re: [PATCH v3 0/4] Support user xattrs in cgroupfs

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 <[email protected]>

2020-03-16 19:57:48

by Tejun Heo

[permalink] [raw]
Subject: Re: [PATCH v3 0/4] Support user xattrs in cgroupfs

Applied to cgroup/for-5.7.

Thanks.

--
tejun