netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Andrew Morton <akpm@linux-foundation.org>
To: avorontsov@ru.mvista.com
Cc: torvalds@linux-foundation.org, a.p.zijlstra@chello.nl,
	oleg@redhat.com, mingo@elte.hu, linux-kernel@vger.kernel.org,
	netdev@vger.kernel.org
Subject: Re: [PATCH/RFC] sched: Remove SYSTEM_RUNNING checks from cond_resched*()
Date: Wed, 8 Jul 2009 14:47:44 -0700	[thread overview]
Message-ID: <20090708144744.5555b88d.akpm@linux-foundation.org> (raw)
In-Reply-To: <20090708213331.GA9346@oksana.dev.rtsoft.ru>

(belatedly cc'ing netdev)

Original diagnosis:

: Using early netconsole and gianfar driver this error pops up:
: 
:   netconsole: timeout waiting for carrier
: 
: It appears that net/core/netpoll.c:netpoll_setup() is using
: cond_resched() in a loop waiting for a carrier.
: 
: The thing is that cond_resched() is a no-op when system_state !=
: SYSTEM_RUNNING, and so drivers/net/phy/phy.c's state_queue is never
: scheduled, therefore link detection doesn't work

> On Thu, 9 Jul 2009 01:33:31 +0400 Anton Vorontsov <avorontsov@ru.mvista.com> wrote:
> On Wed, Jul 08, 2009 at 02:10:24PM -0700, Andrew Morton wrote:
> > > On Wed, 8 Jul 2009 09:12:30 -0700 (PDT) Linus Torvalds <torvalds@linux-foundation.org> wrote:
> > > That said, I do agree that maybe SYSTEM_RUNNING isn't the right check. 
> > > Testing that the scheduler is initialized may be the more correct one. I 
> > > think the SYSTEM_RUNNING one just comes from that being used for other 
> > > debug issues.
> > 
> > Agreed.  system_state is too general.
> > 
> > If we specifically want to know whether it is safe to call schedule() then
> > let's create a global boolean it_is_safe_to_call_schedule and test that,
> > rather than testing something which indirectly and unreliably implies "it
> > is safe to call schedule".  If that boolean already exists then no-brainer.
> > 
> > All that being said, I wonder if the netconsole code should be using
> > msleep(1) instead.  Spinning on cond_resched() is a bit rude.  But one
> > would have to verify that it is safe to call schedule() at this time, and
> > for the netconsole caller, this is dubious.
> 
> What do you mean by "verify that it is safe"? If it works,
> can I assume that it's safe? ;-) It works, fwiw.
> 

netconsole is supposed to be available as early as possible in boot for
obvious reasons.  I'd say there's a decent risk now and in the future that
netconsole will be initialised prior to the scheduler being available.

In fact, if "netconsole: timeout waiting for carrier" newly added to
netpoll_setup() a depedency on the scheduler being available then perhaps
that was an incorrect change.


       reply	other threads:[~2009-07-08 21:49 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <20090707235812.GA12824@oksana.dev.rtsoft.ru>
     [not found] ` <20090708005000.GA12380@redhat.com>
     [not found]   ` <1247034263.9777.24.camel@twins>
     [not found]     ` <alpine.LFD.2.01.0907080907210.3210@localhost.localdomain>
     [not found]       ` <20090708141024.f8b581c5.akpm@linux-foundation.org>
     [not found]         ` <20090708213331.GA9346@oksana.dev.rtsoft.ru>
2009-07-08 21:47           ` Andrew Morton [this message]
2009-07-08 22:20             ` [PATCH] netpoll: Fix carrier detection for drivers that are using phylib Anton Vorontsov
2009-07-09  0:01               ` Linus Torvalds
2009-07-09  3:08                 ` David Miller
2009-07-09  7:56                 ` Peter Zijlstra
2009-07-09 12:56                   ` Matt Mackall
2009-07-09 13:26                 ` Matt Mackall
2009-07-09 13:46                   ` Peter Zijlstra
2009-07-09 14:18                     ` Matt Mackall
2009-07-09 14:31                       ` Peter Zijlstra
2009-07-09 14:43                         ` Matt Mackall
2009-07-09 14:51                           ` Peter Zijlstra
2009-07-09 15:06                             ` Matt Mackall
2009-07-09 17:29                         ` Linus Torvalds
2009-07-09 12:52               ` Matt Mackall

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20090708144744.5555b88d.akpm@linux-foundation.org \
    --to=akpm@linux-foundation.org \
    --cc=a.p.zijlstra@chello.nl \
    --cc=avorontsov@ru.mvista.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@elte.hu \
    --cc=netdev@vger.kernel.org \
    --cc=oleg@redhat.com \
    --cc=torvalds@linux-foundation.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).