* Re: [PATCH] scsi: Remove unnecessary calls to memset after dma_alloc_coherent
@ 2020-04-04 11:14 Markus Elfring
2020-04-07 16:02 ` Alex Dewar
0 siblings, 1 reply; 6+ messages in thread
From: Markus Elfring @ 2020-04-04 11:14 UTC (permalink / raw)
To: Alex Dewar, Allison Randal, Arnd Bergmann, Chaitra Basappa,
Greg Kroah-Hartman, Himanshu Madhani, James E. J. Bottomley,
Martin K. Petersen, Richard Fontana, Sathya Prakash,
Suganath Prabu Subramani, Thomas Gleixner, linux-scsi, aacraid,
MPT-FusionLinux.pdl
Cc: linux-kernel
> 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
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] scsi: Remove unnecessary calls to memset after dma_alloc_coherent
2020-04-04 11:14 [PATCH] scsi: Remove unnecessary calls to memset after dma_alloc_coherent Markus Elfring
@ 2020-04-07 16:02 ` Alex Dewar
2020-04-07 16:56 ` Markus Elfring
0 siblings, 1 reply; 6+ messages in thread
From: Alex Dewar @ 2020-04-07 16:02 UTC (permalink / raw)
To: Markus Elfring
Cc: Allison Randal, Arnd Bergmann, Chaitra Basappa,
Greg Kroah-Hartman, Himanshu Madhani, James E. J. Bottomley,
Martin K. Petersen, Richard Fontana, Sathya Prakash,
Suganath Prabu Subramani, Thomas Gleixner, linux-scsi, aacraid,
MPT-FusionLinux.pdl, linux-kernel
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
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] scsi: Remove unnecessary calls to memset after dma_alloc_coherent
2020-04-07 16:02 ` Alex Dewar
@ 2020-04-07 16:56 ` Markus Elfring
2020-04-09 11:42 ` Alex Dewar
0 siblings, 1 reply; 6+ messages in thread
From: Markus Elfring @ 2020-04-07 16:56 UTC (permalink / raw)
To: Alex Dewar
Cc: Allison Randal, Arnd Bergmann, Julia Lawall, Greg Kroah-Hartman,
Himanshu Madhani, James E. J. Bottomley, Martin K. Petersen,
Richard Fontana, Sathya Prakash, Suganath Prabu Subramani,
Thomas Gleixner, linux-scsi, aacraid, MPT-FusionLinux.pdl,
linux-kernel
>> …
>>> +++ 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
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] scsi: Remove unnecessary calls to memset after dma_alloc_coherent
2020-04-07 16:56 ` Markus Elfring
@ 2020-04-09 11:42 ` Alex Dewar
2020-04-09 14:06 ` Markus Elfring
0 siblings, 1 reply; 6+ messages in thread
From: Alex Dewar @ 2020-04-09 11:42 UTC (permalink / raw)
To: Markus Elfring
Cc: Allison Randal, Arnd Bergmann, Julia Lawall, Greg Kroah-Hartman,
Himanshu Madhani, James E. J. Bottomley, Martin K. Petersen,
Richard Fontana, Sathya Prakash, Suganath Prabu Subramani,
Thomas Gleixner, linux-scsi, aacraid, MPT-FusionLinux.pdl,
linux-kernel
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
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] scsi: Remove unnecessary calls to memset after dma_alloc_coherent
2020-04-09 11:42 ` Alex Dewar
@ 2020-04-09 14:06 ` Markus Elfring
0 siblings, 0 replies; 6+ messages in thread
From: Markus Elfring @ 2020-04-09 14:06 UTC (permalink / raw)
To: Alex Dewar, linux-scsi
Cc: Allison Randal, Arnd Bergmann, Julia Lawall, Greg Kroah-Hartman,
Himanshu Madhani, James E. J. Bottomley, Martin K. Petersen,
Richard Fontana, Sathya Prakash, Suganath Prabu Subramani,
Thomas Gleixner, aacraid, MPT-FusionLinux.pdl, linux-kernel
>> * 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
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH] scsi: Remove unnecessary calls to memset after dma_alloc_coherent
@ 2020-04-03 17:58 Alex Dewar
0 siblings, 0 replies; 6+ messages in thread
From: Alex Dewar @ 2020-04-03 17:58 UTC (permalink / raw)
To: alex.dewar
Cc: Adaptec OEM Raid Solutions, James E.J. Bottomley,
Martin K. Petersen, Sathya Prakash, Chaitra P B,
Suganath Prabu Subramani, hmadhani, Richard Fontana,
Arnd Bergmann, Allison Randal, Greg Kroah-Hartman,
Thomas Gleixner, linux-scsi, linux-kernel, MPT-FusionLinux.pdl
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 <alex.dewar@gmx.co.uk>
---
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
^ permalink raw reply related [flat|nested] 6+ messages in thread
end of thread, other threads:[~2020-04-09 14:07 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2020-04-04 11:14 [PATCH] scsi: Remove unnecessary calls to memset after dma_alloc_coherent Markus Elfring
2020-04-07 16:02 ` Alex Dewar
2020-04-07 16:56 ` Markus Elfring
2020-04-09 11:42 ` Alex Dewar
2020-04-09 14:06 ` Markus Elfring
-- strict thread matches above, loose matches on Subject: below --
2020-04-03 17:58 Alex Dewar
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox