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: <20250508204644.GB323143@cmpxchg.org>
Date: Thu, 8 May 2025 16:46:44 -0400
From: Johannes Weiner <hannes@...xchg.org>
To: Zi Yan <ziy@...dia.com>
Cc: David Hildenbrand <david@...hat.com>, linux-mm@...ck.org,
	Andrew Morton <akpm@...ux-foundation.org>,
	Oscar Salvador <osalvador@...e.de>,
	Vlastimil Babka <vbabka@...e.cz>,
	Baolin Wang <baolin.wang@...ux.alibaba.com>,
	"Kirill A . Shutemov" <kirill.shutemov@...ux.intel.com>,
	Mel Gorman <mgorman@...hsingularity.net>,
	Suren Baghdasaryan <surenb@...gle.com>,
	Michal Hocko <mhocko@...e.com>,
	Brendan Jackman <jackmanb@...gle.com>,
	Richard Chang <richardycc@...gle.com>, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v3 1/4] mm/page_isolation: make page isolation a
 standalone bit.

On Thu, May 08, 2025 at 03:17:05PM -0400, Zi Yan wrote:
> 
> >>> @@ -426,7 +460,12 @@ void set_pageblock_migratetype(struct page *page, int migratetype)
> >>>  		     migratetype < MIGRATE_PCPTYPES))
> >>>  		migratetype = MIGRATE_UNMOVABLE;
> >>>
> >>> -	set_pfnblock_flags_mask(page, (unsigned long)migratetype,
> >>> +#ifdef CONFIG_MEMORY_ISOLATION
> >>> +	if (migratetype == MIGRATE_ISOLATE)
> >>> +		set_pageblock_isolate(page);
> >>
> >> Are there paths actually doing this after the second patch?
> >>
> >> There are many instances that want to *read* the migratetype or
> >> MIGRATE_ISOLATE, but only isolation code should be manipulating that
> >> bit through the dedicated set/toggle_pageblock_isolate API.
> >>
> >> If there isn't one, it might be good to enforce this with a VM_WARN
> >> instead.
> >
> > I checked all set_pageblock_migratetype() callers and do not see
> > one using it for pageblock isolation. Let me replace the code
> > with a VM_WARN and add a comment to tell users to use dedicated
> > pageblock isolation APIs.
> >
> 
> Actually, move_freepages_block_isolate() calls __move_freepages_block()
> to move free pages to MIGRATE_ISOLATE pageblock and
> set_pageblock_migratetype() is used inside __move_freepages_block().
> So the branch has to stay. Will use the suggestion below.

Ah, good catch. But looking at the callers, it's:

move_freepages_block()
move_freepages_block_isolate()
try_to_claim_block()

The last one would benefit from having the set_pageblock_migratetype()
there explicitly, as this is what this function is supposed to do. It
also should never set the isolation bit.

move_freepages_block_isolate() has two set_pageblock_migratetype()
calls already. And after the series, it should only manipulate the
isolate bit, not change the actual migratetype anymore, right?

Maybe it makes the most sense to move it into the three callers?

And then fortify set_pageblock_migratetype() after all.

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ