Re: [PATCH v2 4/4] ipc: sem: do not hold ipc lock more than necessary

From: Rik van Riel
Date: Wed Mar 20 2013 - 11:39:03 EST


On 03/20/2013 10:27 AM, Rik van Riel wrote:
On 03/05/2013 04:36 AM, Davidlohr Bueso wrote:

@@ -1476,8 +1539,8 @@ SYSCALL_DEFINE4(semtimedop, int, semid, struct
sembuf __user *, tsops,
queue.sleeper = current;

sleep_again:
- current->state = TASK_INTERRUPTIBLE;
sem_unlock(sma);
+ current->state = TASK_INTERRUPTIBLE;

if (timeout)
jiffies_left = schedule_timeout(jiffies_left);

After modifying my test case to start with a semaphore value of 1 on
every semaphore, and do down followed by up (to have only one process
take each semaphore at a time), I started seeing lost wakeups and the
test case being stuck.

I believe the change above is the cause of that issue.

By unlocking before setting current->state to TASK_INTERRUPTIBLE,
there is a small window where the next lock holder can grab the
lock and wake us up, before we set ourselves to TASK_INTERRUPTIBLE
and go to sleep.

I have reverted your change in my code and am building a test kernel
now.

If things work, I'll clean up the whole patch series for a re-posting
today.

Half a billion semaphore operations later, I am pretty sure
the above was the cause of the semaphore hangups we both
observed :)

I am currently building a kernel with the cleaned up patch
series I put together while building the previous test kernel.

If all goes well, expect a patch series after lunch...

--
All rights reversed
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/