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 08:43:21 -0400	[thread overview]
Message-ID: <20130807124321.GA27006@htj.dyndns.org> (raw)
In-Reply-To: <20130807121836.GF8184@dhcp22.suse.cz>

Hello, Michal.

On Wed, Aug 07, 2013 at 02:18:36PM +0200, Michal Hocko wrote:
> How is it specific to memcg? The fact only memcg uses the interface
> doesn't imply it is memcg specific.

I don't follow.  It's only for memcg.  That is *by definition* memcg
specific.  It's the verbatim meaning of the word.  Now, I do
understand that it can be a concern the implementation details as-is
could be a bit too invasive into cgroup core to be moved to memcg, but
that's something we can work on, right?  Can you at least agree that
the feature is nmemcg specific and it'd be better to be located in
memcg if possible?  That really isn't not much to ask and is a logical
thing to do.

> There are other ways to achieve the same. E.g. not ack new usage of
> register callback users. We have done similar with other things like
> use_hierarchy...

Yes, but those are all inferior to actually moving the code where it
belongs.  Those makes the code harder to follow and people
misunderstand and waste time working on stuff (either in the core or
controllers) which eventually end up getting nacked.  Why do that when
we can easily do better?  What's the rationale behind that?

> The cleanup is removing 2 callbacks with a cost of moving non-memcg
> specific code inside memcg. That is what I am objecting to.

I don't really get your "non-memcg" specific code assertion when it is
by definition memcg-specific.  What are you talking about?

> I will not repeat myself. We seem to disagree on where the code belongs.
> As I've said I will not ack this code, try to find somebody else who
> think it is a good idea. I do not see any added value.

Nacking is part of your authority as maintainer but you should still
provide plausible rationale for that.  Are you saying that even if the
code is restructured so that it's not invasive into cgroup core, you
are still gonna disagree with it because it's still somehow not
memcg-specifc?  Please don't repeat yourself but do explain your
rationale.  That's part of your duty as a maintainer too.

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 12:43 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 [this message]
2013-08-07 13:26                 ` Michal Hocko
2013-08-07 13:36                   ` Tejun Heo
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=20130807124321.GA27006@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