linux-kernel - Re: [PATCH 2/3] mm/vmstat: get fragmentation statistics from per-migragetype count

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20251128120328.8693-1-zhongjinji@honor.com>
Date: Fri, 28 Nov 2025 20:03:28 +0800
From: zhongjinji <zhongjinji@...or.com>
To: <zhanghongru06@...il.com>
CC: <Liam.Howlett@...cle.com>, <akpm@...ux-foundation.org>,
	<axelrasmussen@...gle.com>, <david@...nel.org>, <hannes@...xchg.org>,
	<jackmanb@...gle.com>, <linux-kernel@...r.kernel.org>, <linux-mm@...ck.org>,
	<lorenzo.stoakes@...cle.com>, <mhocko@...e.com>, <rppt@...nel.org>,
	<surenb@...gle.com>, <vbabka@...e.cz>, <weixugc@...gle.com>,
	<yuanchu@...gle.com>, <zhanghongru@...omi.com>, <ziy@...dia.com>
Subject: Re: [PATCH 2/3] mm/vmstat: get fragmentation statistics from per-migragetype count

Hi, Hongru

> diff --git a/mm/page_alloc.c b/mm/page_alloc.c
> index 9431073e7255..a90f2bf735f6 100644
> --- a/mm/page_alloc.c
> +++ b/mm/page_alloc.c
> @@ -818,7 +818,8 @@ static inline void __add_to_free_list(struct page *page, struct zone *zone,
>  	else
>  		list_add(&page->buddy_list, &area->free_list[migratetype]);
>  	area->nr_free++;
> -	area->mt_nr_free[migratetype]++;
> +	WRITE_ONCE(area->mt_nr_free[migratetype],
> +		area->mt_nr_free[migratetype] + 1);
>  
>  	if (order >= pageblock_order && !is_migrate_isolate(migratetype))
>  		__mod_zone_page_state(zone, NR_FREE_PAGES_BLOCKS, nr_pages);
> @@ -841,8 +842,8 @@ static inline void move_to_free_list(struct page *page, struct zone *zone,
>  		     get_pageblock_migratetype(page), old_mt, nr_pages);
>  
>  	list_move_tail(&page->buddy_list, &area->free_list[new_mt]);
> -	area->mt_nr_free[old_mt]--;
> -	area->mt_nr_free[new_mt]++;
> +	WRITE_ONCE(area->mt_nr_free[old_mt], area->mt_nr_free[old_mt] - 1);
> +	WRITE_ONCE(area->mt_nr_free[new_mt], area->mt_nr_free[new_mt] + 1);
>  
>  	account_freepages(zone, -nr_pages, old_mt);
>  	account_freepages(zone, nr_pages, new_mt);
> @@ -873,7 +874,8 @@ static inline void __del_page_from_free_list(struct page *page, struct zone *zon
>  	__ClearPageBuddy(page);
>  	set_page_private(page, 0);
>  	area->nr_free--;
> -	area->mt_nr_free[migratetype]--;
> +	WRITE_ONCE(area->mt_nr_free[migratetype],
> +		area->mt_nr_free[migratetype] - 1);

It doesn't seem like a good idea to use WRITE_ONCE on the hot path.

>  
>  	if (order >= pageblock_order && !is_migrate_isolate(migratetype))
>  		__mod_zone_page_state(zone, NR_FREE_PAGES_BLOCKS, -nr_pages);
> diff --git a/mm/vmstat.c b/mm/vmstat.c
> index bb09c032eecf..9334bbbe1e16 100644
> --- a/mm/vmstat.c
> +++ b/mm/vmstat.c
> @@ -1590,32 +1590,16 @@ static void pagetypeinfo_showfree_print(struct seq_file *m,
>  					zone->name,
>  					migratetype_names[mtype]);
>  		for (order = 0; order < NR_PAGE_ORDERS; ++order) {
> -			unsigned long freecount = 0;
> -			struct free_area *area;
> -			struct list_head *curr;
> +			unsigned long freecount;
>  			bool overflow = false;
>  
> -			area = &(zone->free_area[order]);
> -
> -			list_for_each(curr, &area->free_list[mtype]) {
> -				/*
> -				 * Cap the free_list iteration because it might
> -				 * be really large and we are under a spinlock
> -				 * so a long time spent here could trigger a
> -				 * hard lockup detector. Anyway this is a
> -				 * debugging tool so knowing there is a handful
> -				 * of pages of this order should be more than
> -				 * sufficient.
> -				 */
> -				if (++freecount >= 100000) {
> -					overflow = true;
> -					break;
> -				}
> +			/* Keep the same output format for user-space tools compatibility */
> +			freecount = READ_ONCE(zone->free_area[order].mt_nr_free[mtype]);

I think it might be better for using an array of size NR_PAGE_ORDERS to store
the free count for each order. Like the code below.

unsigned long freecount[NR_PAGE_ORDERS]

spin_lock_irq(&zone->lock)
	for_each_order
		freecount[order] = zone->free_area[order].mt_nr_free[mtype]
spin_unlock_irq(&zone->lock)

for_each_order
	print freecount[order]

> +			if (freecount >= 100000) {
> +				overflow = true;
> +				freecount = 100000;
>  			}
>  			seq_printf(m, "%s%6lu ", overflow ? ">" : "", freecount);
> -			spin_unlock_irq(&zone->lock);
> -			cond_resched();
> -			spin_lock_irq(&zone->lock);
>  		}
>  		seq_putc(m, '\n');
>  	}
> @@ -1633,7 +1617,7 @@ static void pagetypeinfo_showfree(struct seq_file *m, void *arg)
>  		seq_printf(m, "%6d ", order);
>  	seq_putc(m, '\n');
>  
> -	walk_zones_in_node(m, pgdat, true, false, pagetypeinfo_showfree_print);
> +	walk_zones_in_node(m, pgdat, true, true, pagetypeinfo_showfree_print);
>  }