* RE: [PATCH v6 01/10] arm64: hyperv: Add core Hyper-V include files
From: Michael Kelley @ 2020-03-18 0:10 UTC (permalink / raw)
To: Marc Zyngier
Cc: will@kernel.org, ardb@kernel.org, arnd@arndb.de,
catalin.marinas@arm.com, mark.rutland@arm.com,
linux-arm-kernel@lists.infradead.org, gregkh@linuxfoundation.org,
linux-kernel@vger.kernel.org, linux-hyperv@vger.kernel.org,
linux-efi@vger.kernel.org, linux-arch@vger.kernel.org,
olaf@aepfle.de, apw@canonical.com, vkuznets, jasowang@redhat.com,
marcelo.cerri@canonical.com, KY Srinivasan, Sunil Muthuswamy,
boqun.feng@gmail.com
In-Reply-To: <86v9n5zfrm.wl-maz@kernel.org>
From: Marc Zyngier <maz@kernel.org> Sent: Sunday, March 15, 2020 10:31 AM
>
> On Sat, 14 Mar 2020 15:35:10 +0000,
> Hi Michael,
>
> Michael Kelley <mikelley@microsoft.com> wrote:
> >
> > hyperv-tlfs.h defines Hyper-V interfaces from the Hyper-V Top Level
> > Functional Spec (TLFS). The TLFS is distinctly oriented to x86/x64,
> > and Hyper-V has not separated out the architecture-dependent parts into
> > x86/x64 vs. ARM64. So hyperv-tlfs.h includes information for ARM64
> > that is not yet formally published. The TLFS is available here:
> >
> > docs.microsoft.com/en-us/virtualization/hyper-v-on-windows/reference/tlfs
> >
> > mshyperv.h defines Linux-specific structures and routines for
> > interacting with Hyper-V on ARM64, and #includes the architecture-
> > independent part of mshyperv.h in include/asm-generic.
> >
> > Signed-off-by: Michael Kelley <mikelley@microsoft.com>
> > ---
> > MAINTAINERS | 2 +
> > arch/arm64/include/asm/hyperv-tlfs.h | 413
> +++++++++++++++++++++++++++++++++++
> > arch/arm64/include/asm/mshyperv.h | 115 ++++++++++
> > 3 files changed, 530 insertions(+)
> > create mode 100644 arch/arm64/include/asm/hyperv-tlfs.h
> > create mode 100644 arch/arm64/include/asm/mshyperv.h
>
> So this is a pretty large patch, mostly containing constants and other
> data structures that don't necessarily make sense immediately (or at
> least, I can't make sense of it without reading all the other 9
> patches and going back to patch #1).
>
> Could you please consider splitting this into more discreet bits that
> get added as required by the supporting code?
Yes, I'll do this in the next version.
>
> So here's only a few sparse comments:
>
> >
> > diff --git a/MAINTAINERS b/MAINTAINERS
> > index 58bb5c4..398cfdb 100644
> > --- a/MAINTAINERS
> > +++ b/MAINTAINERS
> > @@ -7809,6 +7809,8 @@ F: arch/x86/include/asm/trace/hyperv.h
> > F: arch/x86/include/asm/hyperv-tlfs.h
> > F: arch/x86/kernel/cpu/mshyperv.c
> > F: arch/x86/hyperv
> > +F: arch/arm64/include/asm/hyperv-tlfs.h
> > +F: arch/arm64/include/asm/mshyperv.h
> > F: drivers/clocksource/hyperv_timer.c
> > F: drivers/hid/hid-hyperv.c
> > F: drivers/hv/
> > diff --git a/arch/arm64/include/asm/hyperv-tlfs.h b/arch/arm64/include/asm/hyperv-
> tlfs.h
> > new file mode 100644
> > index 0000000..5e6a087
> > --- /dev/null
> > +++ b/arch/arm64/include/asm/hyperv-tlfs.h
> > @@ -0,0 +1,413 @@
> > +/* SPDX-License-Identifier: GPL-2.0 */
> > +
> > +/*
> > + * This file contains definitions from the Hyper-V Hypervisor Top-Level
> > + * Functional Specification (TLFS):
> > + * https://docs.microsoft.com/en-us/virtualization/hyper-v-on-windows/reference/tlfs
> > + *
> > + * Copyright (C) 2019, Microsoft, Inc.
> > + *
> > + * Author : Michael Kelley <mikelley@microsoft.com>
> > + */
> > +
> > +#ifndef _ASM_HYPERV_TLFS_H
> > +#define _ASM_HYPERV_TLFS_H
> > +
> > +#include <linux/types.h>
> > +
> > +/*
> > + * All data structures defined in the TLFS that are shared between Hyper-V
> > + * and a guest VM use Little Endian byte ordering. This matches the default
> > + * byte ordering of Linux running on ARM64, so no special handling is required.
> > + */
> > +
> > +
> > +/*
> > + * While not explicitly listed in the TLFS, Hyper-V always runs with a page
> > + * size of 4096. These definitions are used when communicating with Hyper-V
> > + * using guest physical pages and guest physical page addresses, since the
> > + * guest page size may not be 4096 on ARM64.
> > + */
> > +#define HV_HYP_PAGE_SHIFT 12
> > +#define HV_HYP_PAGE_SIZE (1 << HV_HYP_PAGE_SHIFT)
>
> Probably worth writing this as 1UL to be on the safe side.
Agreed.
>
> > +#define HV_HYP_PAGE_MASK (~(HV_HYP_PAGE_SIZE - 1))
> > +
> > +/*
> > + * These Hyper-V registers provide information equivalent to the CPUID
> > + * instruction on x86/x64.
> > + */
> > +#define HV_REGISTER_HYPERVISOR_VERSION 0x00000100 /*CPUID
> 0x40000002 */
> > +#define HV_REGISTER_PRIVILEGES_AND_FEATURES 0x00000200 /*CPUID
> 0x40000003 */
> > +#define HV_REGISTER_FEATURES 0x00000201 /*CPUID
> 0x40000004 */
> > +#define HV_REGISTER_IMPLEMENTATION_LIMITS 0x00000202 /*CPUID
> 0x40000005 */
> > +#define HV_ARM64_REGISTER_INTERFACE_VERSION 0x00090006 /*CPUID
> 0x40000001 */
> > +
> > +/*
> > + * Feature identification. HvRegisterPrivilegesAndFeaturesInfo returns a
> > + * 128-bit value with flags indicating which features are available to the
> > + * partition based upon the current partition privileges. The 128-bit
> > + * value is broken up with different portions stored in different 32-bit
> > + * fields in the ms_hyperv structure.
> > + */
> > +
> > +/* Partition Reference Counter available*/
> > +#define HV_MSR_TIME_REF_COUNT_AVAILABLE BIT(1)
> > +
> > +/* Synthetic Timers available */
> > +#define HV_MSR_SYNTIMER_AVAILABLE BIT(3)
> > +
> > +/* Reference TSC available */
> > +#define HV_MSR_REFERENCE_TSC_AVAILABLE BIT(9)
> > +
> > +
> > +/*
> > + * This group of flags is in the high order 64-bits of the returned
> > + * 128-bit value. Note that this set of bit positions differs from what
> > + * is used on x86/x64 architecture.
> > + */
> > +
> > +/* Crash MSRs available */
> > +#define HV_FEATURE_GUEST_CRASH_MSR_AVAILABLE BIT(8)
>
> It is confusing that you don't have a single bit space for all these
> flags. It'd probably help if you had a structure describing this
> 128bit value in multiple 32bit or 64bit words, and indicating which
> field the bit position is relevant to.
I'll add this in the next version.
>
> [...]
>
> > +/* Define the hypercall status result */
> > +
> > +union hv_hypercall_status {
> > + u64 as_uint64;
>
> nit: it'd be more consistent if as_uint64 was actually a uint64 type.
Agreed.
>
> > + struct {
> > + u16 status;
> > + u16 reserved;
> > + u16 reps_completed; /* Low 12 bits */
> > + u16 reserved2;
> > + };
> > +};
> > +
> > +/* hypercall status code */
> > +#define HV_STATUS_SUCCESS 0
> > +#define HV_STATUS_INVALID_HYPERCALL_CODE 2
> > +#define HV_STATUS_INVALID_HYPERCALL_INPUT 3
> > +#define HV_STATUS_INVALID_ALIGNMENT 4
> > +#define HV_STATUS_INSUFFICIENT_MEMORY 11
> > +#define HV_STATUS_INVALID_CONNECTION_ID 18
> > +#define HV_STATUS_INSUFFICIENT_BUFFERS 19
> > +
> > +/* Define input and output layout for Get VP Register hypercall */
> > +struct hv_get_vp_register_input {
> > + u64 partitionid;
> > + u32 vpindex;
> > + u8 inputvtl;
> > + u8 padding[3];
> > + u32 name0;
> > + u32 name1;
> > +} __packed;
> > +
> > +struct hv_get_vp_register_output {
> > + u64 registervaluelow;
> > + u64 registervaluehigh;
> > +} __packed;
> > +
> > +#define HV_FLUSH_ALL_PROCESSORS BIT(0)
> > +#define HV_FLUSH_ALL_VIRTUAL_ADDRESS_SPACES BIT(1)
> > +#define HV_FLUSH_NON_GLOBAL_MAPPINGS_ONLY BIT(2)
>
> I"m curious: Are these supposed to be PV'd TLB invalidation
> operations?
Yes, they are. Hyper-V provides PV TLB flush hypercalls on ARM64, but
this patch set doesn't use those hypercalls. I'll remove the definitions.
>
> > +#define HV_FLUSH_USE_EXTENDED_RANGE_FORMAT BIT(3)
> > +
> > +enum HV_GENERIC_SET_FORMAT {
> > + HV_GENERIC_SET_SPARSE_4K,
> > + HV_GENERIC_SET_ALL,
> > +};
> > +
> > +/*
> > + * The Hyper-V TimeRefCount register and the TSC
> > + * page provide a guest VM clock with 100ns tick rate
> > + */
> > +#define HV_CLOCK_HZ (NSEC_PER_SEC/100)
> > +
> > +/*
> > + * The fields in this structure are set by Hyper-V and read
> > + * by the Linux guest. They should be accessed with READ_ONCE()
> > + * so the compiler doesn't optimize in a way that will cause
> > + * problems. The union pads the size out to the page size
> > + * used to communicate with Hyper-V.
> > + */
> > +struct ms_hyperv_tsc_page {
> > + union {
> > + struct {
> > + u32 tsc_sequence;
> > + u32 reserved1;
> > + u64 tsc_scale;
> > + s64 tsc_offset;
> > + } __packed;
> > + u8 reserved2[HV_HYP_PAGE_SIZE];
> > + };
> > +};
> > +
> > +/* Define the number of synthetic interrupt sources. */
> > +#define HV_SYNIC_SINT_COUNT (16)
> > +/* Define the expected SynIC version. */
> > +#define HV_SYNIC_VERSION_1 (0x1)
> > +
> > +#define HV_SYNIC_CONTROL_ENABLE (1ULL << 0)
> > +#define HV_SYNIC_SIMP_ENABLE (1ULL << 0)
> > +#define HV_SYNIC_SIEFP_ENABLE (1ULL << 0)
> > +#define HV_SYNIC_SINT_MASKED (1ULL << 16)
> > +#define HV_SYNIC_SINT_AUTO_EOI (1ULL << 17)
> > +#define HV_SYNIC_SINT_VECTOR_MASK (0xFF)
>
> Let's me guess: a PV interrupt controller? Do you really need this?
> Specially as I don't see any PV irqchip driver in this submission...
>
No, the above definitions aren't needed. I'll remove them.
Hyper-V does provide a limited synthetic interrupt controller that's
implemented entirely in architecture independent code and has been
used on the x86 side since the beginning of Hyper-V support. It
pre-dates me by a lot of years, and I've never considered whether it
should be modeled as an irqchip.
> [...]
>
> > diff --git a/arch/arm64/include/asm/mshyperv.h b/arch/arm64/include/asm/mshyperv.h
> > new file mode 100644
> > index 0000000..60b3f68
> > --- /dev/null
> > +++ b/arch/arm64/include/asm/mshyperv.h
> > @@ -0,0 +1,115 @@
> > +/* SPDX-License-Identifier: GPL-2.0 */
> > +
> > +/*
> > + * Linux-specific definitions for managing interactions with Microsoft's
> > + * Hyper-V hypervisor. The definitions in this file are specific to
> > + * the ARM64 architecture. See include/asm-generic/mshyperv.h for
> > + * definitions are that architecture independent.
> > + *
> > + * Definitions that are specified in the Hyper-V Top Level Functional
> > + * Spec (TLFS) should not go in this file, but should instead go in
> > + * hyperv-tlfs.h.
> > + *
> > + * Copyright (C) 2019, Microsoft, Inc.
> > + *
> > + * Author : Michael Kelley <mikelley@microsoft.com>
> > + */
> > +
> > +#ifndef _ASM_MSHYPERV_H
> > +#define _ASM_MSHYPERV_H
> > +
> > +#include <linux/types.h>
> > +#include <linux/interrupt.h>
> > +#include <linux/clocksource.h>
> > +#include <linux/irq.h>
> > +#include <linux/irqdesc.h>
> > +#include <linux/arm-smccc.h>
> > +#include <asm/hyperv-tlfs.h>
> > +
> > +/*
> > + * Define the IRQ numbers/vectors used by Hyper-V VMbus interrupts
> > + * and by STIMER0 Direct Mode interrupts. Hyper-V should be supplying
> > + * these values through ACPI, but there are no other interrupting
> > + * devices in a Hyper-V VM on ARM64, so it's OK to hard code for now.
>
> I'm not convinced it is OK. If you don't try to do the right thing
> now, what is the incentive to do it later? Starting to hard code
> things is akin to going back to the ARM board files of old. Been
> there, managed to fix it, not going back to it again anytime soon.
I'll check back with the Hyper-V guys on getting appropriate values
assigned in ACPI.
>
> > + * The "CALLBACK_VECTOR" terminology is a left-over from the x86/x64
> > + * world that is used in architecture independent Hyper-V code.
> > + */
> > +#define HYPERVISOR_CALLBACK_VECTOR 16
> > +#define HV_STIMER0_IRQNR 17
> > +
> > +extern u64 hv_do_hvc(u64 control, ...);
> > +extern u64 hv_do_hvc_fast_get(u64 control, u64 input1, u64 input2, u64 input3,
> > + struct hv_get_vp_register_output *output);
> > +
> > +/*
> > + * Declare calls to get and set Hyper-V VP register values on ARM64, which
> > + * requires a hypercall.
> > + */
> > +extern void hv_set_vpreg(u32 reg, u64 value);
> > +extern u64 hv_get_vpreg(u32 reg);
> > +extern void hv_get_vpreg_128(u32 reg, struct hv_get_vp_register_output *result);
> > +
> > +/*
> > + * Use the Hyper-V provided stimer0 as the timer that is made
> > + * available to the architecture independent Hyper-V drivers.
> > + */
> > +#define hv_init_timer(timer, tick) \
> > + hv_set_vpreg(HV_REGISTER_STIMER0_COUNT + (2*timer), tick)
> > +#define hv_init_timer_config(timer, val) \
> > + hv_set_vpreg(HV_REGISTER_STIMER0_CONFIG + (2*timer), val)
> > +#define hv_get_current_tick(tick) \
> > + (tick = hv_get_vpreg(HV_REGISTER_TIME_REFCOUNT))
> > +
> > +#define hv_get_simp(val) (val = hv_get_vpreg(HV_REGISTER_SIPP))
> > +#define hv_set_simp(val) hv_set_vpreg(HV_REGISTER_SIPP, val)
> > +
> > +#define hv_get_siefp(val) (val = hv_get_vpreg(HV_REGISTER_SIFP))
> > +#define hv_set_siefp(val) hv_set_vpreg(HV_REGISTER_SIFP, val)
> > +
> > +#define hv_get_synic_state(val) (val = hv_get_vpreg(HV_REGISTER_SCONTROL))
> > +#define hv_set_synic_state(val) hv_set_vpreg(HV_REGISTER_SCONTROL, val)
> > +
> > +#define hv_get_vp_index(index) (index = hv_get_vpreg(HV_REGISTER_VPINDEX))
> > +
> > +#define hv_signal_eom() hv_set_vpreg(HV_REGISTER_EOM, 0)
> > +
> > +/*
> > + * Hyper-V SINT registers are numbered sequentially, so we can just
> > + * add the SINT number to the register number of SINT0
> > + */
> > +#define hv_get_synint_state(sint_num, val) \
> > + (val = hv_get_vpreg(HV_REGISTER_SINT0 + sint_num))
> > +#define hv_set_synint_state(sint_num, val) \
> > + hv_set_vpreg(HV_REGISTER_SINT0 + sint_num, val)
> > +
> > +#define hv_get_crash_ctl(val) \
> > + (val = hv_get_vpreg(HV_REGISTER_CRASH_CTL))
> > +#define hv_get_time_ref_count(val) \
> > + (val = hv_get_vpreg(HV_REGISTER_TIME_REFCOUNT))
> > +#define hv_get_reference_tsc(val) \
> > + (val = hv_get_vpreg(HV_REGISTER_REFERENCE_TSC))
> > +#define hv_set_reference_tsc(val) \
> > + hv_set_vpreg(HV_REGISTER_REFERENCE_TSC, val)
> > +#define hv_enable_vdso_clocksource()
> > +#define hv_set_clocksource_vdso(val) \
> > + ((val).vdso_clock_mode = VDSO_CLOCKMODE_NONE)
> > +
> > +#if IS_ENABLED(CONFIG_HYPERV)
>
> I don't think this guards anything useful.
You are probably right. I'll double-check.
>
> > +#define hv_enable_stimer0_percpu_irq(irq) enable_percpu_irq(irq, 0)
> > +#define hv_disable_stimer0_percpu_irq(irq) disable_percpu_irq(irq)
>
> and this looks pretty premature.
I'm not sure I understand your comment about "premature". Could
you clarify?
>
> > +#endif
> > +
> > +/* ARM64 specific code to read the hardware clock */
> > +#define hv_get_raw_timer() arch_timer_read_counter()
> > +
> > +/* SMCCC hypercall parameters */
> > +#define HV_SMCCC_FUNC_NUMBER 1
> > +#define HV_FUNC_ID ARM_SMCCC_CALL_VAL( \
> > + ARM_SMCCC_STD_CALL, \
> > + ARM_SMCCC_SMC_64, \
> > + ARM_SMCCC_OWNER_VENDOR_HYP, \
>
> This is only defined in patch #2...
Indeed. :-( I'll fix as part of breaking up this patch into smaller
pieces.
Michael
>
> > + HV_SMCCC_FUNC_NUMBER)
> > +
> > +#include <asm-generic/mshyperv.h>
> > +
> > +#endif
>
> Thanks,
>
> M.
>
> --
> Jazz is not dead, it just smells funny.
^ permalink raw reply
* Re: [PATCH v2 6/8] mm/memory_hotplug: unexport memhp_auto_online
From: Wei Yang @ 2020-03-17 22:24 UTC (permalink / raw)
To: David Hildenbrand
Cc: linux-kernel, linux-mm, linuxppc-dev, linux-hyperv, Andrew Morton,
Michal Hocko, Oscar Salvador, Rafael J. Wysocki, Baoquan He,
Wei Yang
In-Reply-To: <20200317104942.11178-7-david@redhat.com>
On Tue, Mar 17, 2020 at 11:49:40AM +0100, David Hildenbrand wrote:
>All in-tree users except the mm-core are gone. Let's drop the export.
>
>Cc: Andrew Morton <akpm@linux-foundation.org>
>Cc: Michal Hocko <mhocko@kernel.org>
>Cc: Oscar Salvador <osalvador@suse.de>
>Cc: "Rafael J. Wysocki" <rafael@kernel.org>
>Cc: Baoquan He <bhe@redhat.com>
>Cc: Wei Yang <richard.weiyang@gmail.com>
>Signed-off-by: David Hildenbrand <david@redhat.com>
Reviewed-by: Wei Yang <richard.weiyang@gmail.com>
>---
> mm/memory_hotplug.c | 1 -
> 1 file changed, 1 deletion(-)
>
>diff --git a/mm/memory_hotplug.c b/mm/memory_hotplug.c
>index 1a00b5a37ef6..2d2aae830b92 100644
>--- a/mm/memory_hotplug.c
>+++ b/mm/memory_hotplug.c
>@@ -71,7 +71,6 @@ bool memhp_auto_online;
> #else
> bool memhp_auto_online = true;
> #endif
>-EXPORT_SYMBOL_GPL(memhp_auto_online);
>
> static int __init setup_memhp_default_state(char *str)
> {
>--
>2.24.1
--
Wei Yang
Help you, Help me
^ permalink raw reply
* Re: [PATCH v2 4/8] powernv/memtrace: always online added memory blocks
From: Wei Yang @ 2020-03-17 22:04 UTC (permalink / raw)
To: David Hildenbrand
Cc: linux-kernel, linux-mm, linuxppc-dev, linux-hyperv,
Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman,
Andrew Morton, Greg Kroah-Hartman, Michal Hocko, Oscar Salvador,
Rafael J. Wysocki, Baoquan He, Wei Yang
In-Reply-To: <20200317104942.11178-5-david@redhat.com>
On Tue, Mar 17, 2020 at 11:49:38AM +0100, David Hildenbrand wrote:
>Let's always try to online the re-added memory blocks. In case add_memory()
>already onlined the added memory blocks, the first device_online() call
>will fail and stop processing the remaining memory blocks.
>
>This avoids manually having to check memhp_auto_online.
>
>Note: PPC always onlines all hotplugged memory directly from the kernel
>as well - something that is handled by user space on other
>architectures.
>
>Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
>Cc: Paul Mackerras <paulus@samba.org>
>Cc: Michael Ellerman <mpe@ellerman.id.au>
>Cc: Andrew Morton <akpm@linux-foundation.org>
>Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
>Cc: Michal Hocko <mhocko@kernel.org>
>Cc: Oscar Salvador <osalvador@suse.de>
>Cc: "Rafael J. Wysocki" <rafael@kernel.org>
>Cc: Baoquan He <bhe@redhat.com>
>Cc: Wei Yang <richard.weiyang@gmail.com>
>Cc: linuxppc-dev@lists.ozlabs.org
>Signed-off-by: David Hildenbrand <david@redhat.com>
Looks good.
Reviewed-by: Wei Yang <richard.weiyang@gmail.com>
>---
> arch/powerpc/platforms/powernv/memtrace.c | 14 ++++----------
> 1 file changed, 4 insertions(+), 10 deletions(-)
>
>diff --git a/arch/powerpc/platforms/powernv/memtrace.c b/arch/powerpc/platforms/powernv/memtrace.c
>index d6d64f8718e6..13b369d2cc45 100644
>--- a/arch/powerpc/platforms/powernv/memtrace.c
>+++ b/arch/powerpc/platforms/powernv/memtrace.c
>@@ -231,16 +231,10 @@ static int memtrace_online(void)
> continue;
> }
>
>- /*
>- * If kernel isn't compiled with the auto online option
>- * we need to online the memory ourselves.
>- */
>- if (!memhp_auto_online) {
>- lock_device_hotplug();
>- walk_memory_blocks(ent->start, ent->size, NULL,
>- online_mem_block);
>- unlock_device_hotplug();
>- }
>+ lock_device_hotplug();
>+ walk_memory_blocks(ent->start, ent->size, NULL,
>+ online_mem_block);
>+ unlock_device_hotplug();
>
> /*
> * Memory was added successfully so clean up references to it
>--
>2.24.1
--
Wei Yang
Help you, Help me
^ permalink raw reply
* Re: [PATCH v2 5/8] hv_balloon: don't check for memhp_auto_online manually
From: David Hildenbrand @ 2020-03-17 18:46 UTC (permalink / raw)
To: linux-kernel
Cc: linux-mm, linuxppc-dev, linux-hyperv, K. Y. Srinivasan,
Haiyang Zhang, Stephen Hemminger, Wei Liu, Andrew Morton,
Michal Hocko, Oscar Salvador, Rafael J. Wysocki, Baoquan He,
Wei Yang, Vitaly Kuznetsov
In-Reply-To: <20200317104942.11178-6-david@redhat.com>
> @@ -1707,6 +1701,7 @@ static int balloon_probe(struct hv_device *dev,
> #ifdef CONFIG_MEMORY_HOTPLUG
> set_online_page_callback(&hv_online_page);
> register_memory_notifier(&hv_memory_nb);
> + init_completion(&dm_device.ol_waitevent);
I'll move this one line up.
> #endif
>
> hv_set_drvdata(dev, &dm_device);
>
--
Thanks,
David / dhildenb
^ permalink raw reply
* Re: [PATCH 2/4] x86/Hyper-V: Free hv_panic_page when fail to register kmsg dump
From: Wei Liu @ 2020-03-17 17:36 UTC (permalink / raw)
To: ltykernel
Cc: kys, haiyangz, sthemmin, liuwe, tglx, mingo, bp, hpa, x86,
michael.h.kelley, Tianyu Lan, linux-hyperv, linux-kernel,
vkuznets, Wei Liu
In-Reply-To: <20200317132523.1508-3-Tianyu.Lan@microsoft.com>
On Tue, Mar 17, 2020 at 06:25:21AM -0700, ltykernel@gmail.com wrote:
> From: Tianyu Lan <Tianyu.Lan@microsoft.com>
>
> If fail to register kmsg dump on Hyper-V platform, hv_panic_page
> will not be used anywhere. So free and reset it.
>
> Signed-off-by: Tianyu Lan <Tianyu.Lan@microsoft.com>
> ---
> drivers/hv/vmbus_drv.c | 6 +++++-
> 1 file changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/hv/vmbus_drv.c b/drivers/hv/vmbus_drv.c
> index b56b9fb9bd90..b043efea092a 100644
> --- a/drivers/hv/vmbus_drv.c
> +++ b/drivers/hv/vmbus_drv.c
> @@ -1385,9 +1385,13 @@ static int vmbus_bus_init(void)
> hv_panic_page = (void *)hv_alloc_hyperv_zeroed_page();
> if (hv_panic_page) {
> ret = kmsg_dump_register(&hv_kmsg_dumper);
> - if (ret)
> + if (ret) {
> pr_err("Hyper-V: kmsg dump register "
> "error 0x%x\n", ret);
> + hv_free_hyperv_page(
> + (unsigned long)hv_panic_page);
> + hv_panic_page = NULL;
> + }
While this modification looks correct to me, there is a call to free
hv_panic_page in the err_alloc path. That makes the error handling a bit
confusing here.
I think you can just remove that function call in err_alloc path.
Wei.
> } else
> pr_err("Hyper-V: panic message page memory "
> "allocation failed");
> --
> 2.14.5
>
^ permalink raw reply
* Re: [PATCH 0/4] x86/Hyper-V: Unload vmbus channel in hv panic callback
From: Wei Liu @ 2020-03-17 17:35 UTC (permalink / raw)
To: ltykernel
Cc: kys, haiyangz, sthemmin, liuwe, tglx, mingo, bp, hpa, x86,
michael.h.kelley, Tianyu Lan, linux-hyperv, linux-kernel,
vkuznets, Wei Liu
In-Reply-To: <20200317132523.1508-2-Tianyu.Lan@microsoft.com>
On Tue, Mar 17, 2020 at 06:25:20AM -0700, ltykernel@gmail.com wrote:
> From: Tianyu Lan <Tianyu.Lan@microsoft.com>
>
> Customer reported Hyper-V VM still responded network traffic
> ack packets after kernel panic with kernel parameter "panic=0”.
> This becauses vmbus driver interrupt handler still works
> on the panic cpu after kernel panic. Panic cpu falls into
> infinite loop of panic() with interrupt enabled at that point.
> Vmbus driver can still handle network traffic.
>
> This confuses remote service that the panic system is still
> alive when it gets ack packets. Unload vmbus channel in hv panic
> callback and fix it.
>
> vmbus_initiate_unload() maybe double called during panic process
> (e.g, hyperv_panic_event() and hv_crash_handler()). So check
> and set connection state in vmbus_initiate_unload() to resolve
> reenter issue.
>
> Signed-off-by: Tianyu Lan <Tianyu.Lan@microsoft.com>
> ---
> drivers/hv/channel_mgmt.c | 5 +++++
> drivers/hv/vmbus_drv.c | 17 +++++++++--------
> 2 files changed, 14 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/hv/channel_mgmt.c b/drivers/hv/channel_mgmt.c
> index 0370364169c4..893493f2b420 100644
> --- a/drivers/hv/channel_mgmt.c
> +++ b/drivers/hv/channel_mgmt.c
> @@ -839,6 +839,9 @@ void vmbus_initiate_unload(bool crash)
> {
> struct vmbus_channel_message_header hdr;
>
> + if (vmbus_connection.conn_state == DISCONNECTED)
> + return;
> +
> /* Pre-Win2012R2 hosts don't support reconnect */
> if (vmbus_proto_version < VERSION_WIN8_1)
> return;
> @@ -857,6 +860,8 @@ void vmbus_initiate_unload(bool crash)
> wait_for_completion(&vmbus_connection.unload_event);
> else
> vmbus_wait_for_unload();
> +
> + vmbus_connection.conn_state = DISCONNECTED;
This is only set at the end of the function. I don't see how this solve
the re-entrant issue with the check at the beginning. Do I miss anything
here?
Maybe this function should check and set the state to
DISCONNECTING/DISCONNECTED at the beginning of this function?
Wei.
^ permalink raw reply
* Re: [PATCH v2 5/8] hv_balloon: don't check for memhp_auto_online manually
From: David Hildenbrand @ 2020-03-17 16:33 UTC (permalink / raw)
To: Vitaly Kuznetsov, linux-kernel
Cc: linux-mm, linuxppc-dev, linux-hyperv, K. Y. Srinivasan,
Haiyang Zhang, Stephen Hemminger, Wei Liu, Andrew Morton,
Michal Hocko, Oscar Salvador, Rafael J. Wysocki, Baoquan He,
Wei Yang
In-Reply-To: <877dzj3pyi.fsf@vitty.brq.redhat.com>
On 17.03.20 17:29, Vitaly Kuznetsov wrote:
> David Hildenbrand <david@redhat.com> writes:
>
>> We get the MEM_ONLINE notifier call if memory is added right from the
>> kernel via add_memory() or later from user space.
>>
>> Let's get rid of the "ha_waiting" flag - the wait event has an inbuilt
>> mechanism (->done) for that. Initialize the wait event only once and
>> reinitialize before adding memory. Unconditionally call complete() and
>> wait_for_completion_timeout().
>>
>> If there are no waiters, complete() will only increment ->done - which
>> will be reset by reinit_completion(). If complete() has already been
>> called, wait_for_completion_timeout() will not wait.
>>
>> There is still the chance for a small race between concurrent
>> reinit_completion() and complete(). If complete() wins, we would not
>> wait - which is tolerable (and the race exists in current code as
>> well).
>
> How can we see concurent reinit_completion() and complete()? Obvioulsy,
> we are not onlining new memory in kernel and hv_mem_hot_add() calls are
> serialized, we're waiting up to 5*HZ for the added block to come online
> before proceeding to the next one. Or do you mean we actually hit this
> 5*HZ timeout, proceeded to the next block and immediately after
> reinit_completion() we saw complete() for the previously added block?
Yes exactly - or if an admin manually offlines+re-onlines a random
memory block.
> This is tolerable indeed, we're making forward progress (and this all is
> 'best effort' anyway).
Exactly my thoughts.
[...]
>
> Reviewed-by: Vitaly Kuznetsov <vkuznets@redhat.com>
>
Thanks!
--
Thanks,
David / dhildenb
^ permalink raw reply
* Re: [PATCH v2 5/8] hv_balloon: don't check for memhp_auto_online manually
From: Vitaly Kuznetsov @ 2020-03-17 16:29 UTC (permalink / raw)
To: David Hildenbrand, linux-kernel
Cc: linux-mm, linuxppc-dev, linux-hyperv, David Hildenbrand,
K. Y. Srinivasan, Haiyang Zhang, Stephen Hemminger, Wei Liu,
Andrew Morton, Michal Hocko, Oscar Salvador, Rafael J. Wysocki,
Baoquan He, Wei Yang
In-Reply-To: <20200317104942.11178-6-david@redhat.com>
David Hildenbrand <david@redhat.com> writes:
> We get the MEM_ONLINE notifier call if memory is added right from the
> kernel via add_memory() or later from user space.
>
> Let's get rid of the "ha_waiting" flag - the wait event has an inbuilt
> mechanism (->done) for that. Initialize the wait event only once and
> reinitialize before adding memory. Unconditionally call complete() and
> wait_for_completion_timeout().
>
> If there are no waiters, complete() will only increment ->done - which
> will be reset by reinit_completion(). If complete() has already been
> called, wait_for_completion_timeout() will not wait.
>
> There is still the chance for a small race between concurrent
> reinit_completion() and complete(). If complete() wins, we would not
> wait - which is tolerable (and the race exists in current code as
> well).
How can we see concurent reinit_completion() and complete()? Obvioulsy,
we are not onlining new memory in kernel and hv_mem_hot_add() calls are
serialized, we're waiting up to 5*HZ for the added block to come online
before proceeding to the next one. Or do you mean we actually hit this
5*HZ timeout, proceeded to the next block and immediately after
reinit_completion() we saw complete() for the previously added block?
This is tolerable indeed, we're making forward progress (and this all is
'best effort' anyway).
>
> Note: We only wait for "some" memory to get onlined, which seems to be
> good enough for now.
>
> Cc: "K. Y. Srinivasan" <kys@microsoft.com>
> Cc: Haiyang Zhang <haiyangz@microsoft.com>
> Cc: Stephen Hemminger <sthemmin@microsoft.com>
> Cc: Wei Liu <wei.liu@kernel.org>
> Cc: Andrew Morton <akpm@linux-foundation.org>
> Cc: Michal Hocko <mhocko@kernel.org>
> Cc: Oscar Salvador <osalvador@suse.de>
> Cc: "Rafael J. Wysocki" <rafael@kernel.org>
> Cc: Baoquan He <bhe@redhat.com>
> Cc: Wei Yang <richard.weiyang@gmail.com>
> Cc: Vitaly Kuznetsov <vkuznets@redhat.com>
> Cc: linux-hyperv@vger.kernel.org
> Signed-off-by: David Hildenbrand <david@redhat.com>
> ---
> drivers/hv/hv_balloon.c | 25 ++++++++++---------------
> 1 file changed, 10 insertions(+), 15 deletions(-)
>
> diff --git a/drivers/hv/hv_balloon.c b/drivers/hv/hv_balloon.c
> index a02ce43d778d..af5e09f08130 100644
> --- a/drivers/hv/hv_balloon.c
> +++ b/drivers/hv/hv_balloon.c
> @@ -533,7 +533,6 @@ struct hv_dynmem_device {
> * State to synchronize hot-add.
> */
> struct completion ol_waitevent;
> - bool ha_waiting;
> /*
> * This thread handles hot-add
> * requests from the host as well as notifying
> @@ -634,10 +633,7 @@ static int hv_memory_notifier(struct notifier_block *nb, unsigned long val,
> switch (val) {
> case MEM_ONLINE:
> case MEM_CANCEL_ONLINE:
> - if (dm_device.ha_waiting) {
> - dm_device.ha_waiting = false;
> - complete(&dm_device.ol_waitevent);
> - }
> + complete(&dm_device.ol_waitevent);
> break;
>
> case MEM_OFFLINE:
> @@ -726,8 +722,7 @@ static void hv_mem_hot_add(unsigned long start, unsigned long size,
> has->covered_end_pfn += processed_pfn;
> spin_unlock_irqrestore(&dm_device.ha_lock, flags);
>
> - init_completion(&dm_device.ol_waitevent);
> - dm_device.ha_waiting = !memhp_auto_online;
> + reinit_completion(&dm_device.ol_waitevent);
>
> nid = memory_add_physaddr_to_nid(PFN_PHYS(start_pfn));
> ret = add_memory(nid, PFN_PHYS((start_pfn)),
> @@ -753,15 +748,14 @@ static void hv_mem_hot_add(unsigned long start, unsigned long size,
> }
>
> /*
> - * Wait for the memory block to be onlined when memory onlining
> - * is done outside of kernel (memhp_auto_online). Since the hot
> - * add has succeeded, it is ok to proceed even if the pages in
> - * the hot added region have not been "onlined" within the
> - * allowed time.
> + * Wait for memory to get onlined. If the kernel onlined the
> + * memory when adding it, this will return directly. Otherwise,
> + * it will wait for user space to online the memory. This helps
> + * to avoid adding memory faster than it is getting onlined. As
> + * adding succeeded, it is ok to proceed even if the memory was
> + * not onlined in time.
> */
> - if (dm_device.ha_waiting)
> - wait_for_completion_timeout(&dm_device.ol_waitevent,
> - 5*HZ);
> + wait_for_completion_timeout(&dm_device.ol_waitevent, 5 * HZ);
> post_status(&dm_device);
> }
> }
> @@ -1707,6 +1701,7 @@ static int balloon_probe(struct hv_device *dev,
> #ifdef CONFIG_MEMORY_HOTPLUG
> set_online_page_callback(&hv_online_page);
> register_memory_notifier(&hv_memory_nb);
> + init_completion(&dm_device.ol_waitevent);
> #endif
>
> hv_set_drvdata(dev, &dm_device);
Reviewed-by: Vitaly Kuznetsov <vkuznets@redhat.com>
--
Vitaly
^ permalink raw reply
* [PATCH 3/4] x86/Hyper-V: Trigger crash enlightenment only once during system crash.
From: ltykernel @ 2020-03-17 13:25 UTC (permalink / raw)
To: kys, haiyangz, sthemmin, liuwe, tglx, mingo, bp, hpa, x86,
michael.h.kelley
Cc: Tianyu Lan, linux-hyperv, linux-kernel, vkuznets
In-Reply-To: <20200317132523.1508-1-Tianyu.Lan@microsoft.com>
From: Tianyu Lan <Tianyu.Lan@microsoft.com>
Hyper-V expects guest only triggers crash enlightenment.
The second crash notify will be ignored by Hyper-V.
Current code may trigger crash enlightenment during system
panic twice.
1) The enlightenment is triggered in hyperv_panic/die_event()
via hyperv_report_panic().
2) hv_kmsg_dump() reports kmsg to host via hyperv_report_panic_msg().
Fix it. If kmsg dump is registered successfully, just report
kmsg via hyperv_report_panic_msg() and not report register values
via hyperv_report_panic().
Signed-off-by: Tianyu Lan <Tianyu.Lan@microsoft.com>
---
drivers/hv/vmbus_drv.c | 14 ++++++++++++--
1 file changed, 12 insertions(+), 2 deletions(-)
diff --git a/drivers/hv/vmbus_drv.c b/drivers/hv/vmbus_drv.c
index b043efea092a..1787d6246251 100644
--- a/drivers/hv/vmbus_drv.c
+++ b/drivers/hv/vmbus_drv.c
@@ -55,7 +55,12 @@ static int hyperv_panic_event(struct notifier_block *nb, unsigned long val,
vmbus_initiate_unload(true);
- if (ms_hyperv.misc_features & HV_FEATURE_GUEST_CRASH_MSR_AVAILABLE) {
+ /*
+ * Crash notify only can be triggered once. If crash notify
+ * message is available, just report kmsg to crash buffer.
+ */
+ if (ms_hyperv.misc_features & HV_FEATURE_GUEST_CRASH_MSR_AVAILABLE
+ && !hv_panic_page) {
regs = current_pt_regs();
hyperv_report_panic(regs, val);
}
@@ -68,7 +73,12 @@ static int hyperv_die_event(struct notifier_block *nb, unsigned long val,
struct die_args *die = (struct die_args *)args;
struct pt_regs *regs = die->regs;
- hyperv_report_panic(regs, val);
+ /*
+ * Crash notify only can be triggered once. If crash notify
+ * message is available, just report kmsg to crash buffer.
+ */
+ if (!hv_panic_page)
+ hyperv_report_panic(regs, val);
return NOTIFY_DONE;
}
--
2.14.5
^ permalink raw reply related
* [PATCH 2/4] x86/Hyper-V: Free hv_panic_page when fail to register kmsg dump
From: ltykernel @ 2020-03-17 13:25 UTC (permalink / raw)
To: kys, haiyangz, sthemmin, liuwe, tglx, mingo, bp, hpa, x86,
michael.h.kelley
Cc: Tianyu Lan, linux-hyperv, linux-kernel, vkuznets
In-Reply-To: <20200317132523.1508-1-Tianyu.Lan@microsoft.com>
From: Tianyu Lan <Tianyu.Lan@microsoft.com>
If fail to register kmsg dump on Hyper-V platform, hv_panic_page
will not be used anywhere. So free and reset it.
Signed-off-by: Tianyu Lan <Tianyu.Lan@microsoft.com>
---
drivers/hv/vmbus_drv.c | 6 +++++-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/drivers/hv/vmbus_drv.c b/drivers/hv/vmbus_drv.c
index b56b9fb9bd90..b043efea092a 100644
--- a/drivers/hv/vmbus_drv.c
+++ b/drivers/hv/vmbus_drv.c
@@ -1385,9 +1385,13 @@ static int vmbus_bus_init(void)
hv_panic_page = (void *)hv_alloc_hyperv_zeroed_page();
if (hv_panic_page) {
ret = kmsg_dump_register(&hv_kmsg_dumper);
- if (ret)
+ if (ret) {
pr_err("Hyper-V: kmsg dump register "
"error 0x%x\n", ret);
+ hv_free_hyperv_page(
+ (unsigned long)hv_panic_page);
+ hv_panic_page = NULL;
+ }
} else
pr_err("Hyper-V: panic message page memory "
"allocation failed");
--
2.14.5
^ permalink raw reply related
* [PATCH 0/4] x86/Hyper-V: Unload vmbus channel in hv panic callback
From: ltykernel @ 2020-03-17 13:25 UTC (permalink / raw)
To: kys, haiyangz, sthemmin, liuwe, tglx, mingo, bp, hpa, x86,
michael.h.kelley
Cc: Tianyu Lan, linux-hyperv, linux-kernel, vkuznets
In-Reply-To: <20200317132523.1508-1-Tianyu.Lan@microsoft.com>
From: Tianyu Lan <Tianyu.Lan@microsoft.com>
Customer reported Hyper-V VM still responded network traffic
ack packets after kernel panic with kernel parameter "panic=0”.
This becauses vmbus driver interrupt handler still works
on the panic cpu after kernel panic. Panic cpu falls into
infinite loop of panic() with interrupt enabled at that point.
Vmbus driver can still handle network traffic.
This confuses remote service that the panic system is still
alive when it gets ack packets. Unload vmbus channel in hv panic
callback and fix it.
vmbus_initiate_unload() maybe double called during panic process
(e.g, hyperv_panic_event() and hv_crash_handler()). So check
and set connection state in vmbus_initiate_unload() to resolve
reenter issue.
Signed-off-by: Tianyu Lan <Tianyu.Lan@microsoft.com>
---
drivers/hv/channel_mgmt.c | 5 +++++
drivers/hv/vmbus_drv.c | 17 +++++++++--------
2 files changed, 14 insertions(+), 8 deletions(-)
diff --git a/drivers/hv/channel_mgmt.c b/drivers/hv/channel_mgmt.c
index 0370364169c4..893493f2b420 100644
--- a/drivers/hv/channel_mgmt.c
+++ b/drivers/hv/channel_mgmt.c
@@ -839,6 +839,9 @@ void vmbus_initiate_unload(bool crash)
{
struct vmbus_channel_message_header hdr;
+ if (vmbus_connection.conn_state == DISCONNECTED)
+ return;
+
/* Pre-Win2012R2 hosts don't support reconnect */
if (vmbus_proto_version < VERSION_WIN8_1)
return;
@@ -857,6 +860,8 @@ void vmbus_initiate_unload(bool crash)
wait_for_completion(&vmbus_connection.unload_event);
else
vmbus_wait_for_unload();
+
+ vmbus_connection.conn_state = DISCONNECTED;
}
static void check_ready_for_resume_event(void)
diff --git a/drivers/hv/vmbus_drv.c b/drivers/hv/vmbus_drv.c
index 029378c27421..b56b9fb9bd90 100644
--- a/drivers/hv/vmbus_drv.c
+++ b/drivers/hv/vmbus_drv.c
@@ -53,9 +53,12 @@ static int hyperv_panic_event(struct notifier_block *nb, unsigned long val,
{
struct pt_regs *regs;
- regs = current_pt_regs();
+ vmbus_initiate_unload(true);
- hyperv_report_panic(regs, val);
+ if (ms_hyperv.misc_features & HV_FEATURE_GUEST_CRASH_MSR_AVAILABLE) {
+ regs = current_pt_regs();
+ hyperv_report_panic(regs, val);
+ }
return NOTIFY_DONE;
}
@@ -1391,10 +1394,12 @@ static int vmbus_bus_init(void)
}
register_die_notifier(&hyperv_die_block);
- atomic_notifier_chain_register(&panic_notifier_list,
- &hyperv_panic_block);
}
+ /* Vmbus channel is unloaded in panic callback when panic happens.*/
+ atomic_notifier_chain_register(&panic_notifier_list,
+ &hyperv_panic_block);
+
vmbus_request_offers();
return 0;
@@ -2204,8 +2209,6 @@ static int vmbus_bus_suspend(struct device *dev)
vmbus_initiate_unload(false);
- vmbus_connection.conn_state = DISCONNECTED;
-
/* Reset the event for the next resume. */
reinit_completion(&vmbus_connection.ready_for_resume_event);
@@ -2289,7 +2292,6 @@ static void hv_kexec_handler(void)
{
hv_stimer_global_cleanup();
vmbus_initiate_unload(false);
- vmbus_connection.conn_state = DISCONNECTED;
/* Make sure conn_state is set as hv_synic_cleanup checks for it */
mb();
cpuhp_remove_state(hyperv_cpuhp_online);
@@ -2306,7 +2308,6 @@ static void hv_crash_handler(struct pt_regs *regs)
* doing the cleanup for current CPU only. This should be sufficient
* for kdump.
*/
- vmbus_connection.conn_state = DISCONNECTED;
cpu = smp_processor_id();
hv_stimer_cleanup(cpu);
hv_synic_disable_regs(cpu);
--
2.14.5
^ permalink raw reply related
* [PATCH 4/4] x86/Hyper-V: Report crash register data or ksmg before running crash kernel
From: ltykernel @ 2020-03-17 13:25 UTC (permalink / raw)
To: kys, haiyangz, sthemmin, liuwe, tglx, mingo, bp, hpa, x86,
michael.h.kelley
Cc: Tianyu Lan, linux-hyperv, linux-kernel, vkuznets
In-Reply-To: <20200317132523.1508-1-Tianyu.Lan@microsoft.com>
From: Tianyu Lan <Tianyu.Lan@microsoft.com>
Hyper-V expects to get crash register data or kmsg via crash
enlightenment when guest crash happens. crash_kexec_post_notifiers
is default to be false and crash kernel runs before calling hv
panic callback and dumping kmsg. In this case, Hyper-V doesn't
get crash register data or kmsg from guest. Set crash_kexec_post
_notifiers to be true for Hyper-V VM and fix it.
Signed-off-by: Tianyu Lan <Tianyu.Lan@microsoft.com>
---
arch/x86/kernel/cpu/mshyperv.c | 10 ++++++++++
1 file changed, 10 insertions(+)
diff --git a/arch/x86/kernel/cpu/mshyperv.c b/arch/x86/kernel/cpu/mshyperv.c
index caa032ce3fe3..5e296a7e6036 100644
--- a/arch/x86/kernel/cpu/mshyperv.c
+++ b/arch/x86/kernel/cpu/mshyperv.c
@@ -263,6 +263,16 @@ static void __init ms_hyperv_init_platform(void)
cpuid_eax(HYPERV_CPUID_NESTED_FEATURES);
}
+ /*
+ * Hyper-V expects to get crash register data or kmsg when
+ * crash enlightment is available and system crashes. Set
+ * crash_kexec_post_notifiers to be true to make sure that
+ * calling crash enlightment interface before running kdump
+ * kernel.
+ */
+ if (ms_hyperv.misc_features & HV_FEATURE_GUEST_CRASH_MSR_AVAILABLE)
+ crash_kexec_post_notifiers = true;
+
#ifdef CONFIG_X86_LOCAL_APIC
if (ms_hyperv.features & HV_X64_ACCESS_FREQUENCY_MSRS &&
ms_hyperv.misc_features & HV_FEATURE_FREQUENCY_MSRS_AVAILABLE) {
--
2.14.5
^ permalink raw reply related
* [PATCH 0/4] x86/Hyper-V: Panic code path fixes
From: ltykernel @ 2020-03-17 13:25 UTC (permalink / raw)
To: kys, haiyangz, sthemmin, liuwe, tglx, mingo, bp, hpa, x86,
michael.h.kelley
Cc: Tianyu Lan, linux-hyperv, linux-kernel, vkuznets
From: Tianyu Lan <Tianyu.Lan@microsoft.com>
This patchset fixes some issues in the Hyper-V panic code path.
Patch 1 resolves issue that panic system still responses network
packets.
Patch 2-3 resolves crash enlightenment issues.
Patch 4 is to set crash_kexec_post_notifiers to true for Hyper-V
VM in order to report crash data or kmsg to host before running
kdump kernel.
Tianyu Lan (4):
x86/Hyper-V: Unload vmbus channel in hv panic callback
x86/Hyper-V: Free hv_panic_page when fail to register kmsg dump
x86/Hyper-V: Trigger crash enlightenment only once during system
crash.
x86/Hyper-V: Report crash register data or ksmg before running crash
kernel
arch/x86/kernel/cpu/mshyperv.c | 10 ++++++++++
drivers/hv/channel_mgmt.c | 5 +++++
drivers/hv/vmbus_drv.c | 35 +++++++++++++++++++++++++----------
3 files changed, 40 insertions(+), 10 deletions(-)
--
2.14.5
^ permalink raw reply
* Re: [PATCH v2 6/8] mm/memory_hotplug: unexport memhp_auto_online
From: Michal Hocko @ 2020-03-17 10:59 UTC (permalink / raw)
To: David Hildenbrand
Cc: linux-kernel, linux-mm, linuxppc-dev, linux-hyperv, Andrew Morton,
Oscar Salvador, Rafael J. Wysocki, Baoquan He, Wei Yang
In-Reply-To: <20200317104942.11178-7-david@redhat.com>
On Tue 17-03-20 11:49:40, David Hildenbrand wrote:
> All in-tree users except the mm-core are gone. Let's drop the export.
>
> Cc: Andrew Morton <akpm@linux-foundation.org>
> Cc: Michal Hocko <mhocko@kernel.org>
> Cc: Oscar Salvador <osalvador@suse.de>
> Cc: "Rafael J. Wysocki" <rafael@kernel.org>
> Cc: Baoquan He <bhe@redhat.com>
> Cc: Wei Yang <richard.weiyang@gmail.com>
> Signed-off-by: David Hildenbrand <david@redhat.com>
Acked-by: Michal Hocko <mhocko@suse.com>
> ---
> mm/memory_hotplug.c | 1 -
> 1 file changed, 1 deletion(-)
>
> diff --git a/mm/memory_hotplug.c b/mm/memory_hotplug.c
> index 1a00b5a37ef6..2d2aae830b92 100644
> --- a/mm/memory_hotplug.c
> +++ b/mm/memory_hotplug.c
> @@ -71,7 +71,6 @@ bool memhp_auto_online;
> #else
> bool memhp_auto_online = true;
> #endif
> -EXPORT_SYMBOL_GPL(memhp_auto_online);
>
> static int __init setup_memhp_default_state(char *str)
> {
> --
> 2.24.1
--
Michal Hocko
SUSE Labs
^ permalink raw reply
* Re: [PATCH v2 7/8] mm/memory_hotplug: convert memhp_auto_online to store an online_type
From: Michal Hocko @ 2020-03-17 11:00 UTC (permalink / raw)
To: David Hildenbrand
Cc: linux-kernel, linux-mm, linuxppc-dev, linux-hyperv, Wei Yang,
Greg Kroah-Hartman, Andrew Morton, Oscar Salvador,
Rafael J. Wysocki, Baoquan He
In-Reply-To: <20200317104942.11178-8-david@redhat.com>
On Tue 17-03-20 11:49:41, David Hildenbrand wrote:
> ... and rename it to memhp_default_online_type. This is a preparation
> for more detailed default online behavior.
>
> Reviewed-by: Wei Yang <richard.weiyang@gmail.com>
> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Cc: Andrew Morton <akpm@linux-foundation.org>
> Cc: Michal Hocko <mhocko@kernel.org>
> Cc: Oscar Salvador <osalvador@suse.de>
> Cc: "Rafael J. Wysocki" <rafael@kernel.org>
> Cc: Baoquan He <bhe@redhat.com>
> Cc: Wei Yang <richard.weiyang@gmail.com>
> Signed-off-by: David Hildenbrand <david@redhat.com>
Acked-by: Michal Hocko <mhocko@suse.com>
> ---
> drivers/base/memory.c | 10 ++++------
> include/linux/memory_hotplug.h | 3 ++-
> mm/memory_hotplug.c | 11 ++++++-----
> 3 files changed, 12 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/base/memory.c b/drivers/base/memory.c
> index 8a7f29c0bf97..8d3e16dab69f 100644
> --- a/drivers/base/memory.c
> +++ b/drivers/base/memory.c
> @@ -386,10 +386,8 @@ static DEVICE_ATTR_RO(block_size_bytes);
> static ssize_t auto_online_blocks_show(struct device *dev,
> struct device_attribute *attr, char *buf)
> {
> - if (memhp_auto_online)
> - return sprintf(buf, "online\n");
> - else
> - return sprintf(buf, "offline\n");
> + return sprintf(buf, "%s\n",
> + online_type_to_str[memhp_default_online_type]);
> }
>
> static ssize_t auto_online_blocks_store(struct device *dev,
> @@ -397,9 +395,9 @@ static ssize_t auto_online_blocks_store(struct device *dev,
> const char *buf, size_t count)
> {
> if (sysfs_streq(buf, "online"))
> - memhp_auto_online = true;
> + memhp_default_online_type = MMOP_ONLINE;
> else if (sysfs_streq(buf, "offline"))
> - memhp_auto_online = false;
> + memhp_default_online_type = MMOP_OFFLINE;
> else
> return -EINVAL;
>
> diff --git a/include/linux/memory_hotplug.h b/include/linux/memory_hotplug.h
> index c2e06ed5e0e9..c6e090b34c4b 100644
> --- a/include/linux/memory_hotplug.h
> +++ b/include/linux/memory_hotplug.h
> @@ -117,7 +117,8 @@ extern int arch_add_memory(int nid, u64 start, u64 size,
> struct mhp_restrictions *restrictions);
> extern u64 max_mem_size;
>
> -extern bool memhp_auto_online;
> +/* Default online_type (MMOP_*) when new memory blocks are added. */
> +extern int memhp_default_online_type;
> /* If movable_node boot option specified */
> extern bool movable_node_enabled;
> static inline bool movable_node_is_enabled(void)
> diff --git a/mm/memory_hotplug.c b/mm/memory_hotplug.c
> index 2d2aae830b92..1975a2b99a2b 100644
> --- a/mm/memory_hotplug.c
> +++ b/mm/memory_hotplug.c
> @@ -67,17 +67,17 @@ void put_online_mems(void)
> bool movable_node_enabled = false;
>
> #ifndef CONFIG_MEMORY_HOTPLUG_DEFAULT_ONLINE
> -bool memhp_auto_online;
> +int memhp_default_online_type = MMOP_OFFLINE;
> #else
> -bool memhp_auto_online = true;
> +int memhp_default_online_type = MMOP_ONLINE;
> #endif
>
> static int __init setup_memhp_default_state(char *str)
> {
> if (!strcmp(str, "online"))
> - memhp_auto_online = true;
> + memhp_default_online_type = MMOP_ONLINE;
> else if (!strcmp(str, "offline"))
> - memhp_auto_online = false;
> + memhp_default_online_type = MMOP_OFFLINE;
>
> return 1;
> }
> @@ -990,6 +990,7 @@ static int check_hotplug_memory_range(u64 start, u64 size)
>
> static int online_memory_block(struct memory_block *mem, void *arg)
> {
> + mem->online_type = memhp_default_online_type;
> return device_online(&mem->dev);
> }
>
> @@ -1062,7 +1063,7 @@ int __ref add_memory_resource(int nid, struct resource *res)
> mem_hotplug_done();
>
> /* online pages if requested */
> - if (memhp_auto_online)
> + if (memhp_default_online_type != MMOP_OFFLINE)
> walk_memory_blocks(start, size, NULL, online_memory_block);
>
> return ret;
> --
> 2.24.1
--
Michal Hocko
SUSE Labs
^ permalink raw reply
* Re: [PATCH v2 8/8] mm/memory_hotplug: allow to specify a default online_type
From: Michal Hocko @ 2020-03-17 11:01 UTC (permalink / raw)
To: David Hildenbrand
Cc: linux-kernel, linux-mm, linuxppc-dev, linux-hyperv, Wei Yang,
Greg Kroah-Hartman, Andrew Morton, Oscar Salvador,
Rafael J. Wysocki, Baoquan He
In-Reply-To: <20200317104942.11178-9-david@redhat.com>
On Tue 17-03-20 11:49:42, David Hildenbrand wrote:
> For now, distributions implement advanced udev rules to essentially
> - Don't online any hotplugged memory (s390x)
> - Online all memory to ZONE_NORMAL (e.g., most virt environments like
> hyperv)
> - Online all memory to ZONE_MOVABLE in case the zone imbalance is taken
> care of (e.g., bare metal, special virt environments)
>
> In summary: All memory is usually onlined the same way, however, the
> kernel always has to ask user space to come up with the same answer.
> E.g., Hyper-V always waits for a memory block to get onlined before
> continuing, otherwise it might end up adding memory faster than
> hotplugging it, which can result in strange OOM situations. This waiting
> slows down adding of a bigger amount of memory.
>
> Let's allow to specify a default online_type, not just "online" and
> "offline". This allows distributions to configure the default online_type
> when booting up and be done with it.
>
> We can now specify "offline", "online", "online_movable" and
> "online_kernel" via
> - "memhp_default_state=" on the kernel cmdline
> - /sys/devices/system/memory/auto_online_blocks
> just like we are able to specify for a single memory block via
> /sys/devices/system/memory/memoryX/state
>
> Reviewed-by: Wei Yang <richard.weiyang@gmail.com>
> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Cc: Andrew Morton <akpm@linux-foundation.org>
> Cc: Michal Hocko <mhocko@kernel.org>
> Cc: Oscar Salvador <osalvador@suse.de>
> Cc: "Rafael J. Wysocki" <rafael@kernel.org>
> Cc: Baoquan He <bhe@redhat.com>
> Cc: Wei Yang <richard.weiyang@gmail.com>
> Signed-off-by: David Hildenbrand <david@redhat.com>
As I've said earlier and several times already, I really dislike this
interface. But it is fact that this patch doesn't make it any worse.
Quite contrary, so feel free to add
Acked-by: Michal Hocko <mhocko@suse.com>
> ---
> drivers/base/memory.c | 11 +++++------
> include/linux/memory_hotplug.h | 2 ++
> mm/memory_hotplug.c | 8 ++++----
> 3 files changed, 11 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/base/memory.c b/drivers/base/memory.c
> index 8d3e16dab69f..2b09b68b9f78 100644
> --- a/drivers/base/memory.c
> +++ b/drivers/base/memory.c
> @@ -35,7 +35,7 @@ static const char *const online_type_to_str[] = {
> [MMOP_ONLINE_MOVABLE] = "online_movable",
> };
>
> -static int memhp_online_type_from_str(const char *str)
> +int memhp_online_type_from_str(const char *str)
> {
> int i;
>
> @@ -394,13 +394,12 @@ static ssize_t auto_online_blocks_store(struct device *dev,
> struct device_attribute *attr,
> const char *buf, size_t count)
> {
> - if (sysfs_streq(buf, "online"))
> - memhp_default_online_type = MMOP_ONLINE;
> - else if (sysfs_streq(buf, "offline"))
> - memhp_default_online_type = MMOP_OFFLINE;
> - else
> + const int online_type = memhp_online_type_from_str(buf);
> +
> + if (online_type < 0)
> return -EINVAL;
>
> + memhp_default_online_type = online_type;
> return count;
> }
>
> diff --git a/include/linux/memory_hotplug.h b/include/linux/memory_hotplug.h
> index c6e090b34c4b..ef55115320fb 100644
> --- a/include/linux/memory_hotplug.h
> +++ b/include/linux/memory_hotplug.h
> @@ -117,6 +117,8 @@ extern int arch_add_memory(int nid, u64 start, u64 size,
> struct mhp_restrictions *restrictions);
> extern u64 max_mem_size;
>
> +extern int memhp_online_type_from_str(const char *str);
> +
> /* Default online_type (MMOP_*) when new memory blocks are added. */
> extern int memhp_default_online_type;
> /* If movable_node boot option specified */
> diff --git a/mm/memory_hotplug.c b/mm/memory_hotplug.c
> index 1975a2b99a2b..9916977b6ee1 100644
> --- a/mm/memory_hotplug.c
> +++ b/mm/memory_hotplug.c
> @@ -74,10 +74,10 @@ int memhp_default_online_type = MMOP_ONLINE;
>
> static int __init setup_memhp_default_state(char *str)
> {
> - if (!strcmp(str, "online"))
> - memhp_default_online_type = MMOP_ONLINE;
> - else if (!strcmp(str, "offline"))
> - memhp_default_online_type = MMOP_OFFLINE;
> + const int online_type = memhp_online_type_from_str(str);
> +
> + if (online_type >= 0)
> + memhp_default_online_type = online_type;
>
> return 1;
> }
> --
> 2.24.1
--
Michal Hocko
SUSE Labs
^ permalink raw reply
* Re: [PATCH v2 8/8] mm/memory_hotplug: allow to specify a default online_type
From: David Hildenbrand @ 2020-03-17 11:08 UTC (permalink / raw)
To: linux-kernel
Cc: linux-mm, linuxppc-dev, linux-hyperv, Wei Yang,
Greg Kroah-Hartman, Andrew Morton, Michal Hocko, Oscar Salvador,
Rafael J. Wysocki, Baoquan He
In-Reply-To: <20200317104942.11178-9-david@redhat.com>
On 17.03.20 11:49, David Hildenbrand wrote:
> For now, distributions implement advanced udev rules to essentially
> - Don't online any hotplugged memory (s390x)
> - Online all memory to ZONE_NORMAL (e.g., most virt environments like
> hyperv)
> - Online all memory to ZONE_MOVABLE in case the zone imbalance is taken
> care of (e.g., bare metal, special virt environments)
>
> In summary: All memory is usually onlined the same way, however, the
> kernel always has to ask user space to come up with the same answer.
> E.g., Hyper-V always waits for a memory block to get onlined before
> continuing, otherwise it might end up adding memory faster than
> hotplugging it, which can result in strange OOM situations. This waiting
s/hotplugging/onlining/
--
Thanks,
David / dhildenb
^ permalink raw reply
* Re: [PATCH v2 8/8] mm/memory_hotplug: allow to specify a default online_type
From: David Hildenbrand @ 2020-03-17 11:05 UTC (permalink / raw)
To: Michal Hocko
Cc: linux-kernel, linux-mm, linuxppc-dev, linux-hyperv, Wei Yang,
Greg Kroah-Hartman, Andrew Morton, Oscar Salvador,
Rafael J. Wysocki, Baoquan He
In-Reply-To: <20200317110121.GN26018@dhcp22.suse.cz>
On 17.03.20 12:01, Michal Hocko wrote:
> On Tue 17-03-20 11:49:42, David Hildenbrand wrote:
>> For now, distributions implement advanced udev rules to essentially
>> - Don't online any hotplugged memory (s390x)
>> - Online all memory to ZONE_NORMAL (e.g., most virt environments like
>> hyperv)
>> - Online all memory to ZONE_MOVABLE in case the zone imbalance is taken
>> care of (e.g., bare metal, special virt environments)
>>
>> In summary: All memory is usually onlined the same way, however, the
>> kernel always has to ask user space to come up with the same answer.
>> E.g., Hyper-V always waits for a memory block to get onlined before
>> continuing, otherwise it might end up adding memory faster than
>> hotplugging it, which can result in strange OOM situations. This waiting
>> slows down adding of a bigger amount of memory.
>>
>> Let's allow to specify a default online_type, not just "online" and
>> "offline". This allows distributions to configure the default online_type
>> when booting up and be done with it.
>>
>> We can now specify "offline", "online", "online_movable" and
>> "online_kernel" via
>> - "memhp_default_state=" on the kernel cmdline
>> - /sys/devices/system/memory/auto_online_blocks
>> just like we are able to specify for a single memory block via
>> /sys/devices/system/memory/memoryX/state
>>
>> Reviewed-by: Wei Yang <richard.weiyang@gmail.com>
>> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
>> Cc: Andrew Morton <akpm@linux-foundation.org>
>> Cc: Michal Hocko <mhocko@kernel.org>
>> Cc: Oscar Salvador <osalvador@suse.de>
>> Cc: "Rafael J. Wysocki" <rafael@kernel.org>
>> Cc: Baoquan He <bhe@redhat.com>
>> Cc: Wei Yang <richard.weiyang@gmail.com>
>> Signed-off-by: David Hildenbrand <david@redhat.com>
>
> As I've said earlier and several times already, I really dislike this
> interface. But it is fact that this patch doesn't make it any worse.
> Quite contrary, so feel free to add
> Acked-by: Michal Hocko <mhocko@suse.com>
Thanks Michal!
--
Thanks,
David / dhildenb
^ permalink raw reply
* Re: [PATCH v2 4/8] powernv/memtrace: always online added memory blocks
From: Michal Hocko @ 2020-03-17 10:58 UTC (permalink / raw)
To: David Hildenbrand
Cc: linux-kernel, linux-mm, linuxppc-dev, linux-hyperv,
Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman,
Andrew Morton, Greg Kroah-Hartman, Oscar Salvador,
Rafael J. Wysocki, Baoquan He, Wei Yang
In-Reply-To: <20200317104942.11178-5-david@redhat.com>
On Tue 17-03-20 11:49:38, David Hildenbrand wrote:
> Let's always try to online the re-added memory blocks. In case add_memory()
> already onlined the added memory blocks, the first device_online() call
> will fail and stop processing the remaining memory blocks.
>
> This avoids manually having to check memhp_auto_online.
>
> Note: PPC always onlines all hotplugged memory directly from the kernel
> as well - something that is handled by user space on other
> architectures.
>
> Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> Cc: Paul Mackerras <paulus@samba.org>
> Cc: Michael Ellerman <mpe@ellerman.id.au>
> Cc: Andrew Morton <akpm@linux-foundation.org>
> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Cc: Michal Hocko <mhocko@kernel.org>
> Cc: Oscar Salvador <osalvador@suse.de>
> Cc: "Rafael J. Wysocki" <rafael@kernel.org>
> Cc: Baoquan He <bhe@redhat.com>
> Cc: Wei Yang <richard.weiyang@gmail.com>
> Cc: linuxppc-dev@lists.ozlabs.org
> Signed-off-by: David Hildenbrand <david@redhat.com>
Acked-by: Michal Hocko <mhocko@suse.com>
> ---
> arch/powerpc/platforms/powernv/memtrace.c | 14 ++++----------
> 1 file changed, 4 insertions(+), 10 deletions(-)
>
> diff --git a/arch/powerpc/platforms/powernv/memtrace.c b/arch/powerpc/platforms/powernv/memtrace.c
> index d6d64f8718e6..13b369d2cc45 100644
> --- a/arch/powerpc/platforms/powernv/memtrace.c
> +++ b/arch/powerpc/platforms/powernv/memtrace.c
> @@ -231,16 +231,10 @@ static int memtrace_online(void)
> continue;
> }
>
> - /*
> - * If kernel isn't compiled with the auto online option
> - * we need to online the memory ourselves.
> - */
> - if (!memhp_auto_online) {
> - lock_device_hotplug();
> - walk_memory_blocks(ent->start, ent->size, NULL,
> - online_mem_block);
> - unlock_device_hotplug();
> - }
> + lock_device_hotplug();
> + walk_memory_blocks(ent->start, ent->size, NULL,
> + online_mem_block);
> + unlock_device_hotplug();
>
> /*
> * Memory was added successfully so clean up references to it
> --
> 2.24.1
--
Michal Hocko
SUSE Labs
^ permalink raw reply
* [PATCH v2 4/8] powernv/memtrace: always online added memory blocks
From: David Hildenbrand @ 2020-03-17 10:49 UTC (permalink / raw)
To: linux-kernel
Cc: linux-mm, linuxppc-dev, linux-hyperv, David Hildenbrand,
Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman,
Andrew Morton, Greg Kroah-Hartman, Michal Hocko, Oscar Salvador,
Rafael J. Wysocki, Baoquan He, Wei Yang
In-Reply-To: <20200317104942.11178-1-david@redhat.com>
Let's always try to online the re-added memory blocks. In case add_memory()
already onlined the added memory blocks, the first device_online() call
will fail and stop processing the remaining memory blocks.
This avoids manually having to check memhp_auto_online.
Note: PPC always onlines all hotplugged memory directly from the kernel
as well - something that is handled by user space on other
architectures.
Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Michael Ellerman <mpe@ellerman.id.au>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: Michal Hocko <mhocko@kernel.org>
Cc: Oscar Salvador <osalvador@suse.de>
Cc: "Rafael J. Wysocki" <rafael@kernel.org>
Cc: Baoquan He <bhe@redhat.com>
Cc: Wei Yang <richard.weiyang@gmail.com>
Cc: linuxppc-dev@lists.ozlabs.org
Signed-off-by: David Hildenbrand <david@redhat.com>
---
arch/powerpc/platforms/powernv/memtrace.c | 14 ++++----------
1 file changed, 4 insertions(+), 10 deletions(-)
diff --git a/arch/powerpc/platforms/powernv/memtrace.c b/arch/powerpc/platforms/powernv/memtrace.c
index d6d64f8718e6..13b369d2cc45 100644
--- a/arch/powerpc/platforms/powernv/memtrace.c
+++ b/arch/powerpc/platforms/powernv/memtrace.c
@@ -231,16 +231,10 @@ static int memtrace_online(void)
continue;
}
- /*
- * If kernel isn't compiled with the auto online option
- * we need to online the memory ourselves.
- */
- if (!memhp_auto_online) {
- lock_device_hotplug();
- walk_memory_blocks(ent->start, ent->size, NULL,
- online_mem_block);
- unlock_device_hotplug();
- }
+ lock_device_hotplug();
+ walk_memory_blocks(ent->start, ent->size, NULL,
+ online_mem_block);
+ unlock_device_hotplug();
/*
* Memory was added successfully so clean up references to it
--
2.24.1
^ permalink raw reply related
* [PATCH v2 8/8] mm/memory_hotplug: allow to specify a default online_type
From: David Hildenbrand @ 2020-03-17 10:49 UTC (permalink / raw)
To: linux-kernel
Cc: linux-mm, linuxppc-dev, linux-hyperv, David Hildenbrand, Wei Yang,
Greg Kroah-Hartman, Andrew Morton, Michal Hocko, Oscar Salvador,
Rafael J. Wysocki, Baoquan He
In-Reply-To: <20200317104942.11178-1-david@redhat.com>
For now, distributions implement advanced udev rules to essentially
- Don't online any hotplugged memory (s390x)
- Online all memory to ZONE_NORMAL (e.g., most virt environments like
hyperv)
- Online all memory to ZONE_MOVABLE in case the zone imbalance is taken
care of (e.g., bare metal, special virt environments)
In summary: All memory is usually onlined the same way, however, the
kernel always has to ask user space to come up with the same answer.
E.g., Hyper-V always waits for a memory block to get onlined before
continuing, otherwise it might end up adding memory faster than
hotplugging it, which can result in strange OOM situations. This waiting
slows down adding of a bigger amount of memory.
Let's allow to specify a default online_type, not just "online" and
"offline". This allows distributions to configure the default online_type
when booting up and be done with it.
We can now specify "offline", "online", "online_movable" and
"online_kernel" via
- "memhp_default_state=" on the kernel cmdline
- /sys/devices/system/memory/auto_online_blocks
just like we are able to specify for a single memory block via
/sys/devices/system/memory/memoryX/state
Reviewed-by: Wei Yang <richard.weiyang@gmail.com>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Michal Hocko <mhocko@kernel.org>
Cc: Oscar Salvador <osalvador@suse.de>
Cc: "Rafael J. Wysocki" <rafael@kernel.org>
Cc: Baoquan He <bhe@redhat.com>
Cc: Wei Yang <richard.weiyang@gmail.com>
Signed-off-by: David Hildenbrand <david@redhat.com>
---
drivers/base/memory.c | 11 +++++------
include/linux/memory_hotplug.h | 2 ++
mm/memory_hotplug.c | 8 ++++----
3 files changed, 11 insertions(+), 10 deletions(-)
diff --git a/drivers/base/memory.c b/drivers/base/memory.c
index 8d3e16dab69f..2b09b68b9f78 100644
--- a/drivers/base/memory.c
+++ b/drivers/base/memory.c
@@ -35,7 +35,7 @@ static const char *const online_type_to_str[] = {
[MMOP_ONLINE_MOVABLE] = "online_movable",
};
-static int memhp_online_type_from_str(const char *str)
+int memhp_online_type_from_str(const char *str)
{
int i;
@@ -394,13 +394,12 @@ static ssize_t auto_online_blocks_store(struct device *dev,
struct device_attribute *attr,
const char *buf, size_t count)
{
- if (sysfs_streq(buf, "online"))
- memhp_default_online_type = MMOP_ONLINE;
- else if (sysfs_streq(buf, "offline"))
- memhp_default_online_type = MMOP_OFFLINE;
- else
+ const int online_type = memhp_online_type_from_str(buf);
+
+ if (online_type < 0)
return -EINVAL;
+ memhp_default_online_type = online_type;
return count;
}
diff --git a/include/linux/memory_hotplug.h b/include/linux/memory_hotplug.h
index c6e090b34c4b..ef55115320fb 100644
--- a/include/linux/memory_hotplug.h
+++ b/include/linux/memory_hotplug.h
@@ -117,6 +117,8 @@ extern int arch_add_memory(int nid, u64 start, u64 size,
struct mhp_restrictions *restrictions);
extern u64 max_mem_size;
+extern int memhp_online_type_from_str(const char *str);
+
/* Default online_type (MMOP_*) when new memory blocks are added. */
extern int memhp_default_online_type;
/* If movable_node boot option specified */
diff --git a/mm/memory_hotplug.c b/mm/memory_hotplug.c
index 1975a2b99a2b..9916977b6ee1 100644
--- a/mm/memory_hotplug.c
+++ b/mm/memory_hotplug.c
@@ -74,10 +74,10 @@ int memhp_default_online_type = MMOP_ONLINE;
static int __init setup_memhp_default_state(char *str)
{
- if (!strcmp(str, "online"))
- memhp_default_online_type = MMOP_ONLINE;
- else if (!strcmp(str, "offline"))
- memhp_default_online_type = MMOP_OFFLINE;
+ const int online_type = memhp_online_type_from_str(str);
+
+ if (online_type >= 0)
+ memhp_default_online_type = online_type;
return 1;
}
--
2.24.1
^ permalink raw reply related
* [PATCH v2 6/8] mm/memory_hotplug: unexport memhp_auto_online
From: David Hildenbrand @ 2020-03-17 10:49 UTC (permalink / raw)
To: linux-kernel
Cc: linux-mm, linuxppc-dev, linux-hyperv, David Hildenbrand,
Andrew Morton, Michal Hocko, Oscar Salvador, Rafael J. Wysocki,
Baoquan He, Wei Yang
In-Reply-To: <20200317104942.11178-1-david@redhat.com>
All in-tree users except the mm-core are gone. Let's drop the export.
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Michal Hocko <mhocko@kernel.org>
Cc: Oscar Salvador <osalvador@suse.de>
Cc: "Rafael J. Wysocki" <rafael@kernel.org>
Cc: Baoquan He <bhe@redhat.com>
Cc: Wei Yang <richard.weiyang@gmail.com>
Signed-off-by: David Hildenbrand <david@redhat.com>
---
mm/memory_hotplug.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/mm/memory_hotplug.c b/mm/memory_hotplug.c
index 1a00b5a37ef6..2d2aae830b92 100644
--- a/mm/memory_hotplug.c
+++ b/mm/memory_hotplug.c
@@ -71,7 +71,6 @@ bool memhp_auto_online;
#else
bool memhp_auto_online = true;
#endif
-EXPORT_SYMBOL_GPL(memhp_auto_online);
static int __init setup_memhp_default_state(char *str)
{
--
2.24.1
^ permalink raw reply related
* [PATCH v2 7/8] mm/memory_hotplug: convert memhp_auto_online to store an online_type
From: David Hildenbrand @ 2020-03-17 10:49 UTC (permalink / raw)
To: linux-kernel
Cc: linux-mm, linuxppc-dev, linux-hyperv, David Hildenbrand, Wei Yang,
Greg Kroah-Hartman, Andrew Morton, Michal Hocko, Oscar Salvador,
Rafael J. Wysocki, Baoquan He
In-Reply-To: <20200317104942.11178-1-david@redhat.com>
... and rename it to memhp_default_online_type. This is a preparation
for more detailed default online behavior.
Reviewed-by: Wei Yang <richard.weiyang@gmail.com>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Michal Hocko <mhocko@kernel.org>
Cc: Oscar Salvador <osalvador@suse.de>
Cc: "Rafael J. Wysocki" <rafael@kernel.org>
Cc: Baoquan He <bhe@redhat.com>
Cc: Wei Yang <richard.weiyang@gmail.com>
Signed-off-by: David Hildenbrand <david@redhat.com>
---
drivers/base/memory.c | 10 ++++------
include/linux/memory_hotplug.h | 3 ++-
mm/memory_hotplug.c | 11 ++++++-----
3 files changed, 12 insertions(+), 12 deletions(-)
diff --git a/drivers/base/memory.c b/drivers/base/memory.c
index 8a7f29c0bf97..8d3e16dab69f 100644
--- a/drivers/base/memory.c
+++ b/drivers/base/memory.c
@@ -386,10 +386,8 @@ static DEVICE_ATTR_RO(block_size_bytes);
static ssize_t auto_online_blocks_show(struct device *dev,
struct device_attribute *attr, char *buf)
{
- if (memhp_auto_online)
- return sprintf(buf, "online\n");
- else
- return sprintf(buf, "offline\n");
+ return sprintf(buf, "%s\n",
+ online_type_to_str[memhp_default_online_type]);
}
static ssize_t auto_online_blocks_store(struct device *dev,
@@ -397,9 +395,9 @@ static ssize_t auto_online_blocks_store(struct device *dev,
const char *buf, size_t count)
{
if (sysfs_streq(buf, "online"))
- memhp_auto_online = true;
+ memhp_default_online_type = MMOP_ONLINE;
else if (sysfs_streq(buf, "offline"))
- memhp_auto_online = false;
+ memhp_default_online_type = MMOP_OFFLINE;
else
return -EINVAL;
diff --git a/include/linux/memory_hotplug.h b/include/linux/memory_hotplug.h
index c2e06ed5e0e9..c6e090b34c4b 100644
--- a/include/linux/memory_hotplug.h
+++ b/include/linux/memory_hotplug.h
@@ -117,7 +117,8 @@ extern int arch_add_memory(int nid, u64 start, u64 size,
struct mhp_restrictions *restrictions);
extern u64 max_mem_size;
-extern bool memhp_auto_online;
+/* Default online_type (MMOP_*) when new memory blocks are added. */
+extern int memhp_default_online_type;
/* If movable_node boot option specified */
extern bool movable_node_enabled;
static inline bool movable_node_is_enabled(void)
diff --git a/mm/memory_hotplug.c b/mm/memory_hotplug.c
index 2d2aae830b92..1975a2b99a2b 100644
--- a/mm/memory_hotplug.c
+++ b/mm/memory_hotplug.c
@@ -67,17 +67,17 @@ void put_online_mems(void)
bool movable_node_enabled = false;
#ifndef CONFIG_MEMORY_HOTPLUG_DEFAULT_ONLINE
-bool memhp_auto_online;
+int memhp_default_online_type = MMOP_OFFLINE;
#else
-bool memhp_auto_online = true;
+int memhp_default_online_type = MMOP_ONLINE;
#endif
static int __init setup_memhp_default_state(char *str)
{
if (!strcmp(str, "online"))
- memhp_auto_online = true;
+ memhp_default_online_type = MMOP_ONLINE;
else if (!strcmp(str, "offline"))
- memhp_auto_online = false;
+ memhp_default_online_type = MMOP_OFFLINE;
return 1;
}
@@ -990,6 +990,7 @@ static int check_hotplug_memory_range(u64 start, u64 size)
static int online_memory_block(struct memory_block *mem, void *arg)
{
+ mem->online_type = memhp_default_online_type;
return device_online(&mem->dev);
}
@@ -1062,7 +1063,7 @@ int __ref add_memory_resource(int nid, struct resource *res)
mem_hotplug_done();
/* online pages if requested */
- if (memhp_auto_online)
+ if (memhp_default_online_type != MMOP_OFFLINE)
walk_memory_blocks(start, size, NULL, online_memory_block);
return ret;
--
2.24.1
^ permalink raw reply related
* [PATCH v2 5/8] hv_balloon: don't check for memhp_auto_online manually
From: David Hildenbrand @ 2020-03-17 10:49 UTC (permalink / raw)
To: linux-kernel
Cc: linux-mm, linuxppc-dev, linux-hyperv, David Hildenbrand,
K. Y. Srinivasan, Haiyang Zhang, Stephen Hemminger, Wei Liu,
Andrew Morton, Michal Hocko, Oscar Salvador, Rafael J. Wysocki,
Baoquan He, Wei Yang, Vitaly Kuznetsov
In-Reply-To: <20200317104942.11178-1-david@redhat.com>
We get the MEM_ONLINE notifier call if memory is added right from the
kernel via add_memory() or later from user space.
Let's get rid of the "ha_waiting" flag - the wait event has an inbuilt
mechanism (->done) for that. Initialize the wait event only once and
reinitialize before adding memory. Unconditionally call complete() and
wait_for_completion_timeout().
If there are no waiters, complete() will only increment ->done - which
will be reset by reinit_completion(). If complete() has already been
called, wait_for_completion_timeout() will not wait.
There is still the chance for a small race between concurrent
reinit_completion() and complete(). If complete() wins, we would not
wait - which is tolerable (and the race exists in current code as well).
Note: We only wait for "some" memory to get onlined, which seems to be
good enough for now.
Cc: "K. Y. Srinivasan" <kys@microsoft.com>
Cc: Haiyang Zhang <haiyangz@microsoft.com>
Cc: Stephen Hemminger <sthemmin@microsoft.com>
Cc: Wei Liu <wei.liu@kernel.org>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Michal Hocko <mhocko@kernel.org>
Cc: Oscar Salvador <osalvador@suse.de>
Cc: "Rafael J. Wysocki" <rafael@kernel.org>
Cc: Baoquan He <bhe@redhat.com>
Cc: Wei Yang <richard.weiyang@gmail.com>
Cc: Vitaly Kuznetsov <vkuznets@redhat.com>
Cc: linux-hyperv@vger.kernel.org
Signed-off-by: David Hildenbrand <david@redhat.com>
---
drivers/hv/hv_balloon.c | 25 ++++++++++---------------
1 file changed, 10 insertions(+), 15 deletions(-)
diff --git a/drivers/hv/hv_balloon.c b/drivers/hv/hv_balloon.c
index a02ce43d778d..af5e09f08130 100644
--- a/drivers/hv/hv_balloon.c
+++ b/drivers/hv/hv_balloon.c
@@ -533,7 +533,6 @@ struct hv_dynmem_device {
* State to synchronize hot-add.
*/
struct completion ol_waitevent;
- bool ha_waiting;
/*
* This thread handles hot-add
* requests from the host as well as notifying
@@ -634,10 +633,7 @@ static int hv_memory_notifier(struct notifier_block *nb, unsigned long val,
switch (val) {
case MEM_ONLINE:
case MEM_CANCEL_ONLINE:
- if (dm_device.ha_waiting) {
- dm_device.ha_waiting = false;
- complete(&dm_device.ol_waitevent);
- }
+ complete(&dm_device.ol_waitevent);
break;
case MEM_OFFLINE:
@@ -726,8 +722,7 @@ static void hv_mem_hot_add(unsigned long start, unsigned long size,
has->covered_end_pfn += processed_pfn;
spin_unlock_irqrestore(&dm_device.ha_lock, flags);
- init_completion(&dm_device.ol_waitevent);
- dm_device.ha_waiting = !memhp_auto_online;
+ reinit_completion(&dm_device.ol_waitevent);
nid = memory_add_physaddr_to_nid(PFN_PHYS(start_pfn));
ret = add_memory(nid, PFN_PHYS((start_pfn)),
@@ -753,15 +748,14 @@ static void hv_mem_hot_add(unsigned long start, unsigned long size,
}
/*
- * Wait for the memory block to be onlined when memory onlining
- * is done outside of kernel (memhp_auto_online). Since the hot
- * add has succeeded, it is ok to proceed even if the pages in
- * the hot added region have not been "onlined" within the
- * allowed time.
+ * Wait for memory to get onlined. If the kernel onlined the
+ * memory when adding it, this will return directly. Otherwise,
+ * it will wait for user space to online the memory. This helps
+ * to avoid adding memory faster than it is getting onlined. As
+ * adding succeeded, it is ok to proceed even if the memory was
+ * not onlined in time.
*/
- if (dm_device.ha_waiting)
- wait_for_completion_timeout(&dm_device.ol_waitevent,
- 5*HZ);
+ wait_for_completion_timeout(&dm_device.ol_waitevent, 5 * HZ);
post_status(&dm_device);
}
}
@@ -1707,6 +1701,7 @@ static int balloon_probe(struct hv_device *dev,
#ifdef CONFIG_MEMORY_HOTPLUG
set_online_page_callback(&hv_online_page);
register_memory_notifier(&hv_memory_nb);
+ init_completion(&dm_device.ol_waitevent);
#endif
hv_set_drvdata(dev, &dm_device);
--
2.24.1
^ permalink raw reply related
* [PATCH v2 3/8] drivers/base/memory: store mapping between MMOP_* and string in an array
From: David Hildenbrand @ 2020-03-17 10:49 UTC (permalink / raw)
To: linux-kernel
Cc: linux-mm, linuxppc-dev, linux-hyperv, David Hildenbrand, Wei Yang,
Michal Hocko, Greg Kroah-Hartman, Andrew Morton, Michal Hocko,
Oscar Salvador, Rafael J. Wysocki, Baoquan He
In-Reply-To: <20200317104942.11178-1-david@redhat.com>
Let's use a simple array which we can reuse soon. While at it, move the
string->mmop conversion out of the device hotplug lock.
Reviewed-by: Wei Yang <richard.weiyang@gmail.com>
Acked-by: Michal Hocko <mhocko@suse.com>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Michal Hocko <mhocko@kernel.org>
Cc: Oscar Salvador <osalvador@suse.de>
Cc: "Rafael J. Wysocki" <rafael@kernel.org>
Cc: Baoquan He <bhe@redhat.com>
Cc: Wei Yang <richard.weiyang@gmail.com>
Signed-off-by: David Hildenbrand <david@redhat.com>
---
drivers/base/memory.c | 38 +++++++++++++++++++++++---------------
1 file changed, 23 insertions(+), 15 deletions(-)
diff --git a/drivers/base/memory.c b/drivers/base/memory.c
index e7e77cafef80..8a7f29c0bf97 100644
--- a/drivers/base/memory.c
+++ b/drivers/base/memory.c
@@ -28,6 +28,24 @@
#define MEMORY_CLASS_NAME "memory"
+static const char *const online_type_to_str[] = {
+ [MMOP_OFFLINE] = "offline",
+ [MMOP_ONLINE] = "online",
+ [MMOP_ONLINE_KERNEL] = "online_kernel",
+ [MMOP_ONLINE_MOVABLE] = "online_movable",
+};
+
+static int memhp_online_type_from_str(const char *str)
+{
+ int i;
+
+ for (i = 0; i < ARRAY_SIZE(online_type_to_str); i++) {
+ if (sysfs_streq(str, online_type_to_str[i]))
+ return i;
+ }
+ return -EINVAL;
+}
+
#define to_memory_block(dev) container_of(dev, struct memory_block, dev)
static int sections_per_block;
@@ -236,26 +254,17 @@ static int memory_subsys_offline(struct device *dev)
static ssize_t state_store(struct device *dev, struct device_attribute *attr,
const char *buf, size_t count)
{
+ const int online_type = memhp_online_type_from_str(buf);
struct memory_block *mem = to_memory_block(dev);
- int ret, online_type;
+ int ret;
+
+ if (online_type < 0)
+ return -EINVAL;
ret = lock_device_hotplug_sysfs();
if (ret)
return ret;
- if (sysfs_streq(buf, "online_kernel"))
- online_type = MMOP_ONLINE_KERNEL;
- else if (sysfs_streq(buf, "online_movable"))
- online_type = MMOP_ONLINE_MOVABLE;
- else if (sysfs_streq(buf, "online"))
- online_type = MMOP_ONLINE;
- else if (sysfs_streq(buf, "offline"))
- online_type = MMOP_OFFLINE;
- else {
- ret = -EINVAL;
- goto err;
- }
-
switch (online_type) {
case MMOP_ONLINE_KERNEL:
case MMOP_ONLINE_MOVABLE:
@@ -271,7 +280,6 @@ static ssize_t state_store(struct device *dev, struct device_attribute *attr,
ret = -EINVAL; /* should never happen */
}
-err:
unlock_device_hotplug();
if (ret < 0)
--
2.24.1
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox