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: <c7e3152b-4fdd-cb93-a4f0-06502eab59b1@redhat.com>
Date:   Mon, 22 May 2023 10:21:13 +0200
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101
 Thunderbird/102.10.0
To:     =?UTF-8?B?QmrDtnJuIFTDtnBlbA==?= <bjorn@kernel.org>,
        Paul Walmsley <paul.walmsley@sifive.com>,
        Palmer Dabbelt <palmer@dabbelt.com>,
        Albert Ou <aou@eecs.berkeley.edu>,
        linux-riscv@lists.infradead.org,
        Anshuman Khandual <anshuman.khandual@arm.com>
Cc:     =?UTF-8?B?QmrDtnJuIFTDtnBlbA==?= <bjorn@rivosinc.com>,
        linux-kernel@vger.kernel.org, linux-mm@kvack.org,
        Oscar Salvador <osalvador@suse.de>,
        virtualization@lists.linux-foundation.org, linux@rivosinc.com,
        Alexandre Ghiti <alexghiti@rivosinc.com>
References: <20230512145737.985671-1-bjorn@kernel.org>
 <9aa7d030-19b5-01df-70c0-86d8d6ab86a6@redhat.com>
 <87zg62eqm4.fsf@all.your.base.are.belong.to.us>
 <87lehikpu6.fsf@all.your.base.are.belong.to.us>
Content-Language: en-US
From:   David Hildenbrand <david@redhat.com>
Organization: Red Hat
Subject: Re: [PATCH 0/7] riscv: Memory Hot(Un)Plug support
In-Reply-To: <87lehikpu6.fsf@all.your.base.are.belong.to.us>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 8bit
Precedence: bulk

On 21.05.23 11:15, Björn Töpel wrote:
> Hi David and Anshuman!
> 
> Björn Töpel <bjorn@kernel.org> writes:
> 
>> David Hildenbrand <david@redhat.com> writes:
>>
>>> On 12.05.23 16:57, Björn Töpel wrote:
>>>> From: Björn Töpel <bjorn@rivosinc.com>
>>>>
>>>> Memory Hot(Un)Plug support for the RISC-V port
>>>> ==============================================
>>
>> [...]
>>
>>>
>>> Cool stuff! I'm fairly busy right now, so some high-level questions upfront:
>>
>> No worries, and no rush! I'd say the v1 series was mainly for the RISC-V
>> folks, and I've got tons of (offline) comments from Alex -- and with
>> your comments below some more details to figure out.
> 
> One of the major issues with my v1 patch is around init_mm page table
> synchronization, and that'll be part of the v2.
> 
> I've noticed there's a quite a difference between x86-64 and arm64 in
> terms of locking, when updating (add/remove) the init_mm table. x86-64
> uses the usual page table locking mechanisms (used by the generic
> kernel functions), whereas arm64 does not.
> 
> How does arm64 manage to mix the "lock-less" updates (READ/WRITE_ONCE,
> and fences in set_p?d+friends), with the generic kernel ones that uses
> the regular page locking mechanism?
> 
> I'm obviously missing something about the locking rules for memory hot
> add/remove... I've been reading the arm64 memory hot add/remove
> series, but none the wiser! ;-)

In general, memory hot(un)plug is serialized on a high level using the 
mem_hotplug_lock. For example, in pagemap_range() or in 
add_memory_resource(), we grab that lock in write mode. So we'll never 
see memory getting added/removed concurrently from the direct map.

 From what I recall, the locking on the arch level is required for 
concurrent (direct mapping) page table modifications that target virtual 
address ranges adjacent to the ranges we hot(un)plug:
CONFIG_ARCH_HAS_SET_DIRECT_MAP and vmalloc come to mind.

For example, if a range would be mapped using a large PUD, but we have 
to unplug it partially (unplugging memory part of bootmem), we'd have to 
replace the large PUD by a PMD table first. That change (that could 
affect other concurrent page table walkers/operations) has to be 
synchronized.

I guess to which degree this applies to riscv depends on the virtual 
memory layout, direct mapping granularity and features (e.g., 
CONFIG_ARCH_HAS_SET_DIRECT_MAP).


One trick that arm64 implements is, that it only allows hotunplugging 
memory that was hotplugged (see prevent_bootmem_remove_notifier()). That 
might just rule out such problematic cases that require locking 
completely, and the high-level mem_hotplug_lock sufficient.

-- 
Thanks,

David / dhildenb