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 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
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
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 <[email protected]>
---
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
It's not really necessary to have contiguous physical memory for xattr
values. We no longer need to worry about higher order allocations
failing with kvmalloc, especially because the xattr size limit is at
64K.
Signed-off-by: Daniel Xu <[email protected]>
---
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
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
On Thu, 2020-03-05 at 13:16 -0800, Daniel Xu wrote:
> It's not really necessary to have contiguous physical memory for xattr
> values. We no longer need to worry about higher order allocations
> failing with kvmalloc, especially because the xattr size limit is at
> 64K.
So why use vmalloc memory at all?
> diff --git 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);
Why is this sensible?
vmalloc memory is a much more limited resource.
Also, it seems as if the function should set
new_xattr->name to NULL before the return.
On Fri, Mar 06, 2020 at 12:49:51AM -0800, Joe Perches wrote:
> On Thu, 2020-03-05 at 13:16 -0800, Daniel Xu wrote:
> > It's not really necessary to have contiguous physical memory for xattr
> > values. We no longer need to worry about higher order allocations
> > failing with kvmalloc, especially because the xattr size limit is at
> > 64K.
>
> So why use vmalloc memory at all?
>
> > diff --git 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);
>
> Why is this sensible?
See the thread on v1
> vmalloc memory is a much more limited resource.
Large chunks of "len" is much more limited :)
thanks,
greg k-h
Hi Joe,
On Fri Mar 6, 2020 at 12:49 AM, Joe Perches wrote:
> On Thu, 2020-03-05 at 13:16 -0800, Daniel Xu wrote:
> > It's not really necessary to have contiguous physical memory for xattr
> > values. We no longer need to worry about higher order allocations
> > failing with kvmalloc, especially because the xattr size limit is at
> > 64K.
>
>
> So why use vmalloc memory at all?
>
>
> > diff --git 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);
>
>
> Why is this sensible?
> vmalloc memory is a much more limited resource.
What would be the alternative? As Greg said, contiguous memory should be
more scarce.
> Also, it seems as if the function should set
> new_xattr->name to NULL before the return.
>
Will add and send in a different patch.
Thanks,
Daniel
On Mon, 2020-03-09 at 11:21 -0700, Daniel Xu wrote:
> Hi Joe,
Hello Daniel.
> On Fri Mar 6, 2020 at 12:49 AM, Joe Perches wrote:
> > On Thu, 2020-03-05 at 13:16 -0800, Daniel Xu wrote:
> > > It's not really necessary to have contiguous physical memory for xattr
> > > values. We no longer need to worry about higher order allocations
> > > failing with kvmalloc, especially because the xattr size limit is at
> > > 64K.
> >
> > So why use vmalloc memory at all?
> >
> >
> > > diff --git 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);
> >
> > Why is this sensible?
> > vmalloc memory is a much more limited resource.
>
> What would be the alternative? As Greg said, contiguous memory should be
> more scarce.
If the need is to allocate from a single block of memory,
perhaps you need a submemory allocator like gen_pool.
(gennalloc.h)
Dunno. Maybe i just don't quite understand your need.
On Mon, Mar 09, 2020 at 12:41:05PM -0700, Joe Perches wrote:
> If the need is to allocate from a single block of memory,
> perhaps you need a submemory allocator like gen_pool.
> (gennalloc.h)
>
> Dunno. Maybe i just don't quite understand your need.
vmalloc is the right thing to do here. vmalloc space isn't a scarce
resource on any 64bit machines. On 32bits, which basically are tiny
machines at this point, these allocations are both size and quantity
limited by other factors (e.g. each cgroup consumes way more memory).
--
tejun
On Mon, 2020-03-09 at 15:51 -0400, Tejun Heo wrote:
> On Mon, Mar 09, 2020 at 12:41:05PM -0700, Joe Perches wrote:
> > If the need is to allocate from a single block of memory,
> > perhaps you need a submemory allocator like gen_pool.
> > (gennalloc.h)
> >
> > Dunno. Maybe i just don't quite understand your need.
>
> vmalloc is the right thing to do here. vmalloc space isn't a scarce
> resource on any 64bit machines. On 32bits, which basically are tiny
> machines at this point, these allocations are both size and quantity
> limited by other factors (e.g. each cgroup consumes way more memory).
This feels like driving spikes into a living thing
more than into a
corpse.
I've still got more than a few 32-bit devices around.
On Mon, Mar 09, 2020 at 12:58:49PM -0700, Joe Perches wrote:
> This feels like driving spikes into a living thing
> more than into a
> corpse.
>
> I've still got more than a few 32-bit devices around.
Unless you're suggesting we stop using vmallocs altogether, I don't
see how your objection makes sense here.
--
tejun
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 <[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
>
>
Adding Al Viro, who I forgot to add in the initial send. Will remember
on future sends.
Daniel
Hi Daniel,
On Thu, Mar 5, 2020 at 1:16 PM Daniel Xu <[email protected]> wrote:
>
> It's not really necessary to have contiguous physical memory for xattr
> values. We no longer need to worry about higher order allocations
> failing with kvmalloc, especially because the xattr size limit is at
> 64K.
>
> Signed-off-by: Daniel Xu <[email protected]>
The patch looks fine to me. However the commit message is too cryptic
i.e. hard to get the motivation behind the change.
Hi Shakeel,
On Tue Mar 10, 2020 at 12:40 PM, Shakeel Butt wrote:
> Hi Daniel,
>
>
> On Thu, Mar 5, 2020 at 1:16 PM Daniel Xu <[email protected]> wrote:
> >
> > It's not really necessary to have contiguous physical memory for xattr
> > values. We no longer need to worry about higher order allocations
> > failing with kvmalloc, especially because the xattr size limit is at
> > 64K.
> >
> > Signed-off-by: Daniel Xu <[email protected]>
>
>
> The patch looks fine to me. However the commit message is too cryptic
> i.e. hard to get the motivation behind the change.
>
Thanks for taking a look. The real reason I did it was because Tejun
said so :).
Tejun, is there a larger reason?
Thanks,
Daniel
On Tue, Mar 10, 2020 at 1:40 PM Daniel Xu <[email protected]> wrote:
>
> Hi Shakeel,
>
> On Tue Mar 10, 2020 at 12:40 PM, Shakeel Butt wrote:
> > Hi Daniel,
> >
> >
> > On Thu, Mar 5, 2020 at 1:16 PM Daniel Xu <[email protected]> wrote:
> > >
> > > It's not really necessary to have contiguous physical memory for xattr
> > > values. We no longer need to worry about higher order allocations
> > > failing with kvmalloc, especially because the xattr size limit is at
> > > 64K.
> > >
> > > Signed-off-by: Daniel Xu <[email protected]>
> >
> >
> > The patch looks fine to me. However the commit message is too cryptic
> > i.e. hard to get the motivation behind the change.
> >
>
> Thanks for taking a look. The real reason I did it was because Tejun
> said so :).
>
> Tejun, is there a larger reason?
>
I understand the reason. I am just suggesting to rephrase it to be more clear.