ACK/cmnt: [SRU][Artful][v2][PATCH 1/1] UBUNTU: SAUCE: (no-up) s390: fix rwlock implementation

[Joseph Salisbury]

On 05/23/2018 11:25 AM, Stefan Bader wrote:
> On 11.05.2018 15:31, Joseph Salisbury wrote:
>> From: Heiko Carstens <heiko.carstens at de.ibm.com>
>>
>> BugLink: http://bugs.launchpad.net/bugs/1761674
>>
>> Description:  kernel: fix rwlock implementation
>> Symptom:      Kernel hangs, due to deadlock on an rwlock.
>> Problem:      With upstream commit 94232a4332de ("s390/rwlock: improve writer
>>               fairness") rwlock writer fairness was supposed to be
>>               implemented. If a writer tries to take an rwlock it sets
>>               unconditionally the writer bit within the lock word and waits
>>               until all readers have released the lock. This however can lead
>>               to a deadlock since rwlocks can be taken recursively by readers.
>>               If e.g. CPU 0 holds the lock as a reader, and CPU 1 wants to
>>               write-lock the lock, then CPU 1 sets the writer bit and
>>               afterwards busy waits for CPU 0 to release the lock. If now CPU 0
>>               tries to read-lock the lock again (recursively) it will also busy
>>               wait until CPU 1 removes the writer bit, which will never happen,
>>               since it waits for the first reader on CPU 0 to release the lock.
>> Solution:     Revert the rwlock writer fairness semantics again.
>>
>> Signed-off-by: Heiko Carstens <heiko.carstens at de.ibm.com>
>> Signed-off-by: Joseph Salisbury <joseph.salisbury at canonical.com>
> Acked-by: Stefan Bader <stefan.bader at canonical.com>
>
>> ---
> Just to note that this sending v2 (or any other increment) as replies is causing
> very hard to follow threads (at least in thunderbird). It would be much simpler
> to nack the old submission and start a new thread...
>
> -Stefan
I did NAK the v1 here:
https://lists.ubuntu.com/archives/kernel-team/2018-May/092308.html

What's strange though, is the v1 and v2 threads were combined, which you
can see here:
https://lists.ubuntu.com/archives/kernel-team/2018-May/thread.html

It seems adding '[v2]' to the subject is not enough for a new thread to
be started.  I'm using 'git send-email' to send my SRU requests, so
maybe it's something I'm doing wrong.  I'll investigate, so I don't do
it again.  Thanks for pointing this out.

>
>>  arch/s390/lib/spinlock.c | 46 +++++++++-------------------------------------
>>  1 file changed, 9 insertions(+), 37 deletions(-)
>>
>> diff --git a/arch/s390/lib/spinlock.c b/arch/s390/lib/spinlock.c
>> index ffb15bd..b3d5e7a 100644
>> --- a/arch/s390/lib/spinlock.c
>> +++ b/arch/s390/lib/spinlock.c
>> @@ -174,40 +174,18 @@ int _raw_read_trylock_retry(arch_rwlock_t *rw)
>>  EXPORT_SYMBOL(_raw_read_trylock_retry);
>>  
>>  #ifdef CONFIG_HAVE_MARCH_Z196_FEATURES
>> -
>>  void _raw_write_lock_wait(arch_rwlock_t *rw, int prev)
>> -{
>> -	int count = spin_retry;
>> -	int owner, old;
>> -
>> -	owner = 0;
>> -	while (1) {
>> -		if (count-- <= 0) {
>> -			if (owner && arch_vcpu_is_preempted(~owner))
>> -				smp_yield_cpu(~owner);
>> -			count = spin_retry;
>> -		}
>> -		old = ACCESS_ONCE(rw->lock);
>> -		owner = ACCESS_ONCE(rw->owner);
>> -		smp_mb();
>> -		if (old >= 0) {
>> -			prev = __RAW_LOCK(&rw->lock, 0x80000000, __RAW_OP_OR);
>> -			old = prev;
>> -		}
>> -		if ((old & 0x7fffffff) == 0 && prev >= 0)
>> -			break;
>> -	}
>> -}
>> -EXPORT_SYMBOL(_raw_write_lock_wait);
>> -
>> -#else /* CONFIG_HAVE_MARCH_Z196_FEATURES */
>> -
>> +#else
>>  void _raw_write_lock_wait(arch_rwlock_t *rw)
>> +#endif
>>  {
>>  	int count = spin_retry;
>> -	int owner, old, prev;
>> +	int owner, old;
>>  
>> -	prev = 0x80000000;
>> +#ifdef CONFIG_HAVE_MARCH_Z196_FEATURES
>> +	if ((int) prev > 0)
>> +		__RAW_UNLOCK(&rw->lock, 0x7fffffff, __RAW_OP_AND);
>> +#endif
>>  	owner = 0;
>>  	while (1) {
>>  		if (count-- <= 0) {
>> @@ -217,19 +195,13 @@ void _raw_write_lock_wait(arch_rwlock_t *rw)
>>  		}
>>  		old = ACCESS_ONCE(rw->lock);
>>  		owner = ACCESS_ONCE(rw->owner);
>> -		if (old >= 0 &&
>> -		    __atomic_cmpxchg_bool(&rw->lock, old, old | 0x80000000))
>> -			prev = old;
>> -		else
>> -			smp_mb();
>> -		if ((old & 0x7fffffff) == 0 && prev >= 0)
>> +		if (old == 0 && __atomic_cmpxchg_bool(&rw->lock, old, old | 0x80000000))
>>  			break;
>> +		smp_mb();
>>  	}
>>  }
>>  EXPORT_SYMBOL(_raw_write_lock_wait);
>>  
>> -#endif /* CONFIG_HAVE_MARCH_Z196_FEATURES */
>> -
>>  int _raw_write_trylock_retry(arch_rwlock_t *rw)
>>  {
>>  	int count = spin_retry;
>>
>


-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: OpenPGP digital signature
URL: <https://lists.ubuntu.com/archives/kernel-team/attachments/20180523/a81da01d/attachment.sig>