Re: [PATCH v2 1/2] media: staging/intel-ipu3: Fix memory leak in imu_fmt

2021-04-06 Thread Ricardo Ribalda 
Hi Bingbu


Maybe you want to add your Reviewed-by ? ;)

Thanks!
On Wed, Mar 17, 2021 at 7:48 AM Bingbu Cao  wrote:
>
>
> On 3/17/21 1:50 AM, Ricardo Ribalda wrote:
> > Hi Bingbu
> >
> > Thanks for your review
> >
> > On Tue, Mar 16, 2021 at 12:29 PM Bingbu Cao  
> > wrote:
> >>
> >> Hi, Ricardo
> >>
> >> Thanks for your patch.
> >> It looks fine for me, do you mind squash 2 patchsets into 1 commit?
> >
> > Are you sure? There are two different issues that we are solving.
>
> Oh, I see. I thought you were fixing 1 issue here.
> Thanks!
>
> >
> > Best regards!
> >
> >>
> >> On 3/15/21 8:34 PM, Ricardo Ribalda wrote:
> >>> We are losing the reference to an allocated memory if try. Change the
> >>> order of the check to avoid that.
> >>>
> >>> Cc: sta...@vger.kernel.org
> >>> Fixes: 6d5f26f2e045 ("media: staging/intel-ipu3-v4l: reduce kernel stack 
> >>> usage")
> >>> Signed-off-by: Ricardo Ribalda 
> >>> ---
> >>>  drivers/staging/media/ipu3/ipu3-v4l2.c | 11 +++
> >>>  1 file changed, 7 insertions(+), 4 deletions(-)
> >>>
> >>> diff --git a/drivers/staging/media/ipu3/ipu3-v4l2.c 
> >>> b/drivers/staging/media/ipu3/ipu3-v4l2.c
> >>> index 60aa02eb7d2a..35a74d99322f 100644
> >>> --- a/drivers/staging/media/ipu3/ipu3-v4l2.c
> >>> +++ b/drivers/staging/media/ipu3/ipu3-v4l2.c
> >>> @@ -693,6 +693,13 @@ static int imgu_fmt(struct imgu_device *imgu, 
> >>> unsigned int pipe, int node,
> >>>   if (inode == IMGU_NODE_STAT_3A || inode == IMGU_NODE_PARAMS)
> >>>   continue;
> >>>
> >>> + /* CSS expects some format on OUT queue */
> >>> + if (i != IPU3_CSS_QUEUE_OUT &&
> >>> + !imgu_pipe->nodes[inode].enabled) {
> >>> + fmts[i] = NULL;
> >>> + continue;
> >>> + }
> >>> +
> >>>   if (try) {
> >>>   fmts[i] = 
> >>> kmemdup(_pipe->nodes[inode].vdev_fmt.fmt.pix_mp,
> >>> sizeof(struct 
> >>> v4l2_pix_format_mplane),
> >>> @@ -705,10 +712,6 @@ static int imgu_fmt(struct imgu_device *imgu, 
> >>> unsigned int pipe, int node,
> >>>   fmts[i] = 
> >>> _pipe->nodes[inode].vdev_fmt.fmt.pix_mp;
> >>>   }
> >>>
> >>> - /* CSS expects some format on OUT queue */
> >>> - if (i != IPU3_CSS_QUEUE_OUT &&
> >>> - !imgu_pipe->nodes[inode].enabled)
> >>> - fmts[i] = NULL;
> >>>   }
> >>>
> >>>   if (!try) {
> >>>
> >>
> >> --
> >> Best regards,
> >> Bingbu Cao
> >
> >
> >
>
> --
> Best regards,
> Bingbu Cao



-- 
Ricardo Ribalda

Re: [PATCH v2 1/2] media: staging/intel-ipu3: Fix memory leak in imu_fmt

2021-03-17 Thread Bingbu Cao 


On 3/17/21 1:50 AM, Ricardo Ribalda wrote:
> Hi Bingbu
> 
> Thanks for your review
> 
> On Tue, Mar 16, 2021 at 12:29 PM Bingbu Cao  
> wrote:
>>
>> Hi, Ricardo
>>
>> Thanks for your patch.
>> It looks fine for me, do you mind squash 2 patchsets into 1 commit?
> 
> Are you sure? There are two different issues that we are solving.

Oh, I see. I thought you were fixing 1 issue here.
Thanks!

> 
> Best regards!
> 
>>
>> On 3/15/21 8:34 PM, Ricardo Ribalda wrote:
>>> We are losing the reference to an allocated memory if try. Change the
>>> order of the check to avoid that.
>>>
>>> Cc: sta...@vger.kernel.org
>>> Fixes: 6d5f26f2e045 ("media: staging/intel-ipu3-v4l: reduce kernel stack 
>>> usage")
>>> Signed-off-by: Ricardo Ribalda 
>>> ---
>>>  drivers/staging/media/ipu3/ipu3-v4l2.c | 11 +++
>>>  1 file changed, 7 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/drivers/staging/media/ipu3/ipu3-v4l2.c 
>>> b/drivers/staging/media/ipu3/ipu3-v4l2.c
>>> index 60aa02eb7d2a..35a74d99322f 100644
>>> --- a/drivers/staging/media/ipu3/ipu3-v4l2.c
>>> +++ b/drivers/staging/media/ipu3/ipu3-v4l2.c
>>> @@ -693,6 +693,13 @@ static int imgu_fmt(struct imgu_device *imgu, unsigned 
>>> int pipe, int node,
>>>   if (inode == IMGU_NODE_STAT_3A || inode == IMGU_NODE_PARAMS)
>>>   continue;
>>>
>>> + /* CSS expects some format on OUT queue */
>>> + if (i != IPU3_CSS_QUEUE_OUT &&
>>> + !imgu_pipe->nodes[inode].enabled) {
>>> + fmts[i] = NULL;
>>> + continue;
>>> + }
>>> +
>>>   if (try) {
>>>   fmts[i] = 
>>> kmemdup(_pipe->nodes[inode].vdev_fmt.fmt.pix_mp,
>>> sizeof(struct 
>>> v4l2_pix_format_mplane),
>>> @@ -705,10 +712,6 @@ static int imgu_fmt(struct imgu_device *imgu, unsigned 
>>> int pipe, int node,
>>>   fmts[i] = 
>>> _pipe->nodes[inode].vdev_fmt.fmt.pix_mp;
>>>   }
>>>
>>> - /* CSS expects some format on OUT queue */
>>> - if (i != IPU3_CSS_QUEUE_OUT &&
>>> - !imgu_pipe->nodes[inode].enabled)
>>> - fmts[i] = NULL;
>>>   }
>>>
>>>   if (!try) {
>>>
>>
>> --
>> Best regards,
>> Bingbu Cao
> 
> 
> 

-- 
Best regards,
Bingbu Cao

Re: [PATCH v2 1/2] media: staging/intel-ipu3: Fix memory leak in imu_fmt

2021-03-16 Thread Ricardo Ribalda 
Hi Bingbu

Thanks for your review

On Tue, Mar 16, 2021 at 12:29 PM Bingbu Cao  wrote:
>
> Hi, Ricardo
>
> Thanks for your patch.
> It looks fine for me, do you mind squash 2 patchsets into 1 commit?

Are you sure? There are two different issues that we are solving.

Best regards!

>
> On 3/15/21 8:34 PM, Ricardo Ribalda wrote:
> > We are losing the reference to an allocated memory if try. Change the
> > order of the check to avoid that.
> >
> > Cc: sta...@vger.kernel.org
> > Fixes: 6d5f26f2e045 ("media: staging/intel-ipu3-v4l: reduce kernel stack 
> > usage")
> > Signed-off-by: Ricardo Ribalda 
> > ---
> >  drivers/staging/media/ipu3/ipu3-v4l2.c | 11 +++
> >  1 file changed, 7 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/staging/media/ipu3/ipu3-v4l2.c 
> > b/drivers/staging/media/ipu3/ipu3-v4l2.c
> > index 60aa02eb7d2a..35a74d99322f 100644
> > --- a/drivers/staging/media/ipu3/ipu3-v4l2.c
> > +++ b/drivers/staging/media/ipu3/ipu3-v4l2.c
> > @@ -693,6 +693,13 @@ static int imgu_fmt(struct imgu_device *imgu, unsigned 
> > int pipe, int node,
> >   if (inode == IMGU_NODE_STAT_3A || inode == IMGU_NODE_PARAMS)
> >   continue;
> >
> > + /* CSS expects some format on OUT queue */
> > + if (i != IPU3_CSS_QUEUE_OUT &&
> > + !imgu_pipe->nodes[inode].enabled) {
> > + fmts[i] = NULL;
> > + continue;
> > + }
> > +
> >   if (try) {
> >   fmts[i] = 
> > kmemdup(_pipe->nodes[inode].vdev_fmt.fmt.pix_mp,
> > sizeof(struct 
> > v4l2_pix_format_mplane),
> > @@ -705,10 +712,6 @@ static int imgu_fmt(struct imgu_device *imgu, unsigned 
> > int pipe, int node,
> >   fmts[i] = 
> > _pipe->nodes[inode].vdev_fmt.fmt.pix_mp;
> >   }
> >
> > - /* CSS expects some format on OUT queue */
> > - if (i != IPU3_CSS_QUEUE_OUT &&
> > - !imgu_pipe->nodes[inode].enabled)
> > - fmts[i] = NULL;
> >   }
> >
> >   if (!try) {
> >
>
> --
> Best regards,
> Bingbu Cao



-- 
Ricardo Ribalda

Re: [PATCH v2 1/2] media: staging/intel-ipu3: Fix memory leak in imu_fmt

2021-03-16 Thread Bingbu Cao 
Hi, Ricardo

Thanks for your patch.
It looks fine for me, do you mind squash 2 patchsets into 1 commit?

On 3/15/21 8:34 PM, Ricardo Ribalda wrote:
> We are losing the reference to an allocated memory if try. Change the
> order of the check to avoid that.
> 
> Cc: sta...@vger.kernel.org
> Fixes: 6d5f26f2e045 ("media: staging/intel-ipu3-v4l: reduce kernel stack 
> usage")
> Signed-off-by: Ricardo Ribalda 
> ---
>  drivers/staging/media/ipu3/ipu3-v4l2.c | 11 +++
>  1 file changed, 7 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/staging/media/ipu3/ipu3-v4l2.c 
> b/drivers/staging/media/ipu3/ipu3-v4l2.c
> index 60aa02eb7d2a..35a74d99322f 100644
> --- a/drivers/staging/media/ipu3/ipu3-v4l2.c
> +++ b/drivers/staging/media/ipu3/ipu3-v4l2.c
> @@ -693,6 +693,13 @@ static int imgu_fmt(struct imgu_device *imgu, unsigned 
> int pipe, int node,
>   if (inode == IMGU_NODE_STAT_3A || inode == IMGU_NODE_PARAMS)
>   continue;
>  
> + /* CSS expects some format on OUT queue */
> + if (i != IPU3_CSS_QUEUE_OUT &&
> + !imgu_pipe->nodes[inode].enabled) {
> + fmts[i] = NULL;
> + continue;
> + }
> +
>   if (try) {
>   fmts[i] = 
> kmemdup(_pipe->nodes[inode].vdev_fmt.fmt.pix_mp,
> sizeof(struct v4l2_pix_format_mplane),
> @@ -705,10 +712,6 @@ static int imgu_fmt(struct imgu_device *imgu, unsigned 
> int pipe, int node,
>   fmts[i] = _pipe->nodes[inode].vdev_fmt.fmt.pix_mp;
>   }
>  
> - /* CSS expects some format on OUT queue */
> - if (i != IPU3_CSS_QUEUE_OUT &&
> - !imgu_pipe->nodes[inode].enabled)
> - fmts[i] = NULL;
>   }
>  
>   if (!try) {
> 

-- 
Best regards,
Bingbu Cao

[PATCH v2 1/2] media: staging/intel-ipu3: Fix memory leak in imu_fmt

2021-03-15 Thread Ricardo Ribalda 
We are losing the reference to an allocated memory if try. Change the
order of the check to avoid that.

Cc: sta...@vger.kernel.org
Fixes: 6d5f26f2e045 ("media: staging/intel-ipu3-v4l: reduce kernel stack usage")
Signed-off-by: Ricardo Ribalda 
---
 drivers/staging/media/ipu3/ipu3-v4l2.c | 11 +++
 1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/drivers/staging/media/ipu3/ipu3-v4l2.c 
b/drivers/staging/media/ipu3/ipu3-v4l2.c
index 60aa02eb7d2a..35a74d99322f 100644
--- a/drivers/staging/media/ipu3/ipu3-v4l2.c
+++ b/drivers/staging/media/ipu3/ipu3-v4l2.c
@@ -693,6 +693,13 @@ static int imgu_fmt(struct imgu_device *imgu, unsigned int 
pipe, int node,
if (inode == IMGU_NODE_STAT_3A || inode == IMGU_NODE_PARAMS)
continue;
 
+   /* CSS expects some format on OUT queue */
+   if (i != IPU3_CSS_QUEUE_OUT &&
+   !imgu_pipe->nodes[inode].enabled) {
+   fmts[i] = NULL;
+   continue;
+   }
+
if (try) {
fmts[i] = 
kmemdup(_pipe->nodes[inode].vdev_fmt.fmt.pix_mp,
  sizeof(struct v4l2_pix_format_mplane),
@@ -705,10 +712,6 @@ static int imgu_fmt(struct imgu_device *imgu, unsigned int 
pipe, int node,
fmts[i] = _pipe->nodes[inode].vdev_fmt.fmt.pix_mp;
}
 
-   /* CSS expects some format on OUT queue */
-   if (i != IPU3_CSS_QUEUE_OUT &&
-   !imgu_pipe->nodes[inode].enabled)
-   fmts[i] = NULL;
}
 
if (!try) {
-- 
2.31.0.rc2.261.g7f71774620-goog