linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] scsi/mvsas/mv_init.c: Use dma_zalloc_coherent
@ 2018-11-21 13:48 Sabyasachi Gupta
  2018-12-01 13:10 ` Sabyasachi Gupta
  0 siblings, 1 reply; 9+ messages in thread
From: Sabyasachi Gupta @ 2018-11-21 13:48 UTC (permalink / raw)
  To: jejb, martin.petersen, jthumshirn
  Cc: jrdr.linux, linux-scsi, linux-kernel, brajeswar.linux

Replace dma_alloc_coherent + memset with dma_zalloc_coherent

Signed-off-by: Sabyasachi Gupta <sabyasachi.linux@gmail.com>
---
 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


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [PATCH] scsi/mvsas/mv_init.c: Use dma_zalloc_coherent
  2018-11-21 13:48 [PATCH] scsi/mvsas/mv_init.c: Use dma_zalloc_coherent Sabyasachi Gupta
@ 2018-12-01 13:10 ` Sabyasachi Gupta
  2018-12-19 13:19   ` Sabyasachi Gupta
  0 siblings, 1 reply; 9+ messages in thread
From: Sabyasachi Gupta @ 2018-12-01 13:10 UTC (permalink / raw)
  To: jejb, martin.petersen, jthumshirn
  Cc: Souptick Joarder, linux-scsi, linux-kernel, Brajeswar Ghosh

On Wed, Nov 21, 2018 at 7:18 PM Sabyasachi Gupta
<sabyasachi.linux@gmail.com> wrote:
>
> Replace dma_alloc_coherent + memset with dma_zalloc_coherent
>
> Signed-off-by: Sabyasachi Gupta <sabyasachi.linux@gmail.com>

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
>

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] scsi/mvsas/mv_init.c: Use dma_zalloc_coherent
  2018-12-01 13:10 ` Sabyasachi Gupta
@ 2018-12-19 13:19   ` Sabyasachi Gupta
  2019-01-04 12:48     ` Sabyasachi Gupta
  0 siblings, 1 reply; 9+ messages in thread
From: Sabyasachi Gupta @ 2018-12-19 13:19 UTC (permalink / raw)
  To: jejb, martin.petersen, jthumshirn
  Cc: Souptick Joarder, linux-scsi, linux-kernel, Brajeswar Ghosh

On Sat, Dec 1, 2018 at 6:40 PM Sabyasachi Gupta
<sabyasachi.linux@gmail.com> wrote:
>
> On Wed, Nov 21, 2018 at 7:18 PM Sabyasachi Gupta
> <sabyasachi.linux@gmail.com> wrote:
> >
> > Replace dma_alloc_coherent + memset with dma_zalloc_coherent
> >
> > Signed-off-by: Sabyasachi Gupta <sabyasachi.linux@gmail.com>
>
> 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
> >

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] scsi/mvsas/mv_init.c: Use dma_zalloc_coherent
  2018-12-19 13:19   ` Sabyasachi Gupta
@ 2019-01-04 12:48     ` Sabyasachi Gupta
  2019-01-04 13:13       ` John Garry
  0 siblings, 1 reply; 9+ messages in thread
From: Sabyasachi Gupta @ 2019-01-04 12:48 UTC (permalink / raw)
  To: jejb, martin.petersen, jthumshirn
  Cc: Souptick Joarder, linux-scsi, linux-kernel, Brajeswar Ghosh

On Wed, Dec 19, 2018 at 6:49 PM Sabyasachi Gupta
<sabyasachi.linux@gmail.com> wrote:
>
> On Sat, Dec 1, 2018 at 6:40 PM Sabyasachi Gupta
> <sabyasachi.linux@gmail.com> wrote:
> >
> > On Wed, Nov 21, 2018 at 7:18 PM Sabyasachi Gupta
> > <sabyasachi.linux@gmail.com> wrote:
> > >
> > > Replace dma_alloc_coherent + memset with dma_zalloc_coherent
> > >
> > > Signed-off-by: Sabyasachi Gupta <sabyasachi.linux@gmail.com>
> >
> > 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
> > >

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] scsi/mvsas/mv_init.c: Use dma_zalloc_coherent
  2019-01-04 12:48     ` Sabyasachi Gupta
@ 2019-01-04 13:13       ` John Garry
  2019-01-04 15:11         ` Sabyasachi Gupta
  0 siblings, 1 reply; 9+ messages in thread
From: John Garry @ 2019-01-04 13:13 UTC (permalink / raw)
  To: Sabyasachi Gupta, jejb, martin.petersen, jthumshirn
  Cc: Souptick Joarder, linux-scsi, linux-kernel, Brajeswar Ghosh

On 04/01/2019 12:48, Sabyasachi Gupta wrote:
> On Wed, Dec 19, 2018 at 6:49 PM Sabyasachi Gupta
> <sabyasachi.linux@gmail.com> wrote:
>>
>> On Sat, Dec 1, 2018 at 6:40 PM Sabyasachi Gupta
>> <sabyasachi.linux@gmail.com> wrote:
>>>
>>> On Wed, Nov 21, 2018 at 7:18 PM Sabyasachi Gupta
>>> <sabyasachi.linux@gmail.com> 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 <sabyasachi.linux@gmail.com>
>>>
>>> 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
>>>>
>
> .
>



^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] scsi/mvsas/mv_init.c: Use dma_zalloc_coherent
  2019-01-04 13:13       ` John Garry
@ 2019-01-04 15:11         ` Sabyasachi Gupta
  2019-01-04 16:04           ` John Garry
  0 siblings, 1 reply; 9+ messages in thread
From: Sabyasachi Gupta @ 2019-01-04 15:11 UTC (permalink / raw)
  To: John Garry
  Cc: jejb, martin.petersen, jthumshirn, Souptick Joarder, linux-scsi,
	linux-kernel, Brajeswar Ghosh

On Fri, Jan 4, 2019 at 6:43 PM John Garry <john.garry@huawei.com> wrote:
>
> On 04/01/2019 12:48, Sabyasachi Gupta wrote:
> > On Wed, Dec 19, 2018 at 6:49 PM Sabyasachi Gupta
> > <sabyasachi.linux@gmail.com> wrote:
> >>
> >> On Sat, Dec 1, 2018 at 6:40 PM Sabyasachi Gupta
> >> <sabyasachi.linux@gmail.com> wrote:
> >>>
> >>> On Wed, Nov 21, 2018 at 7:18 PM Sabyasachi Gupta
> >>> <sabyasachi.linux@gmail.com> 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 <sabyasachi.linux@gmail.com>
> >>>
> >>> 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
> >>>>
> >
> > .
> >
>
>

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] scsi/mvsas/mv_init.c: Use dma_zalloc_coherent
  2019-01-04 15:11         ` Sabyasachi Gupta
@ 2019-01-04 16:04           ` John Garry
  2019-01-04 19:49             ` Souptick Joarder
  0 siblings, 1 reply; 9+ messages in thread
From: John Garry @ 2019-01-04 16:04 UTC (permalink / raw)
  To: Sabyasachi Gupta
  Cc: jejb, martin.petersen, jthumshirn, Souptick Joarder, linux-scsi,
	linux-kernel, Brajeswar Ghosh

On 04/01/2019 15:11, Sabyasachi Gupta wrote:
> On Fri, Jan 4, 2019 at 6:43 PM John Garry <john.garry@huawei.com> wrote:
>>
>> On 04/01/2019 12:48, Sabyasachi Gupta wrote:
>>> On Wed, Dec 19, 2018 at 6:49 PM Sabyasachi Gupta
>>> <sabyasachi.linux@gmail.com> wrote:
>>>>
>>>> On Sat, Dec 1, 2018 at 6:40 PM Sabyasachi Gupta
>>>> <sabyasachi.linux@gmail.com> wrote:
>>>>>
>>>>> On Wed, Nov 21, 2018 at 7:18 PM Sabyasachi Gupta
>>>>> <sabyasachi.linux@gmail.com> 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 <sabyasachi.linux@gmail.com>
>>>>>
>>>>> 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
>>>>>>
>>>
>>> .
>>>
>>
>>
>
> .
>



^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] scsi/mvsas/mv_init.c: Use dma_zalloc_coherent
  2019-01-04 16:04           ` John Garry
@ 2019-01-04 19:49             ` Souptick Joarder
  2019-01-07 13:50               ` John Garry
  0 siblings, 1 reply; 9+ messages in thread
From: Souptick Joarder @ 2019-01-04 19:49 UTC (permalink / raw)
  To: John Garry
  Cc: Sabyasachi Gupta, James Bottomley, Martin K. Petersen,
	Johannes Thumshirn, linux-scsi, linux-kernel, Brajeswar Ghosh

On Fri, Jan 4, 2019 at 9:34 PM John Garry <john.garry@huawei.com> wrote:
>
> On 04/01/2019 15:11, Sabyasachi Gupta wrote:
> > On Fri, Jan 4, 2019 at 6:43 PM John Garry <john.garry@huawei.com> wrote:
> >>
> >> On 04/01/2019 12:48, Sabyasachi Gupta wrote:
> >>> On Wed, Dec 19, 2018 at 6:49 PM Sabyasachi Gupta
> >>> <sabyasachi.linux@gmail.com> wrote:
> >>>>
> >>>> On Sat, Dec 1, 2018 at 6:40 PM Sabyasachi Gupta
> >>>> <sabyasachi.linux@gmail.com> wrote:
> >>>>>
> >>>>> On Wed, Nov 21, 2018 at 7:18 PM Sabyasachi Gupta
> >>>>> <sabyasachi.linux@gmail.com> 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 <sabyasachi.linux@gmail.com>
> >>>>>
> >>>>> 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
> >>>>>>
> >>>
> >>> .
> >>>
> >>
> >>
> >
> > .
> >
>
>

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] scsi/mvsas/mv_init.c: Use dma_zalloc_coherent
  2019-01-04 19:49             ` Souptick Joarder
@ 2019-01-07 13:50               ` John Garry
  0 siblings, 0 replies; 9+ messages in thread
From: John Garry @ 2019-01-07 13:50 UTC (permalink / raw)
  To: Souptick Joarder
  Cc: Sabyasachi Gupta, James Bottomley, Martin K. Petersen,
	Johannes Thumshirn, linux-scsi, linux-kernel, Brajeswar Ghosh

On 04/01/2019 19:49, Souptick Joarder wrote:
> On Fri, Jan 4, 2019 at 9:34 PM John Garry <john.garry@huawei.com> wrote:
>>
>> On 04/01/2019 15:11, Sabyasachi Gupta wrote:
>>> On Fri, Jan 4, 2019 at 6:43 PM John Garry <john.garry@huawei.com> wrote:
>>>>
>>>> On 04/01/2019 12:48, Sabyasachi Gupta wrote:
>>>>> On Wed, Dec 19, 2018 at 6:49 PM Sabyasachi Gupta
>>>>> <sabyasachi.linux@gmail.com> wrote:
>>>>>>
>>>>>> On Sat, Dec 1, 2018 at 6:40 PM Sabyasachi Gupta
>>>>>> <sabyasachi.linux@gmail.com> wrote:
>>>>>>>
>>>>>>> On Wed, Nov 21, 2018 at 7:18 PM Sabyasachi Gupta
>>>>>>> <sabyasachi.linux@gmail.com> 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 <sabyasachi.linux@gmail.com>
>>>>>>>
>>>>>>> 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
>>>>>>>>
>>>>>
>>>>> .
>>>>>
>>>>
>>>>
>>>
>>> .
>>>
>>
>>
>
> .
>



^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2019-01-07 13:50 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-11-21 13:48 [PATCH] scsi/mvsas/mv_init.c: Use dma_zalloc_coherent Sabyasachi Gupta
2018-12-01 13:10 ` Sabyasachi Gupta
2018-12-19 13:19   ` Sabyasachi Gupta
2019-01-04 12:48     ` Sabyasachi Gupta
2019-01-04 13:13       ` John Garry
2019-01-04 15:11         ` Sabyasachi Gupta
2019-01-04 16:04           ` John Garry
2019-01-04 19:49             ` Souptick Joarder
2019-01-07 13:50               ` John Garry

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).