* [PATCH v2 0/2] clocksource/hyperv: Miscellaneous changes
From: Andrea Parri @ 2020-01-09 16:06 UTC (permalink / raw)
To: linux-kernel, linux-hyperv
Cc: K . Y . Srinivasan, Haiyang Zhang, Stephen Hemminger, Sasha Levin,
Daniel Lezcano, Thomas Gleixner, Michael Kelley, Boqun Feng,
Dexuan Cui, Vitaly Kuznetsov, Andrea Parri
Provide some refactoring and adjustments to the Hyper-V clocksources
code.
Changes since v1 [1]:
- Rebased on Daniel's repo/branch
- Added Reviewed-by: tags
[1] https://lkml.kernel.org/r/20191210093054.32230-1-parri.andrea@gmail.com
Andrea Parri (2):
clocksource/hyperv: Untangle stimers and timesync from clocksources
clocksource/hyperv: Set TSC clocksource as default w/ InvariantTSC
drivers/clocksource/hyperv_timer.c | 48 ++++++++++++++++++++----------
drivers/hv/hv_util.c | 8 ++---
include/clocksource/hyperv_timer.h | 2 +-
3 files changed, 38 insertions(+), 20 deletions(-)
--
2.24.0
^ permalink raw reply
* [PATCH v2 1/2] clocksource/hyperv: Untangle stimers and timesync from clocksources
From: Andrea Parri @ 2020-01-09 16:06 UTC (permalink / raw)
To: linux-kernel, linux-hyperv
Cc: K . Y . Srinivasan, Haiyang Zhang, Stephen Hemminger, Sasha Levin,
Daniel Lezcano, Thomas Gleixner, Michael Kelley, Boqun Feng,
Dexuan Cui, Vitaly Kuznetsov, Andrea Parri
In-Reply-To: <20200109160650.16150-1-parri.andrea@gmail.com>
hyperv_timer.c exports hyperv_cs, which is used by stimers and the
timesync mechanism. However, the clocksource dependency is not
needed: these mechanisms only depend on the partition reference
counter (which can be read via a MSR or via the TSC Reference Page).
Introduce the (function) pointer hv_read_reference_counter, as an
embodiment of the partition reference counter read, and export it
in place of the hyperv_cs pointer. The latter can be removed.
This should clarify that there's no relationship between Hyper-V
stimers & timesync and the Linux clocksource abstractions. No
functional or semantic change.
Suggested-by: Michael Kelley <mikelley@microsoft.com>
Signed-off-by: Andrea Parri <parri.andrea@gmail.com>
Reviewed-by: Michael Kelley <mikelley@microsoft.com>
---
drivers/clocksource/hyperv_timer.c | 36 +++++++++++++++++++-----------
drivers/hv/hv_util.c | 8 +++----
include/clocksource/hyperv_timer.h | 2 +-
3 files changed, 28 insertions(+), 18 deletions(-)
diff --git a/drivers/clocksource/hyperv_timer.c b/drivers/clocksource/hyperv_timer.c
index 51ff43274db7b..be98d201d585d 100644
--- a/drivers/clocksource/hyperv_timer.c
+++ b/drivers/clocksource/hyperv_timer.c
@@ -56,7 +56,7 @@ static int hv_ce_set_next_event(unsigned long delta,
{
u64 current_tick;
- current_tick = hyperv_cs->read(NULL);
+ current_tick = hv_read_reference_counter();
current_tick += delta;
hv_init_timer(0, current_tick);
return 0;
@@ -210,8 +210,8 @@ EXPORT_SYMBOL_GPL(hv_stimer_global_cleanup);
* Hyper-V and 32-bit x86. The TSC reference page version is preferred.
*/
-struct clocksource *hyperv_cs;
-EXPORT_SYMBOL_GPL(hyperv_cs);
+u64 (*hv_read_reference_counter)(void);
+EXPORT_SYMBOL_GPL(hv_read_reference_counter);
static union {
struct ms_hyperv_tsc_page page;
@@ -224,7 +224,7 @@ struct ms_hyperv_tsc_page *hv_get_tsc_page(void)
}
EXPORT_SYMBOL_GPL(hv_get_tsc_page);
-static u64 notrace read_hv_clock_tsc(struct clocksource *arg)
+static u64 notrace read_hv_clock_tsc(void)
{
u64 current_tick = hv_read_tsc_page(hv_get_tsc_page());
@@ -234,9 +234,14 @@ static u64 notrace read_hv_clock_tsc(struct clocksource *arg)
return current_tick;
}
+static u64 notrace read_hv_clock_tsc_cs(struct clocksource *arg)
+{
+ return read_hv_clock_tsc();
+}
+
static u64 read_hv_sched_clock_tsc(void)
{
- return read_hv_clock_tsc(NULL) - hv_sched_clock_offset;
+ return read_hv_clock_tsc() - hv_sched_clock_offset;
}
static void suspend_hv_clock_tsc(struct clocksource *arg)
@@ -265,14 +270,14 @@ static void resume_hv_clock_tsc(struct clocksource *arg)
static struct clocksource hyperv_cs_tsc = {
.name = "hyperv_clocksource_tsc_page",
.rating = 400,
- .read = read_hv_clock_tsc,
+ .read = read_hv_clock_tsc_cs,
.mask = CLOCKSOURCE_MASK(64),
.flags = CLOCK_SOURCE_IS_CONTINUOUS,
.suspend= suspend_hv_clock_tsc,
.resume = resume_hv_clock_tsc,
};
-static u64 notrace read_hv_clock_msr(struct clocksource *arg)
+static u64 notrace read_hv_clock_msr(void)
{
u64 current_tick;
/*
@@ -284,15 +289,20 @@ static u64 notrace read_hv_clock_msr(struct clocksource *arg)
return current_tick;
}
+static u64 notrace read_hv_clock_msr_cs(struct clocksource *arg)
+{
+ return read_hv_clock_msr();
+}
+
static u64 read_hv_sched_clock_msr(void)
{
- return read_hv_clock_msr(NULL) - hv_sched_clock_offset;
+ return read_hv_clock_msr() - hv_sched_clock_offset;
}
static struct clocksource hyperv_cs_msr = {
.name = "hyperv_clocksource_msr",
.rating = 400,
- .read = read_hv_clock_msr,
+ .read = read_hv_clock_msr_cs,
.mask = CLOCKSOURCE_MASK(64),
.flags = CLOCK_SOURCE_IS_CONTINUOUS,
};
@@ -305,7 +315,7 @@ static bool __init hv_init_tsc_clocksource(void)
if (!(ms_hyperv.features & HV_MSR_REFERENCE_TSC_AVAILABLE))
return false;
- hyperv_cs = &hyperv_cs_tsc;
+ hv_read_reference_counter = read_hv_clock_tsc;
phys_addr = virt_to_phys(hv_get_tsc_page());
/*
@@ -323,7 +333,7 @@ static bool __init hv_init_tsc_clocksource(void)
hv_set_clocksource_vdso(hyperv_cs_tsc);
clocksource_register_hz(&hyperv_cs_tsc, NSEC_PER_SEC/100);
- hv_sched_clock_offset = hyperv_cs->read(hyperv_cs);
+ hv_sched_clock_offset = hv_read_reference_counter();
hv_setup_sched_clock(read_hv_sched_clock_tsc);
return true;
@@ -345,10 +355,10 @@ void __init hv_init_clocksource(void)
if (!(ms_hyperv.features & HV_MSR_TIME_REF_COUNT_AVAILABLE))
return;
- hyperv_cs = &hyperv_cs_msr;
+ hv_read_reference_counter = read_hv_clock_msr;
clocksource_register_hz(&hyperv_cs_msr, NSEC_PER_SEC/100);
- hv_sched_clock_offset = hyperv_cs->read(hyperv_cs);
+ hv_sched_clock_offset = hv_read_reference_counter();
hv_setup_sched_clock(read_hv_sched_clock_msr);
}
EXPORT_SYMBOL_GPL(hv_init_clocksource);
diff --git a/drivers/hv/hv_util.c b/drivers/hv/hv_util.c
index e32681ee7b9f6..ef338db3d3599 100644
--- a/drivers/hv/hv_util.c
+++ b/drivers/hv/hv_util.c
@@ -211,7 +211,7 @@ static struct timespec64 hv_get_adj_host_time(void)
unsigned long flags;
spin_lock_irqsave(&host_ts.lock, flags);
- reftime = hyperv_cs->read(hyperv_cs);
+ reftime = hv_read_reference_counter();
newtime = host_ts.host_time + (reftime - host_ts.ref_time);
ts = ns_to_timespec64((newtime - WLTIMEDELTA) * 100);
spin_unlock_irqrestore(&host_ts.lock, flags);
@@ -250,7 +250,7 @@ static inline void adj_guesttime(u64 hosttime, u64 reftime, u8 adj_flags)
*/
spin_lock_irqsave(&host_ts.lock, flags);
- cur_reftime = hyperv_cs->read(hyperv_cs);
+ cur_reftime = hv_read_reference_counter();
host_ts.host_time = hosttime;
host_ts.ref_time = cur_reftime;
@@ -315,7 +315,7 @@ static void timesync_onchannelcallback(void *context)
sizeof(struct vmbuspipe_hdr) +
sizeof(struct icmsg_hdr)];
adj_guesttime(timedatap->parenttime,
- hyperv_cs->read(hyperv_cs),
+ hv_read_reference_counter(),
timedatap->flags);
}
}
@@ -523,7 +523,7 @@ static struct ptp_clock *hv_ptp_clock;
static int hv_timesync_init(struct hv_util_service *srv)
{
/* TimeSync requires Hyper-V clocksource. */
- if (!hyperv_cs)
+ if (!hv_read_reference_counter)
return -ENODEV;
spin_lock_init(&host_ts.lock);
diff --git a/include/clocksource/hyperv_timer.h b/include/clocksource/hyperv_timer.h
index 422f5e5237be4..373a3ece88121 100644
--- a/include/clocksource/hyperv_timer.h
+++ b/include/clocksource/hyperv_timer.h
@@ -29,7 +29,7 @@ extern void hv_stimer_global_cleanup(void);
extern void hv_stimer0_isr(void);
#ifdef CONFIG_HYPERV_TIMER
-extern struct clocksource *hyperv_cs;
+extern u64 (*hv_read_reference_counter)(void);
extern void hv_init_clocksource(void);
extern struct ms_hyperv_tsc_page *hv_get_tsc_page(void);
--
2.24.0
^ permalink raw reply related
* [PATCH v2 2/2] clocksource/hyperv: Set TSC clocksource as default w/ InvariantTSC
From: Andrea Parri @ 2020-01-09 16:06 UTC (permalink / raw)
To: linux-kernel, linux-hyperv
Cc: K . Y . Srinivasan, Haiyang Zhang, Stephen Hemminger, Sasha Levin,
Daniel Lezcano, Thomas Gleixner, Michael Kelley, Boqun Feng,
Dexuan Cui, Vitaly Kuznetsov, Andrea Parri
In-Reply-To: <20200109160650.16150-1-parri.andrea@gmail.com>
Change the Hyper-V clocksource ratings to 250, below the TSC clocksource
rating of 300. In configurations where Hyper-V offers an InvariantTSC,
the TSC is not marked "unstable", so the TSC clocksource is available
and preferred. With the higher rating, it will be the default. On
older hardware and Hyper-V versions, the TSC is marked "unstable", so no
TSC clocksource is created and the selected Hyper-V clocksource will be
the default.
Signed-off-by: Andrea Parri <parri.andrea@gmail.com>
Reviewed-by: Michael Kelley <mikelley@microsoft.com>
---
drivers/clocksource/hyperv_timer.c | 12 ++++++++++--
1 file changed, 10 insertions(+), 2 deletions(-)
diff --git a/drivers/clocksource/hyperv_timer.c b/drivers/clocksource/hyperv_timer.c
index be98d201d585d..3881c2bf987e4 100644
--- a/drivers/clocksource/hyperv_timer.c
+++ b/drivers/clocksource/hyperv_timer.c
@@ -208,6 +208,14 @@ EXPORT_SYMBOL_GPL(hv_stimer_global_cleanup);
* the other that uses the TSC reference page feature as defined in the
* TLFS. The MSR version is for compatibility with old versions of
* Hyper-V and 32-bit x86. The TSC reference page version is preferred.
+ *
+ * The Hyper-V clocksource ratings of 250 are chosen to be below the
+ * TSC clocksource rating of 300. In configurations where Hyper-V offers
+ * an InvariantTSC, the TSC is not marked "unstable", so the TSC clocksource
+ * is available and preferred. With the higher rating, it will be the
+ * default. On older hardware and Hyper-V versions, the TSC is marked
+ * "unstable", so no TSC clocksource is created and the selected Hyper-V
+ * clocksource will be the default.
*/
u64 (*hv_read_reference_counter)(void);
@@ -269,7 +277,7 @@ static void resume_hv_clock_tsc(struct clocksource *arg)
static struct clocksource hyperv_cs_tsc = {
.name = "hyperv_clocksource_tsc_page",
- .rating = 400,
+ .rating = 250,
.read = read_hv_clock_tsc_cs,
.mask = CLOCKSOURCE_MASK(64),
.flags = CLOCK_SOURCE_IS_CONTINUOUS,
@@ -301,7 +309,7 @@ static u64 read_hv_sched_clock_msr(void)
static struct clocksource hyperv_cs_msr = {
.name = "hyperv_clocksource_msr",
- .rating = 400,
+ .rating = 250,
.read = read_hv_clock_msr_cs,
.mask = CLOCKSOURCE_MASK(64),
.flags = CLOCK_SOURCE_IS_CONTINUOUS,
--
2.24.0
^ permalink raw reply related
* Re: [PATCH 0/2] clocksource/hyperv: Miscellaneous changes
From: Andrea Parri @ 2020-01-09 16:12 UTC (permalink / raw)
To: Daniel Lezcano
Cc: linux-kernel, linux-hyperv, K . Y . Srinivasan, Haiyang Zhang,
Stephen Hemminger, Sasha Levin, Thomas Gleixner, Michael Kelley,
Boqun Feng, Vitaly Kuznetsov, Dexuan Cui
In-Reply-To: <7069f7ce-4ff7-3569-0d7e-615d1399283a@linaro.org>
On Thu, Jan 09, 2020 at 12:40:04PM +0100, Daniel Lezcano wrote:
> On 08/01/2020 14:33, Andrea Parri wrote:
> > Thomas, Daniel,
> >
> > On Tue, Dec 10, 2019 at 10:30:52AM +0100, Andrea Parri wrote:
> >> Provide some refactoring and adjustments to the Hyper-V clocksources
> >> code. Applies to -rc1 plus Boqun's:
> >>
> >> https://lkml.kernel.org/r/20191126021723.4710-1-boqun.feng@gmail.com
> >>
> >> Thanks,
> >> Andrea
> >>
> >> Andrea Parri (2):
> >> clocksource/hyperv: Untangle stimers and timesync from clocksources
> >> clocksource/hyperv: Set TSC clocksource as default w/ InvariantTSC
> >
> > I'm wondering if you could route these patches via tip; for reference,
> >
> > https://lkml.kernel.org/r/20191210093054.32230-1-parri.andrea@gmail.com
> >
> > Both patches got Reviewed-by: Michael (even though I can't find his replies
> > on lore ATM).
> >
> > Also, I realized that this series has a minor conflict with Dexuan's:
> >
> > https://lkml.kernel.org/r/1578350617-130430-1-git-send-email-decui@microsoft.com
> >
> > (added to Cc:); please let me know if a rebase/resend is needed.
>
> Yes please based on:
>
> https://git.linaro.org/people/daniel.lezcano/linux.git timers/drivers/next
Done: https://lkml.kernel.org/r/20200109160650.16150-1-parri.andrea@gmail.com
Thanks Daniel.
Andrea
^ permalink raw reply
* Re: [PATCH v2 0/2] clocksource/hyperv: Miscellaneous changes
From: Daniel Lezcano @ 2020-01-09 18:40 UTC (permalink / raw)
To: Andrea Parri, linux-kernel, linux-hyperv
Cc: K . Y . Srinivasan, Haiyang Zhang, Stephen Hemminger, Sasha Levin,
Thomas Gleixner, Michael Kelley, Boqun Feng, Dexuan Cui,
Vitaly Kuznetsov
In-Reply-To: <20200109160650.16150-1-parri.andrea@gmail.com>
On 09/01/2020 17:06, Andrea Parri wrote:
> Provide some refactoring and adjustments to the Hyper-V clocksources
> code.
>
> Changes since v1 [1]:
> - Rebased on Daniel's repo/branch
> - Added Reviewed-by: tags
>
> [1] https://lkml.kernel.org/r/20191210093054.32230-1-parri.andrea@gmail.com
>
> Andrea Parri (2):
> clocksource/hyperv: Untangle stimers and timesync from clocksources
> clocksource/hyperv: Set TSC clocksource as default w/ InvariantTSC
>
> drivers/clocksource/hyperv_timer.c | 48 ++++++++++++++++++++----------
> drivers/hv/hv_util.c | 8 ++---
> include/clocksource/hyperv_timer.h | 2 +-
> 3 files changed, 38 insertions(+), 20 deletions(-)
Applied, thanks.
--
<http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs
Follow Linaro: <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog
^ permalink raw reply
* [PATCH] Drivers: hv: vmbus: Ignore CHANNELMSG_TL_CONNECT_RESULT(23)
From: Dexuan Cui @ 2020-01-10 1:25 UTC (permalink / raw)
To: kys, haiyangz, sthemmin, sashal, linux-hyperv, linux-kernel,
mikelley, Alexander.Levin
Cc: sunilmut, Andrea.Parri, Dexuan Cui
When a Linux hv_sock app tries to connect to a Service GUID on which no
host app is listening, a recent host (RS3+) sends a
CHANNELMSG_TL_CONNECT_RESULT (23) message to Linux and this triggers such
a warning:
unknown msgtype=23
WARNING: CPU: 2 PID: 0 at drivers/hv/vmbus_drv.c:1031 vmbus_on_msg_dpc
Actually Linux can safely ignore the message because the Linux app's
connect() will time out in 2 seconds: see VSOCK_DEFAULT_CONNECT_TIMEOUT
and vsock_stream_connect(). We don't bother to make use of the message
because: 1) it's only supported on recent hosts; 2) a non-trivial effort
is required to use the message in Linux, but the benefit is small.
So, let's not see the warning by silently ignoring the message.
Signed-off-by: Dexuan Cui <decui@microsoft.com>
---
drivers/hv/vmbus_drv.c | 5 +++++
include/linux/hyperv.h | 2 ++
2 files changed, 7 insertions(+)
diff --git a/drivers/hv/vmbus_drv.c b/drivers/hv/vmbus_drv.c
index 4ef5a66df680..c838b6f5f726 100644
--- a/drivers/hv/vmbus_drv.c
+++ b/drivers/hv/vmbus_drv.c
@@ -1033,6 +1033,11 @@ void vmbus_on_msg_dpc(unsigned long data)
}
entry = &channel_message_table[hdr->msgtype];
+
+ /* Linux ignores some messages, e.g. CHANNELMSG_TL_CONNECT_RESULT. */
+ if (!entry->message_handler)
+ goto msg_handled;
+
if (entry->handler_type == VMHT_BLOCKING) {
ctx = kmalloc(sizeof(*ctx), GFP_ATOMIC);
if (ctx == NULL)
diff --git a/include/linux/hyperv.h b/include/linux/hyperv.h
index 26f3aeeae1ca..41c58011431e 100644
--- a/include/linux/hyperv.h
+++ b/include/linux/hyperv.h
@@ -425,6 +425,8 @@ enum vmbus_channel_message_type {
CHANNELMSG_19 = 19,
CHANNELMSG_20 = 20,
CHANNELMSG_TL_CONNECT_REQUEST = 21,
+ CHANNELMSG_22 = 22,
+ CHANNELMSG_TL_CONNECT_RESULT = 23,
CHANNELMSG_COUNT
};
--
2.19.1
^ permalink raw reply related
* [PATCH] x86/hyper-v: remove unnecessary conversions to bool
From: Chen Zhou @ 2020-01-10 7:20 UTC (permalink / raw)
To: tglx, mingo; +Cc: linux-hyperv, linux-kernel, chenzhou10
The conversions to bool are not needed, remove these.
Signed-off-by: Chen Zhou <chenzhou10@huawei.com>
---
arch/x86/hyperv/hv_apic.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/arch/x86/hyperv/hv_apic.c b/arch/x86/hyperv/hv_apic.c
index 40e0e32..3112cf6 100644
--- a/arch/x86/hyperv/hv_apic.c
+++ b/arch/x86/hyperv/hv_apic.c
@@ -133,7 +133,7 @@ static bool __send_ipi_mask_ex(const struct cpumask *mask, int vector)
ipi_mask_ex_done:
local_irq_restore(flags);
- return ((ret == 0) ? true : false);
+ return ret == 0;
}
static bool __send_ipi_mask(const struct cpumask *mask, int vector)
@@ -186,7 +186,7 @@ static bool __send_ipi_mask(const struct cpumask *mask, int vector)
ret = hv_do_fast_hypercall16(HVCALL_SEND_IPI, ipi_arg.vector,
ipi_arg.cpu_mask);
- return ((ret == 0) ? true : false);
+ return ret == 0;
do_ex_hypercall:
return __send_ipi_mask_ex(mask, vector);
--
2.7.4
^ permalink raw reply related
* Re: [PATCH] x86/hyper-v: remove unnecessary conversions to bool
From: Vitaly Kuznetsov @ 2020-01-10 11:59 UTC (permalink / raw)
To: Chen Zhou; +Cc: linux-hyperv, linux-kernel, chenzhou10, tglx, mingo
In-Reply-To: <20200110072047.85398-1-chenzhou10@huawei.com>
Chen Zhou <chenzhou10@huawei.com> writes:
> The conversions to bool are not needed, remove these.
>
> Signed-off-by: Chen Zhou <chenzhou10@huawei.com>
> ---
> arch/x86/hyperv/hv_apic.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/arch/x86/hyperv/hv_apic.c b/arch/x86/hyperv/hv_apic.c
> index 40e0e32..3112cf6 100644
> --- a/arch/x86/hyperv/hv_apic.c
> +++ b/arch/x86/hyperv/hv_apic.c
> @@ -133,7 +133,7 @@ static bool __send_ipi_mask_ex(const struct cpumask *mask, int vector)
>
> ipi_mask_ex_done:
> local_irq_restore(flags);
> - return ((ret == 0) ? true : false);
> + return ret == 0;
> }
>
> static bool __send_ipi_mask(const struct cpumask *mask, int vector)
> @@ -186,7 +186,7 @@ static bool __send_ipi_mask(const struct cpumask *mask, int vector)
>
> ret = hv_do_fast_hypercall16(HVCALL_SEND_IPI, ipi_arg.vector,
> ipi_arg.cpu_mask);
> - return ((ret == 0) ? true : false);
> + return ret == 0;
>
> do_ex_hypercall:
> return __send_ipi_mask_ex(mask, vector);
I'd suggest we get rid of bool functions completely instead, something
like (untested):
diff --git a/arch/x86/hyperv/hv_apic.c b/arch/x86/hyperv/hv_apic.c
index 40e0e322161d..440bda338763 100644
--- a/arch/x86/hyperv/hv_apic.c
+++ b/arch/x86/hyperv/hv_apic.c
@@ -97,16 +97,16 @@ static void hv_apic_eoi_write(u32 reg, u32 val)
/*
* IPI implementation on Hyper-V.
*/
-static bool __send_ipi_mask_ex(const struct cpumask *mask, int vector)
+static u16 __send_ipi_mask_ex(const struct cpumask *mask, int vector)
{
struct hv_send_ipi_ex **arg;
struct hv_send_ipi_ex *ipi_arg;
unsigned long flags;
int nr_bank = 0;
- int ret = 1;
+ u16 ret;
if (!(ms_hyperv.hints & HV_X64_EX_PROCESSOR_MASKS_RECOMMENDED))
- return false;
+ return U16_MAX;
local_irq_save(flags);
arg = (struct hv_send_ipi_ex **)this_cpu_ptr(hyperv_pcpu_input_arg);
@@ -129,29 +129,28 @@ static bool __send_ipi_mask_ex(const struct cpumask *mask, int vector)
ipi_arg->vp_set.format = HV_GENERIC_SET_ALL;
ret = hv_do_rep_hypercall(HVCALL_SEND_IPI_EX, 0, nr_bank,
- ipi_arg, NULL);
+ ipi_arg, NULL);
ipi_mask_ex_done:
local_irq_restore(flags);
- return ((ret == 0) ? true : false);
+ return ret;
}
-static bool __send_ipi_mask(const struct cpumask *mask, int vector)
+static u16 __send_ipi_mask(const struct cpumask *mask, int vector)
{
int cur_cpu, vcpu;
struct hv_send_ipi ipi_arg;
- int ret = 1;
trace_hyperv_send_ipi_mask(mask, vector);
if (cpumask_empty(mask))
- return true;
+ return 0;
if (!hv_hypercall_pg)
- return false;
+ return U16_MAX;
if ((vector < HV_IPI_LOW_VECTOR) || (vector > HV_IPI_HIGH_VECTOR))
- return false;
+ return U16_MAX;
/*
* From the supplied CPU set we need to figure out if we can get away
@@ -172,7 +171,7 @@ static bool __send_ipi_mask(const struct cpumask *mask, int vector)
for_each_cpu(cur_cpu, mask) {
vcpu = hv_cpu_number_to_vp_number(cur_cpu);
if (vcpu == VP_INVAL)
- return false;
+ return U16_MAX;
/*
* This particular version of the IPI hypercall can
@@ -184,41 +183,40 @@ static bool __send_ipi_mask(const struct cpumask *mask, int vector)
__set_bit(vcpu, (unsigned long *)&ipi_arg.cpu_mask);
}
- ret = hv_do_fast_hypercall16(HVCALL_SEND_IPI, ipi_arg.vector,
- ipi_arg.cpu_mask);
- return ((ret == 0) ? true : false);
+ return (u16)hv_do_fast_hypercall16(HVCALL_SEND_IPI, ipi_arg.vector,
+ ipi_arg.cpu_mask);
do_ex_hypercall:
return __send_ipi_mask_ex(mask, vector);
}
-static bool __send_ipi_one(int cpu, int vector)
+static u16 __send_ipi_one(int cpu, int vector)
{
int vp = hv_cpu_number_to_vp_number(cpu);
trace_hyperv_send_ipi_one(cpu, vector);
if (!hv_hypercall_pg || (vp == VP_INVAL))
- return false;
+ return U16_MAX;
if ((vector < HV_IPI_LOW_VECTOR) || (vector > HV_IPI_HIGH_VECTOR))
- return false;
+ return U16_MAX;
if (vp >= 64)
return __send_ipi_mask_ex(cpumask_of(cpu), vector);
- return !hv_do_fast_hypercall16(HVCALL_SEND_IPI, vector, BIT_ULL(vp));
+ return (u16)hv_do_fast_hypercall16(HVCALL_SEND_IPI, vector, BIT_ULL(vp));
}
static void hv_send_ipi(int cpu, int vector)
{
- if (!__send_ipi_one(cpu, vector))
+ if (__send_ipi_one(cpu, vector))
orig_apic.send_IPI(cpu, vector);
}
static void hv_send_ipi_mask(const struct cpumask *mask, int vector)
{
- if (!__send_ipi_mask(mask, vector))
+ if (__send_ipi_mask(mask, vector))
orig_apic.send_IPI_mask(mask, vector);
}
@@ -231,7 +229,7 @@ static void hv_send_ipi_mask_allbutself(const struct cpumask *mask, int vector)
cpumask_copy(&new_mask, mask);
cpumask_clear_cpu(this_cpu, &new_mask);
local_mask = &new_mask;
- if (!__send_ipi_mask(local_mask, vector))
+ if (__send_ipi_mask(local_mask, vector))
orig_apic.send_IPI_mask_allbutself(mask, vector);
}
@@ -242,13 +240,13 @@ static void hv_send_ipi_allbutself(int vector)
static void hv_send_ipi_all(int vector)
{
- if (!__send_ipi_mask(cpu_online_mask, vector))
+ if (__send_ipi_mask(cpu_online_mask, vector))
orig_apic.send_IPI_all(vector);
}
static void hv_send_ipi_self(int vector)
{
- if (!__send_ipi_one(smp_processor_id(), vector))
+ if (__send_ipi_one(smp_processor_id(), vector))
orig_apic.send_IPI_self(vector);
}
--
Vitaly
^ permalink raw reply related
* Re: [RFC PATCH V2 2/10] mm: expose is_mem_section_removable() symbol
From: David Hildenbrand @ 2020-01-10 13:41 UTC (permalink / raw)
To: Michal Hocko, lantianyu1986
Cc: kys, haiyangz, sthemmin, sashal, akpm, michael.h.kelley,
Tianyu Lan, linux-hyperv, linux-kernel, linux-mm, vkuznets,
eric.devolder, vbabka, osalvador, pavel.tatashin, rppt
In-Reply-To: <20200107133623.GJ32178@dhcp22.suse.cz>
On 07.01.20 14:36, Michal Hocko wrote:
> On Tue 07-01-20 21:09:42, lantianyu1986@gmail.com wrote:
>> From: Tianyu Lan <Tianyu.Lan@microsoft.com>
>>
>> Hyper-V balloon driver will use is_mem_section_removable() to
>> check whether memory block is removable or not when receive
>> memory hot remove msg. Expose it.
>
> I do not think this is a good idea. The check is inherently racy. Why
> cannot the balloon driver simply hotremove the region when asked?
>
It's not only racy, it also gives no guarantees. False postives and
false negatives are possible.
If you want to avoid having to loop forever trying to offline when
calling offline_and_remove_memory(), you could try to
alloc_contig_range() the memory first and then play the
PG_offline+notifier game like virtio-mem.
I don't remember clearly, but I think pinned pages can make offlining
loop for a long time. And I remember there were other scenarios as well
(including out of memory conditions and similar).
I sent an RFC [1] for powerpc/memtrace that does the same (just error
handling is more complicated as it wants to offline and remove multiple
consecutive memory blocks) - if you want to try to go down that path.
[1] https://lkml.kernel.org/r/20191217123851.8854-1-david@redhat.com
--
Thanks,
David / dhildenb
^ permalink raw reply
* [PATCH] scsi: storvsc: Correctly set number of hardware queues for IDE disk
From: longli @ 2020-01-11 8:17 UTC (permalink / raw)
To: K. Y. Srinivasan, Haiyang Zhang, Stephen Hemminger, Sasha Levin,
James E.J. Bottomley, Martin K. Petersen, linux-hyperv,
linux-scsi, linux-kernel
Cc: Long Li
From: Long Li <longli@microsoft.com>
Commit 0ed881027690 ("scsi: storvsc: setup 1:1 mapping between hardware queue and CPU queue")
introduced a regression for disks attached to IDE. For these disks the host VSP only offers
one VMBUS channel. Setting multiple queues can overload the VMBUS channel and result in
performance drop for high queue depth workload on system with large number of CPUs.
Fix it by leaving the number of hardware queues to 1 (default value) for IDE
disks.
Fixes: 0ed881027690 ("scsi: storvsc: setup 1:1 mapping between hardware queue and CPU queue")
Signed-off-by: Long Li <longli@microsoft.com>
---
drivers/scsi/storvsc_drv.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/scsi/storvsc_drv.c b/drivers/scsi/storvsc_drv.c
index f8faf8b3d965..992b28e40374 100644
--- a/drivers/scsi/storvsc_drv.c
+++ b/drivers/scsi/storvsc_drv.c
@@ -1842,9 +1842,11 @@ static int storvsc_probe(struct hv_device *device,
*/
host->sg_tablesize = (stor_device->max_transfer_bytes >> PAGE_SHIFT);
/*
+ * For non-IDE disks, the host supports multiple channels.
* Set the number of HW queues we are supporting.
*/
- host->nr_hw_queues = num_present_cpus();
+ if (dev_id->driver_data != IDE_GUID)
+ host->nr_hw_queues = num_present_cpus();
/*
* Set the error handler work queue.
--
2.20.1
^ permalink raw reply related
* RE: [Patch v3 1/2] PCI: hv: Decouple the func definition in hv_dr_state from VSP message
From: Long Li @ 2020-01-11 8:27 UTC (permalink / raw)
To: longli@linuxonhyperv.com, KY Srinivasan, Haiyang Zhang,
Stephen Hemminger, Sasha Levin, Lorenzo Pieralisi, Andrew Murray,
Bjorn Helgaas, linux-hyperv@vger.kernel.org,
linux-pci@vger.kernel.org, linux-kernel@vger.kernel.org
In-Reply-To: <1577389241-108450-1-git-send-email-longli@linuxonhyperv.com>
Hi Bjorn,
I have addressed all the prior comments in this v3 patch. Please take a look.
Thanks
Long
>Subject: [Patch v3 1/2] PCI: hv: Decouple the func definition in hv_dr_state from
>VSP message
>
>From: Long Li <longli@microsoft.com>
>
>hv_dr_state is used to find present PCI devices on the bus. The structure reuses
>struct pci_function_description from VSP message to describe a device.
>
>To prepare support for pci_function_description v2, we need to decouple this
>dependence in hv_dr_state so it can work with both v1 and v2 VSP messages.
>
>There is no functionality change.
>
>Signed-off-by: Long Li <longli@microsoft.com>
>---
>Changes
>v2: changed some spaces to tabs, changed failure code to -ENOMEM
>v3: revised comment for function hv_pci_devices_present(), reformatted patch
>title
>
> drivers/pci/controller/pci-hyperv.c | 101 +++++++++++++++++++---------
> 1 file changed, 70 insertions(+), 31 deletions(-)
>
>diff --git a/drivers/pci/controller/pci-hyperv.c b/drivers/pci/controller/pci-
>hyperv.c
>index f1f300218fab..4452c6bae6cf 100644
>--- a/drivers/pci/controller/pci-hyperv.c
>+++ b/drivers/pci/controller/pci-hyperv.c
>@@ -507,10 +507,24 @@ struct hv_dr_work {
> struct hv_pcibus_device *bus;
> };
>
>+struct hv_pcidev_description {
>+ u16 v_id; /* vendor ID */
>+ u16 d_id; /* device ID */
>+ u8 rev;
>+ u8 prog_intf;
>+ u8 subclass;
>+ u8 base_class;
>+ u32 subsystem_id;
>+ union win_slot_encoding win_slot;
>+ u32 ser; /* serial number */
>+ u32 flags;
>+ u16 virtual_numa_node;
>+};
>+
> struct hv_dr_state {
> struct list_head list_entry;
> u32 device_count;
>- struct pci_function_description func[0];
>+ struct hv_pcidev_description func[0];
> };
>
> enum hv_pcichild_state {
>@@ -527,7 +541,7 @@ struct hv_pci_dev {
> refcount_t refs;
> enum hv_pcichild_state state;
> struct pci_slot *pci_slot;
>- struct pci_function_description desc;
>+ struct hv_pcidev_description desc;
> bool reported_missing;
> struct hv_pcibus_device *hbus;
> struct work_struct wrk;
>@@ -1862,7 +1876,7 @@ static void q_resource_requirements(void *context,
>struct pci_response *resp,
> * Return: Pointer to the new tracking struct
> */
> static struct hv_pci_dev *new_pcichild_device(struct hv_pcibus_device *hbus,
>- struct pci_function_description *desc)
>+ struct hv_pcidev_description *desc)
> {
> struct hv_pci_dev *hpdev;
> struct pci_child_message *res_req;
>@@ -1973,7 +1987,7 @@ static void pci_devices_present_work(struct
>work_struct *work) {
> u32 child_no;
> bool found;
>- struct pci_function_description *new_desc;
>+ struct hv_pcidev_description *new_desc;
> struct hv_pci_dev *hpdev;
> struct hv_pcibus_device *hbus;
> struct list_head removed;
>@@ -2090,43 +2104,26 @@ static void pci_devices_present_work(struct
>work_struct *work)
> put_hvpcibus(hbus);
> kfree(dr);
> }
>-
> /**
>- * hv_pci_devices_present() - Handles list of new children
>+ * hv_pci_start_relations_work() - Queue work to start device discovery
> * @hbus: Root PCI bus, as understood by this driver
>- * @relations: Packet from host listing children
>+ * @dr: The list of children returned from host
> *
>- * This function is invoked whenever a new list of devices for
>- * this bus appears.
>+ * Return: 0 on success, -errno on failure
> */
>-static void hv_pci_devices_present(struct hv_pcibus_device *hbus,
>- struct pci_bus_relations *relations)
>+static int hv_pci_start_relations_work(struct hv_pcibus_device *hbus,
>+ struct hv_dr_state *dr)
> {
>- struct hv_dr_state *dr;
> struct hv_dr_work *dr_wrk;
>- unsigned long flags;
> bool pending_dr;
>+ unsigned long flags;
>
> dr_wrk = kzalloc(sizeof(*dr_wrk), GFP_NOWAIT);
> if (!dr_wrk)
>- return;
>-
>- dr = kzalloc(offsetof(struct hv_dr_state, func) +
>- (sizeof(struct pci_function_description) *
>- (relations->device_count)), GFP_NOWAIT);
>- if (!dr) {
>- kfree(dr_wrk);
>- return;
>- }
>+ return -ENOMEM;
>
> INIT_WORK(&dr_wrk->wrk, pci_devices_present_work);
> dr_wrk->bus = hbus;
>- dr->device_count = relations->device_count;
>- if (dr->device_count != 0) {
>- memcpy(dr->func, relations->func,
>- sizeof(struct pci_function_description) *
>- dr->device_count);
>- }
>
> spin_lock_irqsave(&hbus->device_list_lock, flags);
> /*
>@@ -2144,6 +2141,47 @@ static void hv_pci_devices_present(struct
>hv_pcibus_device *hbus,
> get_hvpcibus(hbus);
> queue_work(hbus->wq, &dr_wrk->wrk);
> }
>+
>+ return 0;
>+}
>+
>+/**
>+ * hv_pci_devices_present() - Handles list of new children
>+ * @hbus: Root PCI bus, as understood by this driver
>+ * @relations: Packet from host listing children
>+ *
>+ * This function processes a new list of devices on the bus. The list
>+of
>+ * devices is discoverd by VSP and sent to us via VSP message
>+ * PCI_BUS_RELATIONS, whenever a new list of devices for this bus appears.
>+ */
>+static void hv_pci_devices_present(struct hv_pcibus_device *hbus,
>+ struct pci_bus_relations *relations) {
>+ struct hv_dr_state *dr;
>+ int i;
>+
>+ dr = kzalloc(offsetof(struct hv_dr_state, func) +
>+ (sizeof(struct hv_pcidev_description) *
>+ (relations->device_count)), GFP_NOWAIT);
>+
>+ if (!dr)
>+ return;
>+
>+ dr->device_count = relations->device_count;
>+ for (i = 0; i < dr->device_count; i++) {
>+ dr->func[i].v_id = relations->func[i].v_id;
>+ dr->func[i].d_id = relations->func[i].d_id;
>+ dr->func[i].rev = relations->func[i].rev;
>+ dr->func[i].prog_intf = relations->func[i].prog_intf;
>+ dr->func[i].subclass = relations->func[i].subclass;
>+ dr->func[i].base_class = relations->func[i].base_class;
>+ dr->func[i].subsystem_id = relations->func[i].subsystem_id;
>+ dr->func[i].win_slot = relations->func[i].win_slot;
>+ dr->func[i].ser = relations->func[i].ser;
>+ }
>+
>+ if (hv_pci_start_relations_work(hbus, dr))
>+ kfree(dr);
> }
>
> /**
>@@ -3018,7 +3056,7 @@ static void hv_pci_bus_exit(struct hv_device *hdev)
> struct pci_packet teardown_packet;
> u8 buffer[sizeof(struct pci_message)];
> } pkt;
>- struct pci_bus_relations relations;
>+ struct hv_dr_state *dr;
> struct hv_pci_compl comp_pkt;
> int ret;
>
>@@ -3030,8 +3068,9 @@ static void hv_pci_bus_exit(struct hv_device *hdev)
> return;
>
> /* Delete any children which might still exist. */
>- memset(&relations, 0, sizeof(relations));
>- hv_pci_devices_present(hbus, &relations);
>+ dr = kzalloc(sizeof(*dr), GFP_KERNEL);
>+ if (dr && hv_pci_start_relations_work(hbus, dr))
>+ kfree(dr);
>
> ret = hv_send_resources_released(hdev);
> if (ret)
>--
>2.17.1
^ permalink raw reply
* Re: [PATCH] scsi: storvsc: Correctly set number of hardware queues for IDE disk
From: Ming Lei @ 2020-01-11 12:08 UTC (permalink / raw)
To: longli
Cc: K. Y. Srinivasan, Haiyang Zhang, Stephen Hemminger, Sasha Levin,
James E.J. Bottomley, Martin K. Petersen, linux-hyperv,
Linux SCSI List, Linux Kernel Mailing List, Long Li
In-Reply-To: <1578730634-109961-1-git-send-email-longli@linuxonhyperv.com>
On Sat, Jan 11, 2020 at 4:17 PM <longli@linuxonhyperv.com> wrote:
>
> From: Long Li <longli@microsoft.com>
>
> Commit 0ed881027690 ("scsi: storvsc: setup 1:1 mapping between hardware queue and CPU queue")
> introduced a regression for disks attached to IDE. For these disks the host VSP only offers
> one VMBUS channel. Setting multiple queues can overload the VMBUS channel and result in
> performance drop for high queue depth workload on system with large number of CPUs.
>
> Fix it by leaving the number of hardware queues to 1 (default value) for IDE
> disks.
>
> Fixes: 0ed881027690 ("scsi: storvsc: setup 1:1 mapping between hardware queue and CPU queue")
> Signed-off-by: Long Li <longli@microsoft.com>
> ---
> drivers/scsi/storvsc_drv.c | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/scsi/storvsc_drv.c b/drivers/scsi/storvsc_drv.c
> index f8faf8b3d965..992b28e40374 100644
> --- a/drivers/scsi/storvsc_drv.c
> +++ b/drivers/scsi/storvsc_drv.c
> @@ -1842,9 +1842,11 @@ static int storvsc_probe(struct hv_device *device,
> */
> host->sg_tablesize = (stor_device->max_transfer_bytes >> PAGE_SHIFT);
> /*
> + * For non-IDE disks, the host supports multiple channels.
> * Set the number of HW queues we are supporting.
> */
> - host->nr_hw_queues = num_present_cpus();
> + if (dev_id->driver_data != IDE_GUID)
> + host->nr_hw_queues = num_present_cpus();
>
> /*
> * Set the error handler work queue.
> --
> 2.20.1
>
Reviewed-by: Ming Lei <ming.lei@redhat.com>
--
Ming Lei
^ permalink raw reply
* Re: [PATCH][RESEND] Input: hyperv-keyboard: Add the support of hibernation
From: Sasha Levin @ 2020-01-11 16:26 UTC (permalink / raw)
To: Dexuan Cui
Cc: kys, haiyangz, sthemmin, dmitry.torokhov, linux-hyperv,
linux-input, linux-kernel, mikelley, Alexander.Levin
In-Reply-To: <1578350444-129991-1-git-send-email-decui@microsoft.com>
On Mon, Jan 06, 2020 at 02:40:44PM -0800, Dexuan Cui wrote:
>Add suspend() and resume() functions so the Hyper-V virtual keyboard
>can participate in VM hibernation.
>
>Note that the keyboard is a "wakeup" device that could abort an in-progress
>hibernation if there is keyboard event. No attempt is made to suppress this
>behavior. If desired, a sysadmin can disable the keyboard as a wakeup device
>using standard mechanisms such as:
>
>echo disabled > /sys/bus/vmbus/drivers/hyperv_keyboard/XXX/power/wakeup
>(where XXX is the device's GUID)
>
>Reviewed-by: Michael Kelley <mikelley@microsoft.com>
>Signed-off-by: Dexuan Cui <decui@microsoft.com>
>---
>
>This is a RESEND of https://lkml.org/lkml/2019/11/24/115 .
>
>Please review.
>
>If it looks good, Sasha Levin, can you please pick it up via the
>hyperv/linux.git tree, as you did last time for this driver?
This will need an ack from the input driver maintainers, unless they
want to give a blanket ack to this type of patches.
--
Thanks,
Sasha
^ permalink raw reply
* Re: [PATCH][RESEND] video: hyperv_fb: Fix hibernation for the deferred IO feature
From: Sasha Levin @ 2020-01-11 16:29 UTC (permalink / raw)
To: Dexuan Cui
Cc: kys, haiyangz, sthemmin, b.zolnierkie, linux-hyperv, dri-devel,
linux-fbdev, linux-kernel, mikelley, Alexander.Levin, weh
In-Reply-To: <1578350511-130150-1-git-send-email-decui@microsoft.com>
On Mon, Jan 06, 2020 at 02:41:51PM -0800, Dexuan Cui wrote:
>fb_deferred_io_work() can access the vmbus ringbuffer by calling
>fbdefio->deferred_io() -> synthvid_deferred_io() -> synthvid_update().
>
>Because the vmbus ringbuffer is inaccessible between hvfb_suspend()
>and hvfb_resume(), we must cancel info->deferred_work before calling
>vmbus_close() and then reschedule it after we reopen the channel
>in hvfb_resume().
>
>Fixes: a4ddb11d297e ("video: hyperv: hyperv_fb: Support deferred IO for Hyper-V frame buffer driver")
>Fixes: 824946a8b6fb ("video: hyperv_fb: Add the support of hibernation")
>Signed-off-by: Dexuan Cui <decui@microsoft.com>
>Reviewed-by: Wei Hu <weh@microsoft.com>
>---
>
>This is a RESEND of https://lkml.org/lkml/2019/11/20/73 .
>
>The only change is the addition of Wei's Review-ed-by.
>
>Please review.
>
>If it looks good, Sasha Levin, can you please pick it up via the
>hyperv/linux.git tree, as you did last time for this driver?
Like with the input driver, if the relevant maintainers here are okay
with this type of patches going through the hyperv tree I'll be happy to
do it, otherwise I need an explicit ack from them on this patch.
--
Thanks,
Sasha
^ permalink raw reply
* Re: [Patch v3 1/2] PCI: hv: Decouple the func definition in hv_dr_state from VSP message
From: Bjorn Helgaas @ 2020-01-11 17:49 UTC (permalink / raw)
To: longli
Cc: K. Y. Srinivasan, Haiyang Zhang, Stephen Hemminger, Sasha Levin,
Lorenzo Pieralisi, Andrew Murray, linux-hyperv, linux-pci,
linux-kernel, Long Li
In-Reply-To: <1577389241-108450-1-git-send-email-longli@linuxonhyperv.com>
On Thu, Dec 26, 2019 at 11:40:40AM -0800, longli@linuxonhyperv.com wrote:
> From: Long Li <longli@microsoft.com>
>
> hv_dr_state is used to find present PCI devices on the bus. The structure
> reuses struct pci_function_description from VSP message to describe a device.
>
> To prepare support for pci_function_description v2, we need to decouple this
> dependence in hv_dr_state so it can work with both v1 and v2 VSP messages.
s/we need to decouple/decouple/
> + * hv_pci_devices_present() - Handles list of new children
> + * @hbus: Root PCI bus, as understood by this driver
> + * @relations: Packet from host listing children
> + *
> + * This function processes a new list of devices on the bus. The list of
> + * devices is discoverd by VSP and sent to us via VSP message
s/Handles list/Handle list/
s/This function processeses/Process/
s/discoverd/discovered/
> + * PCI_BUS_RELATIONS, whenever a new list of devices for this bus appears.
^ permalink raw reply
* Re: [Patch v3 2/2] PCI: hv: Add support for protocol 1.3 and support PCI_BUS_RELATIONS2
From: Bjorn Helgaas @ 2020-01-11 17:51 UTC (permalink / raw)
To: longli
Cc: K. Y. Srinivasan, Haiyang Zhang, Stephen Hemminger, Sasha Levin,
Lorenzo Pieralisi, Andrew Murray, linux-hyperv, linux-pci,
linux-kernel, Long Li
In-Reply-To: <1577389241-108450-2-git-send-email-longli@linuxonhyperv.com>
On Thu, Dec 26, 2019 at 11:40:41AM -0800, longli@linuxonhyperv.com wrote:
> From: Long Li <longli@microsoft.com>
>
> Starting with Hyper-V PCI protocol version 1.3, the host VSP can send
> PCI_BUS_RELATIONS2 and pass the vNUMA node information for devices on the bus.
> The vNUMA node tells which guest NUMA node this device is on based on guest
> VM configuration topology and physical device inforamtion.
>
> The patch adds code to negotiate v1.3 and process PCI_BUS_RELATIONS2.
s/The patch adds code/Add code/
> + * hv_pci_devices_present2() - Handles list of new children
> + * @hbus: Root PCI bus, as understood by this driver
> + * @relations2: Packet from host listing children
> + *
> + * This function is the v2 version of hv_pci_devices_present()
s/Handles list/Handle list/
^ permalink raw reply
* Re: [Patch v3 1/2] PCI: hv: Decouple the func definition in hv_dr_state from VSP message
From: Bjorn Helgaas @ 2020-01-11 17:53 UTC (permalink / raw)
To: Long Li
Cc: longli@linuxonhyperv.com, KY Srinivasan, Haiyang Zhang,
Stephen Hemminger, Sasha Levin, Lorenzo Pieralisi, Andrew Murray,
linux-hyperv@vger.kernel.org, linux-pci@vger.kernel.org,
linux-kernel@vger.kernel.org
In-Reply-To: <BL0PR2101MB1123229A668A200C34A2888ECE3B0@BL0PR2101MB1123.namprd21.prod.outlook.com>
On Sat, Jan 11, 2020 at 08:27:25AM +0000, Long Li wrote:
> Hi Bjorn,
>
> I have addressed all the prior comments in this v3 patch. Please take a look.
Lorenzo normally merges hv updates, so I'm sure this is on his list to
take a look. I pointed out a few spelling and similar nits, but I
didn't review the actual substance of v3.
Bjorn
^ permalink raw reply
* WARNING: bad unlock balance in __dev_queue_xmit
From: syzbot @ 2020-01-11 21:34 UTC (permalink / raw)
To: a, alex.aring, allison, andrew, andy, ap420073, ast, b.a.t.m.a.n,
bridge, cleech, daniel, davem, dsa, f.fainelli, fw, gregkh,
gustavo, haiyangz, info, j.vosburgh, j, jakub.kicinski, jhs, jiri,
johan.hedberg, johannes.berg, john.hurley, jwi, kstewart, kuznet,
kvalo, kys, linmiaohe, linux-bluetooth, linux-hams, linux-hyperv,
linux-kernel, linux-ppp, linux-wireless, linux-wpan, liuhangbin,
marcel, mareklindner, mkubecek, mmanning, netdev, nikolay,
oss-drivers, pabeni, paulus
Hello,
syzbot found the following crash on:
HEAD commit: ae608821 Merge tag 'trace-v5.5-rc5' of git://git.kernel.or..
git tree: upstream
console output: https://syzkaller.appspot.com/x/log.txt?x=12abc515e00000
kernel config: https://syzkaller.appspot.com/x/.config?x=18698c0c240ba616
dashboard link: https://syzkaller.appspot.com/bug?extid=ad4ea1dd5d26131a58a6
compiler: gcc (GCC) 9.0.0 20181231 (experimental)
syz repro: https://syzkaller.appspot.com/x/repro.syz?x=141051b9e00000
C reproducer: https://syzkaller.appspot.com/x/repro.c?x=125e5876e00000
The bug was bisected to:
commit ab92d68fc22f9afab480153bd82a20f6e2533769
Author: Taehee Yoo <ap420073@gmail.com>
Date: Mon Oct 21 18:47:51 2019 +0000
net: core: add generic lockdep keys
bisection log: https://syzkaller.appspot.com/x/bisect.txt?x=107f969ee00000
final crash: https://syzkaller.appspot.com/x/report.txt?x=127f969ee00000
console output: https://syzkaller.appspot.com/x/log.txt?x=147f969ee00000
IMPORTANT: if you fix the bug, please add the following tag to the commit:
Reported-by: syzbot+ad4ea1dd5d26131a58a6@syzkaller.appspotmail.com
Fixes: ab92d68fc22f ("net: core: add generic lockdep keys")
=====================================
WARNING: bad unlock balance detected!
5.5.0-rc5-syzkaller #0 Not tainted
-------------------------------------
swapper/0/0 is trying to release lock (&dev->qdisc_xmit_lock_key) at:
[<ffffffff8625890d>] spin_unlock include/linux/spinlock.h:378 [inline]
[<ffffffff8625890d>] __netif_tx_unlock include/linux/netdevice.h:3966
[inline]
[<ffffffff8625890d>] __dev_queue_xmit+0x2bbd/0x35c0 net/core/dev.c:4016
but there are no more locks to release!
other info that might help us debug this:
4 locks held by swapper/0/0:
#0: ffffc90000007d50 ((&ndev->rs_timer)){+.-.}, at: lockdep_copy_map
include/linux/lockdep.h:172 [inline]
#0: ffffc90000007d50 ((&ndev->rs_timer)){+.-.}, at:
call_timer_fn+0xe0/0x780 kernel/time/timer.c:1394
#1: ffffffff899a5340 (rcu_read_lock){....}, at: ip6_nd_hdr
net/ipv6/ndisc.c:463 [inline]
#1: ffffffff899a5340 (rcu_read_lock){....}, at:
ndisc_send_skb+0x7fe/0x1490 net/ipv6/ndisc.c:499
#2: ffffffff899a5300 (rcu_read_lock_bh){....}, at: lwtunnel_xmit_redirect
include/net/lwtunnel.h:92 [inline]
#2: ffffffff899a5300 (rcu_read_lock_bh){....}, at:
ip6_finish_output2+0x214/0x25c0 net/ipv6/ip6_output.c:102
#3: ffffffff899a5300 (rcu_read_lock_bh){....}, at:
__dev_queue_xmit+0x20a/0x35c0 net/core/dev.c:3948
stack backtrace:
CPU: 0 PID: 0 Comm: swapper/0 Not tainted 5.5.0-rc5-syzkaller #0
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS
Google 01/01/2011
Call Trace:
<IRQ>
__dump_stack lib/dump_stack.c:77 [inline]
dump_stack+0x197/0x210 lib/dump_stack.c:118
print_unlock_imbalance_bug kernel/locking/lockdep.c:4008 [inline]
print_unlock_imbalance_bug.cold+0x114/0x123 kernel/locking/lockdep.c:3984
__lock_release kernel/locking/lockdep.c:4242 [inline]
lock_release+0x5f2/0x960 kernel/locking/lockdep.c:4503
__raw_spin_unlock include/linux/spinlock_api_smp.h:150 [inline]
_raw_spin_unlock+0x16/0x40 kernel/locking/spinlock.c:183
spin_unlock include/linux/spinlock.h:378 [inline]
__netif_tx_unlock include/linux/netdevice.h:3966 [inline]
__dev_queue_xmit+0x2bbd/0x35c0 net/core/dev.c:4016
dev_queue_xmit+0x18/0x20 net/core/dev.c:4046
neigh_hh_output include/net/neighbour.h:499 [inline]
neigh_output include/net/neighbour.h:508 [inline]
ip6_finish_output2+0xfbe/0x25c0 net/ipv6/ip6_output.c:116
__ip6_finish_output+0x444/0xaa0 net/ipv6/ip6_output.c:142
ip6_finish_output+0x38/0x1f0 net/ipv6/ip6_output.c:152
NF_HOOK_COND include/linux/netfilter.h:296 [inline]
ip6_output+0x25e/0x880 net/ipv6/ip6_output.c:175
dst_output include/net/dst.h:436 [inline]
NF_HOOK include/linux/netfilter.h:307 [inline]
ndisc_send_skb+0xf1f/0x1490 net/ipv6/ndisc.c:505
ndisc_send_rs+0x134/0x720 net/ipv6/ndisc.c:699
addrconf_rs_timer+0x30f/0x6e0 net/ipv6/addrconf.c:3879
call_timer_fn+0x1ac/0x780 kernel/time/timer.c:1404
expire_timers kernel/time/timer.c:1449 [inline]
__run_timers kernel/time/timer.c:1773 [inline]
__run_timers kernel/time/timer.c:1740 [inline]
run_timer_softirq+0x6c3/0x1790 kernel/time/timer.c:1786
__do_softirq+0x262/0x98c kernel/softirq.c:292
invoke_softirq kernel/softirq.c:373 [inline]
irq_exit+0x19b/0x1e0 kernel/softirq.c:413
exiting_irq arch/x86/include/asm/apic.h:536 [inline]
smp_apic_timer_interrupt+0x1a3/0x610 arch/x86/kernel/apic/apic.c:1137
apic_timer_interrupt+0xf/0x20 arch/x86/entry/entry_64.S:829
</IRQ>
RIP: 0010:native_safe_halt+0xe/0x10 arch/x86/include/asm/irqflags.h:61
Code: 38 be db f9 eb 8a cc cc cc cc cc cc e9 07 00 00 00 0f 00 2d c4 54 51
00 f4 c3 66 90 e9 07 00 00 00 0f 00 2d b4 54 51 00 fb f4 <c3> cc 55 48 89
e5 41 57 41 56 41 55 41 54 53 e8 5e 92 8b f9 e8 c9
RSP: 0018:ffffffff89807ce8 EFLAGS: 00000286 ORIG_RAX: ffffffffffffff13
RAX: 1ffffffff132669e RBX: ffffffff8987a140 RCX: 0000000000000000
RDX: dffffc0000000000 RSI: 0000000000000006 RDI: ffffffff8987a9d4
RBP: ffffffff89807d18 R08: ffffffff8987a140 R09: 0000000000000000
R10: 0000000000000000 R11: 0000000000000000 R12: dffffc0000000000
R13: ffffffff8a7b8600 R14: 0000000000000000 R15: 0000000000000000
arch_cpu_idle+0xa/0x10 arch/x86/kernel/process.c:690
default_idle_call+0x84/0xb0 kernel/sched/idle.c:94
cpuidle_idle_call kernel/sched/idle.c:154 [inline]
do_idle+0x3c8/0x6e0 kernel/sched/idle.c:269
cpu_startup_entry+0x1b/0x20 kernel/sched/idle.c:361
rest_init+0x23b/0x371 init/main.c:451
arch_call_rest_init+0xe/0x1b
start_kernel+0x904/0x943 init/main.c:784
x86_64_start_reservations+0x29/0x2b arch/x86/kernel/head64.c:490
x86_64_start_kernel+0x77/0x7b arch/x86/kernel/head64.c:471
secondary_startup_64+0xa4/0xb0 arch/x86/kernel/head_64.S:242
kobject: 'brport' (0000000015306f5c): kobject_cleanup, parent
00000000835b0c7d
kobject: 'brport' (0000000015306f5c): calling ktype release
kobject: 'brport': free name
kobject: 'brport' (00000000f618ced7): kobject_cleanup, parent
00000000835b0c7d
kobject: 'brport' (00000000f618ced7): calling ktype release
kobject: 'brport': free name
kobject: 'brport' (0000000090c32451): kobject_cleanup, parent
00000000835b0c7d
kobject: 'brport' (0000000090c32451): calling ktype release
kobject: 'brport': free name
kobject: 'brport' (000000009b39b612): kobject_cleanup, parent
00000000835b0c7d
kobject: 'brport' (000000009b39b612): calling ktype release
kobject: 'brport': free name
kobject: 'brport' (00000000fa23c3a6): kobject_cleanup, parent
00000000835b0c7d
kobject: 'brport' (00000000fa23c3a6): calling ktype release
kobject: 'brport': free name
kobject: 'brport' (0000000059be53cf): kobject_cleanup, parent
00000000835b0c7d
kobject: 'brport' (0000000059be53cf): calling ktype release
kobject: 'brport': free name
kobject: 'brport' (00000000afda9faa): kobject_cleanup, parent
00000000835b0c7d
kobject: 'brport' (00000000afda9faa): calling ktype release
kobject: 'brport': free name
kobject: 'brport' (000000001b8d397b): kobject_cleanup, parent
00000000835b0c7d
kobject: 'brport' (000000001b8d397b): calling ktype release
kobject: 'brport': free name
kobject: 'brport' (00000000c95708c8): kobject_cleanup, parent
00000000835b0c7d
kobject: 'brport' (00000000c95708c8): calling ktype release
kobject: 'brport': free name
kobject: 'brport' (0000000021fa4c47): kobject_cleanup, parent
00000000835b0c7d
kobject: 'brport' (0000000021fa4c47): calling ktype release
kobject: 'brport': free name
kobject: 'brport' (00000000e55a6ea3): kobject_cleanup, parent
00000000835b0c7d
kobject: 'brport' (00000000e55a6ea3): calling ktype release
kobject: 'brport': free name
kobject: 'brport' (000000005f707c44): kobject_cleanup, parent
00000000835b0c7d
kobject: 'brport' (000000005f707c44): calling ktype release
kobject: 'brport': free name
---
This bug is generated by a bot. It may contain errors.
See https://goo.gl/tpsmEJ for more information about syzbot.
syzbot engineers can be reached at syzkaller@googlegroups.com.
syzbot will keep track of this bug report. See:
https://goo.gl/tpsmEJ#status for how to communicate with syzbot.
For information about bisection process see: https://goo.gl/tpsmEJ#bisection
syzbot can test patches for this bug, for details see:
https://goo.gl/tpsmEJ#testing-patches
^ permalink raw reply
* Re: Re: WARNING: bad unlock balance in __dev_queue_xmit
From: syzbot @ 2020-01-11 23:38 UTC (permalink / raw)
To: Cong Wang
Cc: a, alex.aring, allison, andrew, andy, ap420073, ast, b.a.t.m.a.n,
bridge, cleech, daniel, davem, dsa, f.fainelli, fw, gregkh,
gustavo, haiyangz, info, j.vosburgh, j, jakub.kicinski, jhs, jiri,
johan.hedberg, johannes.berg, john.hurley, jwi, kstewart, kuznet,
kvalo, kys, linmiaohe, linux-bluetooth, linux-hams, linux-hyperv,
linux-kernel, linux-ppp, linux-wireless, linux-wpan, liuhangbin,
marcel, mareklindner, mkubecek, mmanning, netdev, nikolay,
oss-drivers, pabeni, paulus, syzkaller-bugs
In-Reply-To: <CAM_iQpWN-SKjjrG_7EQ-x+7UMiu6foaNWMJuwQuwN0BGmayB+A@mail.gmail.com>
> #syz dup: WARNING: bad unlock balance in sch_direct_xmit
Your 'dup:' command is accepted, but please keep
syzkaller-bugs@googlegroups.com mailing list in CC next time. It serves as
a history of what happened with each bug report. Thank you.
^ permalink raw reply
* Re: WARNING: bad unlock balance in __dev_queue_xmit
From: Cong Wang @ 2020-01-11 23:38 UTC (permalink / raw)
To: syzbot
Cc: a, Alexander Aring, allison, andrew, Andy Gospodarek, Taehee Yoo,
ast, b.a.t.m.a.n, bridge, cleech, Daniel Borkmann, David Miller,
David Ahern, Florian Fainelli, Florian Westphal, Greg KH, gustavo,
haiyangz, info, Jay Vosburgh, j, Jakub Kicinski, Jamal Hadi Salim,
Jiri Pirko, Johan Hedberg, Johannes Berg, John Hurley, jwi,
Kate Stewart, Alexey Kuznetsov, Kalle Valo, kys, linmiaohe,
linux-bluetooth, linux-hams, linux-hyperv, LKML, linux-ppp,
linux-wireless, linux-wpan, Hangbin Liu, Marcel Holtmann,
Marek Lindner, Michal Kubecek, mmanning,
Linux Kernel Network Developers, Nikolay Aleksandrov, oss-drivers,
Paolo Abeni, Paul Mackerras
In-Reply-To: <000000000000a06985059be4002e@google.com>
#syz dup: WARNING: bad unlock balance in sch_direct_xmit
^ permalink raw reply
* RE: [PATCH] scsi: storvsc: Correctly set number of hardware queues for IDE disk
From: Michael Kelley @ 2020-01-12 16:27 UTC (permalink / raw)
To: longli@linuxonhyperv.com, KY Srinivasan, Haiyang Zhang,
Stephen Hemminger, Sasha Levin, James E.J. Bottomley,
Martin K. Petersen, linux-hyperv@vger.kernel.org,
linux-scsi@vger.kernel.org, linux-kernel@vger.kernel.org
Cc: Long Li
In-Reply-To: <1578730634-109961-1-git-send-email-longli@linuxonhyperv.com>
From: Long Li <longli@microsoft.com> Sent: Saturday, January 11, 2020 12:17 AM
>
> Commit 0ed881027690 ("scsi: storvsc: setup 1:1 mapping between hardware queue and
> CPU queue")
> introduced a regression for disks attached to IDE. For these disks the host VSP only offers
> one VMBUS channel. Setting multiple queues can overload the VMBUS channel and result
> in
> performance drop for high queue depth workload on system with large number of CPUs.
>
> Fix it by leaving the number of hardware queues to 1 (default value) for IDE
> disks.
>
> Fixes: 0ed881027690 ("scsi: storvsc: setup 1:1 mapping between hardware queue and CPU
> queue")
> Signed-off-by: Long Li <longli@microsoft.com>
> ---
> drivers/scsi/storvsc_drv.c | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/scsi/storvsc_drv.c b/drivers/scsi/storvsc_drv.c
> index f8faf8b3d965..992b28e40374 100644
> --- a/drivers/scsi/storvsc_drv.c
> +++ b/drivers/scsi/storvsc_drv.c
> @@ -1842,9 +1842,11 @@ static int storvsc_probe(struct hv_device *device,
> */
> host->sg_tablesize = (stor_device->max_transfer_bytes >> PAGE_SHIFT);
> /*
> + * For non-IDE disks, the host supports multiple channels.
> * Set the number of HW queues we are supporting.
> */
> - host->nr_hw_queues = num_present_cpus();
> + if (dev_id->driver_data != IDE_GUID)
This function already has a pre-computed value of this test in
the local variable "dev_is_ide". It would be more consistent
to just use it.
Michael
> + host->nr_hw_queues = num_present_cpus();
>
> /*
> * Set the error handler work queue.
> --
> 2.20.1
^ permalink raw reply
* RE: [PATCH] x86/hyper-v: remove unnecessary conversions to bool
From: Michael Kelley @ 2020-01-12 16:43 UTC (permalink / raw)
To: vkuznets, Chen Zhou
Cc: linux-hyperv@vger.kernel.org, linux-kernel@vger.kernel.org,
chenzhou10@huawei.com, tglx@linutronix.de, mingo@redhat.com
In-Reply-To: <875zhjr074.fsf@vitty.brq.redhat.com>
From: Vitaly Kuznetsov <vkuznets@redhat.com> Sent: Friday, January 10, 2020 4:00 AM
>
> Chen Zhou <chenzhou10@huawei.com> writes:
>
> > The conversions to bool are not needed, remove these.
> >
> > Signed-off-by: Chen Zhou <chenzhou10@huawei.com>
> > ---
> > arch/x86/hyperv/hv_apic.c | 4 ++--
> > 1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/arch/x86/hyperv/hv_apic.c b/arch/x86/hyperv/hv_apic.c
> > index 40e0e32..3112cf6 100644
> > --- a/arch/x86/hyperv/hv_apic.c
> > +++ b/arch/x86/hyperv/hv_apic.c
> > @@ -133,7 +133,7 @@ static bool __send_ipi_mask_ex(const struct cpumask *mask, int
> vector)
> >
> > ipi_mask_ex_done:
> > local_irq_restore(flags);
> > - return ((ret == 0) ? true : false);
> > + return ret == 0;
> > }
> >
> > static bool __send_ipi_mask(const struct cpumask *mask, int vector)
> > @@ -186,7 +186,7 @@ static bool __send_ipi_mask(const struct cpumask *mask, int
> vector)
> >
> > ret = hv_do_fast_hypercall16(HVCALL_SEND_IPI, ipi_arg.vector,
> > ipi_arg.cpu_mask);
> > - return ((ret == 0) ? true : false);
> > + return ret == 0;
> >
> > do_ex_hypercall:
> > return __send_ipi_mask_ex(mask, vector);
>
> I'd suggest we get rid of bool functions completely instead, something
> like (untested):
Just curious: Why prefer returning a u16 instead of a bool? To avoid
having to test 'ret' for zero in the return statements, or is there some
broader reason?
>
> diff --git a/arch/x86/hyperv/hv_apic.c b/arch/x86/hyperv/hv_apic.c
> index 40e0e322161d..440bda338763 100644
> --- a/arch/x86/hyperv/hv_apic.c
> +++ b/arch/x86/hyperv/hv_apic.c
> @@ -97,16 +97,16 @@ static void hv_apic_eoi_write(u32 reg, u32 val)
> /*
> * IPI implementation on Hyper-V.
> */
> -static bool __send_ipi_mask_ex(const struct cpumask *mask, int vector)
> +static u16 __send_ipi_mask_ex(const struct cpumask *mask, int vector)
> {
> struct hv_send_ipi_ex **arg;
> struct hv_send_ipi_ex *ipi_arg;
> unsigned long flags;
> int nr_bank = 0;
> - int ret = 1;
> + u16 ret;
>
> if (!(ms_hyperv.hints & HV_X64_EX_PROCESSOR_MASKS_RECOMMENDED))
> - return false;
> + return U16_MAX;
>
> local_irq_save(flags);
> arg = (struct hv_send_ipi_ex **)this_cpu_ptr(hyperv_pcpu_input_arg);
> @@ -129,29 +129,28 @@ static bool __send_ipi_mask_ex(const struct cpumask *mask, int
> vector)
> ipi_arg->vp_set.format = HV_GENERIC_SET_ALL;
>
> ret = hv_do_rep_hypercall(HVCALL_SEND_IPI_EX, 0, nr_bank,
> - ipi_arg, NULL);
> + ipi_arg, NULL);
>
> ipi_mask_ex_done:
> local_irq_restore(flags);
> - return ((ret == 0) ? true : false);
> + return ret;
> }
>
> -static bool __send_ipi_mask(const struct cpumask *mask, int vector)
> +static u16 __send_ipi_mask(const struct cpumask *mask, int vector)
> {
> int cur_cpu, vcpu;
> struct hv_send_ipi ipi_arg;
> - int ret = 1;
>
> trace_hyperv_send_ipi_mask(mask, vector);
>
> if (cpumask_empty(mask))
> - return true;
> + return 0;
>
> if (!hv_hypercall_pg)
> - return false;
> + return U16_MAX;
>
> if ((vector < HV_IPI_LOW_VECTOR) || (vector > HV_IPI_HIGH_VECTOR))
> - return false;
> + return U16_MAX;
>
> /*
> * From the supplied CPU set we need to figure out if we can get away
> @@ -172,7 +171,7 @@ static bool __send_ipi_mask(const struct cpumask *mask, int
> vector)
> for_each_cpu(cur_cpu, mask) {
> vcpu = hv_cpu_number_to_vp_number(cur_cpu);
> if (vcpu == VP_INVAL)
> - return false;
> + return U16_MAX;
>
> /*
> * This particular version of the IPI hypercall can
> @@ -184,41 +183,40 @@ static bool __send_ipi_mask(const struct cpumask *mask, int
> vector)
> __set_bit(vcpu, (unsigned long *)&ipi_arg.cpu_mask);
> }
>
> - ret = hv_do_fast_hypercall16(HVCALL_SEND_IPI, ipi_arg.vector,
> - ipi_arg.cpu_mask);
> - return ((ret == 0) ? true : false);
> + return (u16)hv_do_fast_hypercall16(HVCALL_SEND_IPI, ipi_arg.vector,
> + ipi_arg.cpu_mask);
The cast to u16 seems a bit dangerous. The hypercall status code is indeed
returned in the low 16 bits of the hypercall result value, so it works, and
maybe that is why you suggested u16 as the function return value. But it
is a non-obvious assumption.
Michael
>
> do_ex_hypercall:
> return __send_ipi_mask_ex(mask, vector);
> }
>
> -static bool __send_ipi_one(int cpu, int vector)
> +static u16 __send_ipi_one(int cpu, int vector)
> {
> int vp = hv_cpu_number_to_vp_number(cpu);
>
> trace_hyperv_send_ipi_one(cpu, vector);
>
> if (!hv_hypercall_pg || (vp == VP_INVAL))
> - return false;
> + return U16_MAX;
>
> if ((vector < HV_IPI_LOW_VECTOR) || (vector > HV_IPI_HIGH_VECTOR))
> - return false;
> + return U16_MAX;
>
> if (vp >= 64)
> return __send_ipi_mask_ex(cpumask_of(cpu), vector);
>
> - return !hv_do_fast_hypercall16(HVCALL_SEND_IPI, vector, BIT_ULL(vp));
> + return (u16)hv_do_fast_hypercall16(HVCALL_SEND_IPI, vector, BIT_ULL(vp));
> }
>
> static void hv_send_ipi(int cpu, int vector)
> {
> - if (!__send_ipi_one(cpu, vector))
> + if (__send_ipi_one(cpu, vector))
> orig_apic.send_IPI(cpu, vector);
> }
>
> static void hv_send_ipi_mask(const struct cpumask *mask, int vector)
> {
> - if (!__send_ipi_mask(mask, vector))
> + if (__send_ipi_mask(mask, vector))
> orig_apic.send_IPI_mask(mask, vector);
> }
>
> @@ -231,7 +229,7 @@ static void hv_send_ipi_mask_allbutself(const struct cpumask
> *mask, int vector)
> cpumask_copy(&new_mask, mask);
> cpumask_clear_cpu(this_cpu, &new_mask);
> local_mask = &new_mask;
> - if (!__send_ipi_mask(local_mask, vector))
> + if (__send_ipi_mask(local_mask, vector))
> orig_apic.send_IPI_mask_allbutself(mask, vector);
> }
>
> @@ -242,13 +240,13 @@ static void hv_send_ipi_allbutself(int vector)
>
> static void hv_send_ipi_all(int vector)
> {
> - if (!__send_ipi_mask(cpu_online_mask, vector))
> + if (__send_ipi_mask(cpu_online_mask, vector))
> orig_apic.send_IPI_all(vector);
> }
>
> static void hv_send_ipi_self(int vector)
> {
> - if (!__send_ipi_one(smp_processor_id(), vector))
> + if (__send_ipi_one(smp_processor_id(), vector))
> orig_apic.send_IPI_self(vector);
> }
>
> --
> Vitaly
^ permalink raw reply
* [PATCH v2 0/4] hv_utils: Add the support of hibernation
From: Dexuan Cui @ 2020-01-13 6:29 UTC (permalink / raw)
To: KY Srinivasan, Haiyang Zhang, Stephen Hemminger,
sashal@kernel.org, Sasha Levin, linux-hyperv@vger.kernel.org,
Michael Kelley, vkuznets, linux-kernel@vger.kernel.org
Hi,
This is an updated version of the v1 patchset:
https://lkml.org/lkml/2019/9/11/861
Patch #1 is a new patch that makes the daemons more robust.
Patch #2 is the same as v1.
Patch #3 sends the host-initiated hibernation request to the user space via udev.
(v1 used call_usermodehelper() and "/sbin/hyperv-hibernate".)
Patch #4 handles fcopy/vss specially to avoid possible inconsistent states.
Please review.
Thanks!
Dexuan Cui (4):
Patch #1: Tools: hv: Reopen the devices if read() or write() returns errors
Patch #2: hv_utils: Support host-initiated restart request
Patch #3: hv_utils: Support host-initiated hibernation request
Patch #4: hv_utils: Add the support of hibernation
drivers/hv/hv_fcopy.c | 58 +++++++++++++++-
drivers/hv/hv_kvp.c | 44 +++++++++++-
drivers/hv/hv_snapshot.c | 60 ++++++++++++++++-
drivers/hv/hv_util.c | 133 ++++++++++++++++++++++++++++++++++++-
drivers/hv/hyperv_vmbus.h | 6 ++
include/linux/hyperv.h | 2 +
tools/hv/hv_fcopy_daemon.c | 19 ++++--
tools/hv/hv_kvp_daemon.c | 25 ++++---
tools/hv/hv_vss_daemon.c | 25 +++++--
9 files changed, 344 insertions(+), 28 deletions(-)
--
2.19.1
^ permalink raw reply
* [PATCH v2 1/4] Tools: hv: Reopen the devices if read() or write() returns errors
From: Dexuan Cui @ 2020-01-13 6:30 UTC (permalink / raw)
To: KY Srinivasan, Haiyang Zhang, Stephen Hemminger,
sashal@kernel.org, Sasha Levin, linux-hyperv@vger.kernel.org,
Michael Kelley, vkuznets, linux-kernel@vger.kernel.org
The state machine in the hv_utils driver can run out of order in some
corner cases, e.g. if the kvp daemon doesn't call write() fast enough
due to some reason, kvp_timeout_func() can run first and move the state
to HVUTIL_READY; next, when kvp_on_msg() is called it returns -EINVAL
since kvp_transaction.state is smaller than HVUTIL_USERSPACE_REQ; later,
the daemon's write() gets an error -EINVAL, and the daemon will exit().
We can reproduce the issue by sending a SIGSTOP signal to the daemon, wait
for 1 minute, and send a SIGCONT signal to the daemon: the daemon will
exit() quickly.
We can fix the issue by forcing a reset of the device (which means the
daemon can close() and open() the device again) and doing extra necessary
clean-up.
Signed-off-by: Dexuan Cui <decui@microsoft.com>
---
tools/hv/hv_fcopy_daemon.c | 19 +++++++++++++++----
tools/hv/hv_kvp_daemon.c | 25 ++++++++++++++-----------
tools/hv/hv_vss_daemon.c | 25 +++++++++++++++++++------
3 files changed, 48 insertions(+), 21 deletions(-)
diff --git a/tools/hv/hv_fcopy_daemon.c b/tools/hv/hv_fcopy_daemon.c
index aea2d91ab364..a78a5292589b 100644
--- a/tools/hv/hv_fcopy_daemon.c
+++ b/tools/hv/hv_fcopy_daemon.c
@@ -21,7 +21,7 @@
#include <fcntl.h>
#include <getopt.h>
-static int target_fd;
+static int target_fd = -1;
static char target_fname[PATH_MAX];
static unsigned long long filesize;
@@ -80,6 +80,8 @@ static int hv_start_fcopy(struct hv_start_fcopy *smsg)
error = 0;
done:
+ if (error)
+ memset(target_fname, 0, sizeof(target_fname));
return error;
}
@@ -111,12 +113,16 @@ static int hv_copy_data(struct hv_do_fcopy *cpmsg)
static int hv_copy_finished(void)
{
close(target_fd);
+ target_fd = -1;
+ memset(target_fname, 0, sizeof(target_fname));
return 0;
}
static int hv_copy_cancel(void)
{
close(target_fd);
+ target_fd = -1;
unlink(target_fname);
+ memset(target_fname, 0, sizeof(target_fname));
return 0;
}
@@ -141,7 +147,7 @@ int main(int argc, char *argv[])
struct hv_do_fcopy copy;
__u32 kernel_modver;
} buffer = { };
- int in_handshake = 1;
+ int in_handshake;
static struct option long_options[] = {
{"help", no_argument, 0, 'h' },
@@ -170,6 +176,9 @@ int main(int argc, char *argv[])
openlog("HV_FCOPY", 0, LOG_USER);
syslog(LOG_INFO, "starting; pid is:%d", getpid());
+reopen_fcopy_fd:
+ hv_copy_cancel();
+ in_handshake = 1;
fcopy_fd = open("/dev/vmbus/hv_fcopy", O_RDWR);
if (fcopy_fd < 0) {
@@ -196,7 +205,8 @@ int main(int argc, char *argv[])
len = pread(fcopy_fd, &buffer, sizeof(buffer), 0);
if (len < 0) {
syslog(LOG_ERR, "pread failed: %s", strerror(errno));
- exit(EXIT_FAILURE);
+ close(fcopy_fd);
+ goto reopen_fcopy_fd;
}
if (in_handshake) {
@@ -233,7 +243,8 @@ int main(int argc, char *argv[])
if (pwrite(fcopy_fd, &error, sizeof(int), 0) != sizeof(int)) {
syslog(LOG_ERR, "pwrite failed: %s", strerror(errno));
- exit(EXIT_FAILURE);
+ close(fcopy_fd);
+ goto reopen_fcopy_fd;
}
}
}
diff --git a/tools/hv/hv_kvp_daemon.c b/tools/hv/hv_kvp_daemon.c
index e9ef4ca6a655..3282d48c4487 100644
--- a/tools/hv/hv_kvp_daemon.c
+++ b/tools/hv/hv_kvp_daemon.c
@@ -76,7 +76,7 @@ enum {
DNS
};
-static int in_hand_shake = 1;
+static int in_hand_shake;
static char *os_name = "";
static char *os_major = "";
@@ -1400,14 +1400,6 @@ int main(int argc, char *argv[])
openlog("KVP", 0, LOG_USER);
syslog(LOG_INFO, "KVP starting; pid is:%d", getpid());
- kvp_fd = open("/dev/vmbus/hv_kvp", O_RDWR | O_CLOEXEC);
-
- if (kvp_fd < 0) {
- syslog(LOG_ERR, "open /dev/vmbus/hv_kvp failed; error: %d %s",
- errno, strerror(errno));
- exit(EXIT_FAILURE);
- }
-
/*
* Retrieve OS release information.
*/
@@ -1423,6 +1415,16 @@ int main(int argc, char *argv[])
exit(EXIT_FAILURE);
}
+reopen_kvp_fd:
+ in_hand_shake = 1;
+ kvp_fd = open("/dev/vmbus/hv_kvp", O_RDWR | O_CLOEXEC);
+
+ if (kvp_fd < 0) {
+ syslog(LOG_ERR, "open /dev/vmbus/hv_kvp failed; error: %d %s",
+ errno, strerror(errno));
+ exit(EXIT_FAILURE);
+ }
+
/*
* Register ourselves with the kernel.
*/
@@ -1458,7 +1460,7 @@ int main(int argc, char *argv[])
errno, strerror(errno));
close(kvp_fd);
- return EXIT_FAILURE;
+ goto reopen_kvp_fd;
}
/*
@@ -1623,7 +1625,8 @@ int main(int argc, char *argv[])
if (len != sizeof(struct hv_kvp_msg)) {
syslog(LOG_ERR, "write failed; error: %d %s", errno,
strerror(errno));
- exit(EXIT_FAILURE);
+ close(kvp_fd);
+ goto reopen_kvp_fd;
}
}
diff --git a/tools/hv/hv_vss_daemon.c b/tools/hv/hv_vss_daemon.c
index 92902a88f671..e70fed66a5ae 100644
--- a/tools/hv/hv_vss_daemon.c
+++ b/tools/hv/hv_vss_daemon.c
@@ -28,6 +28,8 @@
#include <stdbool.h>
#include <dirent.h>
+static bool fs_frozen;
+
/* Don't use syslog() in the function since that can cause write to disk */
static int vss_do_freeze(char *dir, unsigned int cmd)
{
@@ -155,8 +157,11 @@ static int vss_operate(int operation)
continue;
}
error |= vss_do_freeze(ent->mnt_dir, cmd);
- if (error && operation == VSS_OP_FREEZE)
- goto err;
+ if (operation == VSS_OP_FREEZE) {
+ if (error)
+ goto err;
+ fs_frozen = true;
+ }
}
endmntent(mounts);
@@ -167,6 +172,9 @@ static int vss_operate(int operation)
goto err;
}
+ if (operation == VSS_OP_THAW && !error)
+ fs_frozen = false;
+
goto out;
err:
save_errno = errno;
@@ -175,6 +183,7 @@ static int vss_operate(int operation)
endmntent(mounts);
}
vss_operate(VSS_OP_THAW);
+ fs_frozen = false;
/* Call syslog after we thaw all filesystems */
if (ent)
syslog(LOG_ERR, "FREEZE of %s failed; error:%d %s",
@@ -202,7 +211,7 @@ int main(int argc, char *argv[])
int op;
struct hv_vss_msg vss_msg[1];
int daemonize = 1, long_index = 0, opt;
- int in_handshake = 1;
+ int in_handshake;
__u32 kernel_modver;
static struct option long_options[] = {
@@ -232,6 +241,10 @@ int main(int argc, char *argv[])
openlog("Hyper-V VSS", 0, LOG_USER);
syslog(LOG_INFO, "VSS starting; pid is:%d", getpid());
+reopen_vss_fd:
+ if (fs_frozen)
+ vss_operate(VSS_OP_THAW);
+ in_handshake = 1;
vss_fd = open("/dev/vmbus/hv_vss", O_RDWR);
if (vss_fd < 0) {
syslog(LOG_ERR, "open /dev/vmbus/hv_vss failed; error: %d %s",
@@ -285,7 +298,7 @@ int main(int argc, char *argv[])
syslog(LOG_ERR, "read failed; error:%d %s",
errno, strerror(errno));
close(vss_fd);
- return EXIT_FAILURE;
+ goto reopen_vss_fd;
}
op = vss_msg->vss_hdr.operation;
@@ -318,8 +331,8 @@ int main(int argc, char *argv[])
syslog(LOG_ERR, "write failed; error: %d %s", errno,
strerror(errno));
- if (op == VSS_OP_FREEZE)
- vss_operate(VSS_OP_THAW);
+ close(vss_fd);
+ goto reopen_vss_fd;
}
}
--
2.19.1
^ permalink raw reply related
* [PATCH v2 2/4] hv_utils: Support host-initiated restart request
From: Dexuan Cui @ 2020-01-13 6:30 UTC (permalink / raw)
To: KY Srinivasan, Haiyang Zhang, Stephen Hemminger,
sashal@kernel.org, Sasha Levin, linux-hyperv@vger.kernel.org,
Michael Kelley, vkuznets, linux-kernel@vger.kernel.org
To test the code, run this command on the host:
Restart-VM $vm -Type Reboot
Signed-off-by: Dexuan Cui <decui@microsoft.com>
---
drivers/hv/hv_util.c | 23 ++++++++++++++++++++++-
1 file changed, 22 insertions(+), 1 deletion(-)
diff --git a/drivers/hv/hv_util.c b/drivers/hv/hv_util.c
index 766bd8457346..fe3a316380c2 100644
--- a/drivers/hv/hv_util.c
+++ b/drivers/hv/hv_util.c
@@ -24,6 +24,8 @@
#define SD_MAJOR 3
#define SD_MINOR 0
+#define SD_MINOR_1 1
+#define SD_VERSION_3_1 (SD_MAJOR << 16 | SD_MINOR_1)
#define SD_VERSION (SD_MAJOR << 16 | SD_MINOR)
#define SD_MAJOR_1 1
@@ -50,8 +52,9 @@ static int sd_srv_version;
static int ts_srv_version;
static int hb_srv_version;
-#define SD_VER_COUNT 2
+#define SD_VER_COUNT 3
static const int sd_versions[] = {
+ SD_VERSION_3_1,
SD_VERSION,
SD_VERSION_1
};
@@ -118,11 +121,21 @@ static void perform_shutdown(struct work_struct *dummy)
orderly_poweroff(true);
}
+static void perform_restart(struct work_struct *dummy)
+{
+ orderly_reboot();
+}
+
/*
* Perform the shutdown operation in a thread context.
*/
static DECLARE_WORK(shutdown_work, perform_shutdown);
+/*
+ * Perform the restart operation in a thread context.
+ */
+static DECLARE_WORK(restart_work, perform_restart);
+
static void shutdown_onchannelcallback(void *context)
{
struct vmbus_channel *channel = context;
@@ -166,6 +179,14 @@ static void shutdown_onchannelcallback(void *context)
pr_info("Shutdown request received -"
" graceful shutdown initiated\n");
break;
+ case 2:
+ case 3:
+ pr_info("Restart request received -"
+ " graceful restart initiated\n");
+ icmsghdrp->status = HV_S_OK;
+
+ schedule_work(&restart_work);
+ break;
default:
icmsghdrp->status = HV_E_FAIL;
execute_shutdown = false;
--
2.19.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