[PATCH v2] powerpc/eeh: drop taken reference to driver on eeh_rmv_device

[PATCH v2] powerpc/eeh: drop taken reference to driver on eeh_rmv_device

[Thadeu Lima de Souza Cascardo]

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;
+
 	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

Re: [PATCH v2] powerpc/eeh: drop taken reference to driver on eeh_rmv_device

[Nishanth Aravamudan]

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
> 

Re: [PATCH v2] powerpc/eeh: drop taken reference to driver on eeh_rmv_device

[Thadeu Lima de Souza Cascardo]

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
>