From mboxrd@z Thu Jan 1 00:00:00 1970 Message-ID: <4B8D0CE7.6020101@domain.hid> Date: Tue, 02 Mar 2010 14:04:39 +0100 From: Gilles Chanteperdrix MIME-Version: 1.0 References: <4B8D0359.4070504@domain.hid> <4B8D0466.7040403@domain.hid> <4B8D0542.1020904@domain.hid> <4B8D05B3.1020202@domain.hid> <4B8D07D7.4040108@domain.hid> <4B8D091A.7070908@domain.hid> <4B8D0BA5.505@domain.hid> In-Reply-To: <4B8D0BA5.505@domain.hid> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit Subject: Re: [Xenomai-core] [Xenomai-git] Jan Kiszka : Native: Fix return code of in-kernel rt_cond_wait[_until] List-Id: Xenomai life and development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Jan Kiszka Cc: Xenomai core Jan Kiszka wrote: > Gilles Chanteperdrix wrote: >> Jan Kiszka wrote: >>> Gilles Chanteperdrix wrote: >>>> Gilles Chanteperdrix wrote: >>>>> Jan Kiszka wrote: >>>>>> Gilles Chanteperdrix wrote: >>>>>>> GIT version control wrote: >>>>>>>> Module: xenomai-jki >>>>>>>> Branch: for-upstream >>>>>>>> Commit: f1dfda551b6997f74141986759932e2723a9f024 >>>>>>>> URL: http://git.xenomai.org/?p=xenomai-jki.git;a=commit;h=f1dfda551b6997f74141986759932e2723a9f024 >>>>>>>> >>>>>>>> Author: Jan Kiszka >>>>>>>> Date: Tue Mar 2 13:17:35 2010 +0100 >>>>>>>> >>>>>>>> Native: Fix return code of in-kernel rt_cond_wait[_until] >>>>>>>> >>>>>>>> In rt_cond_wait_inner, do not let rt_cond_wait_epilogue overwrite the >>>>>>>> primary error code of rt_cond_wait_prologue. This restores the in-kernel >>>>>>>> semantics of rt_cond_wait[_until] that were valid before 97323b3287. >>>>>>>> >>>>>>>> Signed-off-by: Jan Kiszka >>>>>>>> >>>>>>>> --- >>>>>>>> >>>>>>>> ksrc/skins/native/cond.c | 8 ++++---- >>>>>>>> 1 files changed, 4 insertions(+), 4 deletions(-) >>>>>>>> >>>>>>>> diff --git a/ksrc/skins/native/cond.c b/ksrc/skins/native/cond.c >>>>>>>> index 10727d1..2dc0069 100644 >>>>>>>> --- a/ksrc/skins/native/cond.c >>>>>>>> +++ b/ksrc/skins/native/cond.c >>>>>>>> @@ -472,10 +472,10 @@ static int rt_cond_wait_inner(RT_COND *cond, RT_MUTEX *mutex, >>>>>>>> err = rt_cond_wait_prologue(cond, mutex, &lockcnt, >>>>>>>> timeout_mode, timeout); >>>>>>>> >>>>>>>> - if(!err || err == -ETIMEDOUT || err == -EINTR) >>>>>>>> - do { >>>>>>>> - err = rt_cond_wait_epilogue(mutex, lockcnt); >>>>>>>> - } while (err == -EINTR); >>>>>>>> + if (!err || err == -ETIMEDOUT || err == -EINTR) { >>>>>>>> + while (rt_cond_wait_epilogue(mutex, lockcnt) == -EINTR) >>>>>>>> + ; /* empty */ >>>>>>>> + } >>>>>>> Not ok. If rt_cond_wait_epilogue returns an error other than -EINTR, we >>>>>>> want this error to be returned. >>>>>> As I said: This is what we used to do in 2.4 (and early 2.5-rc), and due >>>>>> to the absence of spec on this topic, I would vote for restoring old >>>>>> behavior. >>>>> if rt_cond_wait_epilogue returns an error other than -EINTR, there is an >>>>> error to be signaled, so, it should be returned to the user, not 0 or >>>>> -ETIMEDOUT which would let the user think that he got the mutex back, >>>>> whereas the mutex is probably not properly locked. >>>> And if prologue returned -EINTR, and epilogue ran successfully, we do >>>> not want to return -EINTR to the user either. >>> We surely want as the cond_wait did NOT succeed in that case. >> No, we do not know whether it succeeded, the condition may well have >> been signaled while we no longer were waiting on the synch object. So, >> we should return 0, and let the test the user must have in his code >> decide whether the wakeup was spurious or not. In POSIX land, spurious >> condition wake-up are permitted, I bet we want the same for the >> rt_cond_wait interface. Which is why, BTW, all books on the pthread >> interface recommend to use: >> >> >> while (!cond) >> pthread_cond_wait >> >> and not: >> >> if (!cond) >> pthread_cond_wait >> > > This is not user space, this is (deprecated) kernel space. Let's not > fiddle with this old interface, just restore what we had so that > existing apps can rest in peace. User space is different due to signal > handling anyway. Ok for returning -EINTR, it is documented. Kernel-space is not so different from user-space, rt_task_unblock could wake-up a kernel-space task blocked in a call to rt_cond_wait. However, if the epilogue returns an error, we must return it. > > Jan > -- Gilles.