lkml.org 
[lkml]   [2021]   [Oct]   [20]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
From
SubjectRE: [PATCH v1] init: avoid race condition of update page table in kernel init
Date


> -----Original Message-----
> From: David Hildenbrand <david@redhat.com>
> Sent: Wednesday, October 20, 2021 5:05 PM
> To: Jianyong Wu <Jianyong.Wu@arm.com>; akpm@linux-foundation.org;
> mhiramat@kernel.org; peterz@infradead.org
> Cc: rostedt@goodmis.org; vbabka@suse.cz; linux-kernel@vger.kernel.org;
> Anshuman Khandual <Anshuman.Khandual@arm.com>; Justin He
> <Justin.He@arm.com>; nd <nd@arm.com>
> Subject: Re: [PATCH v1] init: avoid race condition of update page table in
> kernel init
>
> >> But why does it matter on arm64? Can you describe how the exact race
> >> triggers?
> >
> > I don't know much about how x86 does in memory map. Let me show you
> how the race happens on arm64.
> > When virtio-mem workqueue is triggered, arch_memory_add will be called
> where the related page table will be created. The call chain is
> arch_memory_add->__create_pgd_mapping->alloc_init_pud. As the memory
> add may take for serval seconds, it may be concurrent with mark_rodata_ro,
> in which page tables are created either.
>
> > The race can occur in alloc_init_pud. See below:
> >
> /***************************************************************
> ************/
> > Virtio-mem workqueue thread
> mark_rodata_ro thread
> > {
> > ...
> > pudp = pud_set_fixmap_offset(p4dp, addr); // set fixmap
> > do {
> > pud_t old_pud = READ_ONCE(*pudp);
> > ...
> > } while (pudp++, addr = next, addr != end); pudp =
> pud_set_fixmap_offset; //set fixmap
> > pud_clear_fixmap(); // clear fixmap do {
> > }
> pud_t old_pud = READ_ONCE(*pudp);//CRASH
> >
>
> I still don't quite understand how that race can even exist. I assume it's due
> to the weird semantics of the "fixmap". (whatever that is :) ) I don't see
> anything similar happen on other archs, especially x86-64 and s390x, which
> I'm familiar with.
>
> s390x similarly to x86-64 code uses a vmem_mutex to serialize add/remove
> in the direct map and a cpa_mutex to serialize attribute changes (and
> splitting of large mappings).
>

Yeah, I see that there is a spin lock when update page table in x86.
OK, let me poke Anshuman about this.
If this issue won't affect all arches, should we only fix it on arm64? @Anshuman Khandual

Thanks
Jianyong


> The right should be to teach arm64 mmu code that direct mapping updates
> might be concurrent, and that two instances might try messing with the
> fixmap concurrently.
>
>
> On a similar topic: I think we might want to reclaim compeltely empty page
> tables when unplugging memory; I suspect that we also have to mess with
> the fixmap then, whem removing page tables. But I feel like the whole fixmap
> machinery is still a big black box for me.
>

> --
> Thanks,
>
> David / dhildenb

\
 
 \ /
  Last update: 2021-10-20 13:56    [W:0.049 / U:0.056 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site