2017-08-29 11:59:40

by Nikola Pajkovsky

[permalink] [raw]
Subject: [PATCH 1/3] scsi: aacraid: fix indentation errors

fix stupid indent error, no rocket science here.

Signed-off-by: Nikola Pajkovsky <[email protected]>
---
drivers/scsi/aacraid/comminit.c | 6 +++---
drivers/scsi/aacraid/linit.c | 2 +-
2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/scsi/aacraid/comminit.c b/drivers/scsi/aacraid/comminit.c
index 9ee025b1d0e0..97d269f16888 100644
--- a/drivers/scsi/aacraid/comminit.c
+++ b/drivers/scsi/aacraid/comminit.c
@@ -520,9 +520,9 @@ struct aac_dev *aac_init_adapter(struct aac_dev *dev)
dev->raw_io_64 = 1;
dev->sync_mode = aac_sync_mode;
if (dev->a_ops.adapter_comm &&
- (status[1] & AAC_OPT_NEW_COMM)) {
- dev->comm_interface = AAC_COMM_MESSAGE;
- dev->raw_io_interface = 1;
+ (status[1] & AAC_OPT_NEW_COMM)) {
+ dev->comm_interface = AAC_COMM_MESSAGE;
+ dev->raw_io_interface = 1;
if ((status[1] & AAC_OPT_NEW_COMM_TYPE1)) {
/* driver supports TYPE1 (Tupelo) */
dev->comm_interface = AAC_COMM_MESSAGE_TYPE1;
diff --git a/drivers/scsi/aacraid/linit.c b/drivers/scsi/aacraid/linit.c
index 0f277df73af0..d7698a4cf840 100644
--- a/drivers/scsi/aacraid/linit.c
+++ b/drivers/scsi/aacraid/linit.c
@@ -1457,7 +1457,7 @@ static int aac_probe_one(struct pci_dev *pdev, const struct pci_device_id *id)
/*
* Only series 7 needs freset.
*/
- if (pdev->device == PMC_DEVICE_S7)
+ if (pdev->device == PMC_DEVICE_S7)
pdev->needs_freset = 1;

list_for_each_entry(aac, &aac_devices, entry) {
--
2.13.5


2017-08-29 11:59:11

by Nikola Pajkovsky

[permalink] [raw]
Subject: [PATCH 3/3] scsi: aacraid: report -ENOMEM to upper layer from aac_convert_sgraw2()

aac_convert_sgraw2() kmalloc memory and return -1 on error, which
should be -ENOMEM. However, nobody is checking return value, so with
this change, -ENOMEM is propagated to upper layer.

Signed-off-by: Nikola Pajkovsky <[email protected]>
---
drivers/scsi/aacraid/aachba.c | 10 +++++++---
1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/drivers/scsi/aacraid/aachba.c b/drivers/scsi/aacraid/aachba.c
index 3c354766791e..ca38c2b52b47 100644
--- a/drivers/scsi/aacraid/aachba.c
+++ b/drivers/scsi/aacraid/aachba.c
@@ -3956,8 +3956,12 @@ static long aac_build_sgraw2(struct scsi_cmnd *scsicmd,
if (!err_found)
break;
}
- if (i > 0 && nseg_new <= sg_max)
- aac_convert_sgraw2(rio2, i, nseg, nseg_new);
+ if (i > 0 && nseg_new <= sg_max) {
+ int ret = aac_convert_sgraw2(rio2, i, nseg, nseg_new);
+
+ if (ret < 0)
+ return ret;
+ }
} else
rio2->flags |= cpu_to_le16(RIO2_SGL_CONFORMANT);

@@ -3981,7 +3985,7 @@ static int aac_convert_sgraw2(struct aac_raw_io2 *rio2, int pages, int nseg, int

sge = kmalloc(nseg_new * sizeof(struct sge_ieee1212), GFP_ATOMIC);
if (sge == NULL)
- return -1;
+ return -ENOMEM;

for (i = 1, pos = 1; i < nseg-1; ++i) {
for (j = 0; j < rio2->sge[i].length / (pages * PAGE_SIZE); ++j) {
--
2.13.5

2017-08-29 11:59:12

by Nikola Pajkovsky

[permalink] [raw]
Subject: [PATCH 2/3] scsi: aacraid: get rid of one level of indentation

unsigned long byte_count = 0;
nseg = scsi_dma_map(scsicmd);
if (nseg < 0)
return nseg;
if (nseg) {
...
}
return byte_count;

is equal to

unsigned long byte_count = 0;
nseg = scsi_dma_map(scsicmd);
if (nseg <= 0)
return nseg;
...
return byte_count;

No other code has changed.

Signed-off-by: Nikola Pajkovsky <[email protected]>
---
drivers/scsi/aacraid/aachba.c | 267 +++++++++++++++++++++---------------------
1 file changed, 131 insertions(+), 136 deletions(-)

diff --git a/drivers/scsi/aacraid/aachba.c b/drivers/scsi/aacraid/aachba.c
index a1a2c71e1626..3c354766791e 100644
--- a/drivers/scsi/aacraid/aachba.c
+++ b/drivers/scsi/aacraid/aachba.c
@@ -3763,6 +3763,8 @@ static long aac_build_sg(struct scsi_cmnd *scsicmd, struct sgmap *psg)
struct aac_dev *dev;
unsigned long byte_count = 0;
int nseg;
+ struct scatterlist *sg;
+ int i;

dev = (struct aac_dev *)scsicmd->device->host->hostdata;
// Get rid of old data
@@ -3771,32 +3773,29 @@ static long aac_build_sg(struct scsi_cmnd *scsicmd, struct sgmap *psg)
psg->sg[0].count = 0;

nseg = scsi_dma_map(scsicmd);
- if (nseg < 0)
+ if (nseg <= 0)
return nseg;
- if (nseg) {
- struct scatterlist *sg;
- int i;

- psg->count = cpu_to_le32(nseg);
+ psg->count = cpu_to_le32(nseg);

- scsi_for_each_sg(scsicmd, sg, nseg, i) {
- psg->sg[i].addr = cpu_to_le32(sg_dma_address(sg));
- psg->sg[i].count = cpu_to_le32(sg_dma_len(sg));
- byte_count += sg_dma_len(sg);
- }
- /* hba wants the size to be exact */
- if (byte_count > scsi_bufflen(scsicmd)) {
- u32 temp = le32_to_cpu(psg->sg[i-1].count) -
- (byte_count - scsi_bufflen(scsicmd));
- psg->sg[i-1].count = cpu_to_le32(temp);
- byte_count = scsi_bufflen(scsicmd);
- }
- /* Check for command underflow */
- if(scsicmd->underflow && (byte_count < scsicmd->underflow)){
- printk(KERN_WARNING"aacraid: cmd len %08lX cmd underflow %08X\n",
- byte_count, scsicmd->underflow);
- }
+ scsi_for_each_sg(scsicmd, sg, nseg, i) {
+ psg->sg[i].addr = cpu_to_le32(sg_dma_address(sg));
+ psg->sg[i].count = cpu_to_le32(sg_dma_len(sg));
+ byte_count += sg_dma_len(sg);
+ }
+ /* hba wants the size to be exact */
+ if (byte_count > scsi_bufflen(scsicmd)) {
+ u32 temp = le32_to_cpu(psg->sg[i-1].count) -
+ (byte_count - scsi_bufflen(scsicmd));
+ psg->sg[i-1].count = cpu_to_le32(temp);
+ byte_count = scsi_bufflen(scsicmd);
+ }
+ /* Check for command underflow */
+ if(scsicmd->underflow && (byte_count < scsicmd->underflow)){
+ printk(KERN_WARNING"aacraid: cmd len %08lX cmd underflow %08X\n",
+ byte_count, scsicmd->underflow);
}
+
return byte_count;
}

@@ -3807,6 +3806,8 @@ static long aac_build_sg64(struct scsi_cmnd *scsicmd, struct sgmap64 *psg)
unsigned long byte_count = 0;
u64 addr;
int nseg;
+ struct scatterlist *sg;
+ int i;

dev = (struct aac_dev *)scsicmd->device->host->hostdata;
// Get rid of old data
@@ -3816,34 +3817,31 @@ static long aac_build_sg64(struct scsi_cmnd *scsicmd, struct sgmap64 *psg)
psg->sg[0].count = 0;

nseg = scsi_dma_map(scsicmd);
- if (nseg < 0)
+ if (nseg <= 0)
return nseg;
- if (nseg) {
- struct scatterlist *sg;
- int i;
-
- scsi_for_each_sg(scsicmd, sg, nseg, i) {
- int count = sg_dma_len(sg);
- addr = sg_dma_address(sg);
- psg->sg[i].addr[0] = cpu_to_le32(addr & 0xffffffff);
- psg->sg[i].addr[1] = cpu_to_le32(addr>>32);
- psg->sg[i].count = cpu_to_le32(count);
- byte_count += count;
- }
- psg->count = cpu_to_le32(nseg);
- /* hba wants the size to be exact */
- if (byte_count > scsi_bufflen(scsicmd)) {
- u32 temp = le32_to_cpu(psg->sg[i-1].count) -
- (byte_count - scsi_bufflen(scsicmd));
- psg->sg[i-1].count = cpu_to_le32(temp);
- byte_count = scsi_bufflen(scsicmd);
- }
- /* Check for command underflow */
- if(scsicmd->underflow && (byte_count < scsicmd->underflow)){
- printk(KERN_WARNING"aacraid: cmd len %08lX cmd underflow %08X\n",
- byte_count, scsicmd->underflow);
- }
+
+ scsi_for_each_sg(scsicmd, sg, nseg, i) {
+ int count = sg_dma_len(sg);
+ addr = sg_dma_address(sg);
+ psg->sg[i].addr[0] = cpu_to_le32(addr & 0xffffffff);
+ psg->sg[i].addr[1] = cpu_to_le32(addr>>32);
+ psg->sg[i].count = cpu_to_le32(count);
+ byte_count += count;
}
+ psg->count = cpu_to_le32(nseg);
+ /* hba wants the size to be exact */
+ if (byte_count > scsi_bufflen(scsicmd)) {
+ u32 temp = le32_to_cpu(psg->sg[i-1].count) -
+ (byte_count - scsi_bufflen(scsicmd));
+ psg->sg[i-1].count = cpu_to_le32(temp);
+ byte_count = scsi_bufflen(scsicmd);
+ }
+ /* Check for command underflow */
+ if(scsicmd->underflow && (byte_count < scsicmd->underflow)){
+ printk(KERN_WARNING"aacraid: cmd len %08lX cmd underflow %08X\n",
+ byte_count, scsicmd->underflow);
+ }
+
return byte_count;
}

@@ -3851,6 +3849,8 @@ static long aac_build_sgraw(struct scsi_cmnd *scsicmd, struct sgmapraw *psg)
{
unsigned long byte_count = 0;
int nseg;
+ struct scatterlist *sg;
+ int i;

// Get rid of old data
psg->count = 0;
@@ -3862,37 +3862,34 @@ static long aac_build_sgraw(struct scsi_cmnd *scsicmd, struct sgmapraw *psg)
psg->sg[0].flags = 0;

nseg = scsi_dma_map(scsicmd);
- if (nseg < 0)
+ if (nseg <= 0)
return nseg;
- if (nseg) {
- struct scatterlist *sg;
- int i;
-
- scsi_for_each_sg(scsicmd, sg, nseg, i) {
- int count = sg_dma_len(sg);
- u64 addr = sg_dma_address(sg);
- psg->sg[i].next = 0;
- psg->sg[i].prev = 0;
- psg->sg[i].addr[1] = cpu_to_le32((u32)(addr>>32));
- psg->sg[i].addr[0] = cpu_to_le32((u32)(addr & 0xffffffff));
- psg->sg[i].count = cpu_to_le32(count);
- psg->sg[i].flags = 0;
- byte_count += count;
- }
- psg->count = cpu_to_le32(nseg);
- /* hba wants the size to be exact */
- if (byte_count > scsi_bufflen(scsicmd)) {
- u32 temp = le32_to_cpu(psg->sg[i-1].count) -
- (byte_count - scsi_bufflen(scsicmd));
- psg->sg[i-1].count = cpu_to_le32(temp);
- byte_count = scsi_bufflen(scsicmd);
- }
- /* Check for command underflow */
- if(scsicmd->underflow && (byte_count < scsicmd->underflow)){
- printk(KERN_WARNING"aacraid: cmd len %08lX cmd underflow %08X\n",
- byte_count, scsicmd->underflow);
- }
+
+ scsi_for_each_sg(scsicmd, sg, nseg, i) {
+ int count = sg_dma_len(sg);
+ u64 addr = sg_dma_address(sg);
+ psg->sg[i].next = 0;
+ psg->sg[i].prev = 0;
+ psg->sg[i].addr[1] = cpu_to_le32((u32)(addr>>32));
+ psg->sg[i].addr[0] = cpu_to_le32((u32)(addr & 0xffffffff));
+ psg->sg[i].count = cpu_to_le32(count);
+ psg->sg[i].flags = 0;
+ byte_count += count;
+ }
+ psg->count = cpu_to_le32(nseg);
+ /* hba wants the size to be exact */
+ if (byte_count > scsi_bufflen(scsicmd)) {
+ u32 temp = le32_to_cpu(psg->sg[i-1].count) -
+ (byte_count - scsi_bufflen(scsicmd));
+ psg->sg[i-1].count = cpu_to_le32(temp);
+ byte_count = scsi_bufflen(scsicmd);
+ }
+ /* Check for command underflow */
+ if(scsicmd->underflow && (byte_count < scsicmd->underflow)){
+ printk(KERN_WARNING"aacraid: cmd len %08lX cmd underflow %08X\n",
+ byte_count, scsicmd->underflow);
}
+
return byte_count;
}

@@ -3901,75 +3898,73 @@ static long aac_build_sgraw2(struct scsi_cmnd *scsicmd,
{
unsigned long byte_count = 0;
int nseg;
+ struct scatterlist *sg;
+ int i, conformable = 0;
+ u32 min_size = PAGE_SIZE, cur_size;

nseg = scsi_dma_map(scsicmd);
- if (nseg < 0)
+ if (nseg <= 0)
return nseg;
- if (nseg) {
- struct scatterlist *sg;
- int i, conformable = 0;
- u32 min_size = PAGE_SIZE, cur_size;
-
- scsi_for_each_sg(scsicmd, sg, nseg, i) {
- int count = sg_dma_len(sg);
- u64 addr = sg_dma_address(sg);
-
- BUG_ON(i >= sg_max);
- rio2->sge[i].addrHigh = cpu_to_le32((u32)(addr>>32));
- rio2->sge[i].addrLow = cpu_to_le32((u32)(addr & 0xffffffff));
- cur_size = cpu_to_le32(count);
- rio2->sge[i].length = cur_size;
- rio2->sge[i].flags = 0;
- if (i == 0) {
- conformable = 1;
- rio2->sgeFirstSize = cur_size;
- } else if (i == 1) {
- rio2->sgeNominalSize = cur_size;
+
+ scsi_for_each_sg(scsicmd, sg, nseg, i) {
+ int count = sg_dma_len(sg);
+ u64 addr = sg_dma_address(sg);
+
+ BUG_ON(i >= sg_max);
+ rio2->sge[i].addrHigh = cpu_to_le32((u32)(addr>>32));
+ rio2->sge[i].addrLow = cpu_to_le32((u32)(addr & 0xffffffff));
+ cur_size = cpu_to_le32(count);
+ rio2->sge[i].length = cur_size;
+ rio2->sge[i].flags = 0;
+ if (i == 0) {
+ conformable = 1;
+ rio2->sgeFirstSize = cur_size;
+ } else if (i == 1) {
+ rio2->sgeNominalSize = cur_size;
+ min_size = cur_size;
+ } else if ((i+1) < nseg && cur_size != rio2->sgeNominalSize) {
+ conformable = 0;
+ if (cur_size < min_size)
min_size = cur_size;
- } else if ((i+1) < nseg && cur_size != rio2->sgeNominalSize) {
- conformable = 0;
- if (cur_size < min_size)
- min_size = cur_size;
- }
- byte_count += count;
}
+ byte_count += count;
+ }

- /* hba wants the size to be exact */
- if (byte_count > scsi_bufflen(scsicmd)) {
- u32 temp = le32_to_cpu(rio2->sge[i-1].length) -
- (byte_count - scsi_bufflen(scsicmd));
- rio2->sge[i-1].length = cpu_to_le32(temp);
- byte_count = scsi_bufflen(scsicmd);
- }
+ /* hba wants the size to be exact */
+ if (byte_count > scsi_bufflen(scsicmd)) {
+ u32 temp = le32_to_cpu(rio2->sge[i-1].length) -
+ (byte_count - scsi_bufflen(scsicmd));
+ rio2->sge[i-1].length = cpu_to_le32(temp);
+ byte_count = scsi_bufflen(scsicmd);
+ }

- rio2->sgeCnt = cpu_to_le32(nseg);
- rio2->flags |= cpu_to_le16(RIO2_SG_FORMAT_IEEE1212);
- /* not conformable: evaluate required sg elements */
- if (!conformable) {
- int j, nseg_new = nseg, err_found;
- for (i = min_size / PAGE_SIZE; i >= 1; --i) {
- err_found = 0;
- nseg_new = 2;
- for (j = 1; j < nseg - 1; ++j) {
- if (rio2->sge[j].length % (i*PAGE_SIZE)) {
- err_found = 1;
- break;
- }
- nseg_new += (rio2->sge[j].length / (i*PAGE_SIZE));
- }
- if (!err_found)
+ rio2->sgeCnt = cpu_to_le32(nseg);
+ rio2->flags |= cpu_to_le16(RIO2_SG_FORMAT_IEEE1212);
+ /* not conformable: evaluate required sg elements */
+ if (!conformable) {
+ int j, nseg_new = nseg, err_found;
+ for (i = min_size / PAGE_SIZE; i >= 1; --i) {
+ err_found = 0;
+ nseg_new = 2;
+ for (j = 1; j < nseg - 1; ++j) {
+ if (rio2->sge[j].length % (i*PAGE_SIZE)) {
+ err_found = 1;
break;
+ }
+ nseg_new += (rio2->sge[j].length / (i*PAGE_SIZE));
}
- if (i > 0 && nseg_new <= sg_max)
- aac_convert_sgraw2(rio2, i, nseg, nseg_new);
- } else
- rio2->flags |= cpu_to_le16(RIO2_SGL_CONFORMANT);
-
- /* Check for command underflow */
- if (scsicmd->underflow && (byte_count < scsicmd->underflow)) {
- printk(KERN_WARNING"aacraid: cmd len %08lX cmd underflow %08X\n",
- byte_count, scsicmd->underflow);
+ if (!err_found)
+ break;
}
+ if (i > 0 && nseg_new <= sg_max)
+ aac_convert_sgraw2(rio2, i, nseg, nseg_new);
+ } else
+ rio2->flags |= cpu_to_le16(RIO2_SGL_CONFORMANT);
+
+ /* Check for command underflow */
+ if (scsicmd->underflow && (byte_count < scsicmd->underflow)) {
+ printk(KERN_WARNING"aacraid: cmd len %08lX cmd underflow %08X\n",
+ byte_count, scsicmd->underflow);
}

return byte_count;
--
2.13.5

2017-08-30 02:10:32

by Raghava Aditya Renukunta

[permalink] [raw]
Subject: RE: [PATCH 1/3] scsi: aacraid: fix indentation errors



> -----Original Message-----
> From: Nikola Pajkovsky [mailto:[email protected]]
> Sent: Tuesday, August 29, 2017 4:59 AM
> To: dl-esc-Aacraid Linux Driver <[email protected]>
> Cc: Nikola Pajkovsky <[email protected]>; James E.J. Bottomley
> <[email protected]>; Martin K. Petersen
> <[email protected]>; [email protected]; linux-
> [email protected]
> Subject: [PATCH 1/3] scsi: aacraid: fix indentation errors
>
> EXTERNAL EMAIL
>
>
> fix stupid indent error, no rocket science here.
>
> Signed-off-by: Nikola Pajkovsky <[email protected]>
> ---
[.....]
Reviewed-by: Raghava Aditya Renukunta <[email protected]>


2017-08-30 02:11:05

by Raghava Aditya Renukunta

[permalink] [raw]
Subject: RE: [PATCH 2/3] scsi: aacraid: get rid of one level of indentation



> -----Original Message-----
> From: Nikola Pajkovsky [mailto:[email protected]]
> Sent: Tuesday, August 29, 2017 4:59 AM
> To: dl-esc-Aacraid Linux Driver <[email protected]>
> Cc: Nikola Pajkovsky <[email protected]>; James E.J. Bottomley
> <[email protected]>; Martin K. Petersen
> <[email protected]>; [email protected]; linux-
> [email protected]
> Subject: [PATCH 2/3] scsi: aacraid: get rid of one level of indentation
>
> EXTERNAL EMAIL
>
>
> unsigned long byte_count = 0;
> nseg = scsi_dma_map(scsicmd);
> if (nseg < 0)
> return nseg;
> if (nseg) {
> ...
> }
> return byte_count;
>
> is equal to
>
> unsigned long byte_count = 0;
> nseg = scsi_dma_map(scsicmd);
> if (nseg <= 0)
> return nseg;
> ...
> return byte_count;
>
> No other code has changed.
>
> Signed-off-by: Nikola Pajkovsky <[email protected]>
> ---
[.....]
Reviewed-by: Raghava Aditya Renukunta <[email protected]>

2017-08-30 02:11:30

by Raghava Aditya Renukunta

[permalink] [raw]
Subject: RE: [PATCH 3/3] scsi: aacraid: report -ENOMEM to upper layer from aac_convert_sgraw2()



> -----Original Message-----
> From: Nikola Pajkovsky [mailto:[email protected]]
> Sent: Tuesday, August 29, 2017 4:59 AM
> To: dl-esc-Aacraid Linux Driver <[email protected]>
> Cc: Nikola Pajkovsky <[email protected]>; James E.J. Bottomley
> <[email protected]>; Martin K. Petersen
> <[email protected]>; [email protected]; linux-
> [email protected]
> Subject: [PATCH 3/3] scsi: aacraid: report -ENOMEM to upper layer from
> aac_convert_sgraw2()
>
> EXTERNAL EMAIL
>
>
> aac_convert_sgraw2() kmalloc memory and return -1 on error, which
> should be -ENOMEM. However, nobody is checking return value, so with
> this change, -ENOMEM is propagated to upper layer.
>
> Signed-off-by: Nikola Pajkovsky <[email protected]>
> ---
[.....]
Reviewed-by: Raghava Aditya Renukunta <[email protected]>

2017-08-31 02:03:24

by Martin K. Petersen

[permalink] [raw]
Subject: Re: [PATCH 3/3] scsi: aacraid: report -ENOMEM to upper layer from aac_convert_sgraw2()


Nikola,

> aac_convert_sgraw2() kmalloc memory and return -1 on error, which
> should be -ENOMEM. However, nobody is checking return value, so with
> this change, -ENOMEM is propagated to upper layer.

Applied to 4.14/scsi-queue.

--
Martin K. Petersen Oracle Linux Engineering