* [Xenomai] Issue in __threadobj_unlock_sched (forge/mercury) @ 2014-05-01 11:42 Matthias Schneider 2014-05-01 12:54 ` Philippe Gerum 0 siblings, 1 reply; 6+ messages in thread From: Matthias Schneider @ 2014-05-01 11:42 UTC (permalink / raw) To: xenomai Hi all, it seems __threadobj_unlock_sched has a slight bug and does not reset the current->policy value. This leads to the old policy being restored, but no longer being available for the next __threadobj_lock_sched / __threadobj_unlock_sched call pair. The attached patch fixes it. Regards, Matthias -------------- next part -------------- A non-text attachment was scrubbed... Name: unlock_sched_fix.patch Type: text/x-patch Size: 512 bytes Desc: not available URL: <http://www.xenomai.org/pipermail/xenomai/attachments/20140501/95ff33ab/attachment.bin> ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Xenomai] Issue in __threadobj_unlock_sched (forge/mercury) 2014-05-01 11:42 [Xenomai] Issue in __threadobj_unlock_sched (forge/mercury) Matthias Schneider @ 2014-05-01 12:54 ` Philippe Gerum 2014-05-01 14:23 ` Matthias Schneider 0 siblings, 1 reply; 6+ messages in thread From: Philippe Gerum @ 2014-05-01 12:54 UTC (permalink / raw) To: Matthias Schneider, xenomai On 05/01/2014 01:42 PM, Matthias Schneider wrote: > Hi all, > > it seems __threadobj_unlock_sched has a slight bug > and does not reset the current->policy value. > This leads to the old policy being restored, but > no longer being available for the next > __threadobj_lock_sched / __threadobj_unlock_sched > call pair. > That would mean that the client code switches between different scheduling policies while holding the scheduler lock? -- Philippe. ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Xenomai] Issue in __threadobj_unlock_sched (forge/mercury) 2014-05-01 12:54 ` Philippe Gerum @ 2014-05-01 14:23 ` Matthias Schneider 2014-05-02 8:27 ` Philippe Gerum 0 siblings, 1 reply; 6+ messages in thread From: Matthias Schneider @ 2014-05-01 14:23 UTC (permalink / raw) To: Philippe Gerum, xenomai ----- Original Message ----- > From: Philippe Gerum <rpm@xenomai.org> > To: Matthias Schneider <ma30002000@yahoo.de>; "xenomai@xenomai.org" <xenomai@xenomai.org> > Cc: > Sent: Thursday, May 1, 2014 2:54 PM > Subject: Re: [Xenomai] Issue in __threadobj_unlock_sched (forge/mercury) > > On 05/01/2014 01:42 PM, Matthias Schneider wrote: > >> Hi all, >> >> it seems __threadobj_unlock_sched has a slight bug >> and does not reset the current->policy value. >> This leads to the old policy being restored, but >> no longer being available for the next >> __threadobj_lock_sched / __threadobj_unlock_sched >> call pair. >> > > That would mean that the client code switches between different > scheduling policies while holding the scheduler lock? > > -- > Philippe. > Consider the following scenario: a) threads may hold SCHED_RT, SCHED_RR or SCHED_OTHER while not in threadobj_lock_sched(). (see e.g. threadobj_set_priority). b) threadobj_lock_sched() saves prio and policy and sets it to SCHED_RT/threadobj_lock_prio c) threadobj_unlock_sched() will restore (correctly) previous settings, but not update the threadobj's policy d) next time threadobj_lock_sched() is called, it will save prio and priority, howver policy will always be SCHED_RT (which is not the currently set policy before upgrading it) e) threadobj_unlock_sched() will restore prio and incorrectly saved policy, which is always SCHED_RT -> threads that originally had SCHED_RR or SCHED_OTHER now have SCHED_RT Changing prio while holding the scheduler lock should be handled fine due to : threadobj_set_priority(): if (thobj->schedlock_depth > 0) { thobj->core.prio_unlocked = prio; thobj->core.policy_unlocked = prio ? SCHED_RT : SCHED_OTHER; threadobj_unlock(thobj); return 0; } Matthias ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Xenomai] Issue in __threadobj_unlock_sched (forge/mercury) 2014-05-01 14:23 ` Matthias Schneider @ 2014-05-02 8:27 ` Philippe Gerum 2014-05-17 14:20 ` Matthias Schneider 0 siblings, 1 reply; 6+ messages in thread From: Philippe Gerum @ 2014-05-02 8:27 UTC (permalink / raw) To: Matthias Schneider, xenomai On 05/01/2014 04:23 PM, Matthias Schneider wrote: > b) threadobj_lock_sched() saves prio and policy and sets it to > SCHED_RT/threadobj_lock_prio Forcing SCHED_RT in threadobj_lock() is the core issue. Doing so allows SCHED_OTHER callers to hold that lock: does it make sense? Maybe it's handy so that client code does not have to care, on the other hand, this very much sounds like introducing priority inversion by design. What would be the use case for holding the sched lock over SCHED_OTHER? -- Philippe. ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Xenomai] Issue in __threadobj_unlock_sched (forge/mercury) 2014-05-02 8:27 ` Philippe Gerum @ 2014-05-17 14:20 ` Matthias Schneider 2014-05-19 13:38 ` Philippe Gerum 0 siblings, 1 reply; 6+ messages in thread From: Matthias Schneider @ 2014-05-17 14:20 UTC (permalink / raw) To: Philippe Gerum, xenomai ----- Original Message ----- > From: Philippe Gerum <rpm@xenomai.org> > To: Matthias Schneider <ma30002000@yahoo.de>; "xenomai@xenomai.org" <xenomai@xenomai.org> > Cc: > Sent: Friday, May 2, 2014 10:27 AM > Subject: Re: [Xenomai] Issue in __threadobj_unlock_sched (forge/mercury) > > On 05/01/2014 04:23 PM, Matthias Schneider wrote: > >> b) threadobj_lock_sched() saves prio and policy and sets it to >> SCHED_RT/threadobj_lock_prio > > Forcing SCHED_RT in threadobj_lock() is the core issue. Doing so allows > SCHED_OTHER callers to hold that lock: does it make sense? Maybe it's > handy so that client code does not have to care, on the other hand, this > very much sounds like introducing priority inversion by design. > > What would be the use case for holding the sched lock over SCHED_OTHER? > > > -- > Philippe. > There are some points I would like to raise about this issue: a) the policy active before holding the "scheduler lock" is actually reapplied, but the threadobj info about this is left inconsistent b) independent from wether SCHED_OTHER is a use case, SCHED_RR and SCHED_RT/SCHED_FIFO definitely are. c) In my recently published FreeRTOS prototype, the "main" thread calling TaskSchedulerStart is actually a SCHED_OTHER thread that takes the scheduler lock Nevertheless, please have a look at the code again, it seems like a simple bug to me. Consider the following: current task is SCHED_RR, prio 5 threadobj_lock_sched() -> SCHED_RT, prio 99 threadobj_unlock_sched() -> SCHED_RR, prio 5 threadobj_lock_sched() -> SCHED_RT, prio 99 threadobj_unlock_sched() -> SCHED_RT(!), prio 5 Regards, Matthias ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Xenomai] Issue in __threadobj_unlock_sched (forge/mercury) 2014-05-17 14:20 ` Matthias Schneider @ 2014-05-19 13:38 ` Philippe Gerum 0 siblings, 0 replies; 6+ messages in thread From: Philippe Gerum @ 2014-05-19 13:38 UTC (permalink / raw) To: Matthias Schneider, xenomai On 05/17/2014 04:20 PM, Matthias Schneider wrote: > ----- Original Message ----- > >> From: Philippe Gerum <rpm@xenomai.org> >> To: Matthias Schneider <ma30002000@yahoo.de>; "xenomai@xenomai.org" <xenomai@xenomai.org> >> Cc: >> Sent: Friday, May 2, 2014 10:27 AM >> Subject: Re: [Xenomai] Issue in __threadobj_unlock_sched (forge/mercury) >> >> On 05/01/2014 04:23 PM, Matthias Schneider wrote: >> >>> b) threadobj_lock_sched() saves prio and policy and sets it to >>> SCHED_RT/threadobj_lock_prio >> >> Forcing SCHED_RT in threadobj_lock() is the core issue. Doing so allows >> SCHED_OTHER callers to hold that lock: does it make sense? Maybe it's >> handy so that client code does not have to care, on the other hand, this >> very much sounds like introducing priority inversion by design. >> >> What would be the use case for holding the sched lock over SCHED_OTHER? >> >> >> -- >> Philippe. >> > > There are some points I would like to raise about this issue: > > a) the policy active before holding the "scheduler lock" is actually reapplied, > but the threadobj info about this is left inconsistent > b) independent from wether SCHED_OTHER is a use case, SCHED_RR and SCHED_RT/SCHED_FIFO > definitely are. > c) In my recently published FreeRTOS prototype, the "main" thread calling > TaskSchedulerStart is actually a SCHED_OTHER thread that takes the scheduler > lock > > Nevertheless, please have a look at the code again, it seems like a simple bug > to me. > It's on hold for now, as it could be part of a more general rework of the threadobj_set_priority() interface. I will definitely follow up asap on this though. -- Philippe. ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2014-05-19 13:38 UTC | newest] Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2014-05-01 11:42 [Xenomai] Issue in __threadobj_unlock_sched (forge/mercury) Matthias Schneider 2014-05-01 12:54 ` Philippe Gerum 2014-05-01 14:23 ` Matthias Schneider 2014-05-02 8:27 ` Philippe Gerum 2014-05-17 14:20 ` Matthias Schneider 2014-05-19 13:38 ` Philippe Gerum
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.