Re: [PATCH-tip v2 2/2] x86/xen: Deprecate xen_nopvspin

[Juergen Gross]

On 02/11/17 14:25, Waiman Long wrote:
> On 11/01/2017 06:01 PM, Boris Ostrovsky wrote:
>> On 11/01/2017 04:58 PM, Waiman Long wrote:
>>> +/* TODO: To be removed in a future kernel version */
>>>  static __init int xen_parse_nopvspin(char *arg)
>>>  {
>>> -   xen_pvspin = false;
>>> +   pr_warn("xen_nopvspin is deprecated, replace it with 
>>> \"pvlock_type=queued\"!\n");
>>> +   if (!pv_spinlock_type)
>>> +   pv_spinlock_type = locktype_queued;
>> Since we currently end up using unfair locks and because you are
>> deprecating xen_nopvspin I wonder whether it would be better to set this
>> to locktype_unfair so that current behavior doesn't change. (Sorry, I
>> haven't responded to your earlier message before you posted this). Juergen?
> 
> I think the latest patch from Juergen in tip is to use native qspinlock
> when xen_nopvspin is specified. Right? That is why I made the current
> choice. I can certainly change to unfair if it is what you guys want.

No, when we are keeping xen_nopvspin (even as deprecated) it should
behave as designed, so locktype_queued is correct.


Juergen
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: [PATCH-tip v2 2/2] x86/xen: Deprecate xen_nopvspin

[Waiman Long]

On 11/01/2017 06:01 PM, Boris Ostrovsky wrote:
> On 11/01/2017 04:58 PM, Waiman Long wrote:
>> +/* TODO: To be removed in a future kernel version */
>>  static __init int xen_parse_nopvspin(char *arg)
>>  {
>> -xen_pvspin = false;
>> +pr_warn("xen_nopvspin is deprecated, replace it with 
>> \"pvlock_type=queued\"!\n");
>> +if (!pv_spinlock_type)
>> +pv_spinlock_type = locktype_queued;
> Since we currently end up using unfair locks and because you are
> deprecating xen_nopvspin I wonder whether it would be better to set this
> to locktype_unfair so that current behavior doesn't change. (Sorry, I
> haven't responded to your earlier message before you posted this). Juergen?

I think the latest patch from Juergen in tip is to use native qspinlock
when xen_nopvspin is specified. Right? That is why I made the current
choice. I can certainly change to unfair if it is what you guys want.

> I am also not sure I agree with making pv_spinlock an enum *and* a
> bitmask at the same time. I understand that it makes checks easier but I
> think not assuming a value or a pattern would be better, especially
> since none of the uses is on a critical path.
>
> (For example, !pv_spinlock_type is the same as locktype_auto, which is
> defined but never used)

OK, I will take out the enum and make explicit use of locktype_auto.

Cheers,
Longman
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: [PATCH-tip v2 2/2] x86/xen: Deprecate xen_nopvspin

[Boris Ostrovsky]

On 11/01/2017 04:58 PM, Waiman Long wrote:
> +/* TODO: To be removed in a future kernel version */
>  static __init int xen_parse_nopvspin(char *arg)
>  {
> - xen_pvspin = false;
> + pr_warn("xen_nopvspin is deprecated, replace it with 
> \"pvlock_type=queued\"!\n");
> + if (!pv_spinlock_type)
> + pv_spinlock_type = locktype_queued;

Since we currently end up using unfair locks and because you are
deprecating xen_nopvspin I wonder whether it would be better to set this
to locktype_unfair so that current behavior doesn't change. (Sorry, I
haven't responded to your earlier message before you posted this). Juergen?

I am also not sure I agree with making pv_spinlock an enum *and* a
bitmask at the same time. I understand that it makes checks easier but I
think not assuming a value or a pattern would be better, especially
since none of the uses is on a critical path.

(For example, !pv_spinlock_type is the same as locktype_auto, which is
defined but never used)


-boris

>   return 0;
>  }
>  early_param("xen_nopvspin", xen_parse_nopvspin);
> -

___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

[PATCH-tip v2 2/2] x86/xen: Deprecate xen_nopvspin

[Waiman Long]

With the new pvlock_type kernel parameter, xen_nopvspin is no longer
needed. This patch deprecates the xen_nopvspin parameter by removing
its documentation and treating it as an alias of "pvlock_type=queued".

Signed-off-by: Waiman Long 
---
 Documentation/admin-guide/kernel-parameters.txt |  4 
 arch/x86/xen/spinlock.c | 19 +++
 2 files changed, 7 insertions(+), 16 deletions(-)

diff --git a/Documentation/admin-guide/kernel-parameters.txt 
b/Documentation/admin-guide/kernel-parameters.txt
index c98d9c7..683a817 100644
--- a/Documentation/admin-guide/kernel-parameters.txt
+++ b/Documentation/admin-guide/kernel-parameters.txt
@@ -4596,10 +4596,6 @@
the unplug protocol
never -- do not unplug even if version check succeeds
 
-   xen_nopvspin[X86,XEN]
-   Disables the ticketlock slowpath using Xen PV
-   optimizations.
-
xen_nopv[X86]
Disables the PV optimizations forcing the HVM guest to
run as generic HVM guest with no PV drivers.
diff --git a/arch/x86/xen/spinlock.c b/arch/x86/xen/spinlock.c
index d5f79ac..19e2e75 100644
--- a/arch/x86/xen/spinlock.c
+++ b/arch/x86/xen/spinlock.c
@@ -20,7 +20,6 @@
 
 static DEFINE_PER_CPU(int, lock_kicker_irq) = -1;
 static DEFINE_PER_CPU(char *, irq_name);
-static bool xen_pvspin = true;
 
 #include 
 
@@ -81,12 +80,8 @@ void xen_init_lock_cpu(int cpu)
int irq;
char *name;
 
-   if (!xen_pvspin ||
-  (pv_spinlock_type & (locktype_queued|locktype_unfair))) {
-   if ((cpu == 0) && !pv_spinlock_type)
-   static_branch_disable(_spin_lock_key);
+   if (pv_spinlock_type & (locktype_queued|locktype_unfair))
return;
-   }
 
WARN(per_cpu(lock_kicker_irq, cpu) >= 0, "spinlock on CPU%d exists on 
IRQ%d!\n",
 cpu, per_cpu(lock_kicker_irq, cpu));
@@ -110,8 +105,7 @@ void xen_init_lock_cpu(int cpu)
 
 void xen_uninit_lock_cpu(int cpu)
 {
-   if (!xen_pvspin ||
-  (pv_spinlock_type & (locktype_queued|locktype_unfair)))
+   if (pv_spinlock_type & (locktype_queued|locktype_unfair))
return;
 
unbind_from_irqhandler(per_cpu(lock_kicker_irq, cpu), NULL);
@@ -132,8 +126,7 @@ void xen_uninit_lock_cpu(int cpu)
  */
 void __init xen_init_spinlocks(void)
 {
-   if (!xen_pvspin ||
-  (pv_spinlock_type & (locktype_queued|locktype_unfair))) {
+   if (pv_spinlock_type & (locktype_queued|locktype_unfair)) {
printk(KERN_DEBUG "xen: PV spinlocks disabled\n");
return;
}
@@ -147,10 +140,12 @@ void __init xen_init_spinlocks(void)
pv_lock_ops.vcpu_is_preempted = PV_CALLEE_SAVE(xen_vcpu_stolen);
 }
 
+/* TODO: To be removed in a future kernel version */
 static __init int xen_parse_nopvspin(char *arg)
 {
-   xen_pvspin = false;
+   pr_warn("xen_nopvspin is deprecated, replace it with 
\"pvlock_type=queued\"!\n");
+   if (!pv_spinlock_type)
+   pv_spinlock_type = locktype_queued;
return 0;
 }
 early_param("xen_nopvspin", xen_parse_nopvspin);
-
-- 
1.8.3.1

___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization