2018-11-21 14:32:03

by Sabyasachi Gupta

Subject: [PATCH] scsi/mvsas/mv_init.c: Use dma_zalloc_coherent

Replace dma_alloc_coherent + memset with dma_zalloc_coherent

Signed-off-by: Sabyasachi Gupta <[email protected]>
---
drivers/scsi/mvsas/mv_init.c | 12 ++++--------
1 file changed, 4 insertions(+), 8 deletions(-)

diff --git a/drivers/scsi/mvsas/mv_init.c b/drivers/scsi/mvsas/mv_init.c
index 3ac3437..495bddb 100644
--- a/drivers/scsi/mvsas/mv_init.c
+++ b/drivers/scsi/mvsas/mv_init.c
@@ -253,33 +253,29 @@ static int mvs_alloc(struct mvs_info *mvi, struct Scsi_Host *shost)
/*
* alloc and init our DMA areas
*/
- mvi->tx = dma_alloc_coherent(mvi->dev,
+ mvi->tx = dma_zalloc_coherent(mvi->dev,
sizeof(*mvi->tx) * MVS_CHIP_SLOT_SZ,
&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_zalloc_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,
+ mvi->rx = dma_zalloc_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,
+ mvi->slot = dma_zalloc_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,
--
2.7.4

2018-12-01 13:11:17

by Sabyasachi Gupta

Subject: Re: [PATCH] scsi/mvsas/mv_init.c: Use dma_zalloc_coherent

On Wed, Nov 21, 2018 at 7:18 PM Sabyasachi Gupta
<[email protected]> wrote:
>
> Replace dma_alloc_coherent + memset with dma_zalloc_coherent
>
> Signed-off-by: Sabyasachi Gupta <[email protected]>

Any comment on this patch?

> ---
> drivers/scsi/mvsas/mv_init.c | 12 ++++--------
> 1 file changed, 4 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/scsi/mvsas/mv_init.c b/drivers/scsi/mvsas/mv_init.c
> index 3ac3437..495bddb 100644
> --- a/drivers/scsi/mvsas/mv_init.c
> +++ b/drivers/scsi/mvsas/mv_init.c
> @@ -253,33 +253,29 @@ static int mvs_alloc(struct mvs_info *mvi, struct Scsi_Host *shost)
> /*
> * alloc and init our DMA areas
> */
> - mvi->tx = dma_alloc_coherent(mvi->dev,
> + mvi->tx = dma_zalloc_coherent(mvi->dev,
> sizeof(*mvi->tx) * MVS_CHIP_SLOT_SZ,
> &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_zalloc_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,
> + mvi->rx = dma_zalloc_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,
> + mvi->slot = dma_zalloc_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,
> --
> 2.7.4
>

2018-12-19 13:37:47

by Sabyasachi Gupta

Subject: Re: [PATCH] scsi/mvsas/mv_init.c: Use dma_zalloc_coherent

On Sat, Dec 1, 2018 at 6:40 PM Sabyasachi Gupta
<[email protected]> wrote:
>
> On Wed, Nov 21, 2018 at 7:18 PM Sabyasachi Gupta
> <[email protected]> wrote:
> >
> > Replace dma_alloc_coherent + memset with dma_zalloc_coherent
> >
> > Signed-off-by: Sabyasachi Gupta <[email protected]>
>
> Any comment on this patch?

Any comment on this patch?

>
> > ---
> > drivers/scsi/mvsas/mv_init.c | 12 ++++--------
> > 1 file changed, 4 insertions(+), 8 deletions(-)
> >
> > diff --git a/drivers/scsi/mvsas/mv_init.c b/drivers/scsi/mvsas/mv_init.c
> > index 3ac3437..495bddb 100644
> > --- a/drivers/scsi/mvsas/mv_init.c
> > +++ b/drivers/scsi/mvsas/mv_init.c
> > @@ -253,33 +253,29 @@ static int mvs_alloc(struct mvs_info *mvi, struct Scsi_Host *shost)
> > /*
> > * alloc and init our DMA areas
> > */
> > - mvi->tx = dma_alloc_coherent(mvi->dev,
> > + mvi->tx = dma_zalloc_coherent(mvi->dev,
> > sizeof(*mvi->tx) * MVS_CHIP_SLOT_SZ,
> > &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_zalloc_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,
> > + mvi->rx = dma_zalloc_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,
> > + mvi->slot = dma_zalloc_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,
> > --
> > 2.7.4
> >

2019-01-04 14:01:42

by Sabyasachi Gupta

Subject: Re: [PATCH] scsi/mvsas/mv_init.c: Use dma_zalloc_coherent

On Wed, Dec 19, 2018 at 6:49 PM Sabyasachi Gupta
<[email protected]> wrote:
>
> On Sat, Dec 1, 2018 at 6:40 PM Sabyasachi Gupta
> <[email protected]> wrote:
> >
> > On Wed, Nov 21, 2018 at 7:18 PM Sabyasachi Gupta
> > <[email protected]> wrote:
> > >
> > > Replace dma_alloc_coherent + memset with dma_zalloc_coherent
> > >
> > > Signed-off-by: Sabyasachi Gupta <[email protected]>
> >
> > Any comment on this patch?
>
> Any comment on this patch?

Any comment on this patch?

>
> >
> > > ---
> > > drivers/scsi/mvsas/mv_init.c | 12 ++++--------
> > > 1 file changed, 4 insertions(+), 8 deletions(-)
> > >
> > > diff --git a/drivers/scsi/mvsas/mv_init.c b/drivers/scsi/mvsas/mv_init.c
> > > index 3ac3437..495bddb 100644
> > > --- a/drivers/scsi/mvsas/mv_init.c
> > > +++ b/drivers/scsi/mvsas/mv_init.c
> > > @@ -253,33 +253,29 @@ static int mvs_alloc(struct mvs_info *mvi, struct Scsi_Host *shost)
> > > /*
> > > * alloc and init our DMA areas
> > > */
> > > - mvi->tx = dma_alloc_coherent(mvi->dev,
> > > + mvi->tx = dma_zalloc_coherent(mvi->dev,
> > > sizeof(*mvi->tx) * MVS_CHIP_SLOT_SZ,
> > > &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_zalloc_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,
> > > + mvi->rx = dma_zalloc_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,
> > > + mvi->slot = dma_zalloc_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,
> > > --
> > > 2.7.4
> > >

2019-01-04 17:22:46

by John Garry

Subject: Re: [PATCH] scsi/mvsas/mv_init.c: Use dma_zalloc_coherent

On 04/01/2019 12:48, Sabyasachi Gupta wrote:
> On Wed, Dec 19, 2018 at 6:49 PM Sabyasachi Gupta
> <[email protected]> wrote:
>>
>> On Sat, Dec 1, 2018 at 6:40 PM Sabyasachi Gupta
>> <[email protected]> wrote:
>>>
>>> On Wed, Nov 21, 2018 at 7:18 PM Sabyasachi Gupta
>>> <[email protected]> wrote:
>>>>
>>>> Replace dma_alloc_coherent + memset with dma_zalloc_coherent
>>>>

If you're going to make this change, then how about change these to the
managed version, so that we can avoid the explicit free'ing at driver
removal?

>>>> Signed-off-by: Sabyasachi Gupta <[email protected]>
>>>
>>> Any comment on this patch?
>>
>> Any comment on this patch?
>
> Any comment on this patch?
>
>>
>>>
>>>> ---
>>>> drivers/scsi/mvsas/mv_init.c | 12 ++++--------
>>>> 1 file changed, 4 insertions(+), 8 deletions(-)
>>>>
>>>> diff --git a/drivers/scsi/mvsas/mv_init.c b/drivers/scsi/mvsas/mv_init.c
>>>> index 3ac3437..495bddb 100644
>>>> --- a/drivers/scsi/mvsas/mv_init.c
>>>> +++ b/drivers/scsi/mvsas/mv_init.c
>>>> @@ -253,33 +253,29 @@ static int mvs_alloc(struct mvs_info *mvi, struct Scsi_Host *shost)
>>>> /*
>>>> * alloc and init our DMA areas
>>>> */
>>>> - mvi->tx = dma_alloc_coherent(mvi->dev,
>>>> + mvi->tx = dma_zalloc_coherent(mvi->dev,
>>>> sizeof(*mvi->tx) * MVS_CHIP_SLOT_SZ,
>>>> &mvi->tx_dma, GFP_KERNEL);

I'm guessing that this does not pass checkpatch with --strict option.

Thanks,
John

>>>> 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_zalloc_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,
>>>> + mvi->rx = dma_zalloc_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,
>>>> + mvi->slot = dma_zalloc_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,
>>>> --
>>>> 2.7.4
>>>>
>
> .
>

2019-01-04 17:25:19

by Sabyasachi Gupta

Subject: Re: [PATCH] scsi/mvsas/mv_init.c: Use dma_zalloc_coherent

On Fri, Jan 4, 2019 at 6:43 PM John Garry <[email protected]> wrote:
>
> On 04/01/2019 12:48, Sabyasachi Gupta wrote:
> > On Wed, Dec 19, 2018 at 6:49 PM Sabyasachi Gupta
> > <[email protected]> wrote:
> >>
> >> On Sat, Dec 1, 2018 at 6:40 PM Sabyasachi Gupta
> >> <[email protected]> wrote:
> >>>
> >>> On Wed, Nov 21, 2018 at 7:18 PM Sabyasachi Gupta
> >>> <[email protected]> wrote:
> >>>>
> >>>> Replace dma_alloc_coherent + memset with dma_zalloc_coherent
> >>>>
>
> If you're going to make this change, then how about change these to the
> managed version, so that we can avoid the explicit free'ing at driver
> removal?

I can't get it

>
> >>>> Signed-off-by: Sabyasachi Gupta <[email protected]>
> >>>
> >>> Any comment on this patch?
> >>
> >> Any comment on this patch?
> >
> > Any comment on this patch?
> >
> >>
> >>>
> >>>> ---
> >>>> drivers/scsi/mvsas/mv_init.c | 12 ++++--------
> >>>> 1 file changed, 4 insertions(+), 8 deletions(-)
> >>>>
> >>>> diff --git a/drivers/scsi/mvsas/mv_init.c b/drivers/scsi/mvsas/mv_init.c
> >>>> index 3ac3437..495bddb 100644
> >>>> --- a/drivers/scsi/mvsas/mv_init.c
> >>>> +++ b/drivers/scsi/mvsas/mv_init.c
> >>>> @@ -253,33 +253,29 @@ static int mvs_alloc(struct mvs_info *mvi, struct Scsi_Host *shost)
> >>>> /*
> >>>> * alloc and init our DMA areas
> >>>> */
> >>>> - mvi->tx = dma_alloc_coherent(mvi->dev,
> >>>> + mvi->tx = dma_zalloc_coherent(mvi->dev,
> >>>> sizeof(*mvi->tx) * MVS_CHIP_SLOT_SZ,
> >>>> &mvi->tx_dma, GFP_KERNEL);
>
> I'm guessing that this does not pass checkpatch with --strict option.
>
> Thanks,
> John

I have not not checked with --strict option

>
> >>>> 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_zalloc_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,
> >>>> + mvi->rx = dma_zalloc_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,
> >>>> + mvi->slot = dma_zalloc_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,
> >>>> --
> >>>> 2.7.4
> >>>>
> >
> > .
> >
>
>

2019-01-04 17:44:13

by John Garry

Subject: Re: [PATCH] scsi/mvsas/mv_init.c: Use dma_zalloc_coherent

On 04/01/2019 15:11, Sabyasachi Gupta wrote:
> On Fri, Jan 4, 2019 at 6:43 PM John Garry <[email protected]> wrote:
>>
>> On 04/01/2019 12:48, Sabyasachi Gupta wrote:
>>> On Wed, Dec 19, 2018 at 6:49 PM Sabyasachi Gupta
>>> <[email protected]> wrote:
>>>>
>>>> On Sat, Dec 1, 2018 at 6:40 PM Sabyasachi Gupta
>>>> <[email protected]> wrote:
>>>>>
>>>>> On Wed, Nov 21, 2018 at 7:18 PM Sabyasachi Gupta
>>>>> <[email protected]> wrote:
>>>>>>
>>>>>> Replace dma_alloc_coherent + memset with dma_zalloc_coherent
>>>>>>
>>
>> If you're going to make this change, then how about change these to the
>> managed version, so that we can avoid the explicit free'ing at driver
>> removal?
>
> I can't get it

Please see dmam_alloc_coherent(). You can set __GFP_ZERO in the gfp
argument to memset(0).

I would say that this is a more useful change.

>
>>
>>>>>> Signed-off-by: Sabyasachi Gupta <[email protected]>
>>>>>
>>>>> Any comment on this patch?
>>>>
>>>> Any comment on this patch?
>>>
>>> Any comment on this patch?
>>>
>>>>
>>>>>
>>>>>> ---
>>>>>> drivers/scsi/mvsas/mv_init.c | 12 ++++--------
>>>>>> 1 file changed, 4 insertions(+), 8 deletions(-)
>>>>>>
>>>>>> diff --git a/drivers/scsi/mvsas/mv_init.c b/drivers/scsi/mvsas/mv_init.c
>>>>>> index 3ac3437..495bddb 100644
>>>>>> --- a/drivers/scsi/mvsas/mv_init.c
>>>>>> +++ b/drivers/scsi/mvsas/mv_init.c
>>>>>> @@ -253,33 +253,29 @@ static int mvs_alloc(struct mvs_info *mvi, struct Scsi_Host *shost)
>>>>>> /*
>>>>>> * alloc and init our DMA areas
>>>>>> */
>>>>>> - mvi->tx = dma_alloc_coherent(mvi->dev,
>>>>>> + mvi->tx = dma_zalloc_coherent(mvi->dev,
>>>>>> sizeof(*mvi->tx) * MVS_CHIP_SLOT_SZ,
>>>>>> &mvi->tx_dma, GFP_KERNEL);
>>
>> I'm guessing that this does not pass checkpatch with --strict option.
>>
>> Thanks,
>> John
>
> I have not not checked with --strict option

It may warn that you're not maintaining alignment.

>
>>
>>>>>> 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_zalloc_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,
>>>>>> + mvi->rx = dma_zalloc_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,
>>>>>> + mvi->slot = dma_zalloc_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,
>>>>>> --
>>>>>> 2.7.4
>>>>>>
>>>
>>> .
>>>
>>
>>
>
> .
>

2019-01-04 19:47:21

by Souptick Joarder

Subject: Re: [PATCH] scsi/mvsas/mv_init.c: Use dma_zalloc_coherent

On Fri, Jan 4, 2019 at 9:34 PM John Garry <[email protected]> wrote:
>
> On 04/01/2019 15:11, Sabyasachi Gupta wrote:
> > On Fri, Jan 4, 2019 at 6:43 PM John Garry <[email protected]> wrote:
> >>
> >> On 04/01/2019 12:48, Sabyasachi Gupta wrote:
> >>> On Wed, Dec 19, 2018 at 6:49 PM Sabyasachi Gupta
> >>> <[email protected]> wrote:
> >>>>
> >>>> On Sat, Dec 1, 2018 at 6:40 PM Sabyasachi Gupta
> >>>> <[email protected]> wrote:
> >>>>>
> >>>>> On Wed, Nov 21, 2018 at 7:18 PM Sabyasachi Gupta
> >>>>> <[email protected]> wrote:
> >>>>>>
> >>>>>> Replace dma_alloc_coherent + memset with dma_zalloc_coherent
> >>>>>>
> >>
> >> If you're going to make this change, then how about change these to the
> >> managed version, so that we can avoid the explicit free'ing at driver
> >> removal?
> >
> > I can't get it
>
> Please see dmam_alloc_coherent(). You can set __GFP_ZERO in the gfp
> argument to memset(0).

and dma_zalloc_coherent() did the same. So both are same right ? No ?

>
> I would say that this is a more useful change.
>
> >
> >>
> >>>>>> Signed-off-by: Sabyasachi Gupta <[email protected]>
> >>>>>
> >>>>> Any comment on this patch?
> >>>>
> >>>> Any comment on this patch?
> >>>
> >>> Any comment on this patch?
> >>>
> >>>>
> >>>>>
> >>>>>> ---
> >>>>>> drivers/scsi/mvsas/mv_init.c | 12 ++++--------
> >>>>>> 1 file changed, 4 insertions(+), 8 deletions(-)
> >>>>>>
> >>>>>> diff --git a/drivers/scsi/mvsas/mv_init.c b/drivers/scsi/mvsas/mv_init.c
> >>>>>> index 3ac3437..495bddb 100644
> >>>>>> --- a/drivers/scsi/mvsas/mv_init.c
> >>>>>> +++ b/drivers/scsi/mvsas/mv_init.c
> >>>>>> @@ -253,33 +253,29 @@ static int mvs_alloc(struct mvs_info *mvi, struct Scsi_Host *shost)
> >>>>>> /*
> >>>>>> * alloc and init our DMA areas
> >>>>>> */
> >>>>>> - mvi->tx = dma_alloc_coherent(mvi->dev,
> >>>>>> + mvi->tx = dma_zalloc_coherent(mvi->dev,
> >>>>>> sizeof(*mvi->tx) * MVS_CHIP_SLOT_SZ,
> >>>>>> &mvi->tx_dma, GFP_KERNEL);
> >>
> >> I'm guessing that this does not pass checkpatch with --strict option.
> >>
> >> Thanks,
> >> John
> >
> > I have not not checked with --strict option
>
> It may warn that you're not maintaining alignment.
>
> >
> >>
> >>>>>> 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_zalloc_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,
> >>>>>> + mvi->rx = dma_zalloc_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,
> >>>>>> + mvi->slot = dma_zalloc_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,
> >>>>>> --
> >>>>>> 2.7.4
> >>>>>>
> >>>
> >>> .
> >>>
> >>
> >>
> >
> > .
> >
>
>

2019-01-07 16:32:35

by John Garry

Subject: Re: [PATCH] scsi/mvsas/mv_init.c: Use dma_zalloc_coherent

On 04/01/2019 19:49, Souptick Joarder wrote:
> On Fri, Jan 4, 2019 at 9:34 PM John Garry <[email protected]> wrote:
>>
>> On 04/01/2019 15:11, Sabyasachi Gupta wrote:
>>> On Fri, Jan 4, 2019 at 6:43 PM John Garry <[email protected]> wrote:
>>>>
>>>> On 04/01/2019 12:48, Sabyasachi Gupta wrote:
>>>>> On Wed, Dec 19, 2018 at 6:49 PM Sabyasachi Gupta
>>>>> <[email protected]> wrote:
>>>>>>
>>>>>> On Sat, Dec 1, 2018 at 6:40 PM Sabyasachi Gupta
>>>>>> <[email protected]> wrote:
>>>>>>>
>>>>>>> On Wed, Nov 21, 2018 at 7:18 PM Sabyasachi Gupta
>>>>>>> <[email protected]> wrote:
>>>>>>>>
>>>>>>>> Replace dma_alloc_coherent + memset with dma_zalloc_coherent
>>>>>>>>
>>>>
>>>> If you're going to make this change, then how about change these to the
>>>> managed version, so that we can avoid the explicit free'ing at driver
>>>> removal?
>>>
>>> I can't get it
>>
>> Please see dmam_alloc_coherent(). You can set __GFP_ZERO in the gfp
>> argument to memset(0).
>
> and dma_zalloc_coherent() did the same. So both are same right ? No ?
>

Firstly I would to say that I am not so keen on changes like this since
(I assume) it will not be tested at all.

However, having said that, the reason to use dmam_alloc_coherent() is
that you can also drop the dma_free_coherent() in mvs_free(). But
changes like that would actually require some code analysis effort...

John

>>
>> I would say that this is a more useful change.
>>
>>>
>>>>
>>>>>>>> Signed-off-by: Sabyasachi Gupta <[email protected]>
>>>>>>>
>>>>>>> Any comment on this patch?
>>>>>>
>>>>>> Any comment on this patch?
>>>>>
>>>>> Any comment on this patch?
>>>>>
>>>>>>
>>>>>>>
>>>>>>>> ---
>>>>>>>> drivers/scsi/mvsas/mv_init.c | 12 ++++--------
>>>>>>>> 1 file changed, 4 insertions(+), 8 deletions(-)
>>>>>>>>
>>>>>>>> diff --git a/drivers/scsi/mvsas/mv_init.c b/drivers/scsi/mvsas/mv_init.c
>>>>>>>> index 3ac3437..495bddb 100644
>>>>>>>> --- a/drivers/scsi/mvsas/mv_init.c
>>>>>>>> +++ b/drivers/scsi/mvsas/mv_init.c
>>>>>>>> @@ -253,33 +253,29 @@ static int mvs_alloc(struct mvs_info *mvi, struct Scsi_Host *shost)
>>>>>>>> /*
>>>>>>>> * alloc and init our DMA areas
>>>>>>>> */
>>>>>>>> - mvi->tx = dma_alloc_coherent(mvi->dev,
>>>>>>>> + mvi->tx = dma_zalloc_coherent(mvi->dev,
>>>>>>>> sizeof(*mvi->tx) * MVS_CHIP_SLOT_SZ,
>>>>>>>> &mvi->tx_dma, GFP_KERNEL);
>>>>
>>>> I'm guessing that this does not pass checkpatch with --strict option.
>>>>
>>>> Thanks,
>>>> John
>>>
>>> I have not not checked with --strict option
>>
>> It may warn that you're not maintaining alignment.
>>
>>>
>>>>
>>>>>>>> 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_zalloc_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,
>>>>>>>> + mvi->rx = dma_zalloc_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,
>>>>>>>> + mvi->slot = dma_zalloc_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,
>>>>>>>> --
>>>>>>>> 2.7.4
>>>>>>>>
>>>>>
>>>>> .
>>>>>
>>>>
>>>>
>>>
>>> .
>>>
>>
>>
>
> .
>