Hello Christophe, On Sat, 2020-03-28 at 10:19 +0000, Christophe Leroy wrote: > Hi Leonardo, > > > > On 03/27/2020 03:51 PM, Leonardo Bras wrote: > > > > > [SNIP] > > - If the lock is already free, it would change nothing, > > - Otherwise, the lock will wait. > > - Waiting cycle just got bigger. > > - Worst case scenario: running one more cycle, given lock->slock can > > turn to 0 just after checking.Could you please point where I failed to see > > the performance penalty? > > (I need to get better at this :) ) > > You are right that when the lock is free, it changes nothing. However > when it is not, it is not just one cycle.
Sorry, what I meant here is one "waiting cycle", meaning that in WCS there would be 1 extra iteration on that while. Or it would 'spin' one more time. > > Here is arch_spin_lock() without your patch: > > 00000440 <my_lock>: > 440: 39 40 00 01 li r10,1 > 444: 7d 20 18 28 lwarx r9,0,r3 > 448: 2c 09 00 00 cmpwi r9,0 > 44c: 40 82 00 10 bne 45c <my_lock+0x1c> > 450: 7d 40 19 2d stwcx. r10,0,r3 > 454: 40 a2 ff f0 bne 444 <my_lock+0x4> > 458: 4c 00 01 2c isync > 45c: 2f 89 00 00 cmpwi cr7,r9,0 > 460: 4d be 00 20 bclr+ 12,4*cr7+eq > 464: 7c 21 0b 78 mr r1,r1 > 468: 81 23 00 00 lwz r9,0(r3) > 46c: 2f 89 00 00 cmpwi cr7,r9,0 > 470: 40 be ff f4 bne cr7,464 <my_lock+0x24> > 474: 7c 42 13 78 mr r2,r2 > 478: 7d 20 18 28 lwarx r9,0,r3 > 47c: 2c 09 00 00 cmpwi r9,0 > 480: 40 82 00 10 bne 490 <my_lock+0x50> > 484: 7d 40 19 2d stwcx. r10,0,r3 > 488: 40 a2 ff f0 bne 478 <my_lock+0x38> > 48c: 4c 00 01 2c isync > 490: 2f 89 00 00 cmpwi cr7,r9,0 > 494: 40 be ff d0 bne cr7,464 <my_lock+0x24> > 498: 4e 80 00 20 blr > > Here is arch_spin_lock() with your patch. I enclose with === what comes > in addition: > > 00000440 <my_lock>: > 440: 39 40 00 01 li r10,1 > 444: 7d 20 18 28 lwarx r9,0,r3 > 448: 2c 09 00 00 cmpwi r9,0 > 44c: 40 82 00 10 bne 45c <my_lock+0x1c> > 450: 7d 40 19 2d stwcx. r10,0,r3 > 454: 40 a2 ff f0 bne 444 <my_lock+0x4> > 458: 4c 00 01 2c isync > 45c: 2f 89 00 00 cmpwi cr7,r9,0 > 460: 4d be 00 20 bclr+ 12,4*cr7+eq > ===================================================== > 464: 3d 40 00 00 lis r10,0 > 466: R_PPC_ADDR16_HA crash_skip_spinlock > 468: 39 4a 00 00 addi r10,r10,0 > 46a: R_PPC_ADDR16_LO crash_skip_spinlock > 46c: 39 00 00 01 li r8,1 > 470: 89 2a 00 00 lbz r9,0(r10) > 474: 2f 89 00 00 cmpwi cr7,r9,0 > 478: 4c 9e 00 20 bnelr cr7 > ===================================================== > 47c: 7c 21 0b 78 mr r1,r1 > 480: 81 23 00 00 lwz r9,0(r3) > 484: 2f 89 00 00 cmpwi cr7,r9,0 > 488: 40 be ff f4 bne cr7,47c <my_lock+0x3c> > 48c: 7c 42 13 78 mr r2,r2 > 490: 7d 20 18 28 lwarx r9,0,r3 > 494: 2c 09 00 00 cmpwi r9,0 > 498: 40 82 00 10 bne 4a8 <my_lock+0x68> > 49c: 7d 00 19 2d stwcx. r8,0,r3 > 4a0: 40 a2 ff f0 bne 490 <my_lock+0x50> > 4a4: 4c 00 01 2c isync > 4a8: 2f 89 00 00 cmpwi cr7,r9,0 > 4ac: 40 be ff c4 bne cr7,470 <my_lock+0x30> > 4b0: 4e 80 00 20 blr > > > Christophe I agree. When there is waiting, it will usually add some time to it. Accounting that spinlocks are widely used, it will cause a slowdown in the whole system. Thanks for the feedback, Best regards,
signature.asc
Description: This is a digitally signed message part
