* Re: [patch] Allow user.* xattr in tmpfs
2016-07-01 4:36 [patch] Allow user.* xattr in tmpfs Pete Zaitcev
@ 2016-07-05 0:11 ` Hugh Dickins
2016-07-05 2:08 ` Pete Zaitcev
0 siblings, 1 reply; 3+ messages in thread
From: Hugh Dickins @ 2016-07-05 0:11 UTC (permalink / raw)
To: Pete Zaitcev; +Cc: linux-mm, Hugh Dickins
On Thu, 30 Jun 2016, Pete Zaitcev wrote:
> The lack of user extended attributes is a source of annoyance when
> testing something that uses it, such as OpenStack Swift (including
> Hummingbird). We used to "monkey-patch" this in Python, so that
> tests can run on development systems, but it became desirable
> to store correct attributes and existing stubs became impractical.
>
> See:
> - my failed attempt to use /var/tmp:
> https://review.openstack.org/328508
> - Sam Merritt removing monkey-patching:
> https://review.openstack.org/336323
>
> Signed-off-by: Pete Zaitcev <zaitcev@redhat.com>
> ---
I use a very similar patch for testing xattrs on tmpfs with xfstests.
>
> This seems entirely too obvious. I'm getting concerned that we omitted
> the user xattr for a reason. Just can't imagine what it might be.
The reason is that it lets anyone who can write to a tmpfs file
allocate almost all of RAM (or lowmem) to those xattrs; without
even being able to swap them out.
We cannot enable user xattrs on tmpfs for everybody without
limiting them in some way: so NAK to your patch as it stands.
Config option? Too inflexible, not much use here, except to you and me.
MEMCG KMEM charging? A good solution, but we don't want to force every
system which might want tmpfs user xattrs to have to switch on MEMCG.
New mount option to limit the amount? Probably the right solution,
consistent with how we limit swappable data with "nr_blocks=" or "size=".
Extend the "nr_inodes=" mount option to include memory for user xattrs?
I hadn't thought of this before your mail, maybe it's a good answer,
maybe not. It is perfectly reasonable to account the two together:
xattrs are inode extensions, and they both use unswappable lowmem
slab memory. Perhaps it would be the right solution if we invent
a new name to cover both uses, rather like "size=" got added as an
alternative for "nr_blocks=".
(And there is already a peculiar extra use of it in tmpfs: nr_inodes
limits hard links as well as inodes, to limit dentry memory usage.)
The advantage of the last would be, that installations would
automatically get (probably more than enough) space for tmpfs user
xattrs with a new kernel, without needing to mess with mount options.
Or would that be a disadvantage - a new way of quietly using up RAM?
But without a stronger case for user xattrs on tmpfs,
shouldn't you and I just stick with our patches?
Hugh
>
> diff --git a/mm/shmem.c b/mm/shmem.c
> index 24463b6..4ddec69 100644
> --- a/mm/shmem.c
> +++ b/mm/shmem.c
> @@ -2655,6 +2655,12 @@ static int shmem_xattr_handler_set(const struct xattr_handler *handler,
> return simple_xattr_set(&info->xattrs, name, value, size, flags);
> }
>
> +static const struct xattr_handler shmem_user_xattr_handler = {
> + .prefix = XATTR_USER_PREFIX,
> + .get = shmem_xattr_handler_get,
> + .set = shmem_xattr_handler_set,
> +};
> +
> static const struct xattr_handler shmem_security_xattr_handler = {
> .prefix = XATTR_SECURITY_PREFIX,
> .get = shmem_xattr_handler_get,
> @@ -2672,6 +2678,7 @@ static const struct xattr_handler *shmem_xattr_handlers[] = {
> &posix_acl_access_xattr_handler,
> &posix_acl_default_xattr_handler,
> #endif
> + &shmem_user_xattr_handler,
> &shmem_security_xattr_handler,
> &shmem_trusted_xattr_handler,
> NULL
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply [flat|nested] 3+ messages in thread