diff mbox

[07/10] aacraid: Fix AIF triggered IOP_RESET

Message ID 1448973589-9216-8-git-send-email-RaghavaAditya.Renukunta@pmcs.com (mailing list archive)
State New, archived
Headers show

Commit Message

Raghava Aditya Renukunta Dec. 1, 2015, 12:39 p.m. UTC
From: Raghava Aditya Renukunta <raghavaaditya.renukunta@pmcs.com>

while driver removal is in progress or PCI shutdown is invoked, driver
kills AIF aacraid thread, but IOCTL requests from the management tools
re-start AIF thread leading to IOP_RESET.

Fixed by setting adapter_shutdown flag when PCI shutdown is invoked.

Signed-off-by: Raghava Aditya Renukunta <raghavaaditya.renukunta@pmcs.com>
---
 drivers/scsi/aacraid/linit.c | 1 +
 1 file changed, 1 insertion(+)

Comments

Johannes Thumshirn Dec. 2, 2015, 10 a.m. UTC | #1
Hi Raghava,

On Tue, 2015-12-01 at 04:39 -0800, Raghava Aditya Renukunta wrote:
> From: Raghava Aditya Renukunta <raghavaaditya.renukunta@pmcs.com>
> 
> while driver removal is in progress or PCI shutdown is invoked, driver
> kills AIF aacraid thread, but IOCTL requests from the management tools
> re-start AIF thread leading to IOP_RESET.
> 
> Fixed by setting adapter_shutdown flag when PCI shutdown is invoked.
> 
> Signed-off-by: Raghava Aditya Renukunta <raghavaaditya.renukunta@pmcs.com>
> ---
>  drivers/scsi/aacraid/linit.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/scsi/aacraid/linit.c b/drivers/scsi/aacraid/linit.c
> index 6912efd..3a4dbe7 100644
> --- a/drivers/scsi/aacraid/linit.c
> +++ b/drivers/scsi/aacraid/linit.c
> @@ -1454,6 +1454,7 @@ static int aac_suspend(struct pci_dev *pdev,
> pm_message_t state)
>  	struct aac_dev *aac = (struct aac_dev *)shost->hostdata;
>  
>  	scsi_block_requests(shost);
> +	aac->adapter_shutdown = 1;
>  	aac_send_shutdown(aac);
>  
>  	aac_release_resources(aac);

I don't quite undestand that, the following is from aac_send_shutdown():


229         /* FIB should be freed only after getting the response from the F/W
*/
230         if (status != -ERESTARTSYS)
231                 aac_fib_free(fibctx);
232         dev->adapter_shutdown = 1;
233         if ((dev->pdev->device == PMC_DEVICE_S7 ||
234              dev->pdev->device == PMC_DEVICE_S8 ||



in line 232 you're already setting the adapter shutdown flag, why do you need
to pre-set it before calling aac_send_shutdown()? 

Thanks,
	Johannes
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Raghava Aditya Renukunta Dec. 2, 2015, 10:29 p.m. UTC | #2
Hello Johannes,

> -----Original Message-----

> From: Johannes Thumshirn [mailto:jthumshirn@suse.de]

> Sent: Wednesday, December 2, 2015 2:01 AM

> To: Raghava Aditya Renukunta; JBottomley@Parallels.com; linux-

> scsi@vger.kernel.org

> Cc: Mahesh Rajashekhara; Murthy Bhat; Santosh Akula; Gana Sridaran;

> aacraid@pmc-sierra.com; Rich Bono

> Subject: Re: [PATCH 07/10] aacraid: Fix AIF triggered IOP_RESET

> 

> Hi Raghava,

> 

> On Tue, 2015-12-01 at 04:39 -0800, Raghava Aditya Renukunta wrote:

> > From: Raghava Aditya Renukunta <raghavaaditya.renukunta@pmcs.com>

> >

> > while driver removal is in progress or PCI shutdown is invoked, driver

> > kills AIF aacraid thread, but IOCTL requests from the management tools

> > re-start AIF thread leading to IOP_RESET.

> >

> > Fixed by setting adapter_shutdown flag when PCI shutdown is invoked.

> >

> > Signed-off-by: Raghava Aditya Renukunta

> > <raghavaaditya.renukunta@pmcs.com>

> > ---

> >  drivers/scsi/aacraid/linit.c | 1 +

> >  1 file changed, 1 insertion(+)

> >

> > diff --git a/drivers/scsi/aacraid/linit.c

> > b/drivers/scsi/aacraid/linit.c index 6912efd..3a4dbe7 100644

> > --- a/drivers/scsi/aacraid/linit.c

> > +++ b/drivers/scsi/aacraid/linit.c

> > @@ -1454,6 +1454,7 @@ static int aac_suspend(struct pci_dev *pdev,

> > pm_message_t state)

> >  	struct aac_dev *aac = (struct aac_dev *)shost->hostdata;

> >

> >  	scsi_block_requests(shost);

> > +	aac->adapter_shutdown = 1;

> >  	aac_send_shutdown(aac);

> >

> >  	aac_release_resources(aac);

> 

> I don't quite undestand that, the following is from aac_send_shutdown():

> 

> 

> 229         /* FIB should be freed only after getting the response from the F/W

> */

> 230         if (status != -ERESTARTSYS)

> 231                 aac_fib_free(fibctx);

> 232         dev->adapter_shutdown = 1;

> 233         if ((dev->pdev->device == PMC_DEVICE_S7 ||

> 234              dev->pdev->device == PMC_DEVICE_S8 ||

> 

> 

> 

> in line 232 you're already setting the adapter shutdown flag, why do you

> need to pre-set it before calling aac_send_shutdown()?


Originally the problem was that dev->adapter_shutdown was set after the
Shutdown command was sent to the controller, leading to a race
condition  If after the controller was shut down and before
 dev->adapter_shutdown  was set , the driver received  a AIF
 command from management tool , it  would still be sent to the 
controller in aac_cfg_ioctl() leading to a  IOP_RESET.

A fix would  be to set dev->adapter_shutdown at the very 
beginning of aac_send_shutdown().

> 

> Thanks,

> 	Johannes


Thank you,
	Raghava Aditya
Johannes Thumshirn Dec. 3, 2015, 8:03 a.m. UTC | #3
Hi Raghava,

On Wed, 2015-12-02 at 22:29 +0000, Raghava Aditya Renukunta wrote:
> Hello Johannes,
> 
> > -----Original Message-----

[...]

> Originally the problem was that dev->adapter_shutdown was set after the
> Shutdown command was sent to the controller, leading to a race
> condition  If after the controller was shut down and before
>  dev->adapter_shutdown  was set , the driver received  a AIF
>  command from management tool , it  would still be sent to the 
> controller in aac_cfg_ioctl() leading to a  IOP_RESET.
> 
> A fix would  be to set dev->adapter_shutdown at the very 
> beginning of aac_send_shutdown().


That's what I though as well

	Johannes


--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/drivers/scsi/aacraid/linit.c b/drivers/scsi/aacraid/linit.c
index 6912efd..3a4dbe7 100644
--- a/drivers/scsi/aacraid/linit.c
+++ b/drivers/scsi/aacraid/linit.c
@@ -1454,6 +1454,7 @@  static int aac_suspend(struct pci_dev *pdev, pm_message_t state)
 	struct aac_dev *aac = (struct aac_dev *)shost->hostdata;
 
 	scsi_block_requests(shost);
+	aac->adapter_shutdown = 1;
 	aac_send_shutdown(aac);
 
 	aac_release_resources(aac);