* Re: [PATCH v1 2/3] locking/pvqspinlock, hv: Enable PV qspinlock for Hyper-V
2018-09-13 9:13 ` [PATCH v1 2/3] locking/pvqspinlock, hv: Enable PV qspinlock for Hyper-V Yi Sun
@ 2018-09-13 11:06 ` Thomas Gleixner
2018-09-14 8:53 ` Yi Sun
2018-09-13 11:24 ` Thomas Gleixner
` (2 subsequent siblings)
3 siblings, 1 reply; 15+ messages in thread
From: Thomas Gleixner @ 2018-09-13 11:06 UTC (permalink / raw)
To: Yi Sun
Cc: linux-kernel, x86, chao.p.peng, chao.gao, isaku.yamahata,
michael.h.kelly, tianyu.lan, K. Y. Srinivasan, Haiyang Zhang,
Stephen Hemminger
On Thu, 13 Sep 2018, Yi Sun wrote:
> +#include <asm/mshyperv.h>
>
> /* representing HT siblings of each logical CPU */
> DEFINE_PER_CPU_READ_MOSTLY(cpumask_var_t, cpu_sibling_map);
> @@ -1335,6 +1336,7 @@ void __init native_smp_prepare_boot_cpu(void)
> /* already set me in cpu_online_mask in boot_cpu_init() */
> cpumask_set_cpu(me, cpu_callout_mask);
> cpu_set_state_online(me);
> + hv_init_spinlocks();
No. We have smp_ops.smp_prepare_boot_cpu for that.
Thanks,
tglx
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v1 2/3] locking/pvqspinlock, hv: Enable PV qspinlock for Hyper-V
2018-09-13 11:06 ` Thomas Gleixner
@ 2018-09-14 8:53 ` Yi Sun
0 siblings, 0 replies; 15+ messages in thread
From: Yi Sun @ 2018-09-14 8:53 UTC (permalink / raw)
To: Thomas Gleixner
Cc: linux-kernel, x86, chao.p.peng, chao.gao, isaku.yamahata,
michael.h.kelly, tianyu.lan, K. Y. Srinivasan, Haiyang Zhang,
Stephen Hemminger
On 18-09-13 13:06:13, Thomas Gleixner wrote:
> On Thu, 13 Sep 2018, Yi Sun wrote:
> > +#include <asm/mshyperv.h>
> >
> > /* representing HT siblings of each logical CPU */
> > DEFINE_PER_CPU_READ_MOSTLY(cpumask_var_t, cpu_sibling_map);
> > @@ -1335,6 +1336,7 @@ void __init native_smp_prepare_boot_cpu(void)
> > /* already set me in cpu_online_mask in boot_cpu_init() */
> > cpumask_set_cpu(me, cpu_callout_mask);
> > cpu_set_state_online(me);
> > + hv_init_spinlocks();
>
> No. We have smp_ops.smp_prepare_boot_cpu for that.
>
Got it, will change it. Thanks!
> Thanks,
>
> tglx
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v1 2/3] locking/pvqspinlock, hv: Enable PV qspinlock for Hyper-V
2018-09-13 9:13 ` [PATCH v1 2/3] locking/pvqspinlock, hv: Enable PV qspinlock for Hyper-V Yi Sun
2018-09-13 11:06 ` Thomas Gleixner
@ 2018-09-13 11:24 ` Thomas Gleixner
2018-09-14 9:04 ` Yi Sun
2018-09-13 16:16 ` kbuild test robot
2018-09-13 16:42 ` kbuild test robot
3 siblings, 1 reply; 15+ messages in thread
From: Thomas Gleixner @ 2018-09-13 11:24 UTC (permalink / raw)
To: Yi Sun
Cc: linux-kernel, x86, chao.p.peng, chao.gao, isaku.yamahata,
michael.h.kelly, tianyu.lan, K. Y. Srinivasan, Haiyang Zhang,
Stephen Hemminger
On Thu, 13 Sep 2018, Yi Sun wrote:
> +++ b/arch/x86/hyperv/hv_spinlock.c
> @@ -0,0 +1,99 @@
> +// SPDX-License-Identifier: GPL-2.0
> +
> +/*
> + * Hyper-V specific spinlock code.
> + *
> + * Copyright (C) 2018, Intel, Inc.
> + *
> + * Author : Yi Sun <yi.y.sun@linux.intel.com>
> + *
> + * This program is free software; you can redistribute it and/or modify it
> + * under the terms of the GNU General Public License version 2 as published
> + * by the Free Software Foundation.
> + *
> + * This program is distributed in the hope that it will be useful, but
> + * WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY OR FITNESS FOR A PARTICULAR PURPOSE, GOOD TITLE or
> + * NON INFRINGEMENT. See the GNU General Public License for more
> + * details.
Please remove the boilerplate. The SPDX identifier is enough. If in doubt
talk to Intel legal.
> +static bool hv_pvspin = true;
__initdata please.
> +static u32 spin_wait_info = 0;
No need for 0 initialization.
> +
> +static void hv_notify_long_spin_wait(void)
> +{
> + u64 input = spin_wait_info | 0x00000000ffffffff;
What? The result is always 0x00000000ffffffff .....
> + spin_wait_info++;
> + hv_do_fast_hypercall8(HVCALL_NOTIFY_LONG_SPIN_WAIT, input);
> +}
> +
> +static void hv_qlock_kick(int cpu)
> +{
> + spin_wait_info--;
Looking at the above this is completely pointless and I have no idea what
that variable is supposed to do.
> + apic->send_IPI(cpu, X86_PLATFORM_IPI_VECTOR);
> +}
> +
> +/*
> + * Halt the current CPU & release it back to the host
> + */
> +static void hv_qlock_wait(u8 *byte, u8 val)
> +{
> + unsigned long msr_val;
> +
> + if (READ_ONCE(*byte) != val)
> + return;
> +
> + hv_notify_long_spin_wait();
> +
> + rdmsrl(HV_X64_MSR_GUEST_IDLE, msr_val);
Magic rdmsrl(). That wants a comment what this is for.
> +/*
> + * Hyper-V does not support this so far.
> + */
> +bool hv_vcpu_is_preempted(int vcpu)
static ?
> +{
> + return false;
> +}
> +PV_CALLEE_SAVE_REGS_THUNK(hv_vcpu_is_preempted);
> +
> +void __init hv_init_spinlocks(void)
> +{
> + if (!hv_pvspin ||
> + !apic ||
> + !(ms_hyperv.hints & HV_X64_CLUSTER_IPI_RECOMMENDED) ||
> + !(ms_hyperv.features & HV_X64_MSR_GUEST_IDLE_AVAILABLE)) {
> + pr_warn("hv: PV spinlocks disabled\n");
Why does this need to be pr_warn? This is purely informational. Also please
use pr_fmt instead of the 'hv:' prefix.
> +static __init int hv_parse_nopvspin(char *arg)
> +{
> + hv_pvspin = false;
> + return 0;
> +}
> +early_param("hv_nopvspin", hv_parse_nopvspin);
That lacks Documentation of the command line parameter. Wants to be in the
same patch.
Thanks,
tglx
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v1 2/3] locking/pvqspinlock, hv: Enable PV qspinlock for Hyper-V
2018-09-13 11:24 ` Thomas Gleixner
@ 2018-09-14 9:04 ` Yi Sun
2018-09-14 10:42 ` Thomas Gleixner
0 siblings, 1 reply; 15+ messages in thread
From: Yi Sun @ 2018-09-14 9:04 UTC (permalink / raw)
To: Thomas Gleixner
Cc: linux-kernel, x86, chao.p.peng, chao.gao, isaku.yamahata,
michael.h.kelly, tianyu.lan, K. Y. Srinivasan, Haiyang Zhang,
Stephen Hemminger
On 18-09-13 13:24:07, Thomas Gleixner wrote:
> On Thu, 13 Sep 2018, Yi Sun wrote:
> > +++ b/arch/x86/hyperv/hv_spinlock.c
> > @@ -0,0 +1,99 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +
> > +/*
> > + * Hyper-V specific spinlock code.
> > + *
> > + * Copyright (C) 2018, Intel, Inc.
> > + *
> > + * Author : Yi Sun <yi.y.sun@linux.intel.com>
> > + *
> > + * This program is free software; you can redistribute it and/or modify it
> > + * under the terms of the GNU General Public License version 2 as published
> > + * by the Free Software Foundation.
> > + *
> > + * This program is distributed in the hope that it will be useful, but
> > + * WITHOUT ANY WARRANTY; without even the implied warranty of
> > + * MERCHANTABILITY OR FITNESS FOR A PARTICULAR PURPOSE, GOOD TITLE or
> > + * NON INFRINGEMENT. See the GNU General Public License for more
> > + * details.
>
> Please remove the boilerplate. The SPDX identifier is enough. If in doubt
> talk to Intel legal.
>
I will check this. Thanks!
> > +static bool hv_pvspin = true;
>
> __initdata please.
>
> > +static u32 spin_wait_info = 0;
>
> No need for 0 initialization.
>
Will modify them.
> > +
> > +static void hv_notify_long_spin_wait(void)
> > +{
> > + u64 input = spin_wait_info | 0x00000000ffffffff;
>
> What? The result is always 0x00000000ffffffff .....
>
Oh, sorry for such error.
> > + spin_wait_info++;
> > + hv_do_fast_hypercall8(HVCALL_NOTIFY_LONG_SPIN_WAIT, input);
> > +}
> > +
> > +static void hv_qlock_kick(int cpu)
> > +{
> > + spin_wait_info--;
>
> Looking at the above this is completely pointless and I have no idea what
> that variable is supposed to do.
>
Per Microsoft Hypervisor Top Level Functional Specification, the input
parameter of HVCALL_NOTIFY_LONG_SPIN_WAIT is defined as below:
SpinwaitInfo – Specifies the accumulated count the guest was spinning.
So, it is increased when guest is spinning and reduced when spinlock is
released.
I may add comments to explain it.
> > + apic->send_IPI(cpu, X86_PLATFORM_IPI_VECTOR);
> > +}
> > +
> > +/*
> > + * Halt the current CPU & release it back to the host
> > + */
> > +static void hv_qlock_wait(u8 *byte, u8 val)
> > +{
> > + unsigned long msr_val;
> > +
> > + if (READ_ONCE(*byte) != val)
> > + return;
> > +
> > + hv_notify_long_spin_wait();
> > +
> > + rdmsrl(HV_X64_MSR_GUEST_IDLE, msr_val);
>
> Magic rdmsrl(). That wants a comment what this is for.
>
Per above spec, reading HV_X64_MSR_GUEST_IDLE can make guest idle.
I will add comment here.
> > +/*
> > + * Hyper-V does not support this so far.
> > + */
> > +bool hv_vcpu_is_preempted(int vcpu)
>
> static ?
>
PV_CALLEE_SAVE_REGS_THUNK definition is below.
#define PV_CALLEE_SAVE_REGS_THUNK(func) \
extern typeof(func) __raw_callee_save_##func; \
......
So, the hv_vcpu_is_preempted cannot be declared as 'static'. Otherwise,
the make fails with below info.
arch/x86/hyperv/hv_spinlock.o: In function `__raw_callee_save_hv_vcpu_is_preempted':
hv_spinlock.c:(.text+0xd): undefined reference to `hv_vcpu_is_preempted'
> > +{
> > + return false;
> > +}
> > +PV_CALLEE_SAVE_REGS_THUNK(hv_vcpu_is_preempted);
> > +
> > +void __init hv_init_spinlocks(void)
> > +{
> > + if (!hv_pvspin ||
> > + !apic ||
> > + !(ms_hyperv.hints & HV_X64_CLUSTER_IPI_RECOMMENDED) ||
> > + !(ms_hyperv.features & HV_X64_MSR_GUEST_IDLE_AVAILABLE)) {
> > + pr_warn("hv: PV spinlocks disabled\n");
>
> Why does this need to be pr_warn? This is purely informational. Also please
> use pr_fmt instead of the 'hv:' prefix.
>
Got it. Thanks!
> > +static __init int hv_parse_nopvspin(char *arg)
> > +{
> > + hv_pvspin = false;
> > + return 0;
> > +}
> > +early_param("hv_nopvspin", hv_parse_nopvspin);
>
> That lacks Documentation of the command line parameter. Wants to be in the
> same patch.
>
Will merge patch 3 into 2. Thanks!
> Thanks,
>
> tglx
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v1 2/3] locking/pvqspinlock, hv: Enable PV qspinlock for Hyper-V
2018-09-14 9:04 ` Yi Sun
@ 2018-09-14 10:42 ` Thomas Gleixner
2018-09-17 12:54 ` Yi Sun
0 siblings, 1 reply; 15+ messages in thread
From: Thomas Gleixner @ 2018-09-14 10:42 UTC (permalink / raw)
To: Yi Sun
Cc: linux-kernel, x86, chao.p.peng, chao.gao, isaku.yamahata,
michael.h.kelly, tianyu.lan, K. Y. Srinivasan, Haiyang Zhang,
Stephen Hemminger
[-- Attachment #1: Type: text/plain, Size: 1278 bytes --]
On Fri, 14 Sep 2018, Yi Sun wrote:
> > > +static void hv_notify_long_spin_wait(void)
> > > +{
> > > + u64 input = spin_wait_info | 0x00000000ffffffff;
> >
> > What? The result is always 0x00000000ffffffff .....
> >
> Oh, sorry for such error.
>
> > > + spin_wait_info++;
> > > + hv_do_fast_hypercall8(HVCALL_NOTIFY_LONG_SPIN_WAIT, input);
> > > +}
> > > +
> > > +static void hv_qlock_kick(int cpu)
> > > +{
> > > + spin_wait_info--;
> >
> > Looking at the above this is completely pointless and I have no idea what
> > that variable is supposed to do.
> >
> Per Microsoft Hypervisor Top Level Functional Specification, the input
> parameter of HVCALL_NOTIFY_LONG_SPIN_WAIT is defined as below:
>
> SpinwaitInfo – Specifies the accumulated count the guest was spinning.
>
> So, it is increased when guest is spinning and reduced when spinlock is
> released.
But that's a global variable, so it might be incremented and decremented by
several CPUs at once. I don't have the spec and have no time to study it
either, but global does not make any sense to me. The spin wait info comes
from a single guest CPU and the wakeup is targeted at that guest CPU as
well. So why global? It might be defined that way, but then you really want
to explain it proper.
Thanks,
tglx
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v1 2/3] locking/pvqspinlock, hv: Enable PV qspinlock for Hyper-V
2018-09-14 10:42 ` Thomas Gleixner
@ 2018-09-17 12:54 ` Yi Sun
2018-09-20 1:56 ` Yi Sun
0 siblings, 1 reply; 15+ messages in thread
From: Yi Sun @ 2018-09-17 12:54 UTC (permalink / raw)
To: Thomas Gleixner
Cc: linux-kernel, x86, chao.p.peng, chao.gao, isaku.yamahata,
michael.h.kelly, tianyu.lan, K. Y. Srinivasan, Haiyang Zhang,
Stephen Hemminger
On 18-09-14 12:42:33, Thomas Gleixner wrote:
> On Fri, 14 Sep 2018, Yi Sun wrote:
> > > > +static void hv_notify_long_spin_wait(void)
> > > > +{
> > > > + u64 input = spin_wait_info | 0x00000000ffffffff;
> > >
> > > What? The result is always 0x00000000ffffffff .....
> > >
> > Oh, sorry for such error.
> >
> > > > + spin_wait_info++;
> > > > + hv_do_fast_hypercall8(HVCALL_NOTIFY_LONG_SPIN_WAIT, input);
> > > > +}
> > > > +
> > > > +static void hv_qlock_kick(int cpu)
> > > > +{
> > > > + spin_wait_info--;
> > >
> > > Looking at the above this is completely pointless and I have no idea what
> > > that variable is supposed to do.
> > >
> > Per Microsoft Hypervisor Top Level Functional Specification, the input
> > parameter of HVCALL_NOTIFY_LONG_SPIN_WAIT is defined as below:
> >
> > SpinwaitInfo – Specifies the accumulated count the guest was spinning.
> >
> > So, it is increased when guest is spinning and reduced when spinlock is
> > released.
>
> But that's a global variable, so it might be incremented and decremented by
> several CPUs at once. I don't have the spec and have no time to study it
> either, but global does not make any sense to me. The spin wait info comes
> from a single guest CPU and the wakeup is targeted at that guest CPU as
> well. So why global? It might be defined that way, but then you really want
> to explain it proper.
>
> Thanks,
>
> tglx
>
Let me check this more. Will reply to you later. Thanks!
BRs,
Yi Sun
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v1 2/3] locking/pvqspinlock, hv: Enable PV qspinlock for Hyper-V
2018-09-17 12:54 ` Yi Sun
@ 2018-09-20 1:56 ` Yi Sun
0 siblings, 0 replies; 15+ messages in thread
From: Yi Sun @ 2018-09-20 1:56 UTC (permalink / raw)
To: Thomas Gleixner
Cc: linux-kernel, x86, chao.p.peng, chao.gao, isaku.yamahata,
michael.h.kelly, tianyu.lan, K. Y. Srinivasan, Haiyang Zhang,
Stephen Hemminger
Hi,
On 18-09-17 20:54:52, Yi Sun wrote:
> On 18-09-14 12:42:33, Thomas Gleixner wrote:
> > On Fri, 14 Sep 2018, Yi Sun wrote:
> > > > > +static void hv_notify_long_spin_wait(void)
> > > > > +{
> > > > > + u64 input = spin_wait_info | 0x00000000ffffffff;
> > > >
> > > > What? The result is always 0x00000000ffffffff .....
> > > >
> > > Oh, sorry for such error.
> > >
> > > > > + spin_wait_info++;
> > > > > + hv_do_fast_hypercall8(HVCALL_NOTIFY_LONG_SPIN_WAIT, input);
> > > > > +}
> > > > > +
> > > > > +static void hv_qlock_kick(int cpu)
> > > > > +{
> > > > > + spin_wait_info--;
> > > >
> > > > Looking at the above this is completely pointless and I have no idea what
> > > > that variable is supposed to do.
> > > >
> > > Per Microsoft Hypervisor Top Level Functional Specification, the input
> > > parameter of HVCALL_NOTIFY_LONG_SPIN_WAIT is defined as below:
> > >
> > > SpinwaitInfo – Specifies the accumulated count the guest was spinning.
> > >
> > > So, it is increased when guest is spinning and reduced when spinlock is
> > > released.
> >
> > But that's a global variable, so it might be incremented and decremented by
> > several CPUs at once. I don't have the spec and have no time to study it
> > either, but global does not make any sense to me. The spin wait info comes
> > from a single guest CPU and the wakeup is targeted at that guest CPU as
> > well. So why global? It might be defined that way, but then you really want
> > to explain it proper.
> >
> > Thanks,
> >
> > tglx
> >
> Let me check this more. Will reply to you later. Thanks!
>
> BRs,
> Yi Sun
I have got the details from Microsoft.
Notify_Long_Spin_Wait hypercall should be called after a specific times
spinning which can be got through CPUID. When hypervisor receives this,
it will try to ensure that all vcpus are scheduled.
Notify_Long_Spin_Wait is a standalone feature which should be split out
as a new patch set.
So I would like split it into a new patch set and submit it after PV
Hyper-V Spinlock patch set merged.
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v1 2/3] locking/pvqspinlock, hv: Enable PV qspinlock for Hyper-V
2018-09-13 9:13 ` [PATCH v1 2/3] locking/pvqspinlock, hv: Enable PV qspinlock for Hyper-V Yi Sun
2018-09-13 11:06 ` Thomas Gleixner
2018-09-13 11:24 ` Thomas Gleixner
@ 2018-09-13 16:16 ` kbuild test robot
2018-09-13 16:42 ` kbuild test robot
3 siblings, 0 replies; 15+ messages in thread
From: kbuild test robot @ 2018-09-13 16:16 UTC (permalink / raw)
To: Yi Sun
Cc: kbuild-all, linux-kernel, x86, chao.p.peng, chao.gao,
isaku.yamahata, michael.h.kelly, tianyu.lan, Yi Sun,
K. Y. Srinivasan, Haiyang Zhang, Stephen Hemminger
[-- Attachment #1: Type: text/plain, Size: 3485 bytes --]
Hi Yi,
Thank you for the patch! Yet something to improve:
[auto build test ERROR on tip/x86/core]
[also build test ERROR on v4.19-rc3 next-20180913]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
url: https://github.com/0day-ci/linux/commits/Yi-Sun/Enable-PV-qspinlock-for-Hyper-V/20180913-220827
config: x86_64-fedora-25 (attached as .config)
compiler: gcc-7 (Debian 7.3.0-1) 7.3.0
reproduce:
# save the attached .config to linux build tree
make ARCH=x86_64
All errors (new ones prefixed by >>):
arch/x86/hyperv/hv_spinlock.c: In function 'hv_init_spinlocks':
>> arch/x86/hyperv/hv_spinlock.c:86:2: error: implicit declaration of function '__pv_init_lock_hash'; did you mean 'spin_lock_bh'? [-Werror=implicit-function-declaration]
__pv_init_lock_hash();
^~~~~~~~~~~~~~~~~~~
spin_lock_bh
>> arch/x86/hyperv/hv_spinlock.c:87:42: error: '__pv_queued_spin_lock_slowpath' undeclared (first use in this function); did you mean 'queued_spin_lock_slowpath'?
pv_lock_ops.queued_spin_lock_slowpath = __pv_queued_spin_lock_slowpath;
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
queued_spin_lock_slowpath
arch/x86/hyperv/hv_spinlock.c:87:42: note: each undeclared identifier is reported only once for each function it appears in
In file included from arch/x86/include/asm/msr.h:246:0,
from arch/x86/include/asm/processor.h:21,
from arch/x86/include/asm/cpufeature.h:5,
from arch/x86/include/asm/thread_info.h:53,
from include/linux/thread_info.h:38,
from arch/x86/include/asm/preempt.h:7,
from include/linux/preempt.h:81,
from include/linux/smp.h:60,
from include/linux/kernel_stat.h:5,
from arch/x86/hyperv/hv_spinlock.c:22:
arch/x86/include/asm/paravirt.h:775:35: error: '__raw_callee_save___pv_queued_spin_unlock' undeclared (first use in this function); did you mean '__raw_callee_save_hv_vcpu_is_preempted'?
((struct paravirt_callee_save) { __raw_callee_save_##func })
^
arch/x86/hyperv/hv_spinlock.c:88:35: note: in expansion of macro 'PV_CALLEE_SAVE'
pv_lock_ops.queued_spin_unlock = PV_CALLEE_SAVE(__pv_queued_spin_unlock);
^~~~~~~~~~~~~~
cc1: some warnings being treated as errors
vim +86 arch/x86/hyperv/hv_spinlock.c
74
75 void __init hv_init_spinlocks(void)
76 {
77 if (!hv_pvspin ||
78 !apic ||
79 !(ms_hyperv.hints & HV_X64_CLUSTER_IPI_RECOMMENDED) ||
80 !(ms_hyperv.features & HV_X64_MSR_GUEST_IDLE_AVAILABLE)) {
81 pr_warn("hv: PV spinlocks disabled\n");
82 return;
83 }
84 pr_info("hv: PV spinlocks enabled\n");
85
> 86 __pv_init_lock_hash();
> 87 pv_lock_ops.queued_spin_lock_slowpath = __pv_queued_spin_lock_slowpath;
88 pv_lock_ops.queued_spin_unlock = PV_CALLEE_SAVE(__pv_queued_spin_unlock);
89 pv_lock_ops.wait = hv_qlock_wait;
90 pv_lock_ops.kick = hv_qlock_kick;
91 pv_lock_ops.vcpu_is_preempted = PV_CALLEE_SAVE(hv_vcpu_is_preempted);
92 }
93
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 48556 bytes --]
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v1 2/3] locking/pvqspinlock, hv: Enable PV qspinlock for Hyper-V
2018-09-13 9:13 ` [PATCH v1 2/3] locking/pvqspinlock, hv: Enable PV qspinlock for Hyper-V Yi Sun
` (2 preceding siblings ...)
2018-09-13 16:16 ` kbuild test robot
@ 2018-09-13 16:42 ` kbuild test robot
3 siblings, 0 replies; 15+ messages in thread
From: kbuild test robot @ 2018-09-13 16:42 UTC (permalink / raw)
To: Yi Sun
Cc: kbuild-all, linux-kernel, x86, chao.p.peng, chao.gao,
isaku.yamahata, michael.h.kelly, tianyu.lan, Yi Sun,
K. Y. Srinivasan, Haiyang Zhang, Stephen Hemminger
[-- Attachment #1: Type: text/plain, Size: 8999 bytes --]
Hi Yi,
Thank you for the patch! Yet something to improve:
[auto build test ERROR on tip/x86/core]
[also build test ERROR on v4.19-rc3 next-20180913]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
url: https://github.com/0day-ci/linux/commits/Yi-Sun/Enable-PV-qspinlock-for-Hyper-V/20180913-220827
config: x86_64-fedora-25 (attached as .config)
compiler: gcc-7 (Debian 7.3.0-1) 7.3.0
reproduce:
# save the attached .config to linux build tree
make ARCH=x86_64
All error/warnings (new ones prefixed by >>):
arch/x86//hyperv/hv_spinlock.c: In function 'hv_init_spinlocks':
arch/x86//hyperv/hv_spinlock.c:86:2: error: implicit declaration of function '__pv_init_lock_hash'; did you mean 'spin_lock_bh'? [-Werror=implicit-function-declaration]
__pv_init_lock_hash();
^~~~~~~~~~~~~~~~~~~
spin_lock_bh
arch/x86//hyperv/hv_spinlock.c:87:42: error: '__pv_queued_spin_lock_slowpath' undeclared (first use in this function); did you mean 'queued_spin_lock_slowpath'?
pv_lock_ops.queued_spin_lock_slowpath = __pv_queued_spin_lock_slowpath;
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
queued_spin_lock_slowpath
arch/x86//hyperv/hv_spinlock.c:87:42: note: each undeclared identifier is reported only once for each function it appears in
In file included from arch/x86/include/asm/msr.h:246:0,
from arch/x86/include/asm/processor.h:21,
from arch/x86/include/asm/cpufeature.h:5,
from arch/x86/include/asm/thread_info.h:53,
from include/linux/thread_info.h:38,
from arch/x86/include/asm/preempt.h:7,
from include/linux/preempt.h:81,
from include/linux/smp.h:60,
from include/linux/kernel_stat.h:5,
from arch/x86//hyperv/hv_spinlock.c:22:
>> arch/x86/include/asm/paravirt.h:775:35: error: '__raw_callee_save___pv_queued_spin_unlock' undeclared (first use in this function); did you mean '__raw_callee_save_hv_vcpu_is_preempted'?
((struct paravirt_callee_save) { __raw_callee_save_##func })
^
>> arch/x86//hyperv/hv_spinlock.c:88:35: note: in expansion of macro 'PV_CALLEE_SAVE'
pv_lock_ops.queued_spin_unlock = PV_CALLEE_SAVE(__pv_queued_spin_unlock);
^~~~~~~~~~~~~~
cc1: some warnings being treated as errors
--
arch/x86/hyperv/hv_spinlock.c: In function 'hv_init_spinlocks':
arch/x86/hyperv/hv_spinlock.c:86:2: error: implicit declaration of function '__pv_init_lock_hash'; did you mean 'spin_lock_bh'? [-Werror=implicit-function-declaration]
__pv_init_lock_hash();
^~~~~~~~~~~~~~~~~~~
spin_lock_bh
arch/x86/hyperv/hv_spinlock.c:87:42: error: '__pv_queued_spin_lock_slowpath' undeclared (first use in this function); did you mean 'queued_spin_lock_slowpath'?
pv_lock_ops.queued_spin_lock_slowpath = __pv_queued_spin_lock_slowpath;
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
queued_spin_lock_slowpath
arch/x86/hyperv/hv_spinlock.c:87:42: note: each undeclared identifier is reported only once for each function it appears in
In file included from arch/x86/include/asm/msr.h:246:0,
from arch/x86/include/asm/processor.h:21,
from arch/x86/include/asm/cpufeature.h:5,
from arch/x86/include/asm/thread_info.h:53,
from include/linux/thread_info.h:38,
from arch/x86/include/asm/preempt.h:7,
from include/linux/preempt.h:81,
from include/linux/smp.h:60,
from include/linux/kernel_stat.h:5,
from arch/x86/hyperv/hv_spinlock.c:22:
>> arch/x86/include/asm/paravirt.h:775:35: error: '__raw_callee_save___pv_queued_spin_unlock' undeclared (first use in this function); did you mean '__raw_callee_save_hv_vcpu_is_preempted'?
((struct paravirt_callee_save) { __raw_callee_save_##func })
^
arch/x86/hyperv/hv_spinlock.c:88:35: note: in expansion of macro 'PV_CALLEE_SAVE'
pv_lock_ops.queued_spin_unlock = PV_CALLEE_SAVE(__pv_queued_spin_unlock);
^~~~~~~~~~~~~~
cc1: some warnings being treated as errors
vim +775 arch/x86/include/asm/paravirt.h
2e47d3e6 include/asm-x86/paravirt.h Glauber de Oliveira Costa 2008-01-30 744
ecb93d1c arch/x86/include/asm/paravirt.h Jeremy Fitzhardinge 2009-01-28 745 /*
ecb93d1c arch/x86/include/asm/paravirt.h Jeremy Fitzhardinge 2009-01-28 746 * Generate a thunk around a function which saves all caller-save
ecb93d1c arch/x86/include/asm/paravirt.h Jeremy Fitzhardinge 2009-01-28 747 * registers except for the return value. This allows C functions to
ecb93d1c arch/x86/include/asm/paravirt.h Jeremy Fitzhardinge 2009-01-28 748 * be called from assembler code where fewer than normal registers are
ecb93d1c arch/x86/include/asm/paravirt.h Jeremy Fitzhardinge 2009-01-28 749 * available. It may also help code generation around calls from C
ecb93d1c arch/x86/include/asm/paravirt.h Jeremy Fitzhardinge 2009-01-28 750 * code if the common case doesn't use many registers.
ecb93d1c arch/x86/include/asm/paravirt.h Jeremy Fitzhardinge 2009-01-28 751 *
ecb93d1c arch/x86/include/asm/paravirt.h Jeremy Fitzhardinge 2009-01-28 752 * When a callee is wrapped in a thunk, the caller can assume that all
ecb93d1c arch/x86/include/asm/paravirt.h Jeremy Fitzhardinge 2009-01-28 753 * arg regs and all scratch registers are preserved across the
ecb93d1c arch/x86/include/asm/paravirt.h Jeremy Fitzhardinge 2009-01-28 754 * call. The return value in rax/eax will not be saved, even for void
ecb93d1c arch/x86/include/asm/paravirt.h Jeremy Fitzhardinge 2009-01-28 755 * functions.
ecb93d1c arch/x86/include/asm/paravirt.h Jeremy Fitzhardinge 2009-01-28 756 */
87b240cb arch/x86/include/asm/paravirt.h Josh Poimboeuf 2016-01-21 757 #define PV_THUNK_NAME(func) "__raw_callee_save_" #func
ecb93d1c arch/x86/include/asm/paravirt.h Jeremy Fitzhardinge 2009-01-28 758 #define PV_CALLEE_SAVE_REGS_THUNK(func) \
ecb93d1c arch/x86/include/asm/paravirt.h Jeremy Fitzhardinge 2009-01-28 759 extern typeof(func) __raw_callee_save_##func; \
ecb93d1c arch/x86/include/asm/paravirt.h Jeremy Fitzhardinge 2009-01-28 760 \
ecb93d1c arch/x86/include/asm/paravirt.h Jeremy Fitzhardinge 2009-01-28 761 asm(".pushsection .text;" \
87b240cb arch/x86/include/asm/paravirt.h Josh Poimboeuf 2016-01-21 762 ".globl " PV_THUNK_NAME(func) ";" \
87b240cb arch/x86/include/asm/paravirt.h Josh Poimboeuf 2016-01-21 763 ".type " PV_THUNK_NAME(func) ", @function;" \
87b240cb arch/x86/include/asm/paravirt.h Josh Poimboeuf 2016-01-21 764 PV_THUNK_NAME(func) ":" \
87b240cb arch/x86/include/asm/paravirt.h Josh Poimboeuf 2016-01-21 765 FRAME_BEGIN \
ecb93d1c arch/x86/include/asm/paravirt.h Jeremy Fitzhardinge 2009-01-28 766 PV_SAVE_ALL_CALLER_REGS \
ecb93d1c arch/x86/include/asm/paravirt.h Jeremy Fitzhardinge 2009-01-28 767 "call " #func ";" \
ecb93d1c arch/x86/include/asm/paravirt.h Jeremy Fitzhardinge 2009-01-28 768 PV_RESTORE_ALL_CALLER_REGS \
87b240cb arch/x86/include/asm/paravirt.h Josh Poimboeuf 2016-01-21 769 FRAME_END \
ecb93d1c arch/x86/include/asm/paravirt.h Jeremy Fitzhardinge 2009-01-28 770 "ret;" \
ecb93d1c arch/x86/include/asm/paravirt.h Jeremy Fitzhardinge 2009-01-28 771 ".popsection")
ecb93d1c arch/x86/include/asm/paravirt.h Jeremy Fitzhardinge 2009-01-28 772
ecb93d1c arch/x86/include/asm/paravirt.h Jeremy Fitzhardinge 2009-01-28 773 /* Get a reference to a callee-save function */
ecb93d1c arch/x86/include/asm/paravirt.h Jeremy Fitzhardinge 2009-01-28 774 #define PV_CALLEE_SAVE(func) \
ecb93d1c arch/x86/include/asm/paravirt.h Jeremy Fitzhardinge 2009-01-28 @775 ((struct paravirt_callee_save) { __raw_callee_save_##func })
ecb93d1c arch/x86/include/asm/paravirt.h Jeremy Fitzhardinge 2009-01-28 776
:::::: The code at line 775 was first introduced by commit
:::::: ecb93d1ccd0aac63f03be2db3cac3fa974716f4c x86/paravirt: add register-saving thunks to reduce caller register pressure
:::::: TO: Jeremy Fitzhardinge <jeremy@goop.org>
:::::: CC: H. Peter Anvin <hpa@linux.intel.com>
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 48556 bytes --]
^ permalink raw reply [flat|nested] 15+ messages in thread