* Re: [Xenomai-core] [Xenomai-git] Jan Kiszka : Native: Work around for error code corruption in rt_cond_wait[ _until] [not found] <E1NmUAX-0004WJ-Eq@domain.hid> @ 2010-03-02 15:43 ` Gilles Chanteperdrix 2010-03-02 16:00 ` Jan Kiszka 0 siblings, 1 reply; 3+ messages in thread From: Gilles Chanteperdrix @ 2010-03-02 15:43 UTC (permalink / raw) To: Jan Kiszka; +Cc: Xenomai core 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 <jan.kiszka@domain.hid> > 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 <jan.kiszka@domain.hid> > > --- > > 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; -- Gilles. ^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [Xenomai-core] [Xenomai-git] Jan Kiszka : Native: Work around for error code corruption in rt_cond_wait[ _until] 2010-03-02 15:43 ` [Xenomai-core] [Xenomai-git] Jan Kiszka : Native: Work around for error code corruption in rt_cond_wait[ _until] Gilles Chanteperdrix @ 2010-03-02 16:00 ` Jan Kiszka 2010-03-02 16:02 ` Jan Kiszka 0 siblings, 1 reply; 3+ messages in thread From: Jan Kiszka @ 2010-03-02 16:00 UTC (permalink / raw) 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 <jan.kiszka@domain.hid> >> 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 <jan.kiszka@domain.hid> >> >> --- >> >> 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 ^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [Xenomai-core] [Xenomai-git] Jan Kiszka : Native: Work around for error code corruption in rt_cond_wait[ _until] 2010-03-02 16:00 ` Jan Kiszka @ 2010-03-02 16:02 ` Jan Kiszka 0 siblings, 0 replies; 3+ messages in thread From: Jan Kiszka @ 2010-03-02 16:02 UTC (permalink / raw) To: Gilles Chanteperdrix; +Cc: Xenomai core Jan Kiszka wrote: > 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 <jan.kiszka@domain.hid> >>> 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 <jan.kiszka@domain.hid> >>> >>> --- >>> >>> 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 In fact, my current version is wrong /wrt EINTR. Back to the drawing board. > 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 > Jan -- Siemens AG, Corporate Technology, CT T DE IT 1 Corporate Competence Center Embedded Linux ^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2010-03-02 16:02 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <E1NmUAX-0004WJ-Eq@domain.hid>
2010-03-02 15:43 ` [Xenomai-core] [Xenomai-git] Jan Kiszka : Native: Work around for error code corruption in rt_cond_wait[ _until] Gilles Chanteperdrix
2010-03-02 16:00 ` Jan Kiszka
2010-03-02 16:02 ` Jan Kiszka
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.