From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S932872AbcFIRje (ORCPT ); Thu, 9 Jun 2016 13:39:34 -0400 Received: from mx1.redhat.com ([209.132.183.28]:34542 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S932249AbcFIRjd (ORCPT ); Thu, 9 Jun 2016 13:39:33 -0400 Message-ID: <1465493968.15275.3.camel@redhat.com> Subject: Re: [PATCH trivial] include/linux/memcontrol.h: Clean up code only From: Rik van Riel To: chengang@emindsoft.com.cn, akpm@linux-foundation.org, trivial@kernel.org Cc: vdavydov@virtuozzo.com, hannes@cmpxchg.org, mhocko@suse.cz, davem@davemloft.net, tj@kernel.org, linux-kernel@vger.kernel.org, Chen Gang Date: Thu, 09 Jun 2016 13:39:28 -0400 In-Reply-To: <1465485832-24175-1-git-send-email-chengang@emindsoft.com.cn> References: <1465485832-24175-1-git-send-email-chengang@emindsoft.com.cn> Content-Type: multipart/signed; micalg="pgp-sha256"; protocol="application/pgp-signature"; boundary="=-x/L0QkhVEDzfa8OkrTLv" Mime-Version: 1.0 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.30]); Thu, 09 Jun 2016 17:39:32 +0000 (UTC) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org --=-x/L0QkhVEDzfa8OkrTLv Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable On Thu, 2016-06-09 at 23:23 +0800, chengang@emindsoft.com.cn wrote: > From: Chen Gang >=20 > Merge several statements to one return statement, since the new > return > statement is still simple enough. This code is not simple, and any change that makes it harder to read needs a good reason. At least in my opinion, your return statement merging thing makes the code harder to read. > Try to let the second line function parameters almost align with the > first line parameter (try to be within 80 columns, and in one line). >=20 > The comments can fully use 80 columns which can save one line. >=20 > Use parameter name newpage instead of new (which will be key word > color > in vim) for dummy mem_cgroup_migrate(), and real mem_cgroup_migrate() > already uses newpage. >=20 > Signed-off-by: Chen Gang > --- > =C2=A0include/linux/memcontrol.h | 31 +++++++++++-------------------- > =C2=A01 file changed, 11 insertions(+), 20 deletions(-) >=20 > diff --git a/include/linux/memcontrol.h b/include/linux/memcontrol.h > index 2d03975..a03204e 100644 > --- a/include/linux/memcontrol.h > +++ b/include/linux/memcontrol.h > @@ -327,10 +327,7 @@ void mem_cgroup_iter_break(struct mem_cgroup *, > struct mem_cgroup *); > =C2=A0 > =C2=A0static inline unsigned short mem_cgroup_id(struct mem_cgroup *memcg= ) > =C2=A0{ > - if (mem_cgroup_disabled()) > - return 0; > - > - return memcg->css.id; > + return mem_cgroup_disabled() ? 0 : memcg->css.id; > =C2=A0} > =C2=A0 > =C2=A0/** > @@ -341,10 +338,7 @@ static inline unsigned short > mem_cgroup_id(struct mem_cgroup *memcg) > =C2=A0 */ > =C2=A0static inline struct mem_cgroup *mem_cgroup_from_id(unsigned short > id) > =C2=A0{ > - struct cgroup_subsys_state *css; > - > - css =3D css_from_id(id, &memory_cgrp_subsys); > - return mem_cgroup_from_css(css); > + return mem_cgroup_from_css(css_from_id(id, > &memory_cgrp_subsys)); > =C2=A0} > =C2=A0 > =C2=A0/** > @@ -390,9 +384,7 @@ ino_t page_cgroup_ino(struct page *page); > =C2=A0 > =C2=A0static inline bool mem_cgroup_online(struct mem_cgroup *memcg) > =C2=A0{ > - if (mem_cgroup_disabled()) > - return true; > - return !!(memcg->css.flags & CSS_ONLINE); > + return mem_cgroup_disabled() || (memcg->css.flags & > CSS_ONLINE); > =C2=A0} > =C2=A0 > =C2=A0/* > @@ -401,7 +393,7 @@ static inline bool mem_cgroup_online(struct > mem_cgroup *memcg) > =C2=A0int mem_cgroup_select_victim_node(struct mem_cgroup *memcg); > =C2=A0 > =C2=A0void mem_cgroup_update_lru_size(struct lruvec *lruvec, enum lru_lis= t > lru, > - int nr_pages); > + int nr_pages); > =C2=A0 > =C2=A0unsigned long mem_cgroup_node_nr_lru_pages(struct mem_cgroup *memcg= , > =C2=A0 =C2=A0=C2=A0=C2=A0int nid, unsigned int > lru_mask); > @@ -452,9 +444,8 @@ void unlock_page_memcg(struct page *page); > =C2=A0 * @idx: page state item to account > =C2=A0 * @val: number of pages (positive or negative) > =C2=A0 * > - * The @page must be locked or the caller must use lock_page_memcg() > - * to prevent double accounting when the page is concurrently being > - * moved to another memcg: > + * The @page must be locked or the caller must use lock_page_memcg() > to prevent > + * double accounting when the page is concurrently being moved to > another memcg: > =C2=A0 * > =C2=A0 *=C2=A0=C2=A0=C2=A0lock_page(page) or lock_page_memcg(page) > =C2=A0 *=C2=A0=C2=A0=C2=A0if (TestClearPageState(page)) > @@ -462,7 +453,7 @@ void unlock_page_memcg(struct page *page); > =C2=A0 *=C2=A0=C2=A0=C2=A0unlock_page(page) or unlock_page_memcg(page) > =C2=A0 */ > =C2=A0static inline void mem_cgroup_update_page_stat(struct page *page, > - =C2=A0enum mem_cgroup_stat_index idx, int > val) > + enum mem_cgroup_stat_index > idx, int val) > =C2=A0{ > =C2=A0 VM_BUG_ON(!(rcu_read_lock_held() || PageLocked(page))); > =C2=A0 > @@ -569,7 +560,7 @@ static inline void > mem_cgroup_uncharge_list(struct list_head *page_list) > =C2=A0{ > =C2=A0} > =C2=A0 > -static inline void mem_cgroup_migrate(struct page *old, struct page > *new) > +static inline void mem_cgroup_migrate(struct page *old, struct page > *newpage) > =C2=A0{ > =C2=A0} > =C2=A0 > @@ -586,7 +577,7 @@ static inline struct lruvec > *mem_cgroup_page_lruvec(struct page *page, > =C2=A0} > =C2=A0 > =C2=A0static inline bool mm_match_cgroup(struct mm_struct *mm, > - struct mem_cgroup *memcg) > + =C2=A0=C2=A0=C2=A0struct mem_cgroup *memcg) > =C2=A0{ > =C2=A0 return true; > =C2=A0} > @@ -798,7 +789,7 @@ static inline int memcg_cache_id(struct > mem_cgroup *memcg) > =C2=A0 * @val: number of pages (positive or negative) > =C2=A0 */ > =C2=A0static inline void memcg_kmem_update_page_stat(struct page *page, > - enum mem_cgroup_stat_index idx, int > val) > + enum mem_cgroup_stat_index > idx, int val) > =C2=A0{ > =C2=A0 if (memcg_kmem_enabled() && page->mem_cgroup) > =C2=A0 this_cpu_add(page->mem_cgroup->stat->count[idx], > val); > @@ -827,7 +818,7 @@ static inline void memcg_put_cache_ids(void) > =C2=A0} > =C2=A0 > =C2=A0static inline void memcg_kmem_update_page_stat(struct page *page, > - enum mem_cgroup_stat_index idx, int > val) > + enum mem_cgroup_stat_index > idx, int val) > =C2=A0{ > =C2=A0} > =C2=A0#endif /* CONFIG_MEMCG && !CONFIG_SLOB */ --=20 All rights reversed --=-x/L0QkhVEDzfa8OkrTLv Content-Type: application/pgp-signature; name="signature.asc" Content-Description: This is a digitally signed message part Content-Transfer-Encoding: 7bit -----BEGIN PGP SIGNATURE----- Version: GnuPG v2 iQEcBAABCAAGBQJXWanQAAoJEM553pKExN6DrdYH/iQZ19t4GIKSxxOto3KRsiG+ 1sVdBrx0U3oxzTF8gI7h58/86+7Ha8VEJFl45UDzeKJiFt32ZTZP5uGV1Tf1fBVy rGEyVZu2859L2hTTe35TA3X+sVJRuZm0l+Uir1vYbJCDBcpY+vNuUouD67SX9WxY F0Z8YXi7ejIO2J2E4j9uJa/azAcuMHTy/vxeE4lZqQq2q3W1L6f2CPqlsO2/fQIg alPFlRawS15RA1Pd5bERmKVlfjPtvxOxzadyJdhMNYU3iiUrw2Ke/3bRfxUdZyH6 Md1DSvemW99i4bZGjeO943x3eqH6wJQ+6GUfWJBBqCU2yO80yPqOMwXlgiX6f5Y= =jUBY -----END PGP SIGNATURE----- --=-x/L0QkhVEDzfa8OkrTLv--