Messages in this thread | | | From | Arnd Bergmann <> | Subject | Re: [PATCH 1/2] sched: Drop the need_resched() loop from cond_resched() | Date | Fri, 10 Jul 2009 17:35:29 +0200 |
| |
On Friday 10 July 2009, Frederic Weisbecker wrote: > On Fri, Jul 10, 2009 at 05:17:38PM +0200, Arnd Bergmann wrote: > > On Friday 10 July 2009, Frederic Weisbecker wrote: > > > --- a/kernel/sched.c > > > +++ b/kernel/sched.c > > > @@ -6613,11 +6613,9 @@ static void __cond_resched(void) > > > * PREEMPT_ACTIVE, which could trigger a second > > > * cond_resched() call. > > > */ > > > - do { > > > - add_preempt_count(PREEMPT_ACTIVE); > > > - schedule(); > > > - sub_preempt_count(PREEMPT_ACTIVE); > > > - } while (need_resched()); > > > + add_preempt_count(PREEMPT_ACTIVE); > > > + schedule(); > > > + sub_preempt_count(PREEMPT_ACTIVE); > > > } > > > > > > > If you drop the loop, then you should also remove the comment that > > explains why it was put there. > > > > Hmm, these comments seem to actually explain why we do the PREEMPT_ACTIVE > trick, which is to prevent from cond_resched() recursion, right? >
I think we both misinterpreted the comment, which seemed to refer to older code added by Ingo in 5bbcfd900 "cond_resched(): fix bogus might_sleep() warning" and removed by Andrew in e7b384043e2 "cond_resched() fix".
The original code in Ingos version looked like
static inline void __cond_resched(void) { /* * The BKS might be reacquired before we have dropped * PREEMPT_ACTIVE, which could trigger a second * cond_resched() call. */ if (unlikely(preempt_count())) return; do { add_preempt_count(PREEMPT_ACTIVE); schedule(); ...
So, it's got nothing to do with the loop, but should still be removed because the 'if (unlikely(preempt_count()))' is no longer there.
Arnd <><
| |