Re: [PATCH] mm/page_alloc: remove prefetchw() on freeing page to buddy system

[Andrew Morton <akpm@linux-foundation.org>]

On Thu, 4 Jul 2024 04:30:50 +0100 Matthew Wilcox <willy@infradead.org> wrote:

> > Something like:
> > 
> > for (;;) {
> > 	...
> > 	if (++loop >= nr_pages)
> > 		break;
> > 	p++;
> > }
> > 
> > 
> > Might generate slightly better code, because we know that we execute the
> > loop body at least once. We use that in set_ptes(), for example.
> 
> I don't think it's worth doing.  Keep the loop simple and obvious.
> set_ptes() is different because we actually expect to execute the loop
> exactly once (ie most folios are small).  So two compares per call to
> set_ptes() instead of one makes a difference.  Here, we're expecting
> to execute this loop, what, a million times?  Doing a million-and-one
> compares instead of a million makes no observable difference.
> 
> I would like to see v2 of this patch dropped, please Andrew.

thud.

Are you supportive of v1?

--- a/mm/page_alloc.c~mm-page_alloc-remove-prefetchw-on-freeing-page-to-buddy-system
+++ a/mm/page_alloc.c
@@ -1236,16 +1236,11 @@ void __free_pages_core(struct page *page
 	 */
 	if (IS_ENABLED(CONFIG_MEMORY_HOTPLUG) &&
 	    unlikely(context == MEMINIT_HOTPLUG)) {
-		prefetchw(p);
-		for (loop = 0; loop < (nr_pages - 1); loop++, p++) {
-			prefetchw(p + 1);
+		for (loop = 0; loop < nr_pages; loop++, p++) {
 			VM_WARN_ON_ONCE(PageReserved(p));
 			__ClearPageOffline(p);
 			set_page_count(p, 0);
 		}
-		VM_WARN_ON_ONCE(PageReserved(p));
-		__ClearPageOffline(p);
-		set_page_count(p, 0);
 
 		/*
 		 * Freeing the page with debug_pagealloc enabled will try to
@@ -1255,14 +1250,10 @@ void __free_pages_core(struct page *page
 		debug_pagealloc_map_pages(page, nr_pages);
 		adjust_managed_page_count(page, nr_pages);
 	} else {
-		prefetchw(p);
-		for (loop = 0; loop < (nr_pages - 1); loop++, p++) {
-			prefetchw(p + 1);
+		for (loop = 0; loop < nr_pages; loop++, p++) {
 			__ClearPageReserved(p);
 			set_page_count(p, 0);
 		}
-		__ClearPageReserved(p);
-		set_page_count(p, 0);
 
 		/* memblock adjusts totalram_pages() manually. */
 		atomic_long_add(nr_pages, &page_zone(page)->managed_pages);
_