[PATCH 0/3] pfn_valid_within() HOLES_WITHIN_ZONES helper

[PATCH 0/3] pfn_valid_within() HOLES_WITHIN_ZONES helper

[Andy Whitcroft]

The thought of having a helper for the holes within zones code
has come up on two different threads in the last couple of days.
So I took the pfn_valid_within() patch I had developed for the
linear reclaim series and pulled it forward to 2.6.21-rc4-mm1.
I have split it into a three patch series to better align with the
affected patch sets within -mm.

add-pfn_valid_within-helper-for-sub-MAX_ORDER-hole-detection --
  adds the base helper and utilises it within the buddy allocator,

anti-fragmentation-switch-over-to-pfn_valid_within() -- changes
  references within Mel Gormans anti-fragmentation patch series, and

lumpy-move-to-using-pfn_valid_within() -- changes references with
  my lumpy reclaim patch series.

-apw

[PATCH 1/3] add pfn_valid_within helper for sub-MAX_ORDER hole detection

[Andy Whitcroft]

Generally we work under the assumption that memory the mem_map
array is contigious and valid out to MAX_ORDER_NR_PAGES block
of pages, ie. that if we have validated any page within this
MAX_ORDER_NR_PAGES block we need not check any other.  This is not
true when CONFIG_HOLES_IN_ZONE is set and we must check each and
every reference we make from a pfn.

Add a pfn_valid_within() helper which should be used when scanning
pages within a MAX_ORDER_NR_PAGES block when we have already
checked the validility of the block normally with pfn_valid().
This can then be optimised away when we do not have holes within
a MAX_ORDER_NR_PAGES block of pages.

Signed-off-by: Andy Whitcroft <apw@shadowen.org>
Acked-by: Mel Gorman <mel@csn.ul.ie>
---
diff --git a/include/linux/mmzone.h b/include/linux/mmzone.h
index 7206c77..8c87d79 100644
--- a/include/linux/mmzone.h
+++ b/include/linux/mmzone.h
@@ -837,6 +837,18 @@ void sparse_init(void);
 void memory_present(int nid, unsigned long start, unsigned long end);
 unsigned long __init node_memmap_size_bytes(int, unsigned long, unsigned long);
 
+/*
+ * If it is possible to have holes within a MAX_ORDER_NR_PAGES, then we
+ * need to check pfn validility within that MAX_ORDER_NR_PAGES block.
+ * pfn_valid_within() should be used in this case; we optimise this away
+ * when we have no holes within a MAX_ORDER_NR_PAGES block.
+ */
+#ifdef CONFIG_HOLES_IN_ZONE
+#define pfn_valid_within(pfn) pfn_valid(pfn)
+#else
+#define pfn_valid_within(pfn) (1)
+#endif
+
 #endif /* !__ASSEMBLY__ */
 #endif /* __KERNEL__ */
 #endif /* _LINUX_MMZONE_H */
diff --git a/mm/page_alloc.c b/mm/page_alloc.c
index 3d7a9e2..695b5a6 100644
--- a/mm/page_alloc.c
+++ b/mm/page_alloc.c
@@ -206,10 +206,8 @@ static int page_outside_zone_boundaries(struct zone *zone, struct page *page)
 
 static int page_is_consistent(struct zone *zone, struct page *page)
 {
-#ifdef CONFIG_HOLES_IN_ZONE
-	if (!pfn_valid(page_to_pfn(page)))
+	if (!pfn_valid_within(page_to_pfn(page)))
 		return 0;
-#endif
 	if (zone != page_zone(page))
 		return 0;
 
@@ -411,10 +409,8 @@ __find_combined_index(unsigned long page_idx, unsigned int order)
 static inline int page_is_buddy(struct page *page, struct page *buddy,
 								int order)
 {
-#ifdef CONFIG_HOLES_IN_ZONE
-	if (!pfn_valid(page_to_pfn(buddy)))
+	if (!pfn_valid_within(page_to_pfn(buddy)))
 		return 0;
-#endif
 
 	if (page_zone_id(page) != page_zone_id(buddy))
 		return 0;

[PATCH 2/3] anti-fragmentation: switch over to pfn_valid_within()

[Andy Whitcroft]

Move to using pfn_valid_within().

Signed-off-by: Andy Whitcroft <andyw@uk.ibm.com>
Acked-by: Mel Gorman <mel@csn.ul.ie>
---
diff --git a/mm/page_alloc.c b/mm/page_alloc.c
index 695b5a6..3d7c29e 100644
--- a/mm/page_alloc.c
+++ b/mm/page_alloc.c
@@ -747,12 +747,10 @@ int move_freepages(struct zone *zone,
 #endif
 
 	for (page = start_page; page < end_page;) {
-#ifdef CONFIG_HOLES_IN_ZONE
-		if (!pfn_valid(page_to_pfn(page))) {
+		if (!pfn_valid_within(page_to_pfn(page))) {
 			page++;
 			continue;
 		}
-#endif
 
 		if (!PageBuddy(page)) {
 			page++;

[PATCH 3/3] lumpy: move to using pfn_valid_within()

[Andy Whitcroft]

Switch to using pfn_valid_within() in lumpy reclaim.

Signed-off-by: Andy Whitcroft <apw@shadowen.org>
Acked-by: Mel Gorman <mel@csn.ul.ie>
---
diff --git a/mm/vmscan.c b/mm/vmscan.c
index c3dc544..cf55c57 100644
--- a/mm/vmscan.c
+++ b/mm/vmscan.c
@@ -722,11 +722,10 @@ static unsigned long isolate_lru_pages(unsigned long nr_to_scan,
 			/* The target page is in the block, ignore it. */
 			if (unlikely(pfn == page_pfn))
 				continue;
-#ifdef CONFIG_HOLES_IN_ZONE
+
 			/* Avoid holes within the zone. */
-			if (unlikely(!pfn_valid(pfn)))
+			if (unlikely(!pfn_valid_within(pfn)))
 				break;
-#endif
 
 			cursor_page = pfn_to_page(pfn);
 			/* Check that we have not crossed a zone boundary. */

Re: [PATCH 0/3] pfn_valid_within() HOLES_WITHIN_ZONES helper

[Bob Picco]

Andy Wihitcroft wrote:	[Wed Mar 21 2007, 02:14:55PM EST]
> The thought of having a helper for the holes within zones code
> has come up on two different threads in the last couple of days.
> So I took the pfn_valid_within() patch I had developed for the
> linear reclaim series and pulled it forward to 2.6.21-rc4-mm1.
> I have split it into a three patch series to better align with the
> affected patch sets within -mm.
> 
> add-pfn_valid_within-helper-for-sub-MAX_ORDER-hole-detection --
>   adds the base helper and utilises it within the buddy allocator,
> 
> anti-fragmentation-switch-over-to-pfn_valid_within() -- changes
>   references within Mel Gormans anti-fragmentation patch series, and
> 
> lumpy-move-to-using-pfn_valid_within() -- changes references with
>   my lumpy reclaim patch series.
> 
> -apw
Andy,

Thanks for doing this.

Acked-by: Bob Picco <bob.picco@hp.com>

Re: [PATCH 1/3] add pfn_valid_within helper for sub-MAX_ORDER hole detection

[Nick Piggin]

Andy Whitcroft wrote:
> Generally we work under the assumption that memory the mem_map
> array is contigious and valid out to MAX_ORDER_NR_PAGES block
> of pages, ie. that if we have validated any page within this
> MAX_ORDER_NR_PAGES block we need not check any other.  This is not
> true when CONFIG_HOLES_IN_ZONE is set and we must check each and
> every reference we make from a pfn.
> 
> Add a pfn_valid_within() helper which should be used when scanning
> pages within a MAX_ORDER_NR_PAGES block when we have already
> checked the validility of the block normally with pfn_valid().
> This can then be optimised away when we do not have holes within
> a MAX_ORDER_NR_PAGES block of pages.

Nice cleanup. Horrible name ;) Calls read like "is the pfn valid
within pfn".

I can't think of anything really good, but I think, say,
pfn_valid_within_block or pfn_valid_within_valid_block would be a
bit better. You still get a slight net savings in keystrokes!

Thanks,
Nick

> 
> Signed-off-by: Andy Whitcroft <apw@shadowen.org>
> Acked-by: Mel Gorman <mel@csn.ul.ie>
> ---
> diff --git a/include/linux/mmzone.h b/include/linux/mmzone.h
> index 7206c77..8c87d79 100644
> --- a/include/linux/mmzone.h
> +++ b/include/linux/mmzone.h
> @@ -837,6 +837,18 @@ void sparse_init(void);
>  void memory_present(int nid, unsigned long start, unsigned long end);
>  unsigned long __init node_memmap_size_bytes(int, unsigned long, unsigned long);
>  
> +/*
> + * If it is possible to have holes within a MAX_ORDER_NR_PAGES, then we
> + * need to check pfn validility within that MAX_ORDER_NR_PAGES block.
> + * pfn_valid_within() should be used in this case; we optimise this away
> + * when we have no holes within a MAX_ORDER_NR_PAGES block.
> + */
> +#ifdef CONFIG_HOLES_IN_ZONE
> +#define pfn_valid_within(pfn) pfn_valid(pfn)
> +#else
> +#define pfn_valid_within(pfn) (1)
> +#endif
> +
>  #endif /* !__ASSEMBLY__ */
>  #endif /* __KERNEL__ */
>  #endif /* _LINUX_MMZONE_H */
> diff --git a/mm/page_alloc.c b/mm/page_alloc.c
> index 3d7a9e2..695b5a6 100644
> --- a/mm/page_alloc.c
> +++ b/mm/page_alloc.c
> @@ -206,10 +206,8 @@ static int page_outside_zone_boundaries(struct zone *zone, struct page *page)
>  
>  static int page_is_consistent(struct zone *zone, struct page *page)
>  {
> -#ifdef CONFIG_HOLES_IN_ZONE
> -	if (!pfn_valid(page_to_pfn(page)))
> +	if (!pfn_valid_within(page_to_pfn(page)))
>  		return 0;
> -#endif
>  	if (zone != page_zone(page))
>  		return 0;
>  
> @@ -411,10 +409,8 @@ __find_combined_index(unsigned long page_idx, unsigned int order)
>  static inline int page_is_buddy(struct page *page, struct page *buddy,
>  								int order)
>  {
> -#ifdef CONFIG_HOLES_IN_ZONE
> -	if (!pfn_valid(page_to_pfn(buddy)))
> +	if (!pfn_valid_within(page_to_pfn(buddy)))
>  		return 0;
> -#endif
>  
>  	if (page_zone_id(page) != page_zone_id(buddy))
>  		return 0;
> -
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/
> 


-- 
SUSE Labs, Novell Inc.
Send instant messages to your online friends http://au.messenger.yahoo.com 

Re: [PATCH 1/3] add pfn_valid_within helper for sub-MAX_ORDER hole detection

[Andrew Morton]

On Thu, 22 Mar 2007 10:23:27 +1100
Nick Piggin <nickpiggin@yahoo.com.au> wrote:

> Andy Whitcroft wrote:
> > Generally we work under the assumption that memory the mem_map
> > array is contigious and valid out to MAX_ORDER_NR_PAGES block
> > of pages, ie. that if we have validated any page within this
> > MAX_ORDER_NR_PAGES block we need not check any other.  This is not
> > true when CONFIG_HOLES_IN_ZONE is set and we must check each and
> > every reference we make from a pfn.
> > 
> > Add a pfn_valid_within() helper which should be used when scanning
> > pages within a MAX_ORDER_NR_PAGES block when we have already
> > checked the validility of the block normally with pfn_valid().
> > This can then be optimised away when we do not have holes within
> > a MAX_ORDER_NR_PAGES block of pages.
> 
> Nice cleanup. Horrible name ;) Calls read like "is the pfn valid
> within pfn".

yeah
 
> I can't think of anything really good, but I think, say,
> pfn_valid_within_block or pfn_valid_within_valid_block would be a
> bit better. You still get a slight net savings in keystrokes!

Neither of those identifiers seem to really fit, and I can't think of anything
suitable either.  Oh well, at least pfn_valid_within() has a nice comment
explaining what it does.