From: Joel Schopp <jschopp@austin.ibm.com>
To: Yasunori Goto <y-goto@jp.fujitsu.com>
Cc: Linux Kernel ML <linux-kernel@vger.kernel.org>,
linux-mm <linux-mm@kvack.org>,
Linux Hotplug Memory Support <lhms-devel@lists.sourceforge.net>
Subject: Re: [Patch] New zone ZONE_EASY_RECLAIM take 4. (change build_zonelists)[3/8]
Date: Tue, 03 Jan 2006 15:24:40 -0600 [thread overview]
Message-ID: <43BAEB98.8060906@austin.ibm.com> (raw)
In-Reply-To: <20051220172910.1B0C.Y-GOTO@jp.fujitsu.com>
> - BUG_ON(zone_type > ZONE_HIGHMEM);
> + BUG_ON(zone_type > ZONE_EASY_RECLAIM);
It might be nice to check ifndef CONFIG_HIGHMEM that the zone isn't
particularly ZONE_HIGHMEM.
> int res = ZONE_NORMAL;
> - if (zone_bits & (__force int)__GFP_HIGHMEM)
> - res = ZONE_HIGHMEM;
> - if (zone_bits & (__force int)__GFP_DMA32)
> - res = ZONE_DMA32;
> - if (zone_bits & (__force int)__GFP_DMA)
> +
> + if (zone_bits == fls((__force int)__GFP_DMA))
> res = ZONE_DMA;
> + if (zone_bits == fls((__force int)__GFP_DMA32) &&
> + (__force int)__GFP_DMA32 == 0x02)
> + res = ZONE_DMA32;
> + if (zone_bits == fls((__force int)__GFP_HIGHMEM))
> + res = ZONE_HIGHMEM;
> + if (zone_bits == fls((__force int)__GFP_EASY_RECLAIM))
> + res = ZONE_EASY_RECLAIM;
> +
> return res;
> }
It is incredibly silly to check a constant for a value. When it is zero
instead of 2 the first part of the statement will be false anyway.
Which reminds me. Why are we using fls again? I don't see why we
aren't just (zone_bits & value) the types. It seems much easier to
understand that way.
>
> Index: zone_reclaim/include/linux/gfp.h
> ===================================================================
> --- zone_reclaim.orig/include/linux/gfp.h 2005-12-19 20:19:37.000000000 +0900
> +++ zone_reclaim/include/linux/gfp.h 2005-12-19 20:19:56.000000000 +0900
> @@ -81,7 +81,7 @@ struct vm_area_struct;
>
> static inline int gfp_zone(gfp_t gfp)
> {
> - int zone = GFP_ZONEMASK & (__force int) gfp;
> + int zone = fls(GFP_ZONEMASK & (__force int) gfp);
> BUG_ON(zone >= GFP_ZONETYPES);
> return zone;
> }
>
WARNING: multiple messages have this Message-ID (diff)
From: Joel Schopp <jschopp@austin.ibm.com>
To: Yasunori Goto <y-goto@jp.fujitsu.com>
Cc: Linux Kernel ML <linux-kernel@vger.kernel.org>,
linux-mm <linux-mm@kvack.org>,
Linux Hotplug Memory Support <lhms-devel@lists.sourceforge.net>
Subject: Re: [Patch] New zone ZONE_EASY_RECLAIM take 4. (change build_zonelists)[3/8]
Date: Tue, 03 Jan 2006 15:24:40 -0600 [thread overview]
Message-ID: <43BAEB98.8060906@austin.ibm.com> (raw)
In-Reply-To: <20051220172910.1B0C.Y-GOTO@jp.fujitsu.com>
> - BUG_ON(zone_type > ZONE_HIGHMEM);
> + BUG_ON(zone_type > ZONE_EASY_RECLAIM);
It might be nice to check ifndef CONFIG_HIGHMEM that the zone isn't
particularly ZONE_HIGHMEM.
> int res = ZONE_NORMAL;
> - if (zone_bits & (__force int)__GFP_HIGHMEM)
> - res = ZONE_HIGHMEM;
> - if (zone_bits & (__force int)__GFP_DMA32)
> - res = ZONE_DMA32;
> - if (zone_bits & (__force int)__GFP_DMA)
> +
> + if (zone_bits == fls((__force int)__GFP_DMA))
> res = ZONE_DMA;
> + if (zone_bits == fls((__force int)__GFP_DMA32) &&
> + (__force int)__GFP_DMA32 == 0x02)
> + res = ZONE_DMA32;
> + if (zone_bits == fls((__force int)__GFP_HIGHMEM))
> + res = ZONE_HIGHMEM;
> + if (zone_bits == fls((__force int)__GFP_EASY_RECLAIM))
> + res = ZONE_EASY_RECLAIM;
> +
> return res;
> }
It is incredibly silly to check a constant for a value. When it is zero
instead of 2 the first part of the statement will be false anyway.
Which reminds me. Why are we using fls again? I don't see why we
aren't just (zone_bits & value) the types. It seems much easier to
understand that way.
>
> Index: zone_reclaim/include/linux/gfp.h
> ===================================================================
> --- zone_reclaim.orig/include/linux/gfp.h 2005-12-19 20:19:37.000000000 +0900
> +++ zone_reclaim/include/linux/gfp.h 2005-12-19 20:19:56.000000000 +0900
> @@ -81,7 +81,7 @@ struct vm_area_struct;
>
> static inline int gfp_zone(gfp_t gfp)
> {
> - int zone = GFP_ZONEMASK & (__force int) gfp;
> + int zone = fls(GFP_ZONEMASK & (__force int) gfp);
> BUG_ON(zone >= GFP_ZONETYPES);
> return zone;
> }
>
--
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:[~2006-01-03 21:25 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2005-12-20 8:52 [Patch] New zone ZONE_EASY_RECLAIM take 4. (change build_zonelists)[3/8] Yasunori Goto
2005-12-20 8:52 ` Yasunori Goto
2006-01-03 21:24 ` Joel Schopp [this message]
2006-01-03 21:24 ` Joel Schopp
2006-01-05 5:42 ` Yasunori Goto
2006-01-05 5:42 ` Yasunori Goto
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=43BAEB98.8060906@austin.ibm.com \
--to=jschopp@austin.ibm.com \
--cc=lhms-devel@lists.sourceforge.net \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-mm@kvack.org \
--cc=y-goto@jp.fujitsu.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 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.