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: <1df06854-f5e6-493f-98e6-6426bd37da3c@redhat.com>
Date: Mon, 30 Jun 2025 16:38:24 +0200
From: David Hildenbrand <david@...hat.com>
To: Anshuman Khandual <anshuman.khandual@....com>,
 Gerald Schaefer <gerald.schaefer@...ux.ibm.com>
Cc: Andrew Morton <akpm@...ux-foundation.org>,
 Matthew Wilcox <willy@...radead.org>, LKML <linux-kernel@...r.kernel.org>,
 linux-mm <linux-mm@...ck.org>, linux-s390@...r.kernel.org
Subject: Re: [RFC PATCH 1/1] mm/debug_vm_pgtable: Use a swp_entry_t input
 value for swap tests

On 30.06.25 06:18, Anshuman Khandual wrote:
> 
> 
> On 25/06/25 10:17 PM, David Hildenbrand wrote:
>>> [...]
>>>>> @@ -1166,6 +1173,7 @@ static void __init init_fixed_pfns(struct pgtable_debug_args *args)
>>>>>      static int __init init_args(struct pgtable_debug_args *args)
>>>>>    {
>>>>> +    unsigned long max_swap_offset;
>>>>>        struct page *page = NULL;
>>>>>        int ret = 0;
>>>>>    @@ -1248,6 +1256,11 @@ static int __init init_args(struct pgtable_debug_args *args)
>>>>>          init_fixed_pfns(args);
>>>>>    +    /* See generic_max_swapfile_size(): probe the maximum offset */
>>>>> +    max_swap_offset = swp_offset(pte_to_swp_entry(swp_entry_to_pte(swp_entry(0, ~0UL))));
>>>> Why not directly use generic_max_swapfile_size() which is doing exact same thing.
>>>>
>>>> unsigned long generic_max_swapfile_size(void)
>>>> {
>>>>      return swp_offset(pte_to_swp_entry(
>>>>              swp_entry_to_pte(swp_entry(0, ~0UL)))) + 1;
>>>> }
>>>
>>> Good question. I just moved this code here from pte_swap_exclusive_tests(),
>>> see above, and did not think about that. Now I also wonder why
>>> generic_max_swapfile_size() wasn't used before.
>>>
>>> But it is not exactly the same thing, there is an extra "+ 1" there.
>>> Maybe that is the reason, but I don't really understand the details /
>>> difference, and therefore would not want to change it.
>>>
>>> David, do you remember why you didn't use generic_max_swapfile_size()
>>> in your pte_swap_exclusive_tests()?
>>
>> Excellent question. If only I would remember :)
>>
>> generic_max_swapfile_size() resides in mm/swapfile.c, which is only around with CONFIG_SWAP.
>>
>> It makes sense to have that function only if there are ... actual swapfiles.
>>
>> These checks here are independent of CONFIG_SWAP (at least in theory -- for migration entries etc we don't need CONFIG_SWAP), and we simply want to construct a swap PTE with all possible bits set.
> 
> After this modification of PMD based swap test - there will be now
> two uses for generic_max_swapfile_size(). Rather than refactoring
> these into a similar helper in mm/debug_vm_pgtable.c - should the
> existing helper just be moved outside of CONFIG_SWAP, thus making
> it available in general ?

As I said, having generic_max_swapfile_size() that returns something != 
0 with !CONFIG_SWAP feels rather odd.

No swap -> no swapfile -> no swapfile max size.

No strong opinion though,, but desperately trying to de-duplicate 2 LOC 
might not be worth the churn at all.

-- 
Cheers,

David / dhildenb


Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ