linuxppc-dev.lists.ozlabs.org archive mirror
 help / color / mirror / Atom feed
From: Benjamin Herrenschmidt <benh@kernel.crashing.org>
To: Bjorn Helgaas <helgaas@kernel.org>,
	"Bryant G. Ly" <bryantly@linux.vnet.ibm.com>
Cc: paulus@samba.org, mpe@ellerman.id.au, seroyer@linux.vnet.ibm.com,
	jjalvare@linux.vnet.ibm.com, alex.williamson@redhat.com,
	aik@ozlabs.ru, ruscur@russell.cc, linux-pci@vger.kernel.org,
	linuxppc-dev@lists.ozlabs.org, bodong@mellanox.com,
	eli@mellanox.com, saeedm@mellanox.com,
	Keith Busch <keith.busch@intel.com>,
	Gabriele Paoloni <gabriele.paoloni@huawei.com>,
	Dongdong Liu <liudongdong3@huawei.com>
Subject: Re: [PATCH v2 2/7] powerpc/kernel: Add uevents in EEH error/resume
Date: Tue, 19 Dec 2017 17:27:58 +1100	[thread overview]
Message-ID: <1513664878.2743.8.camel@kernel.crashing.org> (raw)
In-Reply-To: <20171219045009.GC14941@bhelgaas-glaptop.roam.corp.google.com>

On Mon, 2017-12-18 at 22:50 -0600, Bjorn Helgaas wrote:
> [+cc Keith, Gabriele, Dongdong]
> 
> On Mon, Dec 18, 2017 at 04:38:03PM -0600, Bryant G. Ly wrote:
> > Devices can go offline when EEH is reported. This patch adds
> > a change to the kernel object and lets udev know of error.
> > When device resumes a change is also set reporting device as
> > online. Therefore, EEH events are better propagated to user
> > space for devices in powerpc arch.
> 
> I'm on vacation and can't review this in detail, but I wonder if you
> can compare this with the uevents we emit for DPC, AER, and hotplug
> events (if any).  I hope we don't end up with userspace having to be
> aware of the differences between EEH, DPC, AER, etc.
> 
> > From a very quick look, I only see a few uevents even mentioned in
> 
> drivers/pci: KOBJ_ADD in __pci_hp_register() and KOBJ_CHANGE in the
> SR-IOV code.  I'm worried that we're missing some important uevents in
> the PCI core.  That's not an argument against what you're doing here;
> it just would be nice to fill in any missing pieces in the core also,
> and hopefully make them consistent with these EEH events.

We also need to be careful about what specific EEH activity we are
talking about, and if we bring into the picture things like DPDK, it
gets even more murky...

The basic way EEH is supposed to work for recovery (minus all sort of
implementation nasties which hopefully Russell and Sam are trying to
cleanup and fix) is that either:

	- The driver of the device has recovery callbacks, in which
case the driver participates in the recovery process, the device
doesn't "go away" (though it shouldn't be accessed during that process
by other entities, userspace originated config space could be a problem
and needs to be blocked...). The recovery typically involves a reset of
the device but in sync with the driver.

	- The driver doesn't have the callbacks. In this case, we
simulate an unplug, reset the device, and replug.

So it makes sense for the second case to emit the same uevents as a
normal PCI(e) hotplug.

For the former case I'm less sure.... Do we really need userspace to be
notified ? If yes, what for precisely ?

Cheers,
Ben.
 

  parent reply	other threads:[~2017-12-19  6:28 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-12-18 22:38 [PATCH v1 0/7] SR-IOV Enablement on PowerVM Bryant G. Ly
2017-12-18 22:38 ` [PATCH v2 1/7] platform/pseries: Update VF config space after EEH Bryant G. Ly
2017-12-18 22:38 ` [PATCH v2 2/7] powerpc/kernel: Add uevents in EEH error/resume Bryant G. Ly
2017-12-19  4:50   ` Bjorn Helgaas
2017-12-19  4:59     ` Russell Currey
2017-12-21  3:04       ` Juan Alvarez
2017-12-19  6:27     ` Benjamin Herrenschmidt [this message]
2017-12-21  3:04       ` Juan Alvarez
2017-12-28 23:22         ` Bjorn Helgaas
2017-12-29  0:02           ` Benjamin Herrenschmidt
2017-12-18 22:38 ` [PATCH v2 3/7] platforms/pseries: Set eeh_pe of EEH_PE_VF type Bryant G. Ly
2017-12-18 22:38 ` [PATCH v2 4/7] powerpc/kernel Add EEH operations to notify resume Bryant G. Ly
2017-12-18 22:38 ` [PATCH v2 5/7] powerpc/kernel: Add EEH notify resume sysfs Bryant G. Ly
2017-12-18 22:38 ` [PATCH v2 6/7] pseries/pci: Associate PEs to VFs in configure SR-IOV Bryant G. Ly
2017-12-18 22:38 ` [PATCH v2 7/7] pseries/setup: Add Initialization of VF Bars Bryant G. Ly

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=1513664878.2743.8.camel@kernel.crashing.org \
    --to=benh@kernel.crashing.org \
    --cc=aik@ozlabs.ru \
    --cc=alex.williamson@redhat.com \
    --cc=bodong@mellanox.com \
    --cc=bryantly@linux.vnet.ibm.com \
    --cc=eli@mellanox.com \
    --cc=gabriele.paoloni@huawei.com \
    --cc=helgaas@kernel.org \
    --cc=jjalvare@linux.vnet.ibm.com \
    --cc=keith.busch@intel.com \
    --cc=linux-pci@vger.kernel.org \
    --cc=linuxppc-dev@lists.ozlabs.org \
    --cc=liudongdong3@huawei.com \
    --cc=mpe@ellerman.id.au \
    --cc=paulus@samba.org \
    --cc=ruscur@russell.cc \
    --cc=saeedm@mellanox.com \
    --cc=seroyer@linux.vnet.ibm.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).