netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Florian Fainelli <f.fainelli@gmail.com>
To: David Miller <davem@davemloft.net>, vivien.didelot@savoirfairelinux.com
Cc: netdev@vger.kernel.org, linux-kernel@vger.kernel.org,
	kernel@savoirfairelinux.com
Subject: Re: [PATCH] net: mdio-gpio: support access that may sleep
Date: Fri, 24 Apr 2015 08:56:34 -0700	[thread overview]
Message-ID: <553A67B2.3040400@gmail.com> (raw)
In-Reply-To: <20150424.110455.2117537129018647564.davem@davemloft.net>

On 24/04/15 08:04, David Miller wrote:
> From: Vivien Didelot <vivien.didelot@savoirfairelinux.com>
> Date: Wed, 22 Apr 2015 13:06:54 -0400
> 
>> Some systems using mdio-gpio may use gpio on message based busses, which
>> require sleeping (e.g. gpio from an I2C I/O expander).
>>
>> Since this driver does not use IRQ handler, it is safe to use the
>> _cansleep suffixed gpio accessors.
>>
>> Signed-off-by: Vivien Didelot <vivien.didelot@savoirfairelinux.com>
> 
> Since this is down underneath the layer of an MII bus, you cannot
> universally say that these routines are always called in a sleepable
> context.
> 
> The PHY layer, and the driver itself above that, might call these
> routines from timers, interruptes etc.

The PHY library calls these routines from its state machine workqueue
for that reason, or from process context (when invoked via ethtool
ioctl). The only special case is phy_mac_interrupt() which is callable
from interrupt context, but schedules the state machine workqueue anyway
to circumvent the "in-interrupt" context.

If we were not doing that, there would be a number of things broken, for
instance the per-MDIO bus mutex would not protect us from anything.

> 
> In fact, since the whole point of this driver is to provide a specific
> implementation for programming registers over an MII bus, it's quite
> rediculuous to say that just because interrupts are not used in this
> implementation it means that sleeping is always valid.
> 
> You have to look at all of the (real and potential) users, all the way
> up into the specific ethernet drivers.

It seems to me like this patch in itself is ok, but if there are
particular drivers you believe are at risk, then yes, we definitively
need to audit those.
-- 
Florian

  reply	other threads:[~2015-04-24 15:57 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-04-22 17:06 [PATCH] net: mdio-gpio: support access that may sleep Vivien Didelot
2015-04-24 15:04 ` David Miller
2015-04-24 15:56   ` Florian Fainelli [this message]
2015-04-24 16:01     ` David Miller
2015-04-24 16:19       ` Florian Fainelli
2015-04-24 16:25         ` David Miller
2015-04-24 17:25     ` Sergei Shtylyov
2015-04-24 17:36       ` Florian Fainelli
2015-04-24 17:42         ` Sergei Shtylyov

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=553A67B2.3040400@gmail.com \
    --to=f.fainelli@gmail.com \
    --cc=davem@davemloft.net \
    --cc=kernel@savoirfairelinux.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=netdev@vger.kernel.org \
    --cc=vivien.didelot@savoirfairelinux.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).