linux-kernel - Re: [PATCH v2 1/1] s390/spinlock: Provide vcpu_is

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Tue, 4 Oct 2016 12:17:32 +0200
From:   Christian Borntraeger <borntraeger@...ibm.com>
To:     Pan Xinhui <xinhui@...ux.vnet.ibm.com>,
        Boqun Feng <boqun.feng@...il.com>
Cc:     Peter Zijlstra <peterz@...radead.org>,
        Martin Schwidefsky <schwidefsky@...ibm.com>,
        Heiko Carstens <heiko.carstens@...ibm.com>,
        Pan Xinhui <xinhui.pan@...ux.vnet.ibm.com>,
        linux-kernel@...r.kernel.org,
        virtualization@...ts.linux-foundation.org,
        linux-s390@...r.kernel.org, kvm@...r.kernel.org,
        xen-devel-request@...ts.xenproject.org, benh@...nel.crashing.org,
        paulus@...ba.org, mpe@...erman.id.au, mingo@...hat.com,
        paulmck@...ux.vnet.ibm.com, will.deacon@....com,
        kernellwp@...il.com, jgross@...e.com, pbonzini@...hat.com,
        bsingharora@...il.com
Subject: Re: [PATCH v2 1/1] s390/spinlock: Provide vcpu_is_preempted

On 09/30/2016 08:35 AM, Pan Xinhui wrote:
> 
> 
> 在 2016/9/30 13:52, Boqun Feng 写道:
>> On Fri, Sep 30, 2016 at 12:49:52PM +0800, Pan Xinhui wrote:
>>>
>>>
>>> 在 2016/9/29 23:51, Christian Borntraeger 写道:
>>>> this implements the s390 backend for commit
>>>> "kernel/sched: introduce vcpu preempted check interface"
>>>> by reworking the existing smp_vcpu_scheduled into
>>>> arch_vcpu_is_preempted. We can then also get rid of the
>>>> local cpu_is_preempted function by moving the
>>>> CIF_ENABLED_WAIT test into arch_vcpu_is_preempted.
>>>>
>>>> Signed-off-by: Christian Borntraeger <borntraeger@...ibm.com>
>>>> ---
>>>
>>> hi, Christian
>>>     thanks for your patch!
>>>
>>>>  arch/s390/include/asm/spinlock.h |  3 +++
>>>>  arch/s390/kernel/smp.c           |  9 +++++++--
>>>>  arch/s390/lib/spinlock.c         | 25 ++++++++-----------------
>>>>  3 files changed, 18 insertions(+), 19 deletions(-)
>>>>
>>>> diff --git a/arch/s390/include/asm/spinlock.h b/arch/s390/include/asm/spinlock.h
>>>> index 63ebf37..e16e02f 100644
>>>> --- a/arch/s390/include/asm/spinlock.h
>>>> +++ b/arch/s390/include/asm/spinlock.h
>>>> @@ -21,6 +21,9 @@ _raw_compare_and_swap(unsigned int *lock, unsigned int old, unsigned int new)
>>>>      return __sync_bool_compare_and_swap(lock, old, new);
>>>>  }
>>>>
>>>> +bool arch_vcpu_is_preempted(int cpu);
>>>> +#define vcpu_is_preempted arch_vcpu_is_preempted
>>>> +
>>>>  /*
>>>>   * Simple spin lock operations.  There are two variants, one clears IRQ's
>>>>   * on the local processor, one does not.
>>>> diff --git a/arch/s390/kernel/smp.c b/arch/s390/kernel/smp.c
>>>> index 7b89a75..4aadd16 100644
>>>> --- a/arch/s390/kernel/smp.c
>>>> +++ b/arch/s390/kernel/smp.c
>>>> @@ -376,10 +376,15 @@ int smp_find_processor_id(u16 address)
>>>>      return -1;
>>>>  }
>>>>
>>>> -int smp_vcpu_scheduled(int cpu)
>>> root@...alpine2-lp13:~/linux# git grep -wn smp_vcpu_scheduled arch/s390/
>>> arch/s390/include/asm/smp.h:34:extern int smp_vcpu_scheduled(int cpu);
>>> arch/s390/include/asm/smp.h:56:static inline int smp_vcpu_scheduled(int cpu) { return 1; }
>>> arch/s390/kernel/smp.c:371:int smp_vcpu_scheduled(int cpu)
>>> arch/s390/lib/spinlock.c:44:    if (smp_vcpu_scheduled(cpu))
>>>
>>>> +bool arch_vcpu_is_preempted(int cpu)
>>>>  {
>>>> -    return pcpu_running(pcpu_devices + cpu);
>>>> +    if (test_cpu_flag_of(CIF_ENABLED_WAIT, cpu))
>>>> +        return false;
>>>> +    if (pcpu_running(pcpu_devices + cpu))
>>>> +        return false;
>>> I saw smp_vcpu_scheduled() returns true always on !SMP system.
>>>
>>> maybe we can do somegthing silimar. like below
>>>
>>> #ifndef CONFIG_SMP
>>> static inline bool arch_vcpu_is_preempted(int cpu) { return !test_cpu_flag_of(CIF_ENABLED_WAIT, cpu); }
>>> #else
>>> ...
>>>
>>> but I can't help thinking that if this is a!SMP system, maybe we could only
>>> #ifndef CONFIG_SMP
>>> static inline bool arch_vcpu_is_preempted(int cpu) { return false; }
>>> #else
>>
>> Why do we need a vcpu_is_preempted() implementation for UP? Where will
>> you use it?
>>
> yep, I also wonder that :)
> 
> But there is a definitaion of smp_vcpu_scheduled() for !SMP kernel.
> So I am a little worried that some code has included this spinlock.h for UP kernel also.
> 
> Hi, Christian
>     Could you help confirms that your patch works on UP? :)

My patch as is seems to work fine for !SMP. So it looks like the extra define
is not necessary and we could simply go with v2