Re: [PATCH 1/2] ipc/sem.c: Fix complex_count vs. simple op race

2016-07-15 Thread Davidlohr Bueso
On Wed, 13 Jul 2016, Manfred Spraul wrote: -static void sem_wait_array(struct sem_array *sma) +static void complexmode_enter(struct sem_array *sma) { int i; struct sem *sem; - if (sma->complex_count) { - /* The thread that increased sma->complex_count

[PATCH 1/2] ipc/sem.c: Fix complex_count vs. simple op race

2016-07-12 Thread Manfred Spraul
Commit 6d07b68ce16a ("ipc/sem.c: optimize sem_lock()") introduced a race: sem_lock has a fast path that allows parallel simple operations. There are two reasons why a simple operation cannot run in parallel: - a non-simple operations is ongoing (sma->sem_perm.lock held) - a complex operation is

Re: [PATCH 1/2] ipc/sem.c: Fix complex_count vs. simple op race

2016-07-01 Thread Davidlohr Bueso
On Thu, 30 Jun 2016, Manfred Spraul wrote: On 06/28/2016 07:27 AM, Davidlohr Bueso wrote: If I understand it right, it means that spin_lock() is both an acquire and a release - for qspinlocks. I wouldn't say that: the _Q_LOCKED_VAL stores are still unordered inside the acquire region but

Re: [PATCH 1/2] ipc/sem.c: Fix complex_count vs. simple op race

2016-06-30 Thread Manfred Spraul
On 06/28/2016 07:27 AM, Davidlohr Bueso wrote: On Thu, 23 Jun 2016, Manfred Spraul wrote: What I'm not sure yet is if smp_load_acquire() is sufficient: Thread A: if (!READ_ONCE(sma->complex_mode)) { The code is test_and_test, no barrier requirements for first test Yeah, it would

Re: [PATCH 1/2] ipc/sem.c: Fix complex_count vs. simple op race

2016-06-27 Thread Davidlohr Bueso
On Thu, 23 Jun 2016, Manfred Spraul wrote: What I'm not sure yet is if smp_load_acquire() is sufficient: Thread A: if (!READ_ONCE(sma->complex_mode)) { The code is test_and_test, no barrier requirements for first test Yeah, it would just make us take the big lock unnecessarily,

[PATCH 1/2] ipc/sem.c: Fix complex_count vs. simple op race

2016-06-25 Thread Manfred Spraul
Commit 6d07b68ce16a ("ipc/sem.c: optimize sem_lock()") introduced a race: sem_lock has a fast path that allows parallel simple operations. There are two reasons why a simple operation cannot run in parallel: - a non-simple operations is ongoing (sma->sem_perm.lock held) - a complex operation is

Re: [PATCH 1/2] ipc/sem.c: Fix complex_count vs. simple op race

2016-06-23 Thread Manfred Spraul
On 06/21/2016 02:30 AM, Davidlohr Bueso wrote: On Sat, 18 Jun 2016, Manfred Spraul wrote: diff --git a/include/linux/sem.h b/include/linux/sem.h index 976ce3a..d0efd6e 100644 --- a/include/linux/sem.h +++ b/include/linux/sem.h @@ -21,6 +21,7 @@ struct sem_array { struct list_head

Re: [PATCH 1/2] ipc/sem.c: Fix complex_count vs. simple op race

2016-06-23 Thread Manfred Spraul
On 06/21/2016 01:04 AM, Andrew Morton wrote: On Sat, 18 Jun 2016 22:02:21 +0200 Manfred Spraul wrote: Commit 6d07b68ce16a ("ipc/sem.c: optimize sem_lock()") introduced a race: sem_lock has a fast path that allows parallel simple operations. There are two reasons

Re: [PATCH 1/2] ipc/sem.c: Fix complex_count vs. simple op race

2016-06-20 Thread Davidlohr Bueso
On Sat, 18 Jun 2016, Manfred Spraul wrote: diff --git a/include/linux/sem.h b/include/linux/sem.h index 976ce3a..d0efd6e 100644 --- a/include/linux/sem.h +++ b/include/linux/sem.h @@ -21,6 +21,7 @@ struct sem_array { struct list_headlist_id;/* undo requests on this array

Re: [PATCH 1/2] ipc/sem.c: Fix complex_count vs. simple op race

2016-06-20 Thread Andrew Morton
On Sat, 18 Jun 2016 22:02:21 +0200 Manfred Spraul wrote: > Commit 6d07b68ce16a ("ipc/sem.c: optimize sem_lock()") introduced a race: > > sem_lock has a fast path that allows parallel simple operations. > There are two reasons why a simple operation cannot run in

[PATCH 1/2] ipc/sem.c: Fix complex_count vs. simple op race

2016-06-18 Thread Manfred Spraul
Commit 6d07b68ce16a ("ipc/sem.c: optimize sem_lock()") introduced a race: sem_lock has a fast path that allows parallel simple operations. There are two reasons why a simple operation cannot run in parallel: - a non-simple operations is ongoing (sma->sem_perm.lock held) - a complex operation is