All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Oliver O'Halloran" <oohall@gmail.com>
To: Niklas Schnelle <schnelle@linux.ibm.com>
Cc: Bjorn Helgaas <bhelgaas@google.com>,
	Linas Vepstas <linasvepstas@gmail.com>,
	Russell Currey <ruscur@russell.cc>,
	linuxppc-dev <linuxppc-dev@lists.ozlabs.org>,
	Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
	linux-s390@vger.kernel.org,
	Matthew Rosato <mjrosato@linux.ibm.com>,
	Pierre Morel <pmorel@linux.ibm.com>
Subject: Re: [PATCH 0/5] s390/pci: automatic error recovery
Date: Tue, 7 Sep 2021 12:04:35 +1000	[thread overview]
Message-ID: <CAOSf1CFyuf9FaeSNparj+7W0mKTPvtcM8vxjHDSFsNDC6k_7xQ@mail.gmail.com> (raw)
In-Reply-To: <20210906094927.524106-1-schnelle@linux.ibm.com>

On Mon, Sep 6, 2021 at 7:49 PM Niklas Schnelle <schnelle@linux.ibm.com> wrote:
>
> Patch 3 I already sent separately resulting in the discussion below but without
> a final conclusion.
>
> https://lore.kernel.org/lkml/20210720150145.640727-1-schnelle@linux.ibm.com/
>
> I believe even though there were some doubts about the use of
> pci_dev_is_added() by arch code the existing uses as well as the use in the
> final patch of this series warrant this export.

The use of pci_dev_is_added() in arch/powerpc was because in the past
pci_bus_add_device() could be called before pci_device_add(). That was
fixed a while ago so It should be safe to remove those calls now.

> Patch 4 "PCI: Export pci_dev_lock()" is basically an extension to commit
> e3a9b1212b9d ("PCI: Export pci_dev_trylock() and pci_dev_unlock()") which
> already exported pci_dev_trylock(). In the final patch we make use of
> pci_dev_lock() to wait for any other exclusive uses of the pdev to be finished
> before starting recovery.

Hmm, I noticed the EEH
(arch/powerpc/kernel/eeh_driver.c:eeh_pe_report_edev())  and the
generic PCIe error recovery code (see
drivers/pci/pcie/err.c:report_error_detected()) only call
device_lock() before entering the driver's error handling callbacks. I
wonder if they should be using pci_dev_lock() instead. The only real
difference is that pci_dev_lock() will also block user space from
accessing the device's config space while error recovery is in
progress which seems sensible enough.

WARNING: multiple messages have this Message-ID (diff)
From: "Oliver O'Halloran" <oohall@gmail.com>
To: Niklas Schnelle <schnelle@linux.ibm.com>
Cc: linux-s390@vger.kernel.org, Pierre Morel <pmorel@linux.ibm.com>,
	Matthew Rosato <mjrosato@linux.ibm.com>,
	Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
	Bjorn Helgaas <bhelgaas@google.com>,
	Linas Vepstas <linasvepstas@gmail.com>,
	linuxppc-dev <linuxppc-dev@lists.ozlabs.org>
Subject: Re: [PATCH 0/5] s390/pci: automatic error recovery
Date: Tue, 7 Sep 2021 12:04:35 +1000	[thread overview]
Message-ID: <CAOSf1CFyuf9FaeSNparj+7W0mKTPvtcM8vxjHDSFsNDC6k_7xQ@mail.gmail.com> (raw)
In-Reply-To: <20210906094927.524106-1-schnelle@linux.ibm.com>

On Mon, Sep 6, 2021 at 7:49 PM Niklas Schnelle <schnelle@linux.ibm.com> wrote:
>
> Patch 3 I already sent separately resulting in the discussion below but without
> a final conclusion.
>
> https://lore.kernel.org/lkml/20210720150145.640727-1-schnelle@linux.ibm.com/
>
> I believe even though there were some doubts about the use of
> pci_dev_is_added() by arch code the existing uses as well as the use in the
> final patch of this series warrant this export.

The use of pci_dev_is_added() in arch/powerpc was because in the past
pci_bus_add_device() could be called before pci_device_add(). That was
fixed a while ago so It should be safe to remove those calls now.

> Patch 4 "PCI: Export pci_dev_lock()" is basically an extension to commit
> e3a9b1212b9d ("PCI: Export pci_dev_trylock() and pci_dev_unlock()") which
> already exported pci_dev_trylock(). In the final patch we make use of
> pci_dev_lock() to wait for any other exclusive uses of the pdev to be finished
> before starting recovery.

Hmm, I noticed the EEH
(arch/powerpc/kernel/eeh_driver.c:eeh_pe_report_edev())  and the
generic PCIe error recovery code (see
drivers/pci/pcie/err.c:report_error_detected()) only call
device_lock() before entering the driver's error handling callbacks. I
wonder if they should be using pci_dev_lock() instead. The only real
difference is that pci_dev_lock() will also block user space from
accessing the device's config space while error recovery is in
progress which seems sensible enough.

  parent reply	other threads:[~2021-09-07  2:04 UTC|newest]

Thread overview: 38+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-09-06  9:49 [PATCH 0/5] s390/pci: automatic error recovery Niklas Schnelle
2021-09-06  9:49 ` Niklas Schnelle
2021-09-06  9:49 ` [PATCH 1/5] s390/pci: refresh function handle in iomap Niklas Schnelle
2021-09-06  9:49   ` Niklas Schnelle
2021-09-06  9:49 ` [PATCH 2/5] s390/pci: implement reset_slot for hotplug slot Niklas Schnelle
2021-09-06  9:49   ` Niklas Schnelle
2021-09-06  9:49 ` [PATCH 3/5] PCI: Move pci_dev_is/assign_added() to pci.h Niklas Schnelle
2021-09-06  9:49   ` Niklas Schnelle
2021-09-07  0:22   ` kernel test robot
2021-09-07  0:22     ` kernel test robot
2021-09-07  0:22     ` kernel test robot
2021-09-07  0:25   ` kernel test robot
2021-09-07  0:25     ` kernel test robot
2021-09-07  0:25     ` kernel test robot
2021-09-07  7:51     ` Andy Shevchenko
2021-09-07  7:51       ` Andy Shevchenko
2021-09-07  7:51       ` Andy Shevchenko
2021-09-07  8:14       ` Niklas Schnelle
2021-09-07  8:14         ` Niklas Schnelle
2021-09-07  8:14         ` Niklas Schnelle
2021-09-06  9:49 ` [PATCH 4/5] PCI: Export pci_dev_lock() Niklas Schnelle
2021-09-06  9:49   ` Niklas Schnelle
2021-09-06  9:49 ` [PATCH 5/5] s390/pci: implement minimal PCI error recovery Niklas Schnelle
2021-09-06  9:49   ` Niklas Schnelle
2021-09-07  2:04 ` Oliver O'Halloran [this message]
2021-09-07  2:04   ` [PATCH 0/5] s390/pci: automatic " Oliver O'Halloran
2021-09-07  8:45   ` Niklas Schnelle
2021-09-07  8:45     ` Niklas Schnelle
2021-09-07 12:21     ` Niklas Schnelle
2021-09-07 12:21       ` Niklas Schnelle
2021-09-08  1:37       ` Oliver O'Halloran
2021-09-08  1:37         ` Oliver O'Halloran
2021-09-08  8:09         ` Niklas Schnelle
2021-09-08  8:09           ` Niklas Schnelle
2021-09-07  2:05 ` Linas Vepstas
2021-09-07  2:10   ` Fwd: " Linas Vepstas
2021-09-07  7:49   ` Niklas Schnelle
2021-09-07  7:49     ` Niklas Schnelle

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=CAOSf1CFyuf9FaeSNparj+7W0mKTPvtcM8vxjHDSFsNDC6k_7xQ@mail.gmail.com \
    --to=oohall@gmail.com \
    --cc=bhelgaas@google.com \
    --cc=linasvepstas@gmail.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-s390@vger.kernel.org \
    --cc=linuxppc-dev@lists.ozlabs.org \
    --cc=mjrosato@linux.ibm.com \
    --cc=pmorel@linux.ibm.com \
    --cc=ruscur@russell.cc \
    --cc=schnelle@linux.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 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.