From: Pan Xinhui <hidden> Date: 2016-12-05 10:25:35
pSeries run as a guest and might need pv-qspinlock.
Signed-off-by: Pan Xinhui <redacted>
---
arch/powerpc/kernel/Makefile | 1 +
arch/powerpc/platforms/pseries/Kconfig | 8 ++++++++
2 files changed, 9 insertions(+)
@@ -31,6 +31,14 @@ config ARCH_USE_QUEUED_SPINLOCKSfairlock.Ithasshownagoodperformanceimprovementonx86andalsoppcespeciallyinhighcontentioncases.+configPARAVIRT_SPINLOCKS+bool"Paravirtialization support for qspinlock"+depends onPPC_SPLPAR&&QUEUED_SPINLOCKS+defaulty+help+Ifkernelneedrunasaguestthenenablethisoption.+Generallyitcanletkernelhaveabetterperformace.+configPPC_SPLPARdepends onPPC_PSERIESbool"Support for shared-processor logical partitions"
From: Pan Xinhui <hidden> Date: 2016-12-05 10:25:40
Avoid a function call under native version of qspinlock. On powerNV,
bafore applying this patch, every unlock is expensive. This small
optimizes enhance the performance.
We use static_key with jump_label which removes unnecessary loads of
lppaca and its stuff.
Signed-off-by: Pan Xinhui <redacted>
---
arch/powerpc/include/asm/qspinlock_paravirt.h | 18 +++++++++++++++++-
arch/powerpc/kernel/paravirt.c | 4 ++++
2 files changed, 21 insertions(+), 1 deletion(-)
From: Pan Xinhui <hidden> Date: 2016-12-05 10:25:41
Add two corresponding helper functions to support pv-qspinlock.
For normal use, __spin_yield_cpu will confer current vcpu slices to the
target vcpu(say, a lock holder). If target vcpu is not specified or it
is in running state, such conferging to lpar happens or not depends.
Because hcall itself will introduce latency and a little overhead. And we
do NOT want to suffer any latency on some cases, e.g. in interrupt handler.
The second parameter *confer* can indicate such case.
__spin_wake_cpu is simpiler, it will wake up one vcpu regardless of its
current vcpu state.
Signed-off-by: Pan Xinhui <redacted>
---
arch/powerpc/include/asm/spinlock.h | 4 +++
arch/powerpc/lib/locks.c | 59 +++++++++++++++++++++++++++++++++++++
2 files changed, 63 insertions(+)
@@ -64,9 +64,13 @@ static inline bool vcpu_is_preempted(int cpu)/* We only yield to the hypervisor if we are in shared processor mode */#define SHARED_PROCESSOR (lppaca_shared_proc(local_paca->lppaca_ptr))externvoid__spin_yield(arch_spinlock_t*lock);+externvoid__spin_yield_cpu(intcpu,intconfer);+externvoid__spin_wake_cpu(intcpu);externvoid__rw_yield(arch_rwlock_t*lock);#else /* SPLPAR */#define __spin_yield(x) barrier()+#define __spin_yield_cpu(x, y) barrier()+#define __spin_wake_cpu(x) barrier()#define __rw_yield(x) barrier()#define SHARED_PROCESSOR 0#endif
@@ -23,6 +23,65 @@#include<asm/hvcall.h>#include<asm/smp.h>+/*+*conferourslicestoaspecifiedcpuandreturn.Ifitisinrunningstate+*orcpuis-1,thenwewillcheckconfer.IfconferisNULL,wewillreturn+*otherwiseweconferourslicestolpar.+*/+void__spin_yield_cpu(intcpu,intconfer)+{+unsignedintholder_cpu=cpu,yield_count;++if(cpu==-1)+gotoyield_to_lpar;++BUG_ON(holder_cpu>=nr_cpu_ids);+yield_count=be32_to_cpu(lppaca_of(holder_cpu).yield_count);++/* if cpu is running, confer slices to lpar conditionally*/+if((yield_count&1)==0)+gotoyield_to_lpar;++plpar_hcall_norets(H_CONFER,+get_hard_smp_processor_id(holder_cpu),yield_count);+return;++yield_to_lpar:+if(confer)+plpar_hcall_norets(H_CONFER,-1,0);+}+EXPORT_SYMBOL_GPL(__spin_yield_cpu);++void__spin_wake_cpu(intcpu)+{+unsignedintholder_cpu=cpu;++BUG_ON(holder_cpu>=nr_cpu_ids);+/*+*NOTE:weshouldalwaysdothishcallregardlessof+*theyield_countoftheholder_cpu.+*asthersmightbeacaselikebelow;+*CPU1CPU2+*yielded=true+*if(yielded)+*__spin_wake_cpu()+*__spin_yield_cpu()+*+*Sowemightloseawakeifwechecktheyield_countand+*returndirectlyiftheholder_cpuisrunning.+*IOW.doNOTcodelikebelow.+*yield_count=be32_to_cpu(lppaca_of(holder_cpu).yield_count);+*if((yield_count&1)==0)+*return;+*+*aPRODhcallmarksthetarget_cpuproded,whichcausethenextcede+*orconfercalledonthetarget_cpuinvalid.+*/+plpar_hcall_norets(H_PROD,+get_hard_smp_processor_id(holder_cpu));+}+EXPORT_SYMBOL_GPL(__spin_wake_cpu);+#ifndef CONFIG_QUEUED_SPINLOCKSvoid__spin_yield(arch_spinlock_t*lock){
From: Pan Xinhui <hidden> Date: 2016-12-05 11:01:07
The default pv-qspinlock uses qspinlock(native version of pv-qspinlock).
pv_lock initialization should be done in bootstage with irq disabled.
And if we run as a guest with powerKVM/pHyp shared_processor mode,
restore pv_lock_ops callbacks to pv-qspinlock(pv version) which makes
full use of virtualization.
There is a hash table, we store cpu number into it and the key is lock.
So everytime pv_wait can know who is the lock holder by searching the
lock. Also store the lock in a per_cpu struct, and remove it when we own
the lock. Then pv_wait can know which lock we are spinning on. But the
cpu in the hash table might not be the correct lock holder, as for
performace issue, we does not take care of hash conflict.
Also introduce spin_lock_holder, which tells who owns the lock now.
currently the only user is spin_unlock_wait.
Signed-off-by: Pan Xinhui <redacted>
---
arch/powerpc/include/asm/qspinlock.h | 29 +++-
arch/powerpc/include/asm/qspinlock_paravirt.h | 36 +++++
.../powerpc/include/asm/qspinlock_paravirt_types.h | 13 ++
arch/powerpc/kernel/paravirt.c | 153 +++++++++++++++++++++
arch/powerpc/lib/locks.c | 8 +-
arch/powerpc/platforms/pseries/setup.c | 5 +
6 files changed, 241 insertions(+), 3 deletions(-)
create mode 100644 arch/powerpc/include/asm/qspinlock_paravirt.h
create mode 100644 arch/powerpc/include/asm/qspinlock_paravirt_types.h
create mode 100644 arch/powerpc/kernel/paravirt.c
@@ -27,6 +27,33 @@ static inline int queued_spin_is_locked(struct qspinlock *lock)returnatomic_read(&lock->val);}+#ifdef CONFIG_PARAVIRT_SPINLOCKS+#include<asm/qspinlock_paravirt.h>+/*+*trytoknowwhoisthelockholder,howeveritisnotalwaystrue+*Return:+*-1,wedidnotknowthelockholder.+*othervalue,likelyisthelockholder.+*/+externintspin_lock_holder(void*lock);++staticinlinevoidqueued_spin_lock_slowpath(structqspinlock*lock,u32val)+{+pv_queued_spin_lock(lock,val);+}++staticinlinevoidqueued_spin_unlock(structqspinlock*lock)+{+pv_queued_spin_unlock(lock);+}+#else+#define spin_lock_holder(l) (-1)+staticinlinevoidqueued_spin_unlock(structqspinlock*lock)+{+native_queued_spin_unlock(lock);+}+#endif+#include<asm-generic/qspinlock.h>/* we need override it as ppc has io_sync stuff */
@@ -0,0 +1,36 @@+#ifndef CONFIG_PARAVIRT_SPINLOCKS+#error "do not include this file"+#endif++#ifndef _ASM_QSPINLOCK_PARAVIRT_H+#define _ASM_QSPINLOCK_PARAVIRT_H++#include<asm/qspinlock_paravirt_types.h>++externvoidpv_lock_init(void);+externvoidnative_queued_spin_lock_slowpath(structqspinlock*lock,u32val);+externvoid__pv_init_lock_hash(void);+externvoid__pv_queued_spin_lock_slowpath(structqspinlock*lock,u32val);+externvoid__pv_queued_spin_unlock(structqspinlock*lock);++staticinlinevoidpv_queued_spin_lock(structqspinlock*lock,u32val)+{+pv_lock_op.lock(lock,val);+}++staticinlinevoidpv_queued_spin_unlock(structqspinlock*lock)+{+pv_lock_op.unlock(lock);+}++staticinlinevoidpv_wait(u8*ptr,u8val)+{+pv_lock_op.wait(ptr,val);+}++staticinlinevoidpv_kick(intcpu)+{+pv_lock_op.kick(cpu);+}++#endif
@@ -0,0 +1,153 @@+/*+*Thisprogramisfreesoftware;youcanredistributeitand/ormodify+*itunderthetermsoftheGNUGeneralPublicLicenseaspublishedby+*theFreeSoftwareFoundation;eitherversion2oftheLicense,or+*(atyouroption)anylaterversion.+*/++#include<linux/spinlock.h>+#include<linux/smp.h>+#include<linux/hash.h>+#include<linux/bootmem.h>++/* +2 here is to make sure there is not many conflict*/+#define NUM_LOCK_CPU_ENTRY_SHIFT (order_base_2(NR_CPUS) + 2)+#define NUM_LOCK_CPU_ENTRY (1 << NUM_LOCK_CPU_ENTRY_SHIFT)+/* we can only spin on 4 locks at same time on same cpu*/+#define NUM_LOCKS_PER_CPU 4++staticu16*hash_lock_cpu_ptr;++structlocks_on_cpu{+void*l[NUM_LOCKS_PER_CPU];+intcount;+};++staticDEFINE_PER_CPU(structlocks_on_cpu,node);++staticu16*hash(void*l)+{+intval=hash_ptr(l,NUM_LOCK_CPU_ENTRY_SHIFT);++return&hash_lock_cpu_ptr[val];+}++staticvoid__initinit_hash(void)+{+intsize=NUM_LOCK_CPU_ENTRY*sizeof(*hash_lock_cpu_ptr);++hash_lock_cpu_ptr=memblock_virt_alloc(size,0);+memset(hash_lock_cpu_ptr,0,size);+}++#define lock_get_holder(l) \+((int)(*hash(l)-1))++#define lock_set_holder(l) \+(*hash(l)=raw_smp_processor_id()+1)++intspin_lock_holder(void*lock)+{+/* we might run on PowerNV, which has no hash table ptr*/+if(hash_lock_cpu_ptr)+returnlock_get_holder(lock);+return-1;+}+EXPORT_SYMBOL(spin_lock_holder);++staticvoid*this_cpu_lock(void)+{+structlocks_on_cpu*this_node=this_cpu_ptr(&node);+inti=this_node->count-1;++returnthis_node->l[i];+}++staticvoidcpu_save_lock(void*l)+{+structlocks_on_cpu*this_node=this_cpu_ptr(&node);+inti=this_node->count++;++this_node->l[i]=l;+}++staticvoidcpu_remove_lock(void*l)+{+__this_cpu_dec(node.count);+}++staticvoid__native_queued_spin_unlock(structqspinlock*lock)+{+native_queued_spin_unlock(lock);+}++staticvoid__pv_lock(structqspinlock*lock,u32val)+{+/*+*savethelockwearespinningon+*pv_waitneedknowthislock+*/+cpu_save_lock(lock);++__pv_queued_spin_lock_slowpath(lock,val);++/* as we win the lock, remove it*/+cpu_remove_lock(lock);++/*+*letotherspinnerknowwhoisthelockholder+*wedoesnotneedtounsetlockholderinunlock()+*/+lock_set_holder(lock);+}++staticvoid__pv_wait(u8*ptr,u8val)+{+void*l=this_cpu_lock();+intcpu;+intalways_confer=!in_interrupt();++while(READ_ONCE(*ptr)==val){+HMT_low();+/*+*thelockmightbeunlockedonceandlockedagain+*/+cpu=lock_get_holder(l);++/*+*thedefaultbehaviorof__spin_yield_cpuisyielding+*ourcpuslicestotargetvcpuorlpar(pHyporKVM).+*considerthelatencyofhcallitselfandthepriorityof+*currenttask,wecandoaoptimisation.+*IOW,ifweareininterrupt,andthetargetvcpuisrunning+*wedonotyieldourselftolpar.+*/+__spin_yield_cpu(cpu,always_confer);+}+HMT_medium();+}++staticvoid__pv_kick(intcpu)+{+__spin_wake_cpu(cpu);+}++structpv_lock_opspv_lock_op={+.lock=native_queued_spin_lock_slowpath,+.unlock=__native_queued_spin_unlock,+.wait=NULL,+.kick=NULL,+};+EXPORT_SYMBOL(pv_lock_op);++void__initpv_lock_init(void)+{+if(SHARED_PROCESSOR){+init_hash();+__pv_init_lock_hash();+pv_lock_op.lock=__pv_lock;+pv_lock_op.unlock=__pv_queued_spin_unlock;+pv_lock_op.wait=__pv_wait;+pv_lock_op.kick=__pv_kick;+}+}
From: Pan Xinhui <hidden> Date: 2016-12-05 12:08:49
pSeries/powerNV will use qspinlock from now on.
Signed-off-by: Pan Xinhui <redacted>
---
arch/powerpc/platforms/pseries/Kconfig | 8 ++++++++
1 file changed, 8 insertions(+)
From: Pan Xinhui <hidden> Date: 2016-12-05 13:55:10
This patch add basic code to enable qspinlock on powerpc. qspinlock is
one kind of fairlock implementation. And seen some performance improvement
under some scenarios.
queued_spin_unlock() release the lock by just one write of NULL to the
::locked field which sits at different places in the two endianness
system.
We override some arch_spin_XXX as powerpc has io_sync stuff which makes
sure the io operations are protected by the lock correctly.
There is another special case, see commit
2c610022711 ("locking/qspinlock: Fix spin_unlock_wait() some more")
Signed-off-by: Pan Xinhui <redacted>
---
arch/powerpc/include/asm/qspinlock.h | 66 +++++++++++++++++++++++++++++++
arch/powerpc/include/asm/spinlock.h | 31 +++++++++------
arch/powerpc/include/asm/spinlock_types.h | 4 ++
arch/powerpc/lib/locks.c | 59 +++++++++++++++++++++++++++
4 files changed, 147 insertions(+), 13 deletions(-)
create mode 100644 arch/powerpc/include/asm/qspinlock.h
@@ -60,6 +60,23 @@ static inline bool vcpu_is_preempted(int cpu)}#endif+#if defined(CONFIG_PPC_SPLPAR)+/* We only yield to the hypervisor if we are in shared processor mode */+#define SHARED_PROCESSOR (lppaca_shared_proc(local_paca->lppaca_ptr))+externvoid__spin_yield(arch_spinlock_t*lock);+externvoid__rw_yield(arch_rwlock_t*lock);+#else /* SPLPAR */+#define __spin_yield(x) barrier()+#define __rw_yield(x) barrier()+#define SHARED_PROCESSOR 0+#endif++#ifdef CONFIG_QUEUED_SPINLOCKS+#include<asm/qspinlock.h>+#else++#define arch_spin_relax(lock) __spin_yield(lock)+static__always_inlineintarch_spin_value_unlocked(arch_spinlock_tlock){returnlock.slock==0;
@@ -114,18 +131,6 @@ static inline int arch_spin_trylock(arch_spinlock_t *lock)*held.Conveniently,wehaveawordinthepacathatholdsthis*value.*/--#if defined(CONFIG_PPC_SPLPAR)-/* We only yield to the hypervisor if we are in shared processor mode */-#define SHARED_PROCESSOR (lppaca_shared_proc(local_paca->lppaca_ptr))-externvoid__spin_yield(arch_spinlock_t*lock);-externvoid__rw_yield(arch_rwlock_t*lock);-#else /* SPLPAR */-#define __spin_yield(x) barrier()-#define __rw_yield(x) barrier()-#define SHARED_PROCESSOR 0-#endif-staticinlinevoidarch_spin_lock(arch_spinlock_t*lock){CLEAR_IO_SYNC;
@@ -68,3 +70,60 @@ void __rw_yield(arch_rwlock_t *rw)get_hard_smp_processor_id(holder_cpu),yield_count);}#endif++#ifdef CONFIG_QUEUED_SPINLOCKS+/*+*ThisforbidweloadanoldvalueinanotherLL/SC.BecausetheSChereforce+*anotherLL/SCrepeat.SoweguaranteeallloadsinanotherLLandSCwill+*readcorrectvalue.+*/+staticinlineu32atomic_read_sync(atomic_t*v)+{+u32val;++__asm____volatile__(+"1: "PPC_LWARX(%0,0,%2,0)"\n"+" stwcx. %0, 0, %2\n"+" bne- 1b\n"+:"=&r"(val),"+m"(*v)+:"r"(v)+:"cr0","xer");++returnval;+}++voidqueued_spin_unlock_wait(structqspinlock*lock)+{++u32val;++smp_mb();++/*+*copiedfromgenericqueue_spin_unlock_waitwithlittlemodification+*/+for(;;){+/* need _sync, as we might race with another LL/SC in lock()*/+val=atomic_read_sync(&lock->val);++if(!val)/* not locked, we're done */+gotodone;++if(val&_Q_LOCKED_MASK)/* locked, go wait for unlock */+break;++/* not locked, but pending, wait until we observe the lock */+cpu_relax();+}++/*+*anyunlockisgood.Andneednot_sync,as->valissetbytheSCin+*unlock(),anyloadsinlock()mustseethecorrectvalue.+*/+while(atomic_read(&lock->val)&_Q_LOCKED_MASK)+cpu_relax();+done:+smp_mb();+}+EXPORT_SYMBOL(queued_spin_unlock_wait);+#endif
On Mon, Dec 05, 2016 at 10:19:21AM -0500, Pan Xinhui wrote:
quoted hunk
This patch add basic code to enable qspinlock on powerpc. qspinlock is
one kind of fairlock implementation. And seen some performance improvement
under some scenarios.
queued_spin_unlock() release the lock by just one write of NULL to the
::locked field which sits at different places in the two endianness
system.
We override some arch_spin_XXX as powerpc has io_sync stuff which makes
sure the io operations are protected by the lock correctly.
There is another special case, see commit
2c610022711 ("locking/qspinlock: Fix spin_unlock_wait() some more")
Signed-off-by: Pan Xinhui <redacted>
---
arch/powerpc/include/asm/qspinlock.h | 66 +++++++++++++++++++++++++++++++
arch/powerpc/include/asm/spinlock.h | 31 +++++++++------
arch/powerpc/include/asm/spinlock_types.h | 4 ++
arch/powerpc/lib/locks.c | 59 +++++++++++++++++++++++++++
4 files changed, 147 insertions(+), 13 deletions(-)
create mode 100644 arch/powerpc/include/asm/qspinlock.h
@@ -60,6 +60,23 @@ static inline bool vcpu_is_preempted(int cpu)}#endif+#if defined(CONFIG_PPC_SPLPAR)+/* We only yield to the hypervisor if we are in shared processor mode */+#define SHARED_PROCESSOR (lppaca_shared_proc(local_paca->lppaca_ptr))+externvoid__spin_yield(arch_spinlock_t*lock);+externvoid__rw_yield(arch_rwlock_t*lock);+#else /* SPLPAR */+#define __spin_yield(x) barrier()+#define __rw_yield(x) barrier()+#define SHARED_PROCESSOR 0+#endif++#ifdef CONFIG_QUEUED_SPINLOCKS+#include<asm/qspinlock.h>+#else++#define arch_spin_relax(lock) __spin_yield(lock)+static__always_inlineintarch_spin_value_unlocked(arch_spinlock_tlock){returnlock.slock==0;
@@ -114,18 +131,6 @@ static inline int arch_spin_trylock(arch_spinlock_t *lock)*held.Conveniently,wehaveawordinthepacathatholdsthis*value.*/--#if defined(CONFIG_PPC_SPLPAR)-/* We only yield to the hypervisor if we are in shared processor mode */-#define SHARED_PROCESSOR (lppaca_shared_proc(local_paca->lppaca_ptr))-externvoid__spin_yield(arch_spinlock_t*lock);-externvoid__rw_yield(arch_rwlock_t*lock);-#else /* SPLPAR */-#define __spin_yield(x) barrier()-#define __rw_yield(x) barrier()-#define SHARED_PROCESSOR 0-#endif-staticinlinevoidarch_spin_lock(arch_spinlock_t*lock){CLEAR_IO_SYNC;
@@ -68,3 +70,60 @@ void __rw_yield(arch_rwlock_t *rw)get_hard_smp_processor_id(holder_cpu),yield_count);}#endif++#ifdef CONFIG_QUEUED_SPINLOCKS+/*+*ThisforbidweloadanoldvalueinanotherLL/SC.BecausetheSChereforce+*anotherLL/SCrepeat.SoweguaranteeallloadsinanotherLLandSCwill+*readcorrectvalue.+*/+staticinlineu32atomic_read_sync(atomic_t*v)+{+u32val;++__asm____volatile__(+"1: "PPC_LWARX(%0,0,%2,0)"\n"+" stwcx. %0, 0, %2\n"+" bne- 1b\n"+:"=&r"(val),"+m"(*v)+:"r"(v)+:"cr0","xer");++returnval;+}++voidqueued_spin_unlock_wait(structqspinlock*lock)+{++u32val;++smp_mb();++/*+*copiedfromgenericqueue_spin_unlock_waitwithlittlemodification+*/+for(;;){+/* need _sync, as we might race with another LL/SC in lock()*/+val=atomic_read_sync(&lock->val);++if(!val)/* not locked, we're done */+gotodone;++if(val&_Q_LOCKED_MASK)/* locked, go wait for unlock */+break;++/* not locked, but pending, wait until we observe the lock */+cpu_relax();+}++/*+*anyunlockisgood.Andneednot_sync,as->valissetbytheSCin+*unlock(),anyloadsinlock()mustseethecorrectvalue.+*/
I don't think the comment here about _sync is correct. First, not all
unlock() has a SC part, and for unlock_wait() case there is nothing to
do with whether lock() see the correct value or not. The reason with
_sync is not needed here is:
/*
* _sync() is not needed here, because once we got here, we must already
* read the ->val as LOCKED via a _sync(). Combining the smp_mb()
* before, we guarantee that all the memory accesses before
* unlock_wait() must be observed by the next lock critical section.
*/
Regards,
Boqun
On Mon, Dec 05, 2016 at 10:19:22AM -0500, Pan Xinhui wrote:
quoted hunk
pSeries/powerNV will use qspinlock from now on.
Signed-off-by: Pan Xinhui <redacted>
---
arch/powerpc/platforms/pseries/Kconfig | 8 ++++++++
1 file changed, 8 insertions(+)
I think you just enable qspinlock by default for all PPC platforms. I
guess you need to put
depends on PPC_PSERIES || PPC_POWERNV
here to achieve what you mean in you commit message.
Regards,
Boqun
+ help
+ Enabling this option will let kernel use qspinlock which is a kind of
+ fairlock. It has shown a good performance improvement on x86 and also ppc
+ especially in high contention cases.
+
config PPC_SPLPAR
depends on PPC_PSERIES
bool "Support for shared-processor logical partitions"
--
2.4.11
On Mon, Dec 05, 2016 at 10:19:21AM -0500, Pan Xinhui wrote:
quoted
This patch add basic code to enable qspinlock on powerpc. qspinlock is
one kind of fairlock implementation. And seen some performance improvement
under some scenarios.
queued_spin_unlock() release the lock by just one write of NULL to the
::locked field which sits at different places in the two endianness
system.
We override some arch_spin_XXX as powerpc has io_sync stuff which makes
sure the io operations are protected by the lock correctly.
There is another special case, see commit
2c610022711 ("locking/qspinlock: Fix spin_unlock_wait() some more")
Signed-off-by: Pan Xinhui <redacted>
---
arch/powerpc/include/asm/qspinlock.h | 66 +++++++++++++++++++++++++++++++
arch/powerpc/include/asm/spinlock.h | 31 +++++++++------
arch/powerpc/include/asm/spinlock_types.h | 4 ++
arch/powerpc/lib/locks.c | 59 +++++++++++++++++++++++++++
4 files changed, 147 insertions(+), 13 deletions(-)
create mode 100644 arch/powerpc/include/asm/qspinlock.h
@@ -60,6 +60,23 @@ static inline bool vcpu_is_preempted(int cpu)}#endif+#if defined(CONFIG_PPC_SPLPAR)+/* We only yield to the hypervisor if we are in shared processor mode */+#define SHARED_PROCESSOR (lppaca_shared_proc(local_paca->lppaca_ptr))+externvoid__spin_yield(arch_spinlock_t*lock);+externvoid__rw_yield(arch_rwlock_t*lock);+#else /* SPLPAR */+#define __spin_yield(x) barrier()+#define __rw_yield(x) barrier()+#define SHARED_PROCESSOR 0+#endif++#ifdef CONFIG_QUEUED_SPINLOCKS+#include<asm/qspinlock.h>+#else++#define arch_spin_relax(lock) __spin_yield(lock)+static__always_inlineintarch_spin_value_unlocked(arch_spinlock_tlock){returnlock.slock==0;
@@ -114,18 +131,6 @@ static inline int arch_spin_trylock(arch_spinlock_t *lock)*held.Conveniently,wehaveawordinthepacathatholdsthis*value.*/--#if defined(CONFIG_PPC_SPLPAR)-/* We only yield to the hypervisor if we are in shared processor mode */-#define SHARED_PROCESSOR (lppaca_shared_proc(local_paca->lppaca_ptr))-externvoid__spin_yield(arch_spinlock_t*lock);-externvoid__rw_yield(arch_rwlock_t*lock);-#else /* SPLPAR */-#define __spin_yield(x) barrier()-#define __rw_yield(x) barrier()-#define SHARED_PROCESSOR 0-#endif-staticinlinevoidarch_spin_lock(arch_spinlock_t*lock){CLEAR_IO_SYNC;
@@ -68,3 +70,60 @@ void __rw_yield(arch_rwlock_t *rw)get_hard_smp_processor_id(holder_cpu),yield_count);}#endif++#ifdef CONFIG_QUEUED_SPINLOCKS+/*+*ThisforbidweloadanoldvalueinanotherLL/SC.BecausetheSChereforce+*anotherLL/SCrepeat.SoweguaranteeallloadsinanotherLLandSCwill+*readcorrectvalue.+*/+staticinlineu32atomic_read_sync(atomic_t*v)+{+u32val;++__asm____volatile__(+"1: "PPC_LWARX(%0,0,%2,0)"\n"+" stwcx. %0, 0, %2\n"+" bne- 1b\n"+:"=&r"(val),"+m"(*v)+:"r"(v)+:"cr0","xer");++returnval;+}++voidqueued_spin_unlock_wait(structqspinlock*lock)+{++u32val;++smp_mb();++/*+*copiedfromgenericqueue_spin_unlock_waitwithlittlemodification+*/+for(;;){+/* need _sync, as we might race with another LL/SC in lock()*/+val=atomic_read_sync(&lock->val);++if(!val)/* not locked, we're done */+gotodone;++if(val&_Q_LOCKED_MASK)/* locked, go wait for unlock */+break;++/* not locked, but pending, wait until we observe the lock */+cpu_relax();+}++/*+*anyunlockisgood.Andneednot_sync,as->valissetbytheSCin+*unlock(),anyloadsinlock()mustseethecorrectvalue.+*/
I don't think the comment here about _sync is correct. First, not all
unlock() has a SC part, and for unlock_wait() case there is nothing to
yes, not all unlock has sc, but we are just dealing with sc part or unlock.
do with whether lock() see the correct value or not. The reason with
yep, but loads can be reorded, so the unlock_wait(SC) vs lock(SC) can forbid
any loads between lock(LL) and lock(SC) reading any too new values.
_sync is not needed here is:
/*
* _sync() is not needed here, because once we got here, we must already
* read the ->val as LOCKED via a _sync(). Combining the smp_mb()
* before, we guarantee that all the memory accesses before
* unlock_wait() must be observed by the next lock critical section.
*/
On Mon, Dec 05, 2016 at 10:19:23AM -0500, Pan Xinhui wrote:
quoted hunk
Add two corresponding helper functions to support pv-qspinlock.
For normal use, __spin_yield_cpu will confer current vcpu slices to the
target vcpu(say, a lock holder). If target vcpu is not specified or it
is in running state, such conferging to lpar happens or not depends.
Because hcall itself will introduce latency and a little overhead. And we
do NOT want to suffer any latency on some cases, e.g. in interrupt handler.
The second parameter *confer* can indicate such case.
__spin_wake_cpu is simpiler, it will wake up one vcpu regardless of its
current vcpu state.
Signed-off-by: Pan Xinhui <redacted>
---
arch/powerpc/include/asm/spinlock.h | 4 +++
arch/powerpc/lib/locks.c | 59 +++++++++++++++++++++++++++++++++++++
2 files changed, 63 insertions(+)
@@ -64,9 +64,13 @@ static inline bool vcpu_is_preempted(int cpu)/* We only yield to the hypervisor if we are in shared processor mode */#define SHARED_PROCESSOR (lppaca_shared_proc(local_paca->lppaca_ptr))externvoid__spin_yield(arch_spinlock_t*lock);+externvoid__spin_yield_cpu(intcpu,intconfer);+externvoid__spin_wake_cpu(intcpu);externvoid__rw_yield(arch_rwlock_t*lock);#else /* SPLPAR */#define __spin_yield(x) barrier()+#define __spin_yield_cpu(x, y) barrier()+#define __spin_wake_cpu(x) barrier()#define __rw_yield(x) barrier()#define SHARED_PROCESSOR 0#endif
+
+ if (cpu == -1)
+ goto yield_to_lpar;
+
+ BUG_ON(holder_cpu >= nr_cpu_ids);
+ yield_count = be32_to_cpu(lppaca_of(holder_cpu).yield_count);
+
+ /* if cpu is running, confer slices to lpar conditionally*/
+ if ((yield_count & 1) == 0)
+ goto yield_to_lpar;
+
+ plpar_hcall_norets(H_CONFER,
+ get_hard_smp_processor_id(holder_cpu), yield_count);
+ return;
+
+yield_to_lpar:
+ if (confer)
+ plpar_hcall_norets(H_CONFER, -1, 0);
+}
+EXPORT_SYMBOL_GPL(__spin_yield_cpu);
+
+void __spin_wake_cpu(int cpu)
+{
+ unsigned int holder_cpu = cpu;
And it's even wrong to call the parameter of _wake_cpu() a holder_cpu,
because it's not the current lock holder.
Regards,
Boqun
+
+ BUG_ON(holder_cpu >= nr_cpu_ids);
+ /*
+ * NOTE: we should always do this hcall regardless of
+ * the yield_count of the holder_cpu.
+ * as thers might be a case like below;
+ * CPU 1 CPU 2
+ * yielded = true
+ * if (yielded)
+ * __spin_wake_cpu()
+ * __spin_yield_cpu()
+ *
+ * So we might lose a wake if we check the yield_count and
+ * return directly if the holder_cpu is running.
+ * IOW. do NOT code like below.
+ * yield_count = be32_to_cpu(lppaca_of(holder_cpu).yield_count);
+ * if ((yield_count & 1) == 0)
+ * return;
+ *
+ * a PROD hcall marks the target_cpu proded, which cause the next cede
+ * or confer called on the target_cpu invalid.
+ */
+ plpar_hcall_norets(H_PROD,
+ get_hard_smp_processor_id(holder_cpu));
+}
+EXPORT_SYMBOL_GPL(__spin_wake_cpu);
+
#ifndef CONFIG_QUEUED_SPINLOCKS
void __spin_yield(arch_spinlock_t *lock)
{
--
2.4.11
From: Pan Xinhui <hidden> Date: 2016-12-06 01:24:03
在 2016/12/6 08:58, Boqun Feng 写道:
On Mon, Dec 05, 2016 at 10:19:22AM -0500, Pan Xinhui wrote:
quoted
pSeries/powerNV will use qspinlock from now on.
Signed-off-by: Pan Xinhui <redacted>
---
arch/powerpc/platforms/pseries/Kconfig | 8 ++++++++
1 file changed, 8 insertions(+)
I think you just enable qspinlock by default for all PPC platforms. I
guess you need to put
depends on PPC_PSERIES || PPC_POWERNV
here to achieve what you mean in you commit message.
yes, another good way.
I prefer to put it in pseries/Kconfig as same as pv-qspinlocks config.
when we build nv, it still include pSeries's config anyway.
thanks
xinhui
Regards,
Boqun
quoted
+ help
+ Enabling this option will let kernel use qspinlock which is a kind of
+ fairlock. It has shown a good performance improvement on x86 and also ppc
+ especially in high contention cases.
+
config PPC_SPLPAR
depends on PPC_PSERIES
bool "Support for shared-processor logical partitions"
--
2.4.11
From: Pan Xinhui <hidden> Date: 2016-12-06 01:30:04
在 2016/12/6 09:23, Boqun Feng 写道:
On Mon, Dec 05, 2016 at 10:19:23AM -0500, Pan Xinhui wrote:
quoted
Add two corresponding helper functions to support pv-qspinlock.
For normal use, __spin_yield_cpu will confer current vcpu slices to the
target vcpu(say, a lock holder). If target vcpu is not specified or it
is in running state, such conferging to lpar happens or not depends.
Because hcall itself will introduce latency and a little overhead. And we
do NOT want to suffer any latency on some cases, e.g. in interrupt handler.
The second parameter *confer* can indicate such case.
__spin_wake_cpu is simpiler, it will wake up one vcpu regardless of its
current vcpu state.
Signed-off-by: Pan Xinhui <redacted>
---
arch/powerpc/include/asm/spinlock.h | 4 +++
arch/powerpc/lib/locks.c | 59 +++++++++++++++++++++++++++++++++++++
2 files changed, 63 insertions(+)
@@ -64,9 +64,13 @@ static inline bool vcpu_is_preempted(int cpu)/* We only yield to the hypervisor if we are in shared processor mode */#define SHARED_PROCESSOR (lppaca_shared_proc(local_paca->lppaca_ptr))externvoid__spin_yield(arch_spinlock_t*lock);+externvoid__spin_yield_cpu(intcpu,intconfer);+externvoid__spin_wake_cpu(intcpu);externvoid__rw_yield(arch_rwlock_t*lock);#else /* SPLPAR */#define __spin_yield(x) barrier()+#define __spin_yield_cpu(x, y) barrier()+#define __spin_wake_cpu(x) barrier()#define __rw_yield(x) barrier()#define SHARED_PROCESSOR 0#endif
+
+ if (cpu == -1)
+ goto yield_to_lpar;
+
+ BUG_ON(holder_cpu >= nr_cpu_ids);
+ yield_count = be32_to_cpu(lppaca_of(holder_cpu).yield_count);
+
+ /* if cpu is running, confer slices to lpar conditionally*/
+ if ((yield_count & 1) == 0)
+ goto yield_to_lpar;
+
+ plpar_hcall_norets(H_CONFER,
+ get_hard_smp_processor_id(holder_cpu), yield_count);
+ return;
+
+yield_to_lpar:
+ if (confer)
+ plpar_hcall_norets(H_CONFER, -1, 0);
+}
+EXPORT_SYMBOL_GPL(__spin_yield_cpu);
+
+void __spin_wake_cpu(int cpu)
+{
+ unsigned int holder_cpu = cpu;
And it's even wrong to call the parameter of _wake_cpu() a holder_cpu,
because it's not the current lock holder.
oh, its name is really misleading.
thanks
Regards,
Boqun
quoted
+
+ BUG_ON(holder_cpu >= nr_cpu_ids);
+ /*
+ * NOTE: we should always do this hcall regardless of
+ * the yield_count of the holder_cpu.
+ * as thers might be a case like below;
+ * CPU 1 CPU 2
+ * yielded = true
+ * if (yielded)
+ * __spin_wake_cpu()
+ * __spin_yield_cpu()
+ *
+ * So we might lose a wake if we check the yield_count and
+ * return directly if the holder_cpu is running.
+ * IOW. do NOT code like below.
+ * yield_count = be32_to_cpu(lppaca_of(holder_cpu).yield_count);
+ * if ((yield_count & 1) == 0)
+ * return;
+ *
+ * a PROD hcall marks the target_cpu proded, which cause the next cede
+ * or confer called on the target_cpu invalid.
+ */
+ plpar_hcall_norets(H_PROD,
+ get_hard_smp_processor_id(holder_cpu));
+}
+EXPORT_SYMBOL_GPL(__spin_wake_cpu);
+
#ifndef CONFIG_QUEUED_SPINLOCKS
void __spin_yield(arch_spinlock_t *lock)
{
--
2.4.11
From: Pan Xinhui <hidden> Date: 2016-12-06 02:14:05
在 2016/12/6 09:24, Pan Xinhui 写道:
在 2016/12/6 08:58, Boqun Feng 写道:
quoted
On Mon, Dec 05, 2016 at 10:19:22AM -0500, Pan Xinhui wrote:
quoted
pSeries/powerNV will use qspinlock from now on.
Signed-off-by: Pan Xinhui <redacted>
---
arch/powerpc/platforms/pseries/Kconfig | 8 ++++++++
1 file changed, 8 insertions(+)
I think you just enable qspinlock by default for all PPC platforms. I
guess you need to put
depends on PPC_PSERIES || PPC_POWERNV
here to achieve what you mean in you commit message.
oh, yes, need depends on PPC_PSERIES || PPC_POWERNV.
yes, another good way.
I prefer to put it in pseries/Kconfig as same as pv-qspinlocks config.
when we build nv, it still include pSeries's config anyway.
thanks
xinhui
quoted
Regards,
Boqun
quoted
+ help
+ Enabling this option will let kernel use qspinlock which is a kind of
+ fairlock. It has shown a good performance improvement on x86 and also ppc
+ especially in high contention cases.
+
config PPC_SPLPAR
depends on PPC_PSERIES
bool "Support for shared-processor logical partitions"
--
2.4.11