[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <c3af0b48-f303-456b-bca3-537a61255ec5@redhat.com>
Date: Tue, 30 Jul 2024 17:45:24 +0200
From: David Hildenbrand <david@...hat.com>
To: Marek Szyprowski <m.szyprowski@...sung.com>, linux-kernel@...r.kernel.org
Cc: linux-mm@...ck.org, Andrew Morton <akpm@...ux-foundation.org>,
Muchun Song <muchun.song@...ux.dev>, Peter Xu <peterx@...hat.com>,
Oscar Salvador <osalvador@...e.de>
Subject: Re: [PATCH v1 1/2] mm: let pte_lockptr() consume a pte_t pointer
On 30.07.24 17:30, Marek Szyprowski wrote:
> On 25.07.2024 20:39, David Hildenbrand wrote:
>> pte_lockptr() is the only *_lockptr() function that doesn't consume
>> what would be expected: it consumes a pmd_t pointer instead of a pte_t
>> pointer.
>>
>> Let's change that. The two callers in pgtable-generic.c are easily
>> adjusted. Adjust khugepaged.c:retract_page_tables() to simply do a
>> pte_offset_map_nolock() to obtain the lock, even though we won't actually
>> be traversing the page table.
>>
>> This makes the code more similar to the other variants and avoids other
>> hacks to make the new pte_lockptr() version happy. pte_lockptr() users
>> reside now only in pgtable-generic.c.
>>
>> Maybe, using pte_offset_map_nolock() is the right thing to do because
>> the PTE table could have been removed in the meantime? At least it sounds
>> more future proof if we ever have other means of page table reclaim.
>>
>> It's not quite clear if holding the PTE table lock is really required:
>> what if someone else obtains the lock just after we unlock it? But we'll
>> leave that as is for now, maybe there are good reasons.
>>
>> This is a preparation for adapting hugetlb page table locking logic to
>> take the same locks as core-mm page table walkers would.
>>
>> Signed-off-by: David Hildenbrand <david@...hat.com>
>
> This patch landed in today's linux-next as commit e98970a1d2d4 ("mm: let
> pte_lockptr() consume a pte_t pointer"). Unfortunately it causes the
> following issue on most of my ARM 32bit based test boards:
>
That is ... rather surprising.
The issue below seems to point at __pte_offset_map_lock(), where we
essentially convert from
ptlock_ptr(page_ptdesc(pmd_page(*pmd)));
to
ptlock_ptr(virt_to_ptdesc(pte));
So we would get a NULL ptr from the ptdesc. Either the lock would
actually not be allocated or virt_to_ptdesc() does something unexpected.
Leaf page tables on arm are also a single page, so we cannot possibly be
staring at the wrong page-table-subpage.
Do we maybe have to special-case init-mm? But I don't see how that
special-casing would have happened for now :/
What's your kernel config value of SPLIT_PTLOCK_CPUS?
--
Cheers,
David / dhildenb
Powered by blists - more mailing lists