From: Michal Hocko <mhocko@kernel.org> To: Johannes Weiner <hannes@cmpxchg.org> Cc: Andrew Morton <akpm@linux-foundation.org>, Vladimir Davydov <vdavydov.dev@gmail.com>, linux-mm@kvack.org, cgroups@vger.kernel.org, linux-kernel@vger.kernel.org, kernel-team@fb.com Subject: Re: [PATCH 2/4] mm: memcontrol: re-use global VM event enum Date: Fri, 7 Apr 2017 14:47:02 +0200 [thread overview] Message-ID: <20170407124702.GE16413@dhcp22.suse.cz> (raw) In-Reply-To: <20170404220148.28338-2-hannes@cmpxchg.org> I do agree that we should share global and memcg specific events constants but I am not sure we want to share all of them. Would it make sense to reorganize the global enum and put those that are shared to the beginning? We wouldn't need the memcg specific translation then. Anyway, two comments on the current implementation. On Tue 04-04-17 18:01:46, Johannes Weiner wrote: [...] > +/* Cgroup-specific events, on top of universal VM events */ > +enum memcg_event_item { > + MEMCG_LOW = NR_VM_EVENT_ITEMS, > + MEMCG_HIGH, > + MEMCG_MAX, > + MEMCG_OOM, > + MEMCG_NR_EVENTS, > +}; The above should mention that each supported global VM event should provide the corresponding translation [...] here... > +/* Universal VM events cgroup1 shows, original sort order */ > +unsigned int memcg1_events[] = { > + PGPGIN, > + PGPGOUT, > + PGFAULT, > + PGMAJFAULT, > +}; > + > +static const char *const memcg1_event_names[] = { > + "pgpgin", > + "pgpgout", > + "pgfault", > + "pgmajfault", > +}; the naming doesn't make it easier to undestand why we need this. global2memcg_event? -- Michal Hocko SUSE Labs
WARNING: multiple messages have this Message-ID (diff)
From: Michal Hocko <mhocko@kernel.org> To: Johannes Weiner <hannes@cmpxchg.org> Cc: Andrew Morton <akpm@linux-foundation.org>, Vladimir Davydov <vdavydov.dev@gmail.com>, linux-mm@kvack.org, cgroups@vger.kernel.org, linux-kernel@vger.kernel.org, kernel-team@fb.com Subject: Re: [PATCH 2/4] mm: memcontrol: re-use global VM event enum Date: Fri, 7 Apr 2017 14:47:02 +0200 [thread overview] Message-ID: <20170407124702.GE16413@dhcp22.suse.cz> (raw) In-Reply-To: <20170404220148.28338-2-hannes@cmpxchg.org> I do agree that we should share global and memcg specific events constants but I am not sure we want to share all of them. Would it make sense to reorganize the global enum and put those that are shared to the beginning? We wouldn't need the memcg specific translation then. Anyway, two comments on the current implementation. On Tue 04-04-17 18:01:46, Johannes Weiner wrote: [...] > +/* Cgroup-specific events, on top of universal VM events */ > +enum memcg_event_item { > + MEMCG_LOW = NR_VM_EVENT_ITEMS, > + MEMCG_HIGH, > + MEMCG_MAX, > + MEMCG_OOM, > + MEMCG_NR_EVENTS, > +}; The above should mention that each supported global VM event should provide the corresponding translation [...] here... > +/* Universal VM events cgroup1 shows, original sort order */ > +unsigned int memcg1_events[] = { > + PGPGIN, > + PGPGOUT, > + PGFAULT, > + PGMAJFAULT, > +}; > + > +static const char *const memcg1_event_names[] = { > + "pgpgin", > + "pgpgout", > + "pgfault", > + "pgmajfault", > +}; the naming doesn't make it easier to undestand why we need this. global2memcg_event? -- Michal Hocko SUSE Labs -- 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:[~2017-04-07 12:47 UTC|newest] Thread overview: 29+ messages / expand[flat|nested] mbox.gz Atom feed top 2017-04-04 22:01 [PATCH 1/4] mm: memcontrol: clean up memory.events counting function Johannes Weiner 2017-04-04 22:01 ` Johannes Weiner 2017-04-04 22:01 ` [PATCH 2/4] mm: memcontrol: re-use global VM event enum Johannes Weiner 2017-04-04 22:01 ` Johannes Weiner 2017-04-06 8:49 ` Vladimir Davydov 2017-04-06 8:49 ` Vladimir Davydov 2017-04-10 14:17 ` Johannes Weiner 2017-04-10 14:17 ` Johannes Weiner 2017-04-07 12:47 ` Michal Hocko [this message] 2017-04-07 12:47 ` Michal Hocko 2017-04-10 14:13 ` Johannes Weiner 2017-04-10 14:13 ` Johannes Weiner 2017-04-11 12:30 ` Michal Hocko 2017-04-11 12:30 ` Michal Hocko 2017-04-04 22:01 ` [PATCH 3/4] mm: memcontrol: re-use node VM page state enum Johannes Weiner 2017-04-04 22:01 ` Johannes Weiner 2017-04-06 8:59 ` Vladimir Davydov 2017-04-06 8:59 ` Vladimir Davydov 2017-04-04 22:01 ` [PATCH 4/4] mm: memcontrol: use node page state naming scheme for memcg Johannes Weiner 2017-04-04 22:01 ` Johannes Weiner 2017-04-06 9:01 ` Vladimir Davydov 2017-04-06 9:01 ` Vladimir Davydov 2017-04-07 12:54 ` Michal Hocko 2017-04-07 12:54 ` Michal Hocko 2017-04-07 12:54 ` Michal Hocko 2017-04-06 8:31 ` [PATCH 1/4] mm: memcontrol: clean up memory.events counting function Vladimir Davydov 2017-04-06 8:31 ` Vladimir Davydov 2017-04-07 12:20 ` Michal Hocko 2017-04-07 12:20 ` Michal Hocko
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=20170407124702.GE16413@dhcp22.suse.cz \ --to=mhocko@kernel.org \ --cc=akpm@linux-foundation.org \ --cc=cgroups@vger.kernel.org \ --cc=hannes@cmpxchg.org \ --cc=kernel-team@fb.com \ --cc=linux-kernel@vger.kernel.org \ --cc=linux-mm@kvack.org \ --cc=vdavydov.dev@gmail.com \ /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: linkBe 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.