Messages in this thread | | | Date | Fri, 10 Jul 2009 18:11:41 +0200 | From | Ingo Molnar <> | Subject | Re: [PATCH 1/2] sched: Drop the need_resched() loop from cond_resched() |
| |
* Frederic Weisbecker <fweisbec@gmail.com> wrote:
> On Fri, Jul 10, 2009 at 05:35:29PM +0200, Arnd Bergmann wrote: > > 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. > > > Yeah, but the comment still fits the code after this patch, don't > you think? :-)
... except that there's no Big Kernel Semaphore anymore ;-)
Ingo
| |