2020-04-03 18:01:17

by Alex Dewar

[permalink] [raw]
Subject: [PATCH] scsi: Remove unnecessary calls to memset after dma_alloc_coherent

dma_alloc_coherent() now zeroes memory after allocation, so additional
calls to memset() afterwards are unnecessary. Remove them.

Issue identified with Coccinelle.

Signed-off-by: Alex Dewar <[email protected]>
---
drivers/scsi/dpt_i2o.c | 8 ++------
drivers/scsi/mpt3sas/mpt3sas_base.c | 3 +--
drivers/scsi/mvsas/mv_init.c | 8 +-------
drivers/scsi/pmcraid.c | 1 -
drivers/scsi/qla2xxx/qla_isr.c | 3 +--
drivers/scsi/qla2xxx/qla_mbx.c | 4 +---
6 files changed, 6 insertions(+), 21 deletions(-)

diff --git a/drivers/scsi/dpt_i2o.c b/drivers/scsi/dpt_i2o.c
index 02dff3a684e0..3daf274f85c3 100644
--- a/drivers/scsi/dpt_i2o.c
+++ b/drivers/scsi/dpt_i2o.c
@@ -1331,7 +1331,6 @@ static s32 adpt_i2o_reset_hba(adpt_hba* pHba)
printk(KERN_ERR"IOP reset failed - no free memory.\n");
return -ENOMEM;
}
- memset(status,0,4);

msg[0]=EIGHT_WORD_MSG_SIZE|SGL_OFFSET_0;
msg[1]=I2O_CMD_ADAPTER_RESET<<24|HOST_TID<<12|ADAPTER_TID;
@@ -2784,7 +2783,6 @@ static s32 adpt_i2o_init_outbound_q(adpt_hba* pHba)
pHba->name);
return -ENOMEM;
}
- memset(status, 0, 4);

writel(EIGHT_WORD_MSG_SIZE| SGL_OFFSET_6, &msg[0]);
writel(I2O_CMD_OUTBOUND_INIT<<24 | HOST_TID<<12 | ADAPTER_TID, &msg[1]);
@@ -2838,7 +2836,6 @@ static s32 adpt_i2o_init_outbound_q(adpt_hba* pHba)
printk(KERN_ERR "%s: Could not allocate reply pool\n", pHba->name);
return -ENOMEM;
}
- memset(pHba->reply_pool, 0 , pHba->reply_fifo_size * REPLY_FRAME_SIZE * 4);

for(i = 0; i < pHba->reply_fifo_size; i++) {
writel(pHba->reply_pool_pa + (i * REPLY_FRAME_SIZE * 4),
@@ -3067,13 +3064,12 @@ static int adpt_i2o_build_sys_table(void)
sys_tbl_len = sizeof(struct i2o_sys_tbl) + // Header + IOPs
(hba_count) * sizeof(struct i2o_sys_tbl_entry);

- sys_tbl = dma_alloc_coherent(&pHba->pDev->dev,
- sys_tbl_len, &sys_tbl_pa, GFP_KERNEL);
+ sys_tbl = dma_alloc_coherent(&pHba->pDev->dev, sys_tbl_len,
+ &sys_tbl_pa, GFP_KERNEL);
if (!sys_tbl) {
printk(KERN_WARNING "SysTab Set failed. Out of memory.\n");
return -ENOMEM;
}
- memset(sys_tbl, 0, sys_tbl_len);

sys_tbl->num_entries = hba_count;
sys_tbl->version = I2OVERSION;
diff --git a/drivers/scsi/mpt3sas/mpt3sas_base.c b/drivers/scsi/mpt3sas/mpt3sas_base.c
index 663782bb790d..6144c0910b90 100644
--- a/drivers/scsi/mpt3sas/mpt3sas_base.c
+++ b/drivers/scsi/mpt3sas/mpt3sas_base.c
@@ -5203,7 +5203,7 @@ _base_allocate_memory_pools(struct MPT3SAS_ADAPTER *ioc)

ioc->request_dma_sz = sz;
ioc->request = dma_alloc_coherent(&ioc->pdev->dev, sz,
- &ioc->request_dma, GFP_KERNEL);
+ &ioc->request_dma, GFP_KERNEL);
if (!ioc->request) {
ioc_err(ioc, "request pool: dma_alloc_coherent failed: hba_depth(%d), chains_per_io(%d), frame_sz(%d), total(%d kB)\n",
ioc->hba_queue_depth, ioc->chains_needed_per_io,
@@ -5215,7 +5215,6 @@ _base_allocate_memory_pools(struct MPT3SAS_ADAPTER *ioc)
_base_release_memory_pools(ioc);
goto retry_allocation;
}
- memset(ioc->request, 0, sz);

if (retry_sz)
ioc_err(ioc, "request pool: dma_alloc_coherent succeed: hba_depth(%d), chains_per_io(%d), frame_sz(%d), total(%d kb)\n",
diff --git a/drivers/scsi/mvsas/mv_init.c b/drivers/scsi/mvsas/mv_init.c
index 7af9173c4925..75c9fc37a388 100644
--- a/drivers/scsi/mvsas/mv_init.c
+++ b/drivers/scsi/mvsas/mv_init.c
@@ -244,28 +244,22 @@ static int mvs_alloc(struct mvs_info *mvi, struct Scsi_Host *shost)
&mvi->tx_dma, GFP_KERNEL);
if (!mvi->tx)
goto err_out;
- memset(mvi->tx, 0, sizeof(*mvi->tx) * MVS_CHIP_SLOT_SZ);
mvi->rx_fis = dma_alloc_coherent(mvi->dev, MVS_RX_FISL_SZ,
&mvi->rx_fis_dma, GFP_KERNEL);
if (!mvi->rx_fis)
goto err_out;
- memset(mvi->rx_fis, 0, MVS_RX_FISL_SZ);
-
mvi->rx = dma_alloc_coherent(mvi->dev,
sizeof(*mvi->rx) * (MVS_RX_RING_SZ + 1),
&mvi->rx_dma, GFP_KERNEL);
if (!mvi->rx)
goto err_out;
- memset(mvi->rx, 0, sizeof(*mvi->rx) * (MVS_RX_RING_SZ + 1));
mvi->rx[0] = cpu_to_le32(0xfff);
mvi->rx_cons = 0xfff;

- mvi->slot = dma_alloc_coherent(mvi->dev,
- sizeof(*mvi->slot) * slot_nr,
+ mvi->slot = dma_alloc_coherent(mvi->dev, sizeof(*mvi->slot) * slot_nr,
&mvi->slot_dma, GFP_KERNEL);
if (!mvi->slot)
goto err_out;
- memset(mvi->slot, 0, sizeof(*mvi->slot) * slot_nr);

mvi->bulk_buffer = dma_alloc_coherent(mvi->dev,
TRASH_BUCKET_SIZE,
diff --git a/drivers/scsi/pmcraid.c b/drivers/scsi/pmcraid.c
index 7eb88fe1eb0b..6976013cf4c7 100644
--- a/drivers/scsi/pmcraid.c
+++ b/drivers/scsi/pmcraid.c
@@ -4718,7 +4718,6 @@ static int pmcraid_allocate_host_rrqs(struct pmcraid_instance *pinstance)
return -ENOMEM;
}

- memset(pinstance->hrrq_start[i], 0, buffer_size);
pinstance->hrrq_curr[i] = pinstance->hrrq_start[i];
pinstance->hrrq_end[i] =
pinstance->hrrq_start[i] + PMCRAID_MAX_CMD - 1;
diff --git a/drivers/scsi/qla2xxx/qla_isr.c b/drivers/scsi/qla2xxx/qla_isr.c
index 8d7a905f6247..632fd56cb626 100644
--- a/drivers/scsi/qla2xxx/qla_isr.c
+++ b/drivers/scsi/qla2xxx/qla_isr.c
@@ -79,7 +79,7 @@ qla24xx_process_abts(struct scsi_qla_host *vha, void *pkt)
(uint8_t *)abts, sizeof(*abts));

rsp_els = dma_alloc_coherent(&ha->pdev->dev, sizeof(*rsp_els), &dma,
- GFP_KERNEL);
+ GFP_KERNEL);
if (!rsp_els) {
ql_log(ql_log_warn, vha, 0x0287,
"Failed allocate dma buffer ABTS/ELS RSP.\n");
@@ -87,7 +87,6 @@ qla24xx_process_abts(struct scsi_qla_host *vha, void *pkt)
}

/* terminate exchange */
- memset(rsp_els, 0, sizeof(*rsp_els));
rsp_els->entry_type = ELS_IOCB_TYPE;
rsp_els->entry_count = 1;
rsp_els->nport_handle = ~0;
diff --git a/drivers/scsi/qla2xxx/qla_mbx.c b/drivers/scsi/qla2xxx/qla_mbx.c
index 9fd83d1bffe0..6d8573b870bc 100644
--- a/drivers/scsi/qla2xxx/qla_mbx.c
+++ b/drivers/scsi/qla2xxx/qla_mbx.c
@@ -4887,15 +4887,13 @@ qla25xx_set_els_cmds_supported(scsi_qla_host_t *vha)
"Entered %s.\n", __func__);

els_cmd_map = dma_alloc_coherent(&ha->pdev->dev, ELS_CMD_MAP_SIZE,
- &els_cmd_map_dma, GFP_KERNEL);
+ &els_cmd_map_dma, GFP_KERNEL);
if (!els_cmd_map) {
ql_log(ql_log_warn, vha, 0x7101,
"Failed to allocate RDP els command param.\n");
return QLA_MEMORY_ALLOC_FAILED;
}

- memset(els_cmd_map, 0, ELS_CMD_MAP_SIZE);
-
els_cmd_map[index] |= 1 << bit;

mcp->mb[0] = MBC_SET_RNID_PARAMS;
--
2.26.0


2020-04-04 11:17:39

by Markus Elfring

[permalink] [raw]
Subject: Re: [PATCH] scsi: Remove unnecessary calls to memset after dma_alloc_coherent

> dma_alloc_coherent() now zeroes memory after allocation, so additional
> calls to memset() afterwards are unnecessary. Remove them.

I suggest to reconsider the distribution of recipients also for this patch
according to the fields “Cc” and “To”.



> +++ b/drivers/scsi/dpt_i2o.c

> @@ -3067,13 +3064,12 @@ static int adpt_i2o_build_sys_table(void)
> sys_tbl_len = sizeof(struct i2o_sys_tbl) + // Header + IOPs
> (hba_count) * sizeof(struct i2o_sys_tbl_entry);
>
> - sys_tbl = dma_alloc_coherent(&pHba->pDev->dev,
> - sys_tbl_len, &sys_tbl_pa, GFP_KERNEL);
> + sys_tbl = dma_alloc_coherent(&pHba->pDev->dev, sys_tbl_len,
> + &sys_tbl_pa, GFP_KERNEL);
> if (!sys_tbl) {

> +++ b/drivers/scsi/mvsas/mv_init.c
> @@ -244,28 +244,22 @@ static int mvs_alloc(struct mvs_info *mvi, struct Scsi_Host *shost)

> - mvi->slot = dma_alloc_coherent(mvi->dev,
> - sizeof(*mvi->slot) * slot_nr,
> + mvi->slot = dma_alloc_coherent(mvi->dev, sizeof(*mvi->slot) * slot_nr,
> &mvi->slot_dma, GFP_KERNEL);
> if (!mvi->slot)

> +++ b/drivers/scsi/qla2xxx/qla_isr.c
> @@ -79,7 +79,7 @@ qla24xx_process_abts(struct scsi_qla_host *vha, void *pkt)
> (uint8_t *)abts, sizeof(*abts));
>
> rsp_els = dma_alloc_coherent(&ha->pdev->dev, sizeof(*rsp_els), &dma,
> - GFP_KERNEL);
> + GFP_KERNEL);
> if (!rsp_els) {

> +++ b/drivers/scsi/qla2xxx/qla_mbx.c
> @@ -4887,15 +4887,13 @@ qla25xx_set_els_cmds_supported(scsi_qla_host_t *vha)
> "Entered %s.\n", __func__);
>
> els_cmd_map = dma_alloc_coherent(&ha->pdev->dev, ELS_CMD_MAP_SIZE,
> - &els_cmd_map_dma, GFP_KERNEL);
> + &els_cmd_map_dma, GFP_KERNEL);
> if (!els_cmd_map) {


I find it safer to integrate such source code reformattings by
another update step which will be separated from the proposed deletion
of unwanted function calls.

Regards,
Markus

2020-04-07 16:07:21

by Alex Dewar

[permalink] [raw]
Subject: Re: [PATCH] scsi: Remove unnecessary calls to memset after dma_alloc_coherent

On Sat, Apr 04, 2020 at 01:14:52PM +0200, Markus Elfring wrote:
> > dma_alloc_coherent() now zeroes memory after allocation, so additional
> > calls to memset() afterwards are unnecessary. Remove them.
>
> I suggest to reconsider the distribution of recipients also for this patch
> according to the fields “Cc” and “To”.

Thanks. I'll do this in a v2/RESEND.
>
>
> …
> > +++ b/drivers/scsi/dpt_i2o.c
> …
> > @@ -3067,13 +3064,12 @@ static int adpt_i2o_build_sys_table(void)
> > sys_tbl_len = sizeof(struct i2o_sys_tbl) + // Header + IOPs
> > (hba_count) * sizeof(struct i2o_sys_tbl_entry);
> >
> > - sys_tbl = dma_alloc_coherent(&pHba->pDev->dev,
> > - sys_tbl_len, &sys_tbl_pa, GFP_KERNEL);
> > + sys_tbl = dma_alloc_coherent(&pHba->pDev->dev, sys_tbl_len,
> > + &sys_tbl_pa, GFP_KERNEL);
> > if (!sys_tbl) {
> …
> > +++ b/drivers/scsi/mvsas/mv_init.c
> > @@ -244,28 +244,22 @@ static int mvs_alloc(struct mvs_info *mvi, struct Scsi_Host *shost)
> …
> > - mvi->slot = dma_alloc_coherent(mvi->dev,
> > - sizeof(*mvi->slot) * slot_nr,
> > + mvi->slot = dma_alloc_coherent(mvi->dev, sizeof(*mvi->slot) * slot_nr,
> > &mvi->slot_dma, GFP_KERNEL);
> > if (!mvi->slot)
> …
> > +++ b/drivers/scsi/qla2xxx/qla_isr.c
> > @@ -79,7 +79,7 @@ qla24xx_process_abts(struct scsi_qla_host *vha, void *pkt)
> > (uint8_t *)abts, sizeof(*abts));
> >
> > rsp_els = dma_alloc_coherent(&ha->pdev->dev, sizeof(*rsp_els), &dma,
> > - GFP_KERNEL);
> > + GFP_KERNEL);
> > if (!rsp_els) {
> …
> > +++ b/drivers/scsi/qla2xxx/qla_mbx.c
> > @@ -4887,15 +4887,13 @@ qla25xx_set_els_cmds_supported(scsi_qla_host_t *vha)
> > "Entered %s.\n", __func__);
> >
> > els_cmd_map = dma_alloc_coherent(&ha->pdev->dev, ELS_CMD_MAP_SIZE,
> > - &els_cmd_map_dma, GFP_KERNEL);
> > + &els_cmd_map_dma, GFP_KERNEL);
> > if (!els_cmd_map) {
> …
>
> I find it safer to integrate such source code reformattings by
> another update step which will be separated from the proposed deletion
> of unwanted function calls.

Good point. This whitespace was autoformatted by Coccinelle, probably
due to my bad SmPL skills.

Best,
Alex


>
> Regards,
> Markus

2020-04-07 16:58:05

by Markus Elfring

[permalink] [raw]
Subject: Re: [PATCH] scsi: Remove unnecessary calls to memset after dma_alloc_coherent

>> …
>>> +++ b/drivers/scsi/qla2xxx/qla_mbx.c
>>> @@ -4887,15 +4887,13 @@ qla25xx_set_els_cmds_supported(scsi_qla_host_t *vha)
>>> "Entered %s.\n", __func__);
>>>
>>> els_cmd_map = dma_alloc_coherent(&ha->pdev->dev, ELS_CMD_MAP_SIZE,
>>> - &els_cmd_map_dma, GFP_KERNEL);
>>> + &els_cmd_map_dma, GFP_KERNEL);
>>> if (!els_cmd_map) {
>> …
>>
>> I find it safer to integrate such source code reformattings by
>> another update step which will be separated from the proposed deletion
>> of unwanted function calls.
>
> Good point. This whitespace was autoformatted by Coccinelle,
> probably due to my bad SmPL skills.

Some system factors can be involved here.

* The source code formatting can occasionally be improvable
in further ways (despite of help by a software like Coccinelle).

* A change mixture can become more challenging.

* Would you like to extend your skills in corresponding areas anyhow?

Regards,
Markus

2020-04-09 11:45:09

by Alex Dewar

[permalink] [raw]
Subject: Re: [PATCH] scsi: Remove unnecessary calls to memset after dma_alloc_coherent

On Tue, Apr 07, 2020 at 06:56:28PM +0200, Markus Elfring wrote:
> >> …
> >>> +++ b/drivers/scsi/qla2xxx/qla_mbx.c
> >>> @@ -4887,15 +4887,13 @@ qla25xx_set_els_cmds_supported(scsi_qla_host_t *vha)
> >>> "Entered %s.\n", __func__);
> >>>
> >>> els_cmd_map = dma_alloc_coherent(&ha->pdev->dev, ELS_CMD_MAP_SIZE,
> >>> - &els_cmd_map_dma, GFP_KERNEL);
> >>> + &els_cmd_map_dma, GFP_KERNEL);
> >>> if (!els_cmd_map) {
> >> …
> >>
> >> I find it safer to integrate such source code reformattings by
> >> another update step which will be separated from the proposed deletion
> >> of unwanted function calls.
> >
> > Good point. This whitespace was autoformatted by Coccinelle,
> > probably due to my bad SmPL skills.
>
> Some system factors can be involved here.
>
> * The source code formatting can occasionally be improvable
> in further ways (despite of help by a software like Coccinelle).
>
> * A change mixture can become more challenging.
>
> * Would you like to extend your skills in corresponding areas anyhow?

Sure, I'd love to. Are there any resources you'd recommend? I'm just
starting out with kernel stuff and would be grateful for any pointers
you can offer :-)

Best,
Alex


>
> Regards,
> Markus

2020-04-09 14:11:15

by Markus Elfring

[permalink] [raw]
Subject: Re: [PATCH] scsi: Remove unnecessary calls to memset after dma_alloc_coherent

>> * Would you like to extend your skills in corresponding areas anyhow?
>
> Sure, I'd love to. Are there any resources you'd recommend?

How helpful do you find the available software documentation
(and other communication interfaces)?

Regards,
Markus