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: Mon, 5 Aug 2013 12:29:58 -0400 [thread overview]
Message-ID: <20130805162958.GF19631@mtj.dyndns.org> (raw)
In-Reply-To: <20130805160107.GM10146@dhcp22.suse.cz>
Hello, Michal.
On Mon, Aug 05, 2013 at 06:01:07PM +0200, Michal Hocko wrote:
> Could you be more specific about what is so "overboard" about this
> interface? I am not familiar with internals much, so I cannot judge the
> complexity part, but I thought that eventfd was intended for this kind
> of kernel->userspace notifications.
It's just way over-engineered like many other things in cgroup, most
likely misguided by the appearance that cgroup could be delegated and
accessed by multiple actors concurrently.
The most clear example would be the vmpressure event. When it could
have just called fsnotify_modify() unconditionally when the state
changes, now it involves parsing, dynamic list of events and so on
without actually adding any benefits. For the usage ones,
configurability makes some sense but even then just giving it a single
array of event points of limited size would be sufficient.
It's just way over-done.
> So you think that vmpressure, oom notification or thresholds are
> an abuse of this interface? What would you consider a reasonable
> replacement for those notifications? Or do you think that controller
> shouldn't be signaling any conditions to the userspace at all?
I don't think the ability to generate events are an abuse, just that
the facility itself is way over-engineered. Just generate a file
changed event unconditionally for vmpressure and oom and maybe
implement configureable cadence or single set of threshold array for
threshold events. These are things which can and should be done in a
a few tens of lines of code with far simpler interface. There's no
need for this obsecenely flexible event infrastructure, which of
course leads to things like shared contiguous threshold table without
any size limit and allocated with kmalloc().
So, let's please move towards something simple.
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>
next prev parent reply other threads:[~2013-08-05 16:30 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 [this message]
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
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=20130805162958.GF19631@mtj.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