netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Ian Campbell <icampbell@arcom.com>
To: Nicolas Pitre <nico@cam.org>
Cc: Andrew Morton <akpm@osdl.org>,
	linux-kernel@vger.kernel.org, netdev@oss.sgi.com
Subject: Re: "deadlock" between smc91x driver and link_watch
Date: Wed, 24 Nov 2004 15:52:38 +0000	[thread overview]
Message-ID: <1101311558.31459.21.camel@icampbell-debian> (raw)
In-Reply-To: <Pine.LNX.4.61.0411241014160.8946@xanadu.home>

On Wed, 2004-11-24 at 10:21 -0500, Nicolas Pitre wrote:

> > + * smc_phy_configure_wq
> > + *
> > + * The net_device is referenced when the work was scheduled to avoid
> > + * the need for a flush_scheduled_work() in smc_close(). Drop the
> > + * reference and then do the configuration.
> 
> You probably want to invert the comment here too.

Quite right.

> > @@ -1536,10 +1553,8 @@
> >  	/* clear everything */
> >  	smc_shutdown(dev);
> >  
> > -	if (lp->phy_type != 0) {
> > -		flush_scheduled_work();
> > +	if (lp->phy_type != 0)
> >  		smc_phy_powerdown(dev, lp->mii.phy_id);
> 
> 
> How do you ensure that smc_phy_configure() can't end up being called 
> after smc_phy_powerdown() here?

Hmm, I think that smc_phy_configure() is only called from
smc_drv_resume() and smc_timeout() (via the workqueue).

For smc_drv_resume() I expected that there would be some sort of mutual
exclusion in the generic layers to prevent _close() and _resume() from
happening at the same time.

For smc_timeout() I would also expect that the generic layer would have
cancelled the tx_timeout etc before calling smc_close().

I guess I don't know if either of these things are true -- anyone know?

The other solution might be to set phy_type to 0 in smc_phy_powerdown()
and then redetect it in smc_open() and smc_resume(). Or just use another
flag altogether.

Ian.

-- 
Ian Campbell, Senior Design Engineer
                                        Web: http://www.arcom.com
Arcom, Clifton Road,                    Direct: +44 (0)1223 403 465
Cambridge CB1 7EA, United Kingdom       Phone:  +44 (0)1223 411 200


_____________________________________________________________________
The message in this transmission is sent in confidence for the attention of the addressee only and should not be disclosed to any other party. Unauthorised recipients are requested to preserve this confidentiality. Please advise the sender if the addressee is not resident at the receiving end.  Email to and from Arcom is automatically monitored for operational and lawful business reasons.

This message has been virus scanned by MessageLabs.

  reply	other threads:[~2004-11-24 15:52 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <1101230194.14370.12.camel@icampbell-debian>
2004-11-23 23:31 ` "deadlock" between smc91x driver and link_watch Andrew Morton
2004-11-24  9:41   ` Ian Campbell
2004-11-24  9:46     ` Andrew Morton
2004-11-24  9:58       ` Ian Campbell
2004-11-24 15:21         ` Nicolas Pitre
2004-11-24 15:52           ` Ian Campbell [this message]
2004-11-24 16:57             ` Nicolas Pitre
2004-11-24 17:13               ` Ian Campbell
2004-11-25  9:59               ` Ian Campbell
2004-11-25 16:31                 ` Nicolas Pitre

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=1101311558.31459.21.camel@icampbell-debian \
    --to=icampbell@arcom.com \
    --cc=akpm@osdl.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=netdev@oss.sgi.com \
    --cc=nico@cam.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).