The Linux Kernel Mailing List
 help / color / mirror / Atom feed
From: Hugh Dickins <hughd@google.com>
To: Aristeu Rozanski <aris@ruivo.org>
Cc: Tejun Heo <tj@kernel.org>, Aristeu Rozanski <aris@redhat.com>,
	linux-kernel@vger.kernel.org, Li Zefan <lizefan@huawei.com>,
	Hillf Danton <dhillf@gmail.com>
Subject: Re: [PATCH v3 0/3] cgroup: add xattr support
Date: Sun, 22 Jul 2012 12:12:07 -0700 (PDT)	[thread overview]
Message-ID: <alpine.LSU.2.00.1207221136160.1896@eggly.anvils> (raw)
In-Reply-To: <20120720175959.GA32741@cathedrallabs.org>

On Fri, 20 Jul 2012, Aristeu Rozanski wrote:
> On Wed, Jul 18, 2012 at 06:11:32PM -0700, Hugh Dickins wrote:
> > > But why do we need something completely new?  Can't we hijack some
> > > inodes used by tmpfs and use them for xattr storage?  ie. Would it be
> > > difficult to use tmpfs as backend storage for on-memory xattr?  With
> > > that, we would already have the mechanism and interface(!) for
> > > limiting the size.
> > 
> > That sounds just like what I was suggesting in my last sentence:
> > let userspace manage a tmpfs hierarchy parallel to the cgroupfs one.
> > 
> > Except, perhaps, where I assume "userspace" should be doing the hard work.
> 
> hm, not sure that's what Tejun meant. tmpfs uses anonymous memory for the file
> contents, so reuse that infrastructure to allocate space for the extended
> attributes the same way, instead of using kmem.
> 
> First thing I can think of is to use whole pages for it to prevent further
> complexity. Shouldn't make much difference considering the usecases we have
> now (systemd and containers), right?

Please, do not do this.

It may be fun to implement, but not to review and maintain.

If we're going to start supporting swappable kernel memory, tmpfs
xattrs is not the right place to start, and libfs xattrs certainly not:
they are a poor fit for swappable memory.  (You contemplate using whole
pages above: that will not be very kind to those without swap.)

By all means continue Zefan's work to move xattr support from tmpfs
to libfs (ah, to fs/xattr.c actually, okay), but keep them as kmem.

Support setting and removing user xattrs only if the user has the
appropriate capability (which root will have): looking through the
list of existing capabilities, CAP_IPC_LOCK actually looks appropriate,
although I admit its name certainly does not - it's the "lock down
unlimited amounts of memory" capability.

And support setting and removing user xattrs only if the filesystem
opts in to that: so cgroupfs can opt in, everything else stay out,
and we know where to look when memory goes missing.

Will "lsattr -R" in the cgroupfs mountpoint do enough to judge how
much memory is being used in this way?  I expect not, but I'm
unfamliar with it: you may need to show counts elsewhere.

If we keep an eye on those counts as systemd starts to make use of
this feature, perhaps a real case for making this memory swappable
will emerge; but more likely, a case for systemd to be economical
with them - they may be good for storing paths to data blobs, but
I doubt they're good for large blobs.

Hugh

  parent reply	other threads:[~2012-07-22 19:12 UTC|newest]

Thread overview: 27+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-07-02 14:29 [PATCH v3 0/3] cgroup: add xattr support Aristeu Rozanski
2012-07-02 14:29 ` [PATCH v3 1/3] xattr: extract simple_xattr code from tmpfs Aristeu Rozanski
2012-07-03 16:53   ` [PATCH v4 " Aristeu Rozanski
2012-07-02 14:29 ` [PATCH v3 2/3] cgroup: revise how we re-populate root directory Aristeu Rozanski
2012-07-09 17:17   ` Tejun Heo
2012-07-09 17:22     ` Tejun Heo
2012-07-09 17:28       ` Tejun Heo
2012-07-10 19:27         ` Aristeu Rozanski
2012-07-17 13:56           ` Aristeu Rozanski
2012-07-17 18:38           ` Tejun Heo
2012-07-17 21:29             ` Aristeu Rozanski
2012-07-17 21:40               ` Tejun Heo
2012-07-18 14:16                 ` Aristeu Rozanski
2012-07-18 16:32                   ` Tejun Heo
2012-07-02 14:29 ` [PATCH v3 3/3] cgroup: add xattr support Aristeu Rozanski
2012-07-17 20:41 ` [PATCH v3 0/3] " Tejun Heo
2012-07-18 20:02   ` Hugh Dickins
2012-07-18 22:10     ` Tejun Heo
2012-07-19  1:11       ` Hugh Dickins
2012-07-20 17:59         ` Aristeu Rozanski
2012-07-20 18:04           ` Tejun Heo
2012-08-07 15:22             ` Aristeu Rozanski
2012-07-22 19:12           ` Hugh Dickins [this message]
2012-07-23 18:12             ` Aristeu Rozanski
2012-07-24 18:28               ` Tejun Heo
2012-07-24 21:44                 ` Aristeu Rozanski
2012-07-20 18:10         ` Tejun Heo

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=alpine.LSU.2.00.1207221136160.1896@eggly.anvils \
    --to=hughd@google.com \
    --cc=aris@redhat.com \
    --cc=aris@ruivo.org \
    --cc=dhillf@gmail.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=lizefan@huawei.com \
    --cc=tj@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox