All of lore.kernel.org
 help / color / mirror / Atom feed
From: Russell King - ARM Linux admin <linux@armlinux.org.uk>
To: Robert Hancock <hancock@sedsystems.ca>
Cc: netdev@vger.kernel.org, andrew@lunn.ch, f.fainelli@gmail.com,
	hkallweit1@gmail.com
Subject: Re: [PATCH net-next] net: sfp: Stop SFP polling and interrupt handling during shutdown
Date: Fri, 7 Jun 2019 11:42:51 +0100	[thread overview]
Message-ID: <20190607104251.opqdsgjbyweb2rfg@shell.armlinux.org.uk> (raw)
In-Reply-To: <1a329ee9-4292-44a2-90eb-a82ca3de03f3@sedsystems.ca>

On Thu, Jun 06, 2019 at 02:57:22PM -0600, Robert Hancock wrote:
> The idea there was to deal with the case where GPIO interrupts were
> previously raised before shutdown and not yet handled by the threaded
> interrupt handler by the time shutdown is called. After shutdown on the
> SFP completes, the bus the GPIO stuff is on could potentially be shut
> down at any moment, so we really don't want to be digging into the GPIO
> states after that. Locking the mutex there ensures that we don't read a
> stale value for the shutdown flag in the interrupt handler, since AFAIK
> there's no other synchronization around that value.

There are two cases:

1) The interrupt is raised just as sfp_shutdown() is called but before
   the mutex is taken.  We will get the full processing in this case.

2) The interrupt is raised during the mutex-locked bit of sfp_shutdown()
   or after the mutex in sfp_shutdown() is released.  We will get the
   abbreviated processing.

This means that the mutex doesn't provide any protection against full
interrupt processing if it occurs just prior to or during the initial
execution of sfp_shutdown().

All that we need to ensure is that the state of sfp->shutdown is
visible by the time sfp_shutdown() returns, and that none of the
interrupt and worker functions are executing.  We have the worker
functions covered by the synchronous cancelling of them, but not the
interrupts, and as Florian points out, it's probably better to disable
the interrupts, and again, that can be done synchronously to ensure
that the handlers are not running.

If the workers and interrupt handlers are synchronously disabled, we
can be sure by the end of sfp_shutdown() that none of those paths are
executing, so the next time something attempts to trigger them, they
will see sfp->shutdown is set.

I'm not convinced that we even need sfp->shutdown if we have cancelled
the workers and disabled the interrupts.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps up
According to speedtest.net: 11.9Mbps down 500kbps up

  parent reply	other threads:[~2019-06-07 10:42 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-06-06 18:06 [PATCH net-next] net: sfp: Stop SFP polling and interrupt handling during shutdown Robert Hancock
2019-06-06 18:09 ` Russell King - ARM Linux admin
2019-06-06 20:57   ` Robert Hancock
2019-06-07  3:21     ` Florian Fainelli
2019-06-07 10:26     ` Russell King - ARM Linux admin
2019-06-07 10:42     ` Russell King - ARM Linux admin [this message]
2019-06-07 16:08       ` Robert Hancock

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=20190607104251.opqdsgjbyweb2rfg@shell.armlinux.org.uk \
    --to=linux@armlinux.org.uk \
    --cc=andrew@lunn.ch \
    --cc=f.fainelli@gmail.com \
    --cc=hancock@sedsystems.ca \
    --cc=hkallweit1@gmail.com \
    --cc=netdev@vger.kernel.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.