All of lore.kernel.org
 help / color / mirror / Atom feed
From: Joel Schopp <jschopp@austin.ibm.com>
To: Mel Gorman <mel@csn.ul.ie>
Cc: Dave Hansen <haveblue@us.ibm.com>, Andrew Morton <akpm@osdl.org>,
	kravetz@us.ibm.com,
	Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
	linux-mm <linux-mm@kvack.org>,
	lhms <lhms-devel@lists.sourceforge.net>
Subject: Re: [PATCH 2/8] Fragmentation Avoidance V17: 002_usemap
Date: Thu, 13 Oct 2005 11:33:54 -0500	[thread overview]
Message-ID: <434E8C72.5000909@austin.ibm.com> (raw)
In-Reply-To: <Pine.LNX.4.58.0510131500020.7570@skynet>

>>>@@ -473,6 +491,15 @@ extern struct pglist_data contig_page_da
>>> #if (MAX_ORDER - 1 + PAGE_SHIFT) > SECTION_SIZE_BITS
>>> #error Allocator MAX_ORDER exceeds SECTION_SIZE
>>> #endif
>>>+#if ((SECTION_SIZE_BITS - MAX_ORDER) * BITS_PER_RCLM_TYPE) > 64
>>>+#error free_area_usemap is not big enough
>>>+#endif
>>
>>Every time I look at these patches, I see this #if, and I don't remember
>>what that '64' means.  Can it please get a real name?
>>
> 
> 
> Joel, suggestions?

Oh yeah, blame it on me just because I wrote that bit of code.  How about
#define FREE_AREA_USEMAP_SIZE 64

> 
> 
>>>+/* Usemap initialisation */
>>>+#ifdef CONFIG_SPARSEMEM
>>>+static inline void setup_usemap(struct pglist_data *pgdat,
>>>+				struct zone *zone, unsigned long zonesize) {}
>>>+#endif /* CONFIG_SPARSEMEM */
>>>
>>> struct page;
>>> struct mem_section {
>>>@@ -485,6 +512,7 @@ struct mem_section {
>>> 	 * before using it wrong.
>>> 	 */
>>> 	unsigned long section_mem_map;
>>>+	DECLARE_BITMAP(free_area_usemap,64);
>>> };
>>
>>There's that '64' again!  You need a space after the comma, too.

Ditto.

>>>+ * RCLM_SHIFT is the number of bits that a gfp_mask has to be shifted right
>>>+ * to have just the __GFP_USER and __GFP_KERNRCLM bits. The static check is
>>>+ * made afterwards in case the GFP flags are not updated without updating
>>>+ * this number
>>>+ */
>>>+#define RCLM_SHIFT 19
>>>+#if (__GFP_USER >> RCLM_SHIFT) != RCLM_USER
>>>+#error __GFP_USER not mapping to RCLM_USER
>>>+#endif
>>>+#if (__GFP_KERNRCLM >> RCLM_SHIFT) != RCLM_KERN
>>>+#error __GFP_KERNRCLM not mapping to RCLM_KERN
>>>+#endif
>>
>>Should this really be in page_alloc.c, or should it be close to the
>>RCLM_* definitions?

I had the same first impression, but concluded this was the best place.  The 
compile time checks should keep things from getting out of sync.

WARNING: multiple messages have this Message-ID (diff)
From: Joel Schopp <jschopp@austin.ibm.com>
To: Mel Gorman <mel@csn.ul.ie>
Cc: Dave Hansen <haveblue@us.ibm.com>, Andrew Morton <akpm@osdl.org>,
	kravetz@us.ibm.com,
	Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
	linux-mm <linux-mm@kvack.org>,
	lhms <lhms-devel@lists.sourceforge.net>
Subject: Re: [PATCH 2/8] Fragmentation Avoidance V17: 002_usemap
Date: Thu, 13 Oct 2005 11:33:54 -0500	[thread overview]
Message-ID: <434E8C72.5000909@austin.ibm.com> (raw)
In-Reply-To: <Pine.LNX.4.58.0510131500020.7570@skynet>

>>>@@ -473,6 +491,15 @@ extern struct pglist_data contig_page_da
>>> #if (MAX_ORDER - 1 + PAGE_SHIFT) > SECTION_SIZE_BITS
>>> #error Allocator MAX_ORDER exceeds SECTION_SIZE
>>> #endif
>>>+#if ((SECTION_SIZE_BITS - MAX_ORDER) * BITS_PER_RCLM_TYPE) > 64
>>>+#error free_area_usemap is not big enough
>>>+#endif
>>
>>Every time I look at these patches, I see this #if, and I don't remember
>>what that '64' means.  Can it please get a real name?
>>
> 
> 
> Joel, suggestions?

Oh yeah, blame it on me just because I wrote that bit of code.  How about
#define FREE_AREA_USEMAP_SIZE 64

> 
> 
>>>+/* Usemap initialisation */
>>>+#ifdef CONFIG_SPARSEMEM
>>>+static inline void setup_usemap(struct pglist_data *pgdat,
>>>+				struct zone *zone, unsigned long zonesize) {}
>>>+#endif /* CONFIG_SPARSEMEM */
>>>
>>> struct page;
>>> struct mem_section {
>>>@@ -485,6 +512,7 @@ struct mem_section {
>>> 	 * before using it wrong.
>>> 	 */
>>> 	unsigned long section_mem_map;
>>>+	DECLARE_BITMAP(free_area_usemap,64);
>>> };
>>
>>There's that '64' again!  You need a space after the comma, too.

Ditto.

>>>+ * RCLM_SHIFT is the number of bits that a gfp_mask has to be shifted right
>>>+ * to have just the __GFP_USER and __GFP_KERNRCLM bits. The static check is
>>>+ * made afterwards in case the GFP flags are not updated without updating
>>>+ * this number
>>>+ */
>>>+#define RCLM_SHIFT 19
>>>+#if (__GFP_USER >> RCLM_SHIFT) != RCLM_USER
>>>+#error __GFP_USER not mapping to RCLM_USER
>>>+#endif
>>>+#if (__GFP_KERNRCLM >> RCLM_SHIFT) != RCLM_KERN
>>>+#error __GFP_KERNRCLM not mapping to RCLM_KERN
>>>+#endif
>>
>>Should this really be in page_alloc.c, or should it be close to the
>>RCLM_* definitions?

I had the same first impression, but concluded this was the best place.  The 
compile time checks should keep things from getting out of sync.

--
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>

  parent reply	other threads:[~2005-10-13 16:34 UTC|newest]

Thread overview: 54+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2005-10-11 15:12 [PATCH 0/8] Fragmentation Avoidance V17 Mel Gorman
2005-10-11 15:12 ` Mel Gorman
2005-10-11 15:12 ` [PATCH 1/8] Fragmentation Avoidance V17: 001_antidefrag_flags Mel Gorman
2005-10-11 15:12   ` Mel Gorman
2005-10-11 15:12 ` [PATCH 2/8] Fragmentation Avoidance V17: 002_usemap Mel Gorman
2005-10-11 15:12   ` Mel Gorman
2005-10-13 13:56   ` Dave Hansen
2005-10-13 13:56     ` Dave Hansen
2005-10-13 14:10     ` Mel Gorman
2005-10-13 14:10       ` Mel Gorman
2005-10-13 14:18       ` Dave Hansen
2005-10-13 14:18         ` Dave Hansen
2005-10-13 14:35         ` Mel Gorman
2005-10-13 14:35           ` Mel Gorman
2005-10-16 20:44         ` Mel Gorman
2005-10-16 20:44           ` Mel Gorman
2005-10-13 16:33       ` Joel Schopp [this message]
2005-10-13 16:33         ` Joel Schopp
2005-10-11 15:12 ` [PATCH 3/8] Fragmentation Avoidance V17: 003_fragcore Mel Gorman
2005-10-11 15:12   ` Mel Gorman
2005-10-11 15:12 ` [PATCH 4/8] Fragmentation Avoidance V17: 004_markfree Mel Gorman
2005-10-11 15:12   ` Mel Gorman
2005-10-11 15:12 ` [PATCH 5/8] Fragmentation Avoidance V17: 005_fallback Mel Gorman
2005-10-11 15:12   ` Mel Gorman
2005-10-12 16:43   ` mike kravetz
2005-10-12 16:43     ` mike kravetz
2005-10-12 17:21     ` Mel Gorman
2005-10-12 17:21       ` Mel Gorman
2005-10-12 17:29       ` [Lhms-devel] " Joel Schopp
2005-10-12 17:29         ` Joel Schopp
2005-10-14  5:12         ` Dave Hansen
2005-10-14  5:12           ` Dave Hansen
2005-10-11 15:12 ` [PATCH 6/8] Fragmentation Avoidance V17: 006_largealloc_tryharder Mel Gorman
2005-10-11 15:12   ` Mel Gorman
2005-10-13 19:07   ` Joel Schopp
2005-10-13 19:07     ` Joel Schopp
2005-10-14  5:36     ` Dave Hansen
2005-10-14  5:36       ` Dave Hansen
2005-10-11 15:12 ` [PATCH 7/8] Fragmentation Avoidance V17: 007_percpu Mel Gorman
2005-10-11 15:12   ` Mel Gorman
2005-10-11 15:13 ` [PATCH 8/8] Fragmentation Avoidance V17: 008_stats Mel Gorman
2005-10-11 15:13   ` Mel Gorman
2005-10-12 11:57   ` [Lhms-devel] " Dave Hansen
2005-10-12 11:57     ` Dave Hansen
2005-10-12 12:19     ` Mel Gorman
2005-10-12 12:19       ` Mel Gorman
2005-10-16  2:52 ` [PATCH 0/8] Fragmentation Avoidance V17 Paul Jackson
2005-10-16  2:52   ` Paul Jackson
2005-10-16 11:59   ` Mel Gorman
2005-10-16 11:59     ` Mel Gorman
2005-10-16 17:53     ` Paul Jackson
2005-10-16 17:53       ` Paul Jackson
2005-10-16 18:03       ` Mel Gorman
2005-10-16 18:03         ` Mel Gorman

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=434E8C72.5000909@austin.ibm.com \
    --to=jschopp@austin.ibm.com \
    --cc=akpm@osdl.org \
    --cc=haveblue@us.ibm.com \
    --cc=kravetz@us.ibm.com \
    --cc=lhms-devel@lists.sourceforge.net \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=mel@csn.ul.ie \
    /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.