On 1/28/21 9:36 AM, Rolf Eike Beer wrote:
> Am Mittwoch, 27. Januar 2021, 22:18:51 CET schrieb Helge Deller:
>> On parisc a spinlock is stored in the next page behind the pgd which
>> protects against parallel accesses to the pgd. That's why one additional
>> page (PGD_ALLOC_ORDER) is allocated for the pgd.
>>
>> Matthew Wilcox suggested that we instead should use a pointer in the
>> struct page table for this spinlock and noted, that the comments for the
>> PGD_ORDER and PMD_ORDER defines were wrong.
>>
>> Both suggestions are addressed in this patch. The pgd spinlock
>> (parisc_pgd_lock) is stored in the struct page table. In
>> switch_mm_irqs_off() the physical address of this lock is loaded into
>> cr28 (tr4) and the pgd into cr25, so that the fault handlers can
>> directly access the lock.
>>
>> The currently implemened Hybrid L2/L3 page table scheme (where the pmd
>> is adjacent to the pgd) is dropped now too.
>>
>> Suggested-by: Matthew Wilcox <willy@xxxxxxxxxxxxx>
>> Fixes: b37d1c1898b2 ("parisc: Use per-pagetable spinlock")
>> Signed-off-by: Helge Deller <deller@xxxxxx>
>> Signed-off-by: John David Anglin <dave.anglin@xxxxxxxx>
>>
[...lots of suggestions by Rolf...]
Rolf, thanks a lot for your review.
I've addressed most of your suggestions and published a v12 patch here:
https://git.kernel.org/pub/scm/linux/kernel/git/deller/parisc-linux.git/log/?h=pgtable_spinlock-v12
>> diff --git a/arch/parisc/mm/init.c b/arch/parisc/mm/init.c
>> index 3ec633b11b54..4f3f180b0b20 100644
>> --- a/arch/parisc/mm/init.c
>> +++ b/arch/parisc/mm/init.c
>> +#ifdef CONFIG_SMP
>> +spinlock_t *pgd_spinlock(pgd_t *pgd)
>> +{
>> + struct page *page;
>> +
>> + page = virt_to_page((unsigned long) pgd);
>> + return page->parisc_pgd_lock;
>> +}
>> +#endif
>
> This is very simple, and I suspect it being called rather often. Wouldn't it
> make sense to make it inline?
No, it's not simple, that's why I haven't inlined it.
The virt_to_page() expands to many asm instructions based on the selected memory model.
Thanks!
Helge
Attachment:
signature.asc
Description: OpenPGP digital signature
