[PATCH 3/3] Fix device detection issues with mvsas driver

[PATCH 3/3] Fix device detection issues with mvsas driver

[Praveen Murali]

[-- Attachment #1: Type: text/plain, Size: 2686 bytes --]

  I am using the Marvell SAS HBA (MV64460/64461/64462 System Controller,
Revision B (rev 01)), on a system running Ubuntu 12.04 stock kernel. And
i am seeing some drive detection issues with SATA drives, mostly during
hotplugging the drives; the issue gets exaggerated when we keep removing
and plugging in drives. 

From dmesg, after starting the LibATA debugs, it looks like the
ata_eh_revalidate_and_attach fails because ata_dev_read_id fails to get
the IDENTIFY command response. I see "both IDENTIFYs aborted, assuming
NODEV" for all these drives that go undetected. At this point, if I
unload and reload the mvsas driver, these drives detect fine. So, I
hooked up a SATA analyzer between the HBA and one of the drives, and
restarted the test. From what I can see, even though the drive responds
to the IDENTIFY command, the upper layer detects an error and discards
the response. On further debugging it was evident that the error was
being detected in the FIS. 

 After going thru the mvsas driver, I see that the driver allocates a
DMA area for an FIS pool (for as many as 33 FIS's) and initializes it to
0. And each ATA command is allocated a FIS area from this pool during
the command prep; but its never cleared. 

This change initializes the FIS area before using it and it fixes the
drive detection issue. I have tested it with kernel 3.2.0. 

---
 drivers/scsi/mvsas/mv_sas.c |   14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/drivers/scsi/mvsas/mv_sas.c b/drivers/scsi/mvsas/mv_sas.c
index a4884a5..31bfd4e 100644
--- a/drivers/scsi/mvsas/mv_sas.c
+++ b/drivers/scsi/mvsas/mv_sas.c
@@ -21,6 +21,16 @@
  * along with this program; if not, write to the Free Software
  * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307
  * USA
+ *
+ * Changelog:
+ *	Praveen Murali <pmurali@logicube.com>	May 15, 2012
+ *		Cleanup and prepare the FIS index before issuing the
+ *		ATA command (during prep: mvs_task_prep_ata). This is
+ *		to overcome the drive detection issue where the SATA drives
+ *		fail to get detected during hotplug since the ATA module
+ *		(libATA) detects errors set in the FIS eventhough the SATA
+ *		analyzer shows that the IDENTIFY command was successfull.
+ *
 */
 
 #include "mv_sas.h"
@@ -458,6 +468,10 @@ static int mvs_task_prep_ata(struct mvs_info *mvi,
 			mvi_dev->device_id);
 		return -EBUSY;
 	}
+
+	/* cleanup and prepare the aloocated FIS index */
+	memset( SATA_RECEIVED_D2H_FIS(mvi_dev->taskfileset), 0, sizeof(struct
dev_to_host_fis) );
+
 	slot = &mvi->slot_info[tag];
 	slot->tx = mvi->tx_prod;
 	del_q = TXQ_MODE_I | tag |
-- 
1.7.10


[-- Attachment #2: 0003-Fix-device-detection-issues-with-mvsas-driver.patch --]
[-- Type: text/x-patch, Size: 1773 bytes --]

From 0ac1c61dca4ce303c7233c3ed65c03eb2bfff28c Mon Sep 17 00:00:00 2001
From: Praveen Murali <pmurali@logicube.com>
Date: Thu, 14 Jun 2012 18:30:01 -0700
Subject: [PATCH 3/3] Fix device detection issues with mvsas driver

This change fixes the drive detection issue where the SATA drives fail to
get detected during hotplug since the ATA module (libATA) detects errors set
in the FIS eventhough the SATA analyzer shows that the IDENTIFY command was
successfull.
---
 drivers/scsi/mvsas/mv_sas.c |   14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/drivers/scsi/mvsas/mv_sas.c b/drivers/scsi/mvsas/mv_sas.c
index a4884a5..31bfd4e 100644
--- a/drivers/scsi/mvsas/mv_sas.c
+++ b/drivers/scsi/mvsas/mv_sas.c
@@ -21,6 +21,16 @@
  * along with this program; if not, write to the Free Software
  * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307
  * USA
+ *
+ * Changelog:
+ *	Praveen Murali <pmurali@logicube.com>	May 15, 2012
+ *		Cleanup and prepare the FIS index before issuing the
+ *		ATA command (during prep: mvs_task_prep_ata). This is
+ *		to overcome the drive detection issue where the SATA drives
+ *		fail to get detected during hotplug since the ATA module
+ *		(libATA) detects errors set in the FIS eventhough the SATA
+ *		analyzer shows that the IDENTIFY command was successfull.
+ *
 */
 
 #include "mv_sas.h"
@@ -458,6 +468,10 @@ static int mvs_task_prep_ata(struct mvs_info *mvi,
 			mvi_dev->device_id);
 		return -EBUSY;
 	}
+
+	/* cleanup and prepare the aloocated FIS index */
+	memset( SATA_RECEIVED_D2H_FIS(mvi_dev->taskfileset), 0, sizeof(struct dev_to_host_fis) );
+
 	slot = &mvi->slot_info[tag];
 	slot->tx = mvi->tx_prod;
 	del_q = TXQ_MODE_I | tag |
-- 
1.7.10

Re: [PATCH 3/3] Fix device detection issues with mvsas driver

[Dan Williams]

On Mon, Jun 18, 2012 at 11:35 AM, Praveen Murali <pmurali@logicube.com> wrote:
>  I am using the Marvell SAS HBA (MV64460/64461/64462 System Controller,
> Revision B (rev 01)), on a system running Ubuntu 12.04 stock kernel. And
> i am seeing some drive detection issues with SATA drives, mostly during
> hotplugging the drives; the issue gets exaggerated when we keep removing
> and plugging in drives.
>
> From dmesg, after starting the LibATA debugs, it looks like the
> ata_eh_revalidate_and_attach fails because ata_dev_read_id fails to get
> the IDENTIFY command response. I see "both IDENTIFYs aborted, assuming
> NODEV" for all these drives that go undetected. At this point, if I
> unload and reload the mvsas driver, these drives detect fine. So, I
> hooked up a SATA analyzer between the HBA and one of the drives, and
> restarted the test. From what I can see, even though the drive responds
> to the IDENTIFY command, the upper layer detects an error and discards
> the response. On further debugging it was evident that the error was
> being detected in the FIS.
>
>  After going thru the mvsas driver, I see that the driver allocates a
> DMA area for an FIS pool (for as many as 33 FIS's) and initializes it to
> 0. And each ATA command is allocated a FIS area from this pool during
> the command prep; but its never cleared.
>
> This change initializes the FIS area before using it and it fixes the
> drive detection issue. I have tested it with kernel 3.2.0.
>
> ---
>  drivers/scsi/mvsas/mv_sas.c |   14 ++++++++++++++
>  1 file changed, 14 insertions(+)
>
> diff --git a/drivers/scsi/mvsas/mv_sas.c b/drivers/scsi/mvsas/mv_sas.c
> index a4884a5..31bfd4e 100644
> --- a/drivers/scsi/mvsas/mv_sas.c
> +++ b/drivers/scsi/mvsas/mv_sas.c
> @@ -21,6 +21,16 @@
>  * along with this program; if not, write to the Free Software
>  * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307
>  * USA
> + *
> + * Changelog:
> + *     Praveen Murali <pmurali@logicube.com>   May 15, 2012
> + *             Cleanup and prepare the FIS index before issuing the
> + *             ATA command (during prep: mvs_task_prep_ata). This is
> + *             to overcome the drive detection issue where the SATA drives
> + *             fail to get detected during hotplug since the ATA module
> + *             (libATA) detects errors set in the FIS eventhough the SATA
> + *             analyzer shows that the IDENTIFY command was successfull.
> + *
>  */

Hi,

The changelog will exist in the git history, no need to also append it
to the file.  I think this should also be tagged for the -stable tree.

--
Dan
--
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

Re: [PATCH 3/3] Fix device detection issues with mvsas driver

[Praveen Murali]

Ok, do you want me to regenerate the patch?

Praveen

On Thu, 2012-06-21 at 09:05 -0700, Dan Williams wrote:
> On Mon, Jun 18, 2012 at 11:35 AM, Praveen Murali <pmurali@logicube.com> wrote:
> >  I am using the Marvell SAS HBA (MV64460/64461/64462 System Controller,
> > Revision B (rev 01)), on a system running Ubuntu 12.04 stock kernel. And
> > i am seeing some drive detection issues with SATA drives, mostly during
> > hotplugging the drives; the issue gets exaggerated when we keep removing
> > and plugging in drives.
> >
> > From dmesg, after starting the LibATA debugs, it looks like the
> > ata_eh_revalidate_and_attach fails because ata_dev_read_id fails to get
> > the IDENTIFY command response. I see "both IDENTIFYs aborted, assuming
> > NODEV" for all these drives that go undetected. At this point, if I
> > unload and reload the mvsas driver, these drives detect fine. So, I
> > hooked up a SATA analyzer between the HBA and one of the drives, and
> > restarted the test. From what I can see, even though the drive responds
> > to the IDENTIFY command, the upper layer detects an error and discards
> > the response. On further debugging it was evident that the error was
> > being detected in the FIS.
> >
> >  After going thru the mvsas driver, I see that the driver allocates a
> > DMA area for an FIS pool (for as many as 33 FIS's) and initializes it to
> > 0. And each ATA command is allocated a FIS area from this pool during
> > the command prep; but its never cleared.
> >
> > This change initializes the FIS area before using it and it fixes the
> > drive detection issue. I have tested it with kernel 3.2.0.
> >
> > ---
> >  drivers/scsi/mvsas/mv_sas.c |   14 ++++++++++++++
> >  1 file changed, 14 insertions(+)
> >
> > diff --git a/drivers/scsi/mvsas/mv_sas.c b/drivers/scsi/mvsas/mv_sas.c
> > index a4884a5..31bfd4e 100644
> > --- a/drivers/scsi/mvsas/mv_sas.c
> > +++ b/drivers/scsi/mvsas/mv_sas.c
> > @@ -21,6 +21,16 @@
> >  * along with this program; if not, write to the Free Software
> >  * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307
> >  * USA
> > + *
> > + * Changelog:
> > + *     Praveen Murali <pmurali@logicube.com>   May 15, 2012
> > + *             Cleanup and prepare the FIS index before issuing the
> > + *             ATA command (during prep: mvs_task_prep_ata). This is
> > + *             to overcome the drive detection issue where the SATA drives
> > + *             fail to get detected during hotplug since the ATA module
> > + *             (libATA) detects errors set in the FIS eventhough the SATA
> > + *             analyzer shows that the IDENTIFY command was successfull.
> > + *
> >  */
> 
> Hi,
> 
> The changelog will exist in the git history, no need to also append it
> to the file.  I think this should also be tagged for the -stable tree.
> 
> --
> Dan

Re: [PATCH 3/3] Fix device detection issues with mvsas driver

[Dan Williams]

On Thu, Jun 21, 2012 at 9:41 AM, Praveen Murali <pmurali@logicube.com> wrote:
> Ok, do you want me to regenerate the patch?

Yes, James will be the one to ultimately apply it and would save him
time to have a new patch.  The patch you attached had a shorter
changelog I think it would be fine to include your entire 4 paragraph
lead in message.

Re: [PATCH 3/3] Fix device detection issues with mvsas driver

[Praveen Murali]

[-- Attachment #1: Type: text/plain, Size: 454 bytes --]

here is the updated patch.

On Thu, 2012-06-21 at 09:48 -0700, Dan Williams wrote:
> On Thu, Jun 21, 2012 at 9:41 AM, Praveen Murali <pmurali@logicube.com> wrote:
> > Ok, do you want me to regenerate the patch?
> 
> Yes, James will be the one to ultimately apply it and would save him
> time to have a new patch.  The patch you attached had a shorter
> changelog I think it would be fine to include your entire 4 paragraph
> lead in message.


[-- Attachment #2: 0002-Fix-mvsas-SATA-disk-detection-issues.patch --]
[-- Type: text/x-patch, Size: 2233 bytes --]

From 6ad9cf691f9d11e8c6de332efc35bd945c0f2840 Mon Sep 17 00:00:00 2001
From: Praveen Murali <pmurali@logicube.com>
Date: Thu, 21 Jun 2012 09:57:03 -0700
Subject: [PATCH v2] Fix mvsas SATA disk detection issues

 I am using the Marvell SAS HBA (MV64460/64461/64462 System Controller,
Revision B (rev 01)), on a system running Ubuntu 12.04 stock kernel. And
i am seeing some drive detection issues with SATA drives, mostly during
hotplugging the drives; the issue gets exaggerated when we keep removing
and plugging in drives.

From dmesg, after starting the LibATA debugs, it looks like the
ata_eh_revalidate_and_attach fails because ata_dev_read_id fails to get
the IDENTIFY command response. I see "both IDENTIFYs aborted, assuming
NODEV" for all these drives that go undetected. At this point, if I
unload and reload the mvsas driver, these drives detect fine. So, I
hooked up a SATA analyzer between the HBA and one of the drives, and
restarted the test. From what I can see, even though the drive responds
to the IDENTIFY command, the upper layer detects an error and discards
the response. On further debugging it was evident that the error was
being detected in the FIS.

 After going thru the mvsas driver, I see that during init the driver 
allocates a DMA area for an FIS pool (for as many as 33 FIS's) and initializes 
it to 0. And each ATA command is allocated a FIS area from this pool during
the command prep; but its never cleared.

The fix is to cleanup and prepare the FIS index before issuing the ATA
command (during prep: mvs_task_prep_ata).
---
 drivers/scsi/mvsas/mv_sas.c |    5 +++++
 1 file changed, 5 insertions(+)

diff --git a/drivers/scsi/mvsas/mv_sas.c b/drivers/scsi/mvsas/mv_sas.c
index a4884a5..73d9816 100644
--- a/drivers/scsi/mvsas/mv_sas.c
+++ b/drivers/scsi/mvsas/mv_sas.c
@@ -458,6 +458,11 @@ static int mvs_task_prep_ata(struct mvs_info *mvi,
 			mvi_dev->device_id);
 		return -EBUSY;
 	}
+
+	/* cleanup and prepare the allocated FIS index */
+	memset( SATA_RECEIVED_D2H_FIS(mvi_dev->taskfileset), 0, 
+				sizeof(struct dev_to_host_fis) );
+
 	slot = &mvi->slot_info[tag];
 	slot->tx = mvi->tx_prod;
 	del_q = TXQ_MODE_I | tag |
-- 
1.7.10

Re: [PATCH 3/3] Fix device detection issues with mvsas driver

[Dan Williams]

On Thu, Jun 21, 2012 at 10:05 AM, Praveen Murali <pmurali@logicube.com> wrote:
> here is the updated patch.
>
> On Thu, 2012-06-21 at 09:48 -0700, Dan Williams wrote:
>> On Thu, Jun 21, 2012 at 9:41 AM, Praveen Murali <pmurali@logicube.com> wrote:
>> > Ok, do you want me to regenerate the patch?
>>
>> Yes, James will be the one to ultimately apply it and would save him
>> time to have a new patch.  The patch you attached had a shorter
>> changelog I think it would be fine to include your entire 4 paragraph
>> lead in message.
>

Hi Praveen,

Your last message prompted me to look back in my archives and I found
this patch which does not appear to be upstream yet.  I assume it is
still needed?

--
Dan

Re: [PATCH 3/3] Fix device detection issues with mvsas driver

[Praveen Murali]

Yes, I checked that today and was meaning to ask you about it. :)

> On Oct 15, 2013, at 6:31 PM, "Dan Williams" <dan.j.williams@intel.com> wrote:
> 
>> On Thu, Jun 21, 2012 at 10:05 AM, Praveen Murali <pmurali@logicube.com> wrote:
>> here is the updated patch.
>> 
>>> On Thu, 2012-06-21 at 09:48 -0700, Dan Williams wrote:
>>>> On Thu, Jun 21, 2012 at 9:41 AM, Praveen Murali <pmurali@logicube.com> wrote:
>>>> Ok, do you want me to regenerate the patch?
>>> 
>>> Yes, James will be the one to ultimately apply it and would save him
>>> time to have a new patch.  The patch you attached had a shorter
>>> changelog I think it would be fine to include your entire 4 paragraph
>>> lead in message.
> 
> Hi Praveen,
> 
> Your last message prompted me to look back in my archives and I found
> this patch which does not appear to be upstream yet.  I assume it is
> still needed?
> 
> --
> Dan

Re: [PATCH 3/3] Fix device detection issues with mvsas driver

[James Bottomley]

On Tue, 2013-10-15 at 19:40 -0700, Praveen Murali wrote:
> Yes, I checked that today and was meaning to ask you about it. :)

Ideally, I need someone from Marvell to ack it ... can you prod them?

Thanks,

James

Re: [PATCH 3/3] Fix device detection issues with mvsas driver

[Praveen Murali]

[-- Attachment #1: Type: text/plain, Size: 363 bytes --]

Hi Xiang, Can you please take a look at the attached patch and ack it?

Thanks,
Praveen

On 10/16/2013 02:45 PM, James Bottomley wrote:
> On Tue, 2013-10-15 at 19:40 -0700, Praveen Murali wrote:
>> Yes, I checked that today and was meaning to ask you about it. :)
> Ideally, I need someone from Marvell to ack it ... can you prod them?
>
> Thanks,
>
> James
>
>


[-- Attachment #2: Fix-mvsas-disk-detection-issues.patch --]
[-- Type: text/x-diff, Size: 2183 bytes --]

>From 6ad9cf691f9d11e8c6de332efc35bd945c0f2840 Mon Sep 17 00:00:00 2001
From: Praveen Murali <pmurali@logicube.com>
Date: Thu, 21 Jun 2012 09:57:03 -0700
Subject: [PATCH v2] Fix mvsas SATA disk detection issues

 I am using the Marvell SAS HBA (MV64460/64461/64462 System Controller,
Revision B (rev 01)), on a system running Ubuntu 12.04 stock kernel. And
i am seeing some drive detection issues with SATA drives, mostly during
hotplugging the drives; the issue gets exaggerated when we keep removing
and plugging in drives.

>From dmesg, after starting the LibATA debugs, it looks like the
ata_eh_revalidate_and_attach fails because ata_dev_read_id fails to get
the IDENTIFY command response. I see "both IDENTIFYs aborted, assuming
NODEV" for all these drives that go undetected. At this point, if I
unload and reload the mvsas driver, these drives detect fine. So, I
hooked up a SATA analyzer between the HBA and one of the drives, and
restarted the test. From what I can see, even though the drive responds
to the IDENTIFY command, the upper layer detects an error and discards
the response. On further debugging it was evident that the error was
being detected in the FIS.

 After going thru the mvsas driver, I see that during init the driver 
allocates a DMA area for an FIS pool (for as many as 33 FIS's) and initializes 
it to 0. And each ATA command is allocated a FIS area from this pool during
the command prep; but its never cleared.

The fix is to cleanup and prepare the FIS index before issuing the ATA
command (during prep: mvs_task_prep_ata).
---
 drivers/scsi/mvsas/mv_sas.c |    5 +++++
 1 file changed, 5 insertions(+)

diff --git a/drivers/scsi/mvsas/mv_sas.c b/drivers/scsi/mvsas/mv_sas.c
index a4884a5..73d9816 100644
--- a/drivers/scsi/mvsas/mv_sas.c
+++ b/drivers/scsi/mvsas/mv_sas.c
@@ -458,6 +458,11 @@ static int mvs_task_prep_ata(struct mvs_info *mvi,
 			mvi_dev->device_id);
 		return -EBUSY;
 	}
+
+	/* cleanup and prepare the allocated FIS index */
+	memset( SATA_RECEIVED_D2H_FIS(mvi_dev->taskfileset), 0, 
+				sizeof(struct dev_to_host_fis) );
+
 	slot = &mvi->slot_info[tag];
 	slot->tx = mvi->tx_prod;
 	del_q = TXQ_MODE_I | tag |
-- 
1.7.10

Re: [PATCH 3/3] Fix device detection issues with mvsas driver

[Praveen Murali]

The only way to reproduce this is to keep hot plugging drives. To speed it up once in a while remove the drive before the detection completes.

> On Oct 16, 2013, at 7:42 PM, "xiangliang yu" <yxlraid@gmail.com> wrote:
> 
> Hi,
> 
> >Subject: Re: [PATCH 3/3] Fix device detection issues with mvsas driver
> 
> >Hi Xiang, Can you please take a look at the attached patch and ack it?
> >	/* cleanup and prepare the allocated FIS index */
> >	memset( SATA_RECEIVED_D2H_FIS(mvi_dev->taskfileset), 0, 
> >				sizeof(struct dev_to_host_fis) );
> 
> According to spec, the FIS memory don't need to be cleared by software.
> can you tell me how to reproduce the issue? thanks!
> 
>