| Message ID | 1391624445-9095-1-git-send-email-cascardo@linux.vnet.ibm.com (mailing list archive) |
|---|---|
| State | Accepted |
| Headers | show |
On 05.02.2014 [16:20:45 -0200], Thadeu Lima de Souza Cascardo wrote: > Commit f5c57710dd62dd06f176934a8b4b8accbf00f9f8 ("powerpc/eeh: Use > partial hotplug for EEH unaware drivers") introduces eeh_rmv_device, > which may grab a reference to a driver, but not release it. > > That prevents a driver from being removed after it has gone through EEH > recovery. > > This patch drops the reference if it was taken. > > Signed-off-by: Thadeu Lima de Souza Cascardo <cascardo@linux.vnet.ibm.com> > Acked-by: Gavin Shan <shangw@linux.vnet.ibm.com> > --- > arch/powerpc/kernel/eeh_driver.c | 8 ++++++-- > 1 files changed, 6 insertions(+), 2 deletions(-) > > diff --git a/arch/powerpc/kernel/eeh_driver.c b/arch/powerpc/kernel/eeh_driver.c > index 7bb30dc..fdc679d 100644 > --- a/arch/powerpc/kernel/eeh_driver.c > +++ b/arch/powerpc/kernel/eeh_driver.c > @@ -362,9 +362,13 @@ static void *eeh_rmv_device(void *data, void *userdata) > */ > if (!dev || (dev->hdr_type & PCI_HEADER_TYPE_BRIDGE)) > return NULL; > + This appears to be unnecessary whitespace change? -Nish > driver = eeh_pcid_get(dev); > - if (driver && driver->err_handler) > - return NULL; > + if (driver) { > + eeh_pcid_put(dev); > + if (driver->err_handler) > + return NULL; > + } > > /* Remove it from PCI subsystem */ > pr_debug("EEH: Removing %s without EEH sensitive driver\n", > -- > 1.7.1 > > _______________________________________________ > Linuxppc-dev mailing list > Linuxppc-dev@lists.ozlabs.org > https://lists.ozlabs.org/listinfo/linuxppc-dev >
On Wed, Feb 05, 2014 at 10:43:38AM -0800, Nishanth Aravamudan wrote: > On 05.02.2014 [16:20:45 -0200], Thadeu Lima de Souza Cascardo wrote: > > Commit f5c57710dd62dd06f176934a8b4b8accbf00f9f8 ("powerpc/eeh: Use > > partial hotplug for EEH unaware drivers") introduces eeh_rmv_device, > > which may grab a reference to a driver, but not release it. > > > > That prevents a driver from being removed after it has gone through EEH > > recovery. > > > > This patch drops the reference if it was taken. > > > > Signed-off-by: Thadeu Lima de Souza Cascardo <cascardo@linux.vnet.ibm.com> > > Acked-by: Gavin Shan <shangw@linux.vnet.ibm.com> > > --- > > arch/powerpc/kernel/eeh_driver.c | 8 ++++++-- > > 1 files changed, 6 insertions(+), 2 deletions(-) > > > > diff --git a/arch/powerpc/kernel/eeh_driver.c b/arch/powerpc/kernel/eeh_driver.c > > index 7bb30dc..fdc679d 100644 > > --- a/arch/powerpc/kernel/eeh_driver.c > > +++ b/arch/powerpc/kernel/eeh_driver.c > > @@ -362,9 +362,13 @@ static void *eeh_rmv_device(void *data, void *userdata) > > */ > > if (!dev || (dev->hdr_type & PCI_HEADER_TYPE_BRIDGE)) > > return NULL; > > + > > This appears to be unnecessary whitespace change? > > -Nish > Hi, Nish. I originally add it there for readability, giving both more evidence to the code below, where a driver reference is get and put, and to the code above and its respective comment. Leaving those together could give the impression the comment also applies to the code below. But I have no strong feelings about that. Ben, do you want me to send a new version? Regards. Cascardo. > > driver = eeh_pcid_get(dev); > > - if (driver && driver->err_handler) > > - return NULL; > > + if (driver) { > > + eeh_pcid_put(dev); > > + if (driver->err_handler) > > + return NULL; > > + } > > > > /* Remove it from PCI subsystem */ > > pr_debug("EEH: Removing %s without EEH sensitive driver\n", > > -- > > 1.7.1 > > > > _______________________________________________ > > Linuxppc-dev mailing list > > Linuxppc-dev@lists.ozlabs.org > > https://lists.ozlabs.org/listinfo/linuxppc-dev > > > > _______________________________________________ > Linuxppc-dev mailing list > Linuxppc-dev@lists.ozlabs.org > https://lists.ozlabs.org/listinfo/linuxppc-dev >
diff --git a/arch/powerpc/kernel/eeh_driver.c b/arch/powerpc/kernel/eeh_driver.c index 7bb30dc..fdc679d 100644 --- a/arch/powerpc/kernel/eeh_driver.c +++ b/arch/powerpc/kernel/eeh_driver.c @@ -362,9 +362,13 @@ static void *eeh_rmv_device(void *data, void *userdata) */ if (!dev || (dev->hdr_type & PCI_HEADER_TYPE_BRIDGE)) return NULL; + driver = eeh_pcid_get(dev); - if (driver && driver->err_handler) - return NULL; + if (driver) { + eeh_pcid_put(dev); + if (driver->err_handler) + return NULL; + } /* Remove it from PCI subsystem */ pr_debug("EEH: Removing %s without EEH sensitive driver\n",
Commit f5c57710dd62dd06f176934a8b4b8accbf00f9f8 ("powerpc/eeh: Use partial hotplug for EEH unaware drivers") introduces eeh_rmv_device, which may grab a reference to a driver, but not release it. That prevents a driver from being removed after it has gone through EEH recovery. This patch drops the reference if it was taken. Signed-off-by: Thadeu Lima de Souza Cascardo <cascardo@linux.vnet.ibm.com> Acked-by: Gavin Shan <shangw@linux.vnet.ibm.com> --- arch/powerpc/kernel/eeh_driver.c | 8 ++++++-- 1 files changed, 6 insertions(+), 2 deletions(-)