mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: David Hildenbrand <david@redhat.com>
To: Jianyong Wu <Jianyong.Wu@arm.com>,
	"akpm@linux-foundation.org" <akpm@linux-foundation.org>,
	"mhiramat@kernel.org" <mhiramat@kernel.org>,
	"peterz@infradead.org" <peterz@infradead.org>
Cc: "rostedt@goodmis.org" <rostedt@goodmis.org>,
	"vbabka@suse.cz" <vbabka@suse.cz>,
	"linux-kernel@vger.kernel.org" <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
Date: Wed, 20 Oct 2021 11:05:11 +0200	[thread overview]
Message-ID: <cbe8d847-8a2d-4220-c6a3-775d517e2edd@redhat.com> (raw)
In-Reply-To: <AM9PR08MB7276FEF0AE524A9728F63A4CF4BE9@AM9PR08MB7276.eurprd08.prod.outlook.com>

>> 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).

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


  reply	other threads:[~2021-10-20  9:05 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-10-20  2:07 Jianyong Wu
2021-10-20  7:05 ` David Hildenbrand
2021-10-20  8:17   ` Jianyong Wu
2021-10-20  9:05     ` David Hildenbrand [this message]
2021-10-20 11:54       ` Jianyong Wu

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=cbe8d847-8a2d-4220-c6a3-775d517e2edd@redhat.com \
    --to=david@redhat.com \
    --cc=Anshuman.Khandual@arm.com \
    --cc=Jianyong.Wu@arm.com \
    --cc=Justin.He@arm.com \
    --cc=akpm@linux-foundation.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mhiramat@kernel.org \
    --cc=nd@arm.com \
    --cc=peterz@infradead.org \
    --cc=rostedt@goodmis.org \
    --cc=vbabka@suse.cz \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®