public inbox for linuxppc-dev@ozlabs.org 
 help / color / mirror / Atom feed
From: Michael Ellerman <mpe@ellerman•id.au>
To: bharata@linux•ibm.com
Cc: Nathan Lynch <nathanl@linux•ibm.com>,
	"Aneesh Kumar K.V" <aneesh.kumar@linux•ibm.com>,
	linuxppc-dev@lists•ozlabs.org
Subject: Re: [PATCH v3 0/4] powerpc/mm/radix: Memory unplug fixes
Date: Tue, 21 Jul 2020 22:25:58 +1000	[thread overview]
Message-ID: <87ft9lrr55.fsf@mpe.ellerman.id.au> (raw)
In-Reply-To: <20200721032959.GN7902@in.ibm.com>

Bharata B Rao <bharata@linux•ibm.com> writes:
> On Tue, Jul 21, 2020 at 11:45:20AM +1000, Michael Ellerman wrote:
>> Nathan Lynch <nathanl@linux•ibm.com> writes:
>> > "Aneesh Kumar K.V" <aneesh.kumar@linux•ibm.com> writes:
>> >> This is the next version of the fixes for memory unplug on radix.
>> >> The issues and the fix are described in the actual patches.
>> >
>> > I guess this isn't actually causing problems at runtime right now, but I
>> > notice calls to resize_hpt_for_hotplug() from arch_add_memory() and
>> > arch_remove_memory(), which ought to be mmu-agnostic:
>> >
>> > int __ref arch_add_memory(int nid, u64 start, u64 size,
>> > 			  struct mhp_params *params)
>> > {
>> > 	unsigned long start_pfn = start >> PAGE_SHIFT;
>> > 	unsigned long nr_pages = size >> PAGE_SHIFT;
>> > 	int rc;
>> >
>> > 	resize_hpt_for_hotplug(memblock_phys_mem_size());
>> >
>> > 	start = (unsigned long)__va(start);
>> > 	rc = create_section_mapping(start, start + size, nid,
>> > 				    params->pgprot);
>> > ...
>> 
>> Hmm well spotted.
>> 
>> That does return early if the ops are not setup:
>> 
>> int resize_hpt_for_hotplug(unsigned long new_mem_size)
>> {
>> 	unsigned target_hpt_shift;
>> 
>> 	if (!mmu_hash_ops.resize_hpt)
>> 		return 0;
>> 
>> 
>> And:
>> 
>> void __init hpte_init_pseries(void)
>> {
>> 	...
>> 	if (firmware_has_feature(FW_FEATURE_HPT_RESIZE))
>> 		mmu_hash_ops.resize_hpt = pseries_lpar_resize_hpt;
>> 
>> And that comes in via ibm,hypertas-functions:
>> 
>> 	{FW_FEATURE_HPT_RESIZE,		"hcall-hpt-resize"},
>> 
>> 
>> But firmware is not necessarily going to add/remove that call based on
>> whether we're using hash/radix.
>
> Correct but hpte_init_pseries() will not be called for radix guests.

Yeah, duh. You'd think the function name would have been a sufficient
clue for me :)

>> So I think a follow-up patch is needed to make this more robust.
>> 
>> Aneesh/Bharata what platform did you test this series on? I'm curious
>> how this didn't break.
>
> I have tested memory hotplug/unplug for radix guest on zz platform and
> sanity-tested this for hash guest on P8.
>
> As noted above, mmu_hash_ops.resize_hpt will not be set for radix
> guest and hence we won't see any breakage.

OK.

That's probably fine as it is then. Or maybe just a comment in
resize_hpt_for_hotplug() pointing out that resize_hpt will be NULL if
we're using radix.

cheers

  reply	other threads:[~2020-07-21 12:28 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-07-09 13:19 [PATCH v3 0/4] powerpc/mm/radix: Memory unplug fixes Aneesh Kumar K.V
2020-07-09 13:19 ` [PATCH v3 1/4] powerpc/mm/radix: Fix PTE/PMD fragment count for early page table mappings Aneesh Kumar K.V
2020-07-09 13:19 ` [PATCH v3 2/4] powerpc/mm/radix: Free PUD table when freeing pagetable Aneesh Kumar K.V
2020-07-09 13:19 ` [PATCH v3 3/4] powerpc/mm/radix: Remove split_kernel_mapping() Aneesh Kumar K.V
2020-07-09 13:19 ` [PATCH v3 4/4] powerpc/mm/radix: Create separate mappings for hot-plugged memory Aneesh Kumar K.V
2020-07-16 14:00 ` [PATCH v3 0/4] powerpc/mm/radix: Memory unplug fixes Nathan Lynch
2020-07-21  1:45   ` Michael Ellerman
2020-07-21  3:29     ` Bharata B Rao
2020-07-21 12:25       ` Michael Ellerman [this message]
2020-07-22  6:05         ` Bharata B Rao
2020-07-22  7:51           ` David Gibson
2020-07-24 11:52           ` Michael Ellerman
2020-07-24 12:17             ` Bharata B Rao
2020-07-25  7:37             ` David Gibson
2020-07-21  4:42     ` Aneesh Kumar K.V
2020-07-24 13:24 ` Michael Ellerman

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=87ft9lrr55.fsf@mpe.ellerman.id.au \
    --to=mpe@ellerman$(echo .)id.au \
    --cc=aneesh.kumar@linux$(echo .)ibm.com \
    --cc=bharata@linux$(echo .)ibm.com \
    --cc=linuxppc-dev@lists$(echo .)ozlabs.org \
    --cc=nathanl@linux$(echo .)ibm.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html
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