Skip to contentSkip to conversation

aacraid: Fix 2T+ drives on SmartIOC-2000

PATCH RESEND Applied Dave Carroll 4 participants 7 messages Sep 15, 2017 – Sep 17, 2017

Subsystems (3)
Stable branch SCSI Aacraid SCSI RAID driver
lore
Share & tools
lore

Dave Carroll

author
#0 lore raw

PATCH RESEND 1/1 aacraid: Fix 2T+ drives on SmartIOC-2000

The logic for supporting large drives was previously tied to 4Kn support for SmartIOC-2000. As SmartIOC-2000 does not support volumes using 4Kn drives, use the intended option flag AAC_OPT_NEW_COMM_64 to determine support for volumes greater than 2T.

cc [email protected]
Signed-off-by Dave Carroll <[email protected]>
---
 drivers/scsi/aacraid/aachba.c  | 12 ++++++------
 drivers/scsi/aacraid/aacraid.h |  5 +++++
 2 files changed, 11 insertions(+), 6 deletions(-)
drivers/scsi/aacraid/aachba.c +6 -6
index 4591113..18a1a1f 100644
@@ -695,13 +695,13 @@ static void _aac_probe_container1(void * context, struct fib * fibptr)
 	int status;
 
 	dresp = (struct aac_mount *) fib_data(fibptr);
-	if (!(fibptr->dev->supplement_adapter_info.supported_options2 &
-	    AAC_OPTION_VARIABLE_BLOCK_SIZE))
+	if (!aac_supports_2T(fibptr->dev)) {
 		dresp->mnt[0].capacityhigh = 0;
-	if ((le32_to_cpu(dresp->status) != ST_OK) ||
-	    (le32_to_cpu(dresp->mnt[0].vol) != CT_NONE)) {
-		_aac_probe_container2(context, fibptr);
-		return;
+		if ((le32_to_cpu(dresp->status) == ST_OK) &&
+			(le32_to_cpu(dresp->mnt[0].vol) != CT_NONE)) {
+			_aac_probe_container2(context, fibptr);
+			return;
+		}
 	}
 	scsicmd = (struct scsi_cmnd *) context;
drivers/scsi/aacraid/aacraid.h +5 -0
index 6981299..998fbad 100644
@@ -2701,6 +2701,11 @@ static inline int aac_is_src(struct aac_dev *dev)
 	return 0;
 }
 
+static inline int aac_supports_2T(struct aac_dev *dev)
+{
+	return (dev->adapter_info.options & AAC_OPT_NEW_COMM_64);
+}
+
 char * get_container_type(unsigned type);
 extern int numacb;
 extern char aac_driver_version[];
Signature · 1 line
2.8.4
Reviewed-by on v1 1/1
#1 lore raw

drivers/scsi/aacraid/aachba.c static void _aac_probe_container1(void * context, struct fib * fibptr) · line 703

Quoted 4 lines
drivers/scsi/aacraid/aachba.c +4 -0
+		if ((le32_to_cpu(dresp->status) == ST_OK) &&
+			(le32_to_cpu(dresp->mnt[0].vol) != CT_NONE)) {
+			_aac_probe_container2(context, fibptr);
+			return;

		if (dresp->status == cpu_to_le32(ST_OK) &&
		    dresp->mnt[0].vol != cpu_to_le32(CT_NONE)) {

Otherwise looks good:

Reviewed-by Christoph Hellwig <[email protected]>
author
#2 lore raw

Hi Christoph,

Quoted 8 lines

drivers/scsi/aacraid/aachba.c static void _aac_probe_container1(void * context, struct fib * fibptr) · line 703

Quoted 4 lines
drivers/scsi/aacraid/aachba.c +4 -0
+             if ((le32_to_cpu(dresp->status) == ST_OK) &&
+                     (le32_to_cpu(dresp->mnt[0].vol) != CT_NONE)) {
+                     _aac_probe_container2(context, fibptr);
+                     return;

                if (dresp->status == cpu_to_le32(ST_OK) &&
                    dresp->mnt[0].vol != cpu_to_le32(CT_NONE)) {

I've contemplated this all day, and it looked like sematics, but I assume you were Indicating that the cpu_to_le32() of a constant is a compiler issue, vs le32_to_cpu() is a runtime issue. If so, thanks! If not could you elaborate ...

Thanks, -Dave

#3 lore raw

Dave,

Quoted 7 lines

Hi Christoph,

Quoted 5 lines
Quoted 4 lines
drivers/scsi/aacraid/aachba.c +4 -0
+             if ((le32_to_cpu(dresp->status) == ST_OK) &&
+                     (le32_to_cpu(dresp->mnt[0].vol) != CT_NONE)) {
+                     _aac_probe_container2(context, fibptr);
+                     return;
Quoted 8 lines
Quoted 3 lines
                if (dresp->status == cpu_to_le32(ST_OK) &&
                    dresp->mnt[0].vol != cpu_to_le32(CT_NONE)) {

I've contemplated this all day, and it looked like sematics, but I assume you were Indicating that the cpu_to_le32() of a constant is a compiler issue, vs le32_to_cpu() is a runtime issue. If so, thanks! If not could you elaborate ...

FWIW, I applied your patch as-is for consistency with the rest of the driver. I do prefer Christoph's version, though, but that can be a follow-on patch.

Signature · 1 line
Martin K. Petersen	Oracle Linux Engineering
author
#4 lore raw
Quoted 13 lines

Dave,

Quoted 11 lines

Hi Christoph,

Quoted 8 lines

drivers/scsi/aacraid/aachba.c static void _aac_probe_container1(void * context, struct fib * fibptr) · line 703

Quoted 4 lines
drivers/scsi/aacraid/aachba.c +4 -0
+             if ((le32_to_cpu(dresp->status) == ST_OK) &&
+                     (le32_to_cpu(dresp->mnt[0].vol) != CT_NONE)) {
+                     _aac_probe_container2(context, fibptr);
+                     return;

                if (dresp->status == cpu_to_le32(ST_OK) &&
                    dresp->mnt[0].vol != cpu_to_le32(CT_NONE)) {
Quoted 8 lines
Quoted 4 lines

I've contemplated this all day, and it looked like sematics, but I assume you were Indicating that the cpu_to_le32() of a constant is a compiler issue, vs le32_to_cpu() is a runtime issue. If so, thanks! If not could you elaborate ...

FWIW, I applied your patch as-is for consistency with the rest of the driver. I do prefer Christoph's version, though, but that can be a follow-on patch.

Hi Martin,

If I'm right regarding Christph's intentions, and you seem to imply, which since I've been an LE person most of my life, I will instill in my development team, and propose another patch to fix that ...

Thanks, -Dave

Reviewed-by on v1 1/1
#5 lore raw
In reply to Dave Carroll
Quoted 67 lines

-----Original Message-----
From: Dave Carroll [mailto:[email protected]]
Sent: Friday, September 15, 2017 10:04 AM
To: Martin K . Petersen <[email protected]>; James Bottomley <[email protected]>
Cc: Dave Carroll <[email protected]>; linux-scsi <linux- [email protected]>; dl-esc-Aacraid Linux Driver <[email protected]>; Scott Benesh <[email protected]>; Brian King <[email protected]>; [email protected]
Subject: [PATCH RESEND] aacraid: Fix 2T+ drives on SmartIOC-2000

The logic for supporting large drives was previously tied to 4Kn support for SmartIOC-2000. As SmartIOC-2000 does not support volumes using 4Kn drives, use the intended option flag AAC_OPT_NEW_COMM_64 to determine
support for volumes greater than 2T.

cc [email protected]
Signed-off-by Dave Carroll <[email protected]>
---
 drivers/scsi/aacraid/aachba.c  | 12 ++++++------
 drivers/scsi/aacraid/aacraid.h |  5 +++++
 2 files changed, 11 insertions(+), 6 deletions(-)
drivers/scsi/aacraid/aachba.c +6 -6
index 4591113..18a1a1f 100644
@@ -695,13 +695,13 @@ static void _aac_probe_container1(void * context,
 struct fib * fibptr)
 	int status;
 
 	dresp = (struct aac_mount *) fib_data(fibptr);
-	if (!(fibptr->dev->supplement_adapter_info.supported_options2 &
-	    AAC_OPTION_VARIABLE_BLOCK_SIZE))
+	if (!aac_supports_2T(fibptr->dev)) {
 		dresp->mnt[0].capacityhigh = 0;
-	if ((le32_to_cpu(dresp->status) != ST_OK) ||
-	    (le32_to_cpu(dresp->mnt[0].vol) != CT_NONE)) {
-		_aac_probe_container2(context, fibptr);
-		return;
+		if ((le32_to_cpu(dresp->status) == ST_OK) &&
+			(le32_to_cpu(dresp->mnt[0].vol) != CT_NONE)) {
+			_aac_probe_container2(context, fibptr);
+			return;
+		}
 	}
 	scsicmd = (struct scsi_cmnd *) context;
drivers/scsi/aacraid/aacraid.h +5 -1
index 6981299..998fbad 100644
@@ -2701,6 +2701,11 @@ static inline int aac_is_src(struct aac_dev *dev)
 	return 0;
 }
 
+static inline int aac_supports_2T(struct aac_dev *dev)
+{
+	return (dev->adapter_info.options & AAC_OPT_NEW_COMM_64);
+}
+
 char * get_container_type(unsigned type);
 extern int numacb;
 extern char aac_driver_version[];
--
Quoted 1 line

2.8.4

Reviewed-by Raghava Aditya Renukunta <[email protected]>
Applied
#6 lore raw
In reply to Dave Carroll

Dave,

Quoted 4 lines

The logic for supporting large drives was previously tied to 4Kn support for SmartIOC-2000. As SmartIOC-2000 does not support volumes using 4Kn drives, use the intended option flag AAC_OPT_NEW_COMM_64 to determine support for volumes greater than 2T.

Applied to 4.14/scsi-fixes. Thanks!

Signature · 1 line
Martin K. Petersen	Oracle Linux Engineering