netpoll: Fix carrier detection for drivers that are using phylib

On Thu, 2009-07-09 at 09:18 -0500, Matt Mackall wrote:
> 
> Sorry if I was unclear. I'm suggesting setting the count so the existing
> PREEMPT_ACTIVE test here fires:
> 
> int __sched _cond_resched(void)
> {
>         if (need_resched() && !(preempt_count() & PREEMPT_ACTIVE) &&
>                                         system_state == SYSTEM_RUNNING) {
>                 __cond_resched();
>                 return 1;
>         }
>         return 0;
> }

Right, /me read preempt and thought a simple preempt inc but didn't read
the code. Shame on me.

So something like (utterly untested and such)

---

 arch/x86/include/asm/thread_info.h |    2 +-
 kernel/sched.c                     |    4 ++--
 2 files changed, 3 insertions(+), 3 deletions(-)



--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

On Thu, 2009-07-09 at 16:31 +0200, Peter Zijlstra wrote:
> On Thu, 2009-07-09 at 09:18 -0500, Matt Mackall wrote:
> > 
> > Sorry if I was unclear. I'm suggesting setting the count so the existing
> > PREEMPT_ACTIVE test here fires:
> > 
> > int __sched _cond_resched(void)
> > {
> >         if (need_resched() && !(preempt_count() & PREEMPT_ACTIVE) &&
> >                                         system_state == SYSTEM_RUNNING) {
> >                 __cond_resched();
> >                 return 1;
> >         }
> >         return 0;
> > }
> 
> Right, /me read preempt and thought a simple preempt inc but didn't read
> the code. Shame on me.
> 
> So something like (utterly untested and such)

Yeah, that's what I had in mind. Probably throw in a define:

/* for disabling scheduling in early boot */
#define PREEMPT_EARLY (1 + PREEMPT_ACTIVE)

and slap a comment on the sub_preempt_count().

Does anything actually use scheduler_running yet? Perhaps my tree is
old.

Also, might_sleep's use of system_state probably bears revisiting.

On Thu, 2009-07-09 at 09:43 -0500, Matt Mackall wrote:

> Yeah, that's what I had in mind. Probably throw in a define:
> 
> /* for disabling scheduling in early boot */
> #define PREEMPT_EARLY (1 + PREEMPT_ACTIVE)
> 
> and slap a comment on the sub_preempt_count().

Right, and visit all the other arch init code ;-)

I'll wait to see if Ingo has anything to say about it and then complete
this thing.

> Does anything actually use scheduler_running yet? Perhaps my tree is
> old.

# git grep scheduler_running
kernel/sched.c:static __read_mostly int scheduler_running;
kernel/sched.c: scheduler_running = 1;
kernel/sched_rt.c:      if (unlikely(!scheduler_running))
kernel/sched_rt.c:      if (unlikely(!scheduler_running))

If memory serves there used to be more, but I think that migrated into
kernel/sched_clock.c, which has sched_clock_running.

> Also, might_sleep's use of system_state probably bears revisiting.

Yeah, all that code is from long before we had scheduler_running (which
was introduced around CFS/.23).

--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

On Thu, 2009-07-09 at 16:51 +0200, Peter Zijlstra wrote:
> > Does anything actually use scheduler_running yet? Perhaps my tree is
> > old.
> 
> # git grep scheduler_running
> kernel/sched.c:static __read_mostly int scheduler_running;
> kernel/sched.c: scheduler_running = 1;
> kernel/sched_rt.c:      if (unlikely(!scheduler_running))
> kernel/sched_rt.c:      if (unlikely(!scheduler_running))
> 
> If memory serves there used to be more, but I think that migrated into
> kernel/sched_clock.c, which has sched_clock_running.

The static in the first line makes that a bit of a head-scratcher? Oh, I
see: it's living in sin. Going to avert my eyes for now.

On Thu, 9 Jul 2009, Peter Zijlstra wrote:
> 
> So something like (utterly untested and such)

This looks like a good patch. Please make it so - who knows what other 
uses of cond_resched() we have in module init routines that might have 
deadlocks without it. The netpoll case got fixed, but please just do this.

I'd like to do my system_state movement too (the thing is, when you load 
drivers as modules you _will_ have "system_state == SYSTEM_RUNNING", so 
any initcall that depends on it being "early boot" is already broken), but 
there's no way that patch is appropriate for post-rc2. This one, however, 
looks appropriate (modulo getting some testing, of course)

		Linus
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html