From: Johannes Weiner <hannes@cmpxchg.org>
To: Michal Hocko <mhocko@suse.cz>
Cc: linux-mm@kvack.org, Greg Thelen <gthelen@google.com>,
Dave Hansen <dave@sr71.net>,
cgroups@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [patch] mm: memcontrol: lockless page counters
Date: Mon, 22 Sep 2014 11:50:49 -0400 [thread overview]
Message-ID: <20140922155049.GA6630@cmpxchg.org> (raw)
In-Reply-To: <20140922144436.GG336@dhcp22.suse.cz>
On Mon, Sep 22, 2014 at 04:44:36PM +0200, Michal Hocko wrote:
> On Fri 19-09-14 09:22:08, Johannes Weiner wrote:
> > Memory is internally accounted in bytes, using spinlock-protected
> > 64-bit counters, even though the smallest accounting delta is a page.
> > The counter interface is also convoluted and does too many things.
> >
> > Introduce a new lockless word-sized page counter API, then change all
> > memory accounting over to it and remove the old one. The translation
> > from and to bytes then only happens when interfacing with userspace.
>
> Dunno why but I thought other controllers use res_counter as well. But
> this doesn't seem to be the case so this is perfectly reasonable way
> forward.
You were fooled by its generic name! It really is a lot less generic
than what it was designed for, and there are no new users in sight.
> I have only glanced through the patch and it mostly seems good to me
> (I have to look more closely on the atomicity of hierarchical operations).
>
> Nevertheless I think that the counter should live outside of memcg (it
> is ugly and bad in general to make HUGETLB controller depend on MEMCG
> just to have a counter). If you made kernel/page_counter.c and led both
> containers select CONFIG_PAGE_COUNTER then you do not need a dependency
> on MEMCG and I would find it cleaner in general.
The reason I did it this way is because the hugetlb controller simply
accounts and limits a certain type of memory and in the future I would
like to make it a memcg extension, just like kmem and swap.
Once that is done, page counters can become fully private, but until
then I think it's a good idea to make them part of memcg to express
this relationship and to ensure we are moving in the same direction.
> > Aside from the locking costs, this gets rid of the icky unsigned long
> > long types in the very heart of memcg, which is great for 32 bit and
> > also makes the code a lot more readable.
>
> Definitely. Nice work!
Thanks!
--
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>
WARNING: multiple messages have this Message-ID (diff)
From: Johannes Weiner <hannes@cmpxchg.org>
To: Michal Hocko <mhocko@suse.cz>
Cc: linux-mm@kvack.org, Greg Thelen <gthelen@google.com>,
Dave Hansen <dave@sr71.net>,
cgroups@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [patch] mm: memcontrol: lockless page counters
Date: Mon, 22 Sep 2014 11:50:49 -0400 [thread overview]
Message-ID: <20140922155049.GA6630@cmpxchg.org> (raw)
In-Reply-To: <20140922144436.GG336@dhcp22.suse.cz>
On Mon, Sep 22, 2014 at 04:44:36PM +0200, Michal Hocko wrote:
> On Fri 19-09-14 09:22:08, Johannes Weiner wrote:
> > Memory is internally accounted in bytes, using spinlock-protected
> > 64-bit counters, even though the smallest accounting delta is a page.
> > The counter interface is also convoluted and does too many things.
> >
> > Introduce a new lockless word-sized page counter API, then change all
> > memory accounting over to it and remove the old one. The translation
> > from and to bytes then only happens when interfacing with userspace.
>
> Dunno why but I thought other controllers use res_counter as well. But
> this doesn't seem to be the case so this is perfectly reasonable way
> forward.
You were fooled by its generic name! It really is a lot less generic
than what it was designed for, and there are no new users in sight.
> I have only glanced through the patch and it mostly seems good to me
> (I have to look more closely on the atomicity of hierarchical operations).
>
> Nevertheless I think that the counter should live outside of memcg (it
> is ugly and bad in general to make HUGETLB controller depend on MEMCG
> just to have a counter). If you made kernel/page_counter.c and led both
> containers select CONFIG_PAGE_COUNTER then you do not need a dependency
> on MEMCG and I would find it cleaner in general.
The reason I did it this way is because the hugetlb controller simply
accounts and limits a certain type of memory and in the future I would
like to make it a memcg extension, just like kmem and swap.
Once that is done, page counters can become fully private, but until
then I think it's a good idea to make them part of memcg to express
this relationship and to ensure we are moving in the same direction.
> > Aside from the locking costs, this gets rid of the icky unsigned long
> > long types in the very heart of memcg, which is great for 32 bit and
> > also makes the code a lot more readable.
>
> Definitely. Nice work!
Thanks!
next prev parent reply other threads:[~2014-09-22 15:50 UTC|newest]
Thread overview: 53+ messages / expand[flat|nested] mbox.gz Atom feed top
2014-09-19 13:22 [patch] mm: memcontrol: lockless page counters Johannes Weiner
2014-09-19 13:22 ` Johannes Weiner
2014-09-19 13:29 ` Johannes Weiner
2014-09-19 13:29 ` Johannes Weiner
2014-09-22 14:41 ` Vladimir Davydov
2014-09-22 14:41 ` Vladimir Davydov
2014-09-22 18:57 ` Johannes Weiner
2014-09-22 18:57 ` Johannes Weiner
2014-09-22 18:57 ` Johannes Weiner
[not found] ` <20140922185736.GB6630-druUgvl0LCNAfugRpC6u6w@public.gmane.org>
2014-09-23 11:06 ` Vladimir Davydov
2014-09-23 11:06 ` Vladimir Davydov
2014-09-23 11:06 ` Vladimir Davydov
2014-09-23 13:28 ` Johannes Weiner
2014-09-23 13:28 ` Johannes Weiner
[not found] ` <20140923132801.GA14302-druUgvl0LCNAfugRpC6u6w@public.gmane.org>
2014-09-23 15:21 ` Vladimir Davydov
2014-09-23 15:21 ` Vladimir Davydov
2014-09-23 15:21 ` Vladimir Davydov
2014-09-23 17:05 ` Johannes Weiner
2014-09-23 17:05 ` Johannes Weiner
2014-09-23 17:05 ` Johannes Weiner
2014-09-24 8:02 ` Vladimir Davydov
2014-09-24 8:02 ` Vladimir Davydov
[not found] ` <20140923170525.GA28460-druUgvl0LCNAfugRpC6u6w@public.gmane.org>
2014-09-24 13:33 ` Michal Hocko
2014-09-24 13:33 ` Michal Hocko
2014-09-24 13:33 ` Michal Hocko
2014-09-24 16:51 ` Johannes Weiner
2014-09-24 16:51 ` Johannes Weiner
2014-09-24 14:16 ` Michal Hocko
2014-09-24 14:16 ` Michal Hocko
2014-09-24 17:00 ` Johannes Weiner
2014-09-24 17:00 ` Johannes Weiner
2014-09-25 13:07 ` Michal Hocko
2014-09-25 13:07 ` Michal Hocko
2014-09-22 14:44 ` Michal Hocko
2014-09-22 14:44 ` Michal Hocko
2014-09-22 15:50 ` Johannes Weiner [this message]
2014-09-22 15:50 ` Johannes Weiner
2014-09-22 17:28 ` Michal Hocko
2014-09-22 17:28 ` Michal Hocko
2014-09-22 19:58 ` Johannes Weiner
2014-09-22 19:58 ` Johannes Weiner
[not found] ` <20140922195829.GA5197-druUgvl0LCNAfugRpC6u6w@public.gmane.org>
2014-09-23 13:25 ` Michal Hocko
2014-09-23 13:25 ` Michal Hocko
2014-09-23 13:25 ` Michal Hocko
2014-09-23 14:05 ` Johannes Weiner
2014-09-23 14:05 ` Johannes Weiner
[not found] ` <20140923140526.GA15014-druUgvl0LCNAfugRpC6u6w@public.gmane.org>
2014-09-23 14:28 ` Michal Hocko
2014-09-23 14:28 ` Michal Hocko
2014-09-23 14:28 ` Michal Hocko
2014-09-23 22:33 ` David Rientjes
2014-09-23 22:33 ` David Rientjes
2014-09-23 7:46 ` Kamezawa Hiroyuki
2014-09-23 7:46 ` Kamezawa Hiroyuki
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=20140922155049.GA6630@cmpxchg.org \
--to=hannes@cmpxchg.org \
--cc=cgroups@vger.kernel.org \
--cc=dave@sr71.net \
--cc=gthelen@google.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-mm@kvack.org \
--cc=mhocko@suse.cz \
/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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.