Andrew, here is the patch I promised :)
Everywhere else in the code, we check sma->sem_perm.deleted under
the semaphore array lock, so we should do teh same here.
The easy fix is to simply not drop and re-take the lock, but keep
it locked.
This patch can be folded into ipcsem-fine-grained-locking-for-semtimedop.patch
Signed-off-by: Rik van Riel <[email protected]>
---
ipc/sem.c | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/ipc/sem.c b/ipc/sem.c
index 5711616..efdaee6 100644
--- a/ipc/sem.c
+++ b/ipc/sem.c
@@ -1243,10 +1243,9 @@ static int semctl_main(struct ipc_namespace *ns, int semid, int semnum,
err = -EIDRM;
goto out_free;
}
- sem_unlock(sma, -1);
- }
+ } else
+ sem_lock(sma, NULL, -1);
- sem_lock(sma, NULL, -1);
for (i = 0; i < sma->sem_nsems; i++)
sem_io[i] = sma->sem_base[i].semval;
sem_unlock(sma, -1);
On Tue, 2013-04-23 at 12:29 -0400, Rik van Riel wrote:
> Andrew, here is the patch I promised :)
>
> Everywhere else in the code, we check sma->sem_perm.deleted under
> the semaphore array lock, so we should do teh same here.
> The easy fix is to simply not drop and re-take the lock, but keep
> it locked.
>
CCing Linus, Sedat and Emmanuel.
This looks very much like a patch I asked Sedat to test during the
weekend, which according to him didn't solve his issue. The patch is
correct in any case.
> This patch can be folded into ipcsem-fine-grained-locking-for-semtimedop.patch
>
> Signed-off-by: Rik van Riel <[email protected]>
Acked-by: Davidlohr Bueso <[email protected]>
> ---
> ipc/sem.c | 5 ++---
> 1 file changed, 2 insertions(+), 3 deletions(-)
>
> diff --git a/ipc/sem.c b/ipc/sem.c
> index 5711616..efdaee6 100644
> --- a/ipc/sem.c
> +++ b/ipc/sem.c
> @@ -1243,10 +1243,9 @@ static int semctl_main(struct ipc_namespace *ns, int semid, int semnum,
> err = -EIDRM;
> goto out_free;
> }
> - sem_unlock(sma, -1);
> - }
> + } else
> + sem_lock(sma, NULL, -1);
>
> - sem_lock(sma, NULL, -1);
> for (i = 0; i < sma->sem_nsems; i++)
> sem_io[i] = sma->sem_base[i].semval;
> sem_unlock(sma, -1);
On Tue, Apr 23, 2013 at 7:36 PM, Davidlohr Bueso <[email protected]> wrote:
> On Tue, 2013-04-23 at 12:29 -0400, Rik van Riel wrote:
>> Andrew, here is the patch I promised :)
>>
>> Everywhere else in the code, we check sma->sem_perm.deleted under
>> the semaphore array lock, so we should do teh same here.
>> The easy fix is to simply not drop and re-take the lock, but keep
>> it locked.
>>
>
> CCing Linus, Sedat and Emmanuel.
>
> This looks very much like a patch I asked Sedat to test during the
> weekend, which according to him didn't solve his issue. The patch is
> correct in any case.
>
Your patch looked a bit different (checked for ipc-lock already taken,
both patches - original and refreshed againt -next attached).
- Sedat -
>> This patch can be folded into ipcsem-fine-grained-locking-for-semtimedop.patch
>>
>> Signed-off-by: Rik van Riel <[email protected]>
>
> Acked-by: Davidlohr Bueso <[email protected]>
>
>> ---
>> ipc/sem.c | 5 ++---
>> 1 file changed, 2 insertions(+), 3 deletions(-)
>>
>> diff --git a/ipc/sem.c b/ipc/sem.c
>> index 5711616..efdaee6 100644
>> --- a/ipc/sem.c
>> +++ b/ipc/sem.c
>> @@ -1243,10 +1243,9 @@ static int semctl_main(struct ipc_namespace *ns, int semid, int semnum,
>> err = -EIDRM;
>> goto out_free;
>> }
>> - sem_unlock(sma, -1);
>> - }
>> + } else
>> + sem_lock(sma, NULL, -1);
>>
>> - sem_lock(sma, NULL, -1);
>> for (i = 0; i < sma->sem_nsems; i++)
>> sem_io[i] = sma->sem_base[i].semval;
>> sem_unlock(sma, -1);
>
>