Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <bc50265a-d657-c23b-9cfd-3928f3324906@redhat.com>
Date:   Thu, 8 Dec 2022 09:41:43 +0100
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101
 Thunderbird/102.5.0
Subject: Re: [PATCH v2] mm/swap: fix SWP_PFN_BITS with
 CONFIG_PHYS_ADDR_T_64BIT on 32bit
To:     Yang Shi <shy828301@gmail.com>
Cc:     linux-kernel@vger.kernel.org, linux-mm@kvack.org,
        Andrew Morton <akpm@linux-foundation.org>,
        Peter Xu <peterx@redhat.com>, Hugh Dickins <hughd@google.com>,
        Andrea Arcangeli <aarcange@redhat.com>
References: <20221206105737.69478-1-david@redhat.com>
 <CAHbLzkra0mjn4utzYPNp+-e77W3tb6yJiJMhkK9Kg=Ra_3rQnQ@mail.gmail.com>
Content-Language: en-US
From:   David Hildenbrand <david@redhat.com>
Organization: Red Hat
In-Reply-To: <CAHbLzkra0mjn4utzYPNp+-e77W3tb6yJiJMhkK9Kg=Ra_3rQnQ@mail.gmail.com>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit
Precedence: bulk

On 07.12.22 23:40, Yang Shi wrote:
> On Tue, Dec 6, 2022 at 2:57 AM David Hildenbrand <david@redhat.com> wrote:
>>
>> We use "unsigned long" to store a PFN in the kernel and phys_addr_t to
>> store a physical address.
>>
>> On a 64bit system, both are 64bit wide. However, on a 32bit system, the
>> latter might be 64bit wide. This is, for example, the case on x86 with
>> PAE: phys_addr_t and PTEs are 64bit wide, while "unsigned long" only
>> spans 32bit.
>>
>> The current definition of SWP_PFN_BITS without MAX_PHYSMEM_BITS misses
>> that case, and assumes that the maximum PFN is limited by an 32bit
>> phys_addr_t. This implies, that SWP_PFN_BITS will currently only be able to
>> cover 4 GiB - 1 on any 32bit system with 4k page size, which is wrong.
> 
> Thanks for debugging this. IIUC this means even swap is actually
> broken on x86_32 + PAE?

I saw all different kinds of issues while testing debugging without this 
patch, but they might just be a fallout from previous page migration/THP 
splitting issues.

I think swap should be fine, because SWP_PFN_BITS only affects 
swp_offset_pfn(): only used when is_pfn_swap_entry()==true.

Thanks!

-- 
Thanks,

David / dhildenb