| 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
| ||
|
Message-ID: <d58f8ab8-55da-447a-9fd6-93bebe27f23e@redhat.com>
Date: Wed, 2 Jul 2025 11:08:49 +0200
From: David Hildenbrand <david@...hat.com>
To: Zi Yan <ziy@...dia.com>
Cc: linux-kernel@...r.kernel.org, linux-mm@...ck.org,
Andrew Morton <akpm@...ux-foundation.org>,
"Liam R. Howlett" <Liam.Howlett@...cle.com>,
Lorenzo Stoakes <lorenzo.stoakes@...cle.com>,
Vlastimil Babka <vbabka@...e.cz>, Jann Horn <jannh@...gle.com>,
Mike Rapoport <rppt@...nel.org>, Suren Baghdasaryan <surenb@...gle.com>,
Michal Hocko <mhocko@...e.com>, Matthew Brost <matthew.brost@...el.com>,
Joshua Hahn <joshua.hahnjy@...il.com>, Rakie Kim <rakie.kim@...com>,
Byungchul Park <byungchul@...com>, Gregory Price <gourry@...rry.net>,
Ying Huang <ying.huang@...ux.alibaba.com>,
Alistair Popple <apopple@...dia.com>, Pedro Falcato <pfalcato@...e.de>,
Rik van Riel <riel@...riel.com>, Harry Yoo <harry.yoo@...cle.com>
Subject: Re: [PATCH v1 4/4] mm: remove boolean output parameters from
folio_pte_batch_ext()
On 30.06.25 19:59, Zi Yan wrote:
> On 27 Jun 2025, at 7:55, David Hildenbrand wrote:
>
>> Instead, let's just allow for specifying through flags whether we want
>> to have bits merged into the original PTE.
>>
>> For the madvise() case, simplify by having only a single parameter for
>> merging young+dirty. For madvise_cold_or_pageout_pte_range() merging the
>> dirty bit is not required, but also not harmful. This code is not that
>> performance critical after all to really force all micro-optimizations.
>>
>> As we now have two pte_t * parameters, use PageTable() to make sure we
>> are actually given a pointer at a copy of the PTE, not a pointer into
>> an actual page table.
>>
>> Signed-off-by: David Hildenbrand <david@...hat.com>
>> ---
>> mm/internal.h | 58 +++++++++++++++++++++++++++++++--------------------
>> mm/madvise.c | 26 +++++------------------
>> mm/memory.c | 8 ++-----
>> mm/util.c | 2 +-
>> 4 files changed, 43 insertions(+), 51 deletions(-)
>>
>> diff --git a/mm/internal.h b/mm/internal.h
>> index 6000b683f68ee..fe69e21b34a24 100644
>> --- a/mm/internal.h
>> +++ b/mm/internal.h
>> @@ -208,6 +208,18 @@ typedef int __bitwise fpb_t;
>> /* Compare PTEs honoring the soft-dirty bit. */
>> #define FPB_HONOR_SOFT_DIRTY ((__force fpb_t)BIT(1))
>>
>> +/*
>> + * Merge PTE write bits: if any PTE in the batch is writable, modify the
>> + * PTE at @ptentp to be writable.
>> + */
>> +#define FPB_MERGE_WRITE ((__force fpb_t)BIT(2))
>> +
>> +/*
>> + * Merge PTE young and dirty bits: if any PTE in the batch is young or dirty,
>> + * modify the PTE at @ptentp to be young or dirty, respectively.
>> + */
>> +#define FPB_MERGE_YOUNG_DIRTY ((__force fpb_t)BIT(3))
>> +
>> static inline pte_t __pte_batch_clear_ignored(pte_t pte, fpb_t flags)
>> {
>> if (!(flags & FPB_HONOR_DIRTY))
>> @@ -220,16 +232,11 @@ static inline pte_t __pte_batch_clear_ignored(pte_t pte, fpb_t flags)
>> /**
>> * folio_pte_batch_ext - detect a PTE batch for a large folio
>> * @folio: The large folio to detect a PTE batch for.
>> + * @vma: The VMA. Only relevant with FPB_MERGE_WRITE, otherwise can be NULL.
>> * @ptep: Page table pointer for the first entry.
>> - * @pte: Page table entry for the first page.
>> + * @ptentp: Pointer at a copy of the first page table entry.
>> * @max_nr: The maximum number of table entries to consider.
>> * @flags: Flags to modify the PTE batch semantics.
>> - * @any_writable: Optional pointer to indicate whether any entry except the
>> - * first one is writable.
>> - * @any_young: Optional pointer to indicate whether any entry except the
>> - * first one is young.
>> - * @any_dirty: Optional pointer to indicate whether any entry except the
>> - * first one is dirty.
>> *
>> * Detect a PTE batch: consecutive (present) PTEs that map consecutive
>> * pages of the same large folio in a single VMA and a single page table.
>> @@ -242,28 +249,26 @@ static inline pte_t __pte_batch_clear_ignored(pte_t pte, fpb_t flags)
>> * must be limited by the caller so scanning cannot exceed a single VMA and
>> * a single page table.
>> *
>> + * Depending on the FPB_MERGE_* flags, the pte stored at @ptentp will
>> + * be modified.
>> + *
>> * This function will be inlined to optimize based on the input parameters;
>> * consider using folio_pte_batch() instead if applicable.
>> *
>> * Return: the number of table entries in the batch.
>> */
>> static inline unsigned int folio_pte_batch_ext(struct folio *folio,
>> - pte_t *ptep, pte_t pte, unsigned int max_nr, fpb_t flags,
>> - bool *any_writable, bool *any_young, bool *any_dirty)
>> + struct vm_area_struct *vma, pte_t *ptep, pte_t *ptentp,
>> + unsigned int max_nr, fpb_t flags)
>> {
>> + bool any_writable = false, any_young = false, any_dirty = false;
>> + pte_t expected_pte, pte = *ptentp;
>> unsigned int nr, cur_nr;
>> - pte_t expected_pte;
>> -
>> - if (any_writable)
>> - *any_writable = false;
>> - if (any_young)
>> - *any_young = false;
>> - if (any_dirty)
>> - *any_dirty = false;
>>
>> VM_WARN_ON_FOLIO(!pte_present(pte), folio);
>> VM_WARN_ON_FOLIO(!folio_test_large(folio) || max_nr < 1, folio);
>> VM_WARN_ON_FOLIO(page_folio(pfn_to_page(pte_pfn(pte))) != folio, folio);
>> + VM_WARN_ON(virt_addr_valid(ptentp) && PageTable(virt_to_page(ptentp)));
>
> Why not just VM_WARN_ON(!pte_same(*ptep, *ptentp)) ?
>
> ptep points to the first page table entry and ptentp points to the copy of it.
> I assume ptep should point to a valid page table entry to begin with.
That would also work, only miss the case where someone would by accident
flip both pointers :)
--
Cheers,
David / dhildenb
Powered by blists - more mailing lists