From mboxrd@z Thu Jan 1 00:00:00 1970 Message-ID: <4B8D3600.8090000@domain.hid> Date: Tue, 02 Mar 2010 17:00:00 +0100 From: Jan Kiszka MIME-Version: 1.0 References: <4B8D323C.5050306@domain.hid> In-Reply-To: <4B8D323C.5050306@domain.hid> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit Subject: Re: [Xenomai-core] [Xenomai-git] Jan Kiszka : Native: Work around for error code corruption in rt_cond_wait[ _until] List-Id: Xenomai life and development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Gilles Chanteperdrix Cc: Xenomai core Gilles Chanteperdrix wrote: > GIT version control wrote: >> Module: xenomai-jki >> Branch: for-upstream >> Commit: 4b5370dce6f3a655ac104b93b032f3b639206fd5 >> URL: http://git.xenomai.org/?p=xenomai-jki.git;a=commit;h=4b5370dce6f3a655ac104b93b032f3b639206fd5 >> >> Author: Jan Kiszka >> Date: Tue Mar 2 16:24:59 2010 +0100 >> >> Native: Work around for error code corruption in rt_cond_wait[_until] >> >> User space suffers from losing the error code of the cond wait prologue. >> We actually had to return the prologue error to user space and use that >> one after the epilogue succeeded, but the existing kernel ABI does not >> allow this, nor is existing user space prepared for this. >> >> As a work around, keep the prologue error code in xnthread's errcode and >> restore it from there when user space re-enters the kernel for the >> epilogue. This requires no ABI changes, but suffers from potential >> overwriting if the same or some other errcode touching service is called >> from a user space signal handler in between the calls. Moreover, current >> user space also prevents that we can return -EINTR on interruption while >> blocked on the condition variable, which was possible before. >> >> A true fix will require a new syscall and updated user space libs. >> >> Signed-off-by: Jan Kiszka >> >> --- >> >> ksrc/skins/native/syscall.c | 19 +++++++++++++++---- >> 1 files changed, 15 insertions(+), 4 deletions(-) >> >> diff --git a/ksrc/skins/native/syscall.c b/ksrc/skins/native/syscall.c >> index 462ee7e..930a28d 100644 >> --- a/ksrc/skins/native/syscall.c >> +++ b/ksrc/skins/native/syscall.c >> @@ -1841,7 +1841,7 @@ static int __rt_cond_wait_prologue(struct pt_regs *regs) >> RT_MUTEX *mutex; >> RT_COND *cond; >> RTIME timeout; >> - int err; >> + int err, epilogue_err; >> >> if (__xn_safe_copy_from_user(&cph, (void __user *)__xn_reg_arg1(regs), >> sizeof(cph))) >> @@ -1868,11 +1868,15 @@ static int __rt_cond_wait_prologue(struct pt_regs *regs) >> return -EFAULT; >> >> err = rt_cond_wait_prologue(cond, mutex, &lockcnt, timeout_mode, timeout); >> + xnpod_current_thread()->errcode = err; >> >> /* Reacquire the mutex if it was unlocked in the prologue and we were >> not interrupted. */ >> - if (lockcnt && err != -EINTR) >> - err = rt_cond_wait_epilogue(mutex, lockcnt); >> + if (lockcnt && err != -EINTR) { >> + epilogue_err = rt_cond_wait_epilogue(mutex, lockcnt); >> + if (!err) >> + err = epilogue_err; > > IMO, Here should be: > if (epilogue_err < 0) > err = epilogue_err; > That would be wrong (if it equals -EINTR), but I get your point: What should take precedence, prologue or epilogue error? So far the decision was simple (always prologue before 2.5), since we now also consider the epilogue. So we could - always pick the epilogue (except for -EINTR) or - only do so if the prologue reported a "less fatal" error than the epilogue E.g. if the prologue failed due to -EIDRM, I would prefer to always propagate this instead of whatever the epilogue reports. Jan -- Siemens AG, Corporate Technology, CT T DE IT 1 Corporate Competence Center Embedded Linux