On Thu, 4 Jul 2024 04:30:50 +0100 Matthew Wilcox <willy@xxxxxxxxxxxxx> 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);
_
