linux-mm.kvack.org archive mirror
 help / color / mirror / Atom feed
From: Tejun Heo <tj@kernel.org>
To: Michal Hocko <mhocko@suse.cz>
Cc: lizefan@huawei.com, hannes@cmpxchg.org, bsingharora@gmail.com,
	kamezawa.hiroyu@jp.fujitsu.com, cgroups@vger.kernel.org,
	linux-mm@kvack.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCHSET cgroup/for-3.12] cgroup: make cgroup_event specific to memcg
Date: Wed, 7 Aug 2013 09:36:45 -0400	[thread overview]
Message-ID: <20130807133645.GE27006@htj.dyndns.org> (raw)
In-Reply-To: <20130807132613.GH8184@dhcp22.suse.cz>

Hello, Michal.

On Wed, Aug 07, 2013 at 03:26:13PM +0200, Michal Hocko wrote:
> I would rather see it not changed unless it really is a big win in the
> cgroup core. So far I do not see anything like that (just look at
> __cgroup_from_dentry which needs to be exported to allow for the move).

The end goal is cleaning up cftype so that it becomes a thin wrapper
around seq_file and I'd really like to keep the interface minimal so
that it's difficult to misunderstand.

> You reduce the amount of code in cgroup.c, alright, but the code
> doesn't go away really. It just moves out of your sight and moves the
> same burden on somebody else without providing a new generic interface.

If the implementation details are all that you're objecting, I'll be
happy to restructure it.  I just didn't pay too much attention to it
because I considered it to be mostly deprecated.  I don't think it'll
be too much work and strongly think it'll be worth the effort.  Our
code base is extremely nasty is and I'll try to get any ounce of
cleanup I can get.

> If somebody needs a notification interface (and there is no one available
> right now) then you cannot prevent from such a pointless work anyway...

I'm gonna add one for freezer state transitions.  It'll be simple
"this file changed" thing and will probably apply that to at least oom
and vmpressure.  I'm relatively confident that it's gonna be pretty
simple and that's gonna be the cgroup event mechanism.

> cgroup_event_* don't sound memcg specific at all. They are playing with
> cgroup dentry reference counting and do a generic functionality which
> memcg doesn't need to know about.

Sure, I'll try to clean it up so that it doesn't meddle with cgroup
internals directly.

> I wouldn't object to having non-cgroup internals playing variant. I just
> do not think it makes sense to invest time to something that should go
> away long term.

I suppose it's priority thing.  To me, cleaning up cgroup core API is
quite important and I'd be happy to sink time and effort into it and
it's not like we can drop the event thing in a release cycle or two.
We'd have to carry it for years, so I think the effort is justified.

Thanks.

-- 
tejun

--
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>

  reply	other threads:[~2013-08-07 13:36 UTC|newest]

Thread overview: 30+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-08-04 16:07 Tejun Heo
2013-08-04 16:07 ` [PATCH 1/5] cgroup: implement CFTYPE_NO_PREFIX Tejun Heo
2013-08-04 16:07 ` [PATCH 2/5] cgroup: export __cgroup_from_dentry() and __cgroup_dput() Tejun Heo
2013-08-05  2:58   ` Li Zefan
2013-08-05 15:40     ` Tejun Heo
2013-08-05 17:08   ` [PATCH v2 2/5] cgroup: make __cgroup_from_dentry() and __cgroup_dput() global Tejun Heo
2013-08-04 16:07 ` [PATCH 3/5] cgroup, memcg: move cgroup_event implementation to memcg Tejun Heo
2013-08-05  3:14   ` Li Zefan
2013-08-05 17:09   ` [PATCH v2 " Tejun Heo
2013-08-06  2:02     ` Li Zefan
2013-08-06  2:21       ` Li Zefan
2013-08-06  3:26   ` [PATCH " Balbir Singh
2013-08-06 14:09     ` Tejun Heo
2013-08-06 16:03       ` Balbir Singh
2013-08-04 16:07 ` [PATCH 4/5] cgroup, memcg: move cgroup->event_list[_lock] and event callbacks into memcg Tejun Heo
2013-08-04 16:07 ` [PATCH 5/5] memcg: rename cgroup_event to mem_cgroup_event Tejun Heo
2013-08-05  3:26   ` Li Zefan
2013-08-05 17:10   ` [PATCH v2 " Tejun Heo
2013-08-05 16:01 ` [PATCHSET cgroup/for-3.12] cgroup: make cgroup_event specific to memcg Michal Hocko
2013-08-05 16:29   ` Tejun Heo
2013-08-05 19:16     ` Michal Hocko
2013-08-05 19:44       ` Tejun Heo
2013-08-06 15:58         ` Michal Hocko
2013-08-06 16:15           ` Tejun Heo
2013-08-07 12:18             ` Michal Hocko
2013-08-07 12:43               ` Tejun Heo
2013-08-07 13:26                 ` Michal Hocko
2013-08-07 13:36                   ` Tejun Heo [this message]
2013-08-08  2:53                     ` Li Zefan
2013-08-09  1:00                       ` 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=20130807133645.GE27006@htj.dyndns.org \
    --to=tj@kernel.org \
    --cc=bsingharora@gmail.com \
    --cc=cgroups@vger.kernel.org \
    --cc=hannes@cmpxchg.org \
    --cc=kamezawa.hiroyu@jp.fujitsu.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=lizefan@huawei.com \
    --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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox