gmane.linux.kernel.cgroups

Re: [PATCH v6 12/31] fs: convert inode and dentry shrinking to be node aware

Glauber Costa — 2013-05-18T07:20:33

I forgive you.

Re: [PATCH v6 12/31] fs: convert inode and dentry shrinking to be node aware

Glauber Costa — 2013-05-17T22:54:23

Except of course that struct shrinker is obviously shared between runs,
and this won't cut.

Right now I am inclined to really just put this in the stack. The
alternative, if it becomes a problem, can be to extend the lru apis
to allow us to go for a single node. This way we only need to use 1
extra word in the stack.

Re: [PATCH 5/9] memcg: use css_get/put when charging/uncharging kmem

Tejun Heo — 2013-05-17T18:08:22

Hey,

On Fri, May 17, 2013 at 03:04:06PM +0800, Li Zefan wrote:

Paired with the wmb to achieve what?


The other side is wmb, so there gotta be something which wants to read
which were written before wmb here but the only thing after the
barrier is css_put() which doesn't need such thing, so I'm lost on
what the barrier pair is achieving here.

In general, please be *very* explicit about what's going on whenever
something is depending on barrier pairs.  It'll make it easier for
both the author and reviewers to actually understand what's going on
and why it's necessary.

...

Hmmm?  offline is called on cgroup destruction regardless of css
refcnt.  The above comment seems a bit misleading.


Is the wmb() trying to prevent reordering between css_get() and
memcg_kmem_mark_dead()?  If so, it isn't necessary - the compiler
isn't allowed to reorder two atomic ops (they're all asm volatiles)
and the visibility order is guaranteed by the nature of the two
operations going on here - both perform modify-and-test on o

Re: [patch v3 -mm 1/3] memcg: integrate soft reclaim tighter with zone shrinking code

Tejun Heo — 2013-05-17T17:45:54

Hello,

On Fri, May 17, 2013 at 10:27 AM, Johannes Weiner <hannes-druUgvl0LCNAfugRpC6u6w< at >public.gmane.org> wrote:

Heh, yeah, realized that after writing it but it can be something much
simpler. ie. just linked list of children with soft limit configured.


Yeap.


Thanks.

--
tejun

Re: [PATCH v6 12/31] fs: convert inode and dentry shrinking to be node aware

Glauber Costa — 2013-05-17T14:49:37


All right.

I played a bit today with variations of this patch that will keep the
deferred count per node. I will rebase the whole series ontop of it (the
changes can get quite disruptive) and post. I want to believe that
after this, all our regression problems will be gone (famous last words).

As I have told you, I wasn't seeing problems like you are, and
speculated that this was due to the disk speeds. While this is true,
the patch I came up with makes my workload actually a lot better.
While my caches weren't being emptied, they were being slightly depleted
and then slowly filled again. With my new patch, it is almost
a straight line throughout the whole find run. There is a dent here and
there eventually, but it recovers quickly. It takes some time as well
for steady state to be reached, but once it is, we have all variables
in the equation (dentries, inodes, etc) basically flat. So I guess it
works, and I am confident that it will make your workload better.

My strategy is to modify the shrinker stru

Re: [PATCH V2 0/3] memcg: simply lock of page stat accounting

Michal Hocko — 2013-05-17T12:53:27

But the file is there and we have to keep it for ever. That was my
point. I do not care how do we account.
 

Be careful about that. charge can end up in OOM lock waiting for the
user to handle free some memory. You cannot call charge from within any
locks (well mmap_sem for reading is somehow acceptable because that one
is really hard to get rid of).
 

No, unmap it from all processes if there is no other process from the
same group as the exiting task and charge on the re-fault.
I have heared about some usecases where this could help to age shared
data but the reporter disappeared so I've lost interest in fixing it
because it wouldn't be cheap and I do not remember many details about
the workload which would be necessary to justify such a change.


Well, the world is not ideal. I wasn't involved in the early discussion
about this topic but what I remember from mailing list archives is that
"charge at the first touch" was the most reasonable compromise at the
time.


There are other options as well. We can

Re: [PATCH V2 0/3] memcg: simply lock of page stat accounting

Konstantin Khlebnikov — 2013-05-17T10:29:33

Hmm. Kernel api is just api. It's not supposed to be a tool itself.
It must be usable for making tools which admins can use.


On activation it's mostly free, because we need to lock lru anyway.
If memcg charge/uncharge isn't fast enough for that it should be optimized.


So, you propose to uncharge pages at unmap and charge them to who?
To the next mm in rmap?

What if next map will happens from different memcg, but after that unmap:

A:map
<pages owned by A>
A:unmap
<no new owner in rmap>
B:map
<pages still owned by A>

Anyway this is difficult question and I bring this just to show that currently
used logic isn't perfect in many ways. Switching ownership lazily in particular
places is a best solution which I can see now. So, page will be owned by its last
active user.


Main problem here that it increases complexity of switching pages' ownership.
We need not just charge/uncharge page and move it to the different LRU, but
also synchronize with map/unmap paths in rmap to update nr-mapped counters
carefully

Re: [patch v3 -mm 3/3] vmscan, memcg: Do softlimit reclaim also for targeted reclaim

Michal Hocko — 2013-05-17T07:50:19

Thanks for the review Glauber!

Re: [patch v3 -mm 1/3] memcg: integrate soft reclaim tighter with zone shrinking code

Michal Hocko — 2013-05-17T07:16:55

The testing I have done was on top of the complete series. The last
patch should be irrelevant as I have tested the global reclaim but the
second patch might still influence figures a tiny bit (we still do the
soft limit tree thing). That's why I haven't pushed the numbers here.

I can add that information if people prefer or just ask Andrew to squash
the leader email into the first patch as he tend to do quite often in
other cases as well.

Re: [PATCH 0/12][V3] memcg: make memcg's life cycle the same as cgroup

Li Zefan — 2013-05-17T07:06:56

The subject should be "[PATCH 0/9][v3] ..."

On 2013/5/17 15:02, Li Zefan wrote:

[PATCH 3/9] memcg: use css_get() in sock_update_memcg()

Li Zefan — 2013-05-17T07:03:38

Use css_get/css_put instead of mem_cgroup_get/put.

Note, if at the same time someone is moving < at >current to a different
cgroup and removing the old cgroup, css_tryget() may return false,
and sock->sk_cgrp won't be initialized, which is fine.

Signed-off-by: Li Zefan <lizefan-hv44wF8Li93QT0dZR+AlfA< at >public.gmane.org>
Acked-by: KAMEZAWA Hiroyuki <kamezawa.hiroyu-+CUm20s59erQFUHtdCDX3A< at >public.gmane.org>
Acked-by: Michal Hocko <mhocko-AlSwsSmVLrQ< at >public.gmane.org>
---
 mm/memcontrol.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/mm/memcontrol.c b/mm/memcontrol.c
index 4d0458d..f1320d3 100644
--- a/mm/memcontrol.c
+++ b/mm/memcontrol.c
< at >< at > -561,15 +561,15 < at >< at > void sock_update_memcg(struct sock *sk)
  */
 if (sk->sk_cgrp) {
 BUG_ON(mem_cgroup_is_root(sk->sk_cgrp->memcg));
-mem_cgroup_get(sk->sk_cgrp->memcg);
+css_get(&sk->sk_cgrp->memcg->css);
 return;
 }
 
 rcu_read_lock();
 memcg = mem_cgroup_from_task(current);
 cg_proto = sk->sk_prot->proto_cgroup(memcg);
-

Re: [patch v3 -mm 1/3] memcg: integrate soft reclaim tighter with zone shrinking code

Tejun Heo — 2013-05-16T22:12:00

Sorry about the delay.  Just getting back to memcg.

On Mon, May 13, 2013 at 09:46:10AM +0200, Michal Hocko wrote:
...

ancestors?


Maybe the following is easier to follow?

if (mem_cgroup_should_soft_reclaim(sc)) {
__shrink_zone(zone, sc, true);
if (sc->nr_scanned == nr_scanned)
__shrink_zone(zone, sc, false);
} else {
__shrink_zone(zone, sc, false);
}

But it's a minor point, please feel free to ignore.

  Reviewed-by: Tejun Heo <tj-DgEjT+Ai2ygdnm+yROfE0A< at >public.gmane.org>

Thanks.

Re: [PATCH V2 0/3] memcg: simply lock of page stat accounting

Michal Hocko — 2013-05-16T13:28:46

not for dirty and writeback data which is the next step.


How do you find out whether given pages were charged to the group of
interest - e.g. shared data or taks that has moved from a different
group without move_at_immigrate?


The accounting code is trying to be not intrusive as much as possible.
This patchset makes it more complicated without a good reason and that
is why it has been Nacked by me.

Re: [PATCH v6 12/31] fs: convert inode and dentry shrinking to be node aware

Glauber Costa — 2013-05-16T08:03:30

That makes sense now, thanks. It doesn't seem, however, that any deep
black magic is required to fix this here.

Any reason why proportional scanning wouldn't work ?

If we have a very large nr_in_batch, and many nodes to scan, we can try
dividing them among present nodes in the proportion of existing objects
in the cache.

Or maybe even better, we can make deferring work node aware, so when you
defer, you don't defer to a global pool, but rather to a node copy.

Re: [PATCH 0/4] Rebase device_cgroup v2 patchset

Serge E. Hallyn — 2013-05-16T01:23:10

Quoting Serge E. Hallyn (serge-A9i7LUbDfNHQT0dZR+AlfA< at >public.gmane.org):

I'm terribly sorry, Andrew, I have no idea how that address for you got
into my address book.  (Corrected)  fwiw the thread can be followed at
https://lkml.org/lkml/2013/5/14/363 .

-serge

Re: [PATCH 0/4] Rebase device_cgroup v2 patchset

Serge E. Hallyn — 2013-05-16T01:14:01

Quoting Eric W. Biederman (ebiederm-aS9lmoZGLiVWk0Htik3J/w< at >public.gmane.org):

I'm still thinking through it, which is why I haven't sent a real
patch.  What I'm working on is the unprivileged startup of a container.
Right now most things are not allowed in a private user ns, so device
cgroup is not as useful.  But it should be possible eventually to use
block devices, which the original unprivileged user owned, by chowning
the blockdev to a user mapped into the target userns.

The unprivileged user may want to use devices cgroup so he can chown
the loop file into the container, but only allow read-only mounts, for
instance.


Yes I think that's it - except userns root before forking the container
init (and venturing into the really untrusted category).

...


If they were going to do that over tty, that would be to the malicious
user anyway, so that should just either be ignored, or result in the
program exiting early.


It's also possible that this will end up being worked around by the new
(not-yet-design

Re: [PATCH v6 12/31] fs: convert inode and dentry shrinking to be node aware

Glauber Costa — 2013-05-15T15:27:26

That doesn't totally make sense to me.

Both our scan and count functions will be per-node now. This means we
will always try to keep ourselves within reasonable maximums on a
per-node basis as well.

Re: [PATCH V2 0/3] memcg: simply lock of page stat accounting

Michal Hocko — 2013-05-15T13:41:10

Well, I guess it makes some sense to know how much page cache and anon
memory is charged to the group. I am using that to monitor the per-group
memory usage. I can imagine a even better coverage - something
/proc/meminfo like.

[GIT PULL] blk-throttle: implement proper hierarchy support

Tejun Heo — 2013-05-14T21:09:33

Hello, Jens.

This is the pull request for patches which implement proper hierarchy
support in blk-throttle and remove .broken_hierarchy tagging from
blkcg.

  http://thread.gmane.org/gmane.linux.kernel.cgroups/7119

The implementation is fairly straight-forward in that it just repeats
the same scheduling at each layer until it reaches the top and thus
isn't very scalable.  It also still has an issue where a nested cgroup
could get lower than configured limits as it travels towards root but
the severity is at an acceptable level after Vivke's start time
adjustment patch.  The issue ultimately is a problem in the scheduling
algorithm itself and can also show up in flat hierarchy given the
right (well, wrong) IO pattern.  If it still is an actual problem,
which I don't think is, we should be able to work on it later on in
fairly isolated manner.

While the implementation isn't perfect, it should be good enough in
most cases with a few levels of nesting and this allows the rest of
cgroup to proceed towards unif

Re: [PATCH 0/4] Rebase device_cgroup v2 patchset

Eric W. Biederman — 2013-05-14T21:02:06


That should be looked at very closely.  There are some funny exploits of
setuid root applications writing to files that have required some
additional permission checks on /proc/<pid>/uid_map.  I think the
cgroups files may be vulnerable to some of the same kind of exploits.

Certainly we should be verifying that the opener of the file had the
capabilities we are trying to use to avoid being open to those kinds of
problems.

I am trying to see the utilitity of the proposed patch.  It doesn't
allow mknod.  So what is the benefit of having the user namespace bits?

Is the point to allow the userns root to remove access to selected
devices from it's children even if the DAC permissions would allow the
access?


I think the sendmail website has something as well.  The core problem
was programs not dealing safely with having some of their privileges
removed.  As I recall an unprivileged attacker at one point could drop
the CAP_SYS_SETUID on sendmail and get it to deliver mail as root.

I took a good hard look at

Re: [PATCH v7 04/31] dcache: remove dentries from LRU before putting on dispose list

Dave Chinner — 2013-05-14T20:32:41

Right. Did I mention I was seeing a panic on a later patch? That's
because this patch removed the DCACHE_SHRINK_LIST check. So, the
DCACHE_SHRINK_LIST needs to move into dentry_lru_del(), and
dentry_lru_prune() can still go away. That's what my current version
of this patch does - I just didn't post it last night because it was
still running tests when I went to bed....

Cheers,

Dave.