linux-kernel - Re: [PATCH] mm: move mask update out of the atomic context

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] [day] [month] [year] [list]

Message-ID: <d4b70a87-336a-4bf2-bed9-39e00371d0e1@redhat.com>
Date: Tue, 24 Jun 2025 14:03:23 +0200
From: David Hildenbrand <david@...hat.com>
To: Alexander Gordeev <agordeev@...ux.ibm.com>
Cc: Dev Jain <dev.jain@....com>, Andrew Morton <akpm@...ux-foundation.org>,
 linux-mm@...ck.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] mm: move mask update out of the atomic context

On 24.06.25 14:00, Alexander Gordeev wrote:
> On Tue, Jun 24, 2025 at 11:40:05AM +0200, David Hildenbrand wrote:
>> On 24.06.25 11:37, Alexander Gordeev wrote:
>>> On Mon, Jun 23, 2025 at 09:45:34PM +0200, David Hildenbrand wrote:
>>> ...
>>>> Let's ask the real questions: who checks PGTBL_PTE_MODIFIED?
>>>>
>>>> I see
>>>>
>>>> if (mask & ARCH_PAGE_TABLE_SYNC_MASK)
>>>> 	arch_sync_kernel_mappings(start, start + size);
>>>>
>>>> And then
>>>>
>>>> arch/arm/include/asm/page.h:#define ARCH_PAGE_TABLE_SYNC_MASK   PGTBL_PMD_MODIFIED
>>>> arch/x86/include/asm/pgtable-2level_types.h:#define ARCH_PAGE_TABLE_SYNC_MASK   PGTBL_PMD_MODIFIED
>>>> arch/x86/include/asm/pgtable-3level_types.h:#define ARCH_PAGE_TABLE_SYNC_MASK   PGTBL_PMD_MODIFIED
>>>>
>>>>
>>>> Which makes me wonder why we need PGTBL_PTE_MODIFIED at all? Is there some other check I am missing?
>>>>
>>>> (same question regarding everything excepy PGTBL_PMD_MODIFIED, because that actually seems to be used)
>>>
>>> AFAICT it was thought as architecture-specific:
>>>
>>> /*
>>>    * Architectures can set this mask to a combination of PGTBL_P?D_MODIFIED values
>>>    * and let generic vmalloc and ioremap code know when arch_sync_kernel_mappings()
>>>    * needs to be called.
>>>    */
>>> #ifndef ARCH_PAGE_TABLE_SYNC_MASK
>>> #define ARCH_PAGE_TABLE_SYNC_MASK 0
>>> #endif
>>>
>>> Not sure if that needs to be addressed at all.
>>
>> Okay, if there are no users of PGTBL_PTE_MODIFIED we could just ... remove
>> it. Dead code.
> 
> As you noticed, PGTBL_PMD_MODIFIED bit is used only. Thus, all other
> bits would have to be removed as well, not just PGTBL_PTE_MODIFIED?

Likely, yes.

> 
> That is more or less revert of at least the below commits and rewriting
> it in a PMD-focused manner:
> 
> 2ba3e6947aed ("mm/vmalloc: track which page-table levels were modified")
> d8626138009b ("mm: add functions to track page directory modifications")
> 
> That would be a completely different effort, which I am not aming at ;)

Well, I don't think we should be taking this micro-optimization patch 
here TBH. The real optimization is getting rid of dead code, not 
optimizing dead code.

-- 
Cheers,

David / dhildenb