linux-kernel - Re: [PATCH] mm/hugetlb: remove {Set,Clear}Hpage macros

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: <26e49614-ddb5-4d4c-97c1-a3a8c06d5bb5@oracle.com>
Date: Mon, 20 May 2024 16:44:53 -0700
From: Sidhartha Kumar <sidhartha.kumar@...cle.com>
To: Matthew Wilcox <willy@...radead.org>
Cc: linux-kernel@...r.kernel.org, linux-mm@...ck.org,
        akpm@...ux-foundation.org, vishal.moola@...cle.com,
        muchun.song@...ux.dev, peterx@...hat.com, osalvador@...e.de
Subject: Re: [PATCH] mm/hugetlb: remove {Set,Clear}Hpage macros

On 5/20/24 4:30 PM, Matthew Wilcox wrote:
> On Mon, May 20, 2024 at 03:44:07PM -0700, Sidhartha Kumar wrote:
>> All users have been converted to use the folio version of these macros,
>> we can safely remove the page based interface.
> 
> Yay!
> 
> Reviewed-by: Matthew Wilcox (Oracle) <willy@...radead.org>
> 
> 

There is only one remaining user of page-based Test version of these macros.

in mm/memory-hotplug.c:

		if (!PageHuge(page))
			continue;
		head = compound_head(page);
		/*
		 * This test is racy as we hold no reference or lock.  The
		 * hugetlb page could have been free'ed and head is no longer
		 * a hugetlb page before the following check.  In such unlikely
		 * cases false positives and negatives are possible.  Calling
		 * code must deal with these scenarios.
		 */
		if (HPageMigratable(head))
			goto found;
		skip = compound_nr(head) - (pfn - page_to_pfn(head));

I've previously sent a patch to convert this to folios[1] but got feedback that 
it was unsafe. But I'm not sure why replacing compound_head() with page_folio() 
and using folio_test_hugetlb_migratable(folio) rather than HPageMigratable(head) 
changes the existing behavior. With no reference or lock, can't the head pointer 
also be moved and no longer be a part of page like the comment states. So would 
the folio conversion just be maintaining this level of existing un-safety that 
the calling code should handle anyways?

[1]: 
https://lore.kernel.org/lkml/Y89DK23hYiLtgGNk@casper.infradead.org/T/#mb3a339b98386b1cd0b87f94f45163756ebd7feaa