From: Mel Gorman <mel@csn.ul.ie>
To: Johannes Weiner <hannes@cmpxchg.org>
Cc: Andrew Morton <akpm@linux-foundation.org>,
KOSAKI Motohiro <kosaki.motohiro@jp.fujitsu.com>,
linux-mm@kvack.org
Subject: Re: [patch 1/4] mm: drop unneeded double negations
Date: Tue, 21 Jul 2009 10:33:13 +0100 [thread overview]
Message-ID: <20090721093312.GA25383@csn.ul.ie> (raw)
In-Reply-To: <1248166594-8859-1-git-send-email-hannes@cmpxchg.org>
On Tue, Jul 21, 2009 at 10:56:31AM +0200, Johannes Weiner wrote:
> Remove double negations where the operand is already boolean.
>
> Signed-off-by: Johannes Weiner <hannes@cmpxchg.org>
> ---
> mm/memcontrol.c | 2 +-
> mm/memory.c | 2 +-
> mm/vmscan.c | 10 +++++-----
> 3 files changed, 7 insertions(+), 7 deletions(-)
>
> diff --git a/mm/memcontrol.c b/mm/memcontrol.c
> index 38ad840..8ad148a 100644
> --- a/mm/memcontrol.c
> +++ b/mm/memcontrol.c
> @@ -655,7 +655,7 @@ unsigned long mem_cgroup_isolate_pages(unsigned long nr_to_scan,
> int nid = z->zone_pgdat->node_id;
> int zid = zone_idx(z);
> struct mem_cgroup_per_zone *mz;
> - int lru = LRU_FILE * !!file + !!active;
> + int lru = LRU_FILE * file + active;
> int ret;
>
Ok, this should be ok as file and active appear to be 1 and 0.
> BUG_ON(!mem_cont);
> diff --git a/mm/memory.c b/mm/memory.c
> index 6521619..dd8eb26 100644
> --- a/mm/memory.c
> +++ b/mm/memory.c
> @@ -596,7 +596,7 @@ copy_one_pte(struct mm_struct *dst_mm, struct mm_struct *src_mm,
> if (page) {
> get_page(page);
> page_dup_rmap(page, vma, addr);
> - rss[!!PageAnon(page)]++;
> + rss[PageAnon(page)]++;
> }
Similarly, seems ok.
>
> out_set_pte:
> diff --git a/mm/vmscan.c b/mm/vmscan.c
> index 07fd8aa..46ec6a5 100644
> --- a/mm/vmscan.c
> +++ b/mm/vmscan.c
> @@ -516,7 +516,7 @@ int remove_mapping(struct address_space *mapping, struct page *page)
> void putback_lru_page(struct page *page)
> {
> int lru;
> - int active = !!TestClearPageActive(page);
> + int active = TestClearPageActive(page);
> int was_unevictable = PageUnevictable(page);
>
But are you *sure* about this change?
active it used as an array offset later in this function for evictable pages
so it needs to be 1 or 0 but IIRC, the TestClear functions are not guaranteed
to return 0 or 1 on all architectures. They return 0 or non-zero. I'm 99.999%
certain I've been bitten before by test_bit returning the word with the one
bit set instead of 1. Maybe things have changed since or it's my
imagination but can you double check please?
> VM_BUG_ON(PageLRU(page));
> @@ -966,7 +966,7 @@ static unsigned long isolate_pages_global(unsigned long nr,
> if (file)
> lru += LRU_FILE;
> return isolate_lru_pages(nr, &z->lru[lru].list, dst, scanned, order,
> - mode, !!file);
> + mode, file);
> }
>
> /*
> @@ -1204,7 +1204,7 @@ static unsigned long shrink_inactive_list(unsigned long max_scan,
> lru = page_lru(page);
> add_page_to_lru_list(zone, page, lru);
> if (is_active_lru(lru)) {
> - int file = !!is_file_lru(lru);
> + int file = is_file_lru(lru);
> reclaim_stat->recent_rotated[file]++;
> }
> if (!pagevec_add(&pvec, page)) {
> @@ -1310,7 +1310,7 @@ static void shrink_active_list(unsigned long nr_pages, struct zone *zone,
> if (scanning_global_lru(sc)) {
> zone->pages_scanned += pgscanned;
> }
> - reclaim_stat->recent_scanned[!!file] += nr_taken;
> + reclaim_stat->recent_scanned[file] += nr_taken;
>
> __count_zone_vm_events(PGREFILL, zone, pgscanned);
> if (file)
> @@ -1364,7 +1364,7 @@ static void shrink_active_list(unsigned long nr_pages, struct zone *zone,
> * helps balance scan pressure between file and anonymous pages in
> * get_scan_ratio.
> */
> - reclaim_stat->recent_rotated[!!file] += nr_rotated;
> + reclaim_stat->recent_rotated[file] += nr_rotated;
> __count_vm_events(PGDEACTIVATE, nr_deactivate);
>
> move_active_pages_to_lru(zone, &l_active,
> --
> 1.6.3
>
> --
> 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>
>
--
Mel Gorman
Part-time Phd Student Linux Technology Center
University of Limerick IBM Dublin Software Lab
--
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:[~2009-07-21 9:33 UTC|newest]
Thread overview: 18+ messages / expand[flat|nested] mbox.gz Atom feed top
2009-07-21 8:56 [patch 1/4] mm: drop unneeded double negations Johannes Weiner
2009-07-21 8:56 ` [patch 2/4] mm: introduce page_lru_type() Johannes Weiner
2009-07-22 1:52 ` Minchan Kim
2009-07-22 9:07 ` Johannes Weiner
2009-07-22 12:37 ` Minchan Kim
2009-07-22 16:01 ` Rik van Riel
2009-07-21 8:56 ` [patch 3/4] mm: return boolean from page_is_file_cache() Johannes Weiner
2009-07-21 8:56 ` [patch 4/4] mm: return boolean from page_has_private() Johannes Weiner
2009-07-22 16:49 ` Christoph Lameter
2009-07-22 17:50 ` Johannes Weiner
2009-07-22 17:54 ` [patch 5/4] mm: document is_page_cache_freeable() Johannes Weiner
2009-07-22 19:02 ` Christoph Lameter
2009-07-22 21:55 ` Li, Ming Chun
2009-07-22 22:10 ` Johannes Weiner
2009-07-22 23:51 ` Li, Ming Chun
2009-07-22 23:58 ` Christoph Lameter
2009-07-21 9:33 ` Mel Gorman [this message]
2009-07-21 11:18 ` [patch 1/4] mm: drop unneeded double negations Johannes Weiner
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=20090721093312.GA25383@csn.ul.ie \
--to=mel@csn.ul.ie \
--cc=akpm@linux-foundation.org \
--cc=hannes@cmpxchg.org \
--cc=kosaki.motohiro@jp.fujitsu.com \
--cc=linux-mm@kvack.org \
/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).