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.
next prev 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: linkBe 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.