netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Lukas Wunner <lukas@wunner.de>
To: Andrew Lunn <andrew@lunn.ch>
Cc: Oleksij Rempel <o.rempel@pengutronix.de>,
	Oliver Neukum <oneukum@suse.com>,
	Oleksij Rempel <linux@rempel-privat.de>,
	"netdev@vger.kernel.org" <netdev@vger.kernel.org>,
	Heiner Kallweit <hkallweit1@gmail.com>
Subject: Re: ordering of call to unbind() in usbnet_disconnect
Date: Sat, 26 Mar 2022 14:04:30 +0100	[thread overview]
Message-ID: <20220326130430.GD31022@wunner.de> (raw)
In-Reply-To: <Yj8L2Jl0yyHIyW1m@lunn.ch>

On Sat, Mar 26, 2022 at 01:49:28PM +0100, Andrew Lunn wrote:
> On Sat, Mar 26, 2022 at 01:39:29PM +0100, Lukas Wunner wrote:
> > On Mon, Mar 21, 2022 at 02:10:27PM +0100, Andrew Lunn wrote:
> > > There are two patterns in use at the moment:
> > > 
> > > 1) The phy is attached in open() and detached in close(). There is no
> > >    danger of the netdev disappearing at this time.
> > > 
> > > 2) The PHY is attached during probe, and detached during release.
> > > 
> > > This second case is what is being used here in the USB code. This is
> > > also a common pattern for complex devices. In probe, you get all the
> > > components of a complex devices, stitch them together and then
> > > register the composite device. During release, you unregister the
> > > composite device, and then release all the components. Since this is a
> > > natural model, i think it should work.
> > 
> > I've gone through all drivers and noticed that some of them use a variation
> > of pattern 2 which looks fishy:
> > 
> > On probe, they first attach the PHY, then register the netdev.
> > On remove, they detach the PHY, then unregister the netdev.
> > 
> > Is it legal to detach the PHY from a registered (potentially running)
> > netdev? It looks wrong to me.
> 
> I think the network stack guarantee that the close() method is called
> before unregister completes. It is a common pattern to attach the PHY
> in open() and detach it in close(). The stack itself should not be
> using the PHY when it is down, the exception being IOCTL handlers
> which people often get wrong.

But the PHY is detached from a *running* netdev *before* that netdev
is unregistered (and closed).  Is that really legal?

Thanks,

Lukas

  reply	other threads:[~2022-03-26 13:04 UTC|newest]

Thread overview: 26+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-03-10 11:25 ordering of call to unbind() in usbnet_disconnect Oliver Neukum
2022-03-10 11:38 ` Oleksij Rempel
2022-03-14 18:42   ` Lukas Wunner
2022-03-14 19:14     ` Andrew Lunn
2022-03-15  5:44       ` Oleksij Rempel
2022-03-15  8:32         ` Lukas Wunner
2022-03-15 11:38           ` Oleksij Rempel
2022-03-15 13:28             ` Andrew Lunn
2022-03-17 15:53               ` Oliver Neukum
2022-03-17 21:03                 ` Lukas Wunner
2022-03-21 10:17                 ` Lukas Wunner
2022-03-21 10:43                   ` Oleksij Rempel
2022-03-31  9:35                   ` Oliver Neukum
2022-03-21 10:02               ` Lukas Wunner
2022-03-21 13:10                 ` Andrew Lunn
2022-03-26 12:39                   ` Lukas Wunner
2022-03-26 12:49                     ` Andrew Lunn
2022-03-26 13:04                       ` Lukas Wunner [this message]
2022-03-27  8:37                         ` Oleksij Rempel
2022-03-31  9:20                           ` Oliver Neukum
2022-03-31  9:30                             ` Lukas Wunner
2022-03-31  9:59                               ` Oliver Neukum
2022-03-31 11:22                                 ` Lukas Wunner
2022-03-26 12:25             ` Lukas Wunner
2022-03-26 12:44               ` Andrew Lunn
2022-03-26 13:01                 ` Lukas Wunner

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=20220326130430.GD31022@wunner.de \
    --to=lukas@wunner.de \
    --cc=andrew@lunn.ch \
    --cc=hkallweit1@gmail.com \
    --cc=linux@rempel-privat.de \
    --cc=netdev@vger.kernel.org \
    --cc=o.rempel@pengutronix.de \
    --cc=oneukum@suse.com \
    /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).