From: Mel Gorman <mgorman@techsingularity.net>
To: Joe Perches <joe@perches.com>
Cc: Yaowei Bai <baiyaowei@cmss.chinamobile.com>,
akpm@linux-foundation.org, n-horiguchi@ah.jp.nec.com,
kirill.shutemov@linux.intel.com, rientjes@google.com,
iamjoonsoo.kim@lge.com, hannes@cmpxchg.org,
vdavydov@virtuozzo.com, kuleshovmail@gmail.com, vbabka@suse.cz,
linux-mm@kvack.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH 4/5] mm/lru: is_file/active_lru can be boolean
Date: Wed, 23 Mar 2016 11:44:15 +0000 [thread overview]
Message-ID: <20160323114415.GJ31585@techsingularity.net> (raw)
In-Reply-To: <1458703028.22080.7.camel@perches.com>
On Tue, Mar 22, 2016 at 08:17:08PM -0700, Joe Perches wrote:
> On Wed, 2016-03-23 at 10:26 +0800, Yaowei Bai wrote:
> > This patch makes is_file/active_lru return bool to improve
> > readability due to these particular functions only using either
> > one or zero as their return value.
> >
> > No functional change.
>
> These assignments to int should likely be modified too
>
Which would lead to oddities as the ints are used as offsets within
enums. Patch 2 has a problem where a bool is then used as part of a
bitmask operation.
I stopped looking fairly early on. Conversions from int to bool as part
of a cleanup-only series are almost never useful and sometimes introduce
subtle breakage. It is only worth the conversion when the helper is being
modified for some other purpose.
On that grounds, NAK to the whole series as small problems were quickly
obvious and it's not worth the brainpower to find all the problems when
the end result does not give us a tangible improvement.
--
Mel Gorman
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:[~2016-03-23 11:44 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-03-23 2:26 [PATCH 0/5] mm: make several functions return bool Yaowei Bai
2016-03-23 2:26 ` [PATCH 1/5] mm/hugetlb: is_vm_hugetlb_page can be boolean Yaowei Bai
2016-03-23 2:26 ` [PATCH 2/5] mm/memory_hotplug: is_mem_section_removable " Yaowei Bai
2016-03-23 2:26 ` [PATCH 3/5] mm/vmalloc: is_vmalloc_addr " Yaowei Bai
2016-03-23 2:26 ` [PATCH 4/5] mm/lru: is_file/active_lru " Yaowei Bai
2016-03-23 3:17 ` Joe Perches
2016-03-23 11:44 ` Mel Gorman [this message]
2016-03-23 2:26 ` [PATCH 5/5] mm/mempolicy: vma_migratable " Yaowei Bai
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=20160323114415.GJ31585@techsingularity.net \
--to=mgorman@techsingularity.net \
--cc=akpm@linux-foundation.org \
--cc=baiyaowei@cmss.chinamobile.com \
--cc=hannes@cmpxchg.org \
--cc=iamjoonsoo.kim@lge.com \
--cc=joe@perches.com \
--cc=kirill.shutemov@linux.intel.com \
--cc=kuleshovmail@gmail.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-mm@kvack.org \
--cc=n-horiguchi@ah.jp.nec.com \
--cc=rientjes@google.com \
--cc=vbabka@suse.cz \
--cc=vdavydov@virtuozzo.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: 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;
as well as URLs for NNTP newsgroup(s).