Re: [PATCHv17 12/34] v4l2-ctrls: alloc memory for p_req

2018-08-10 Thread Hans Verkuil
On 08/09/2018 10:19 PM, Mauro Carvalho Chehab wrote:
> Em Sat,  4 Aug 2018 14:45:04 +0200
> Hans Verkuil  escreveu:
> 
>> From: Hans Verkuil 
>>
>> To store request data the handler_new_ref() allocates memory
>> for it if needed.
>>
>> Signed-off-by: Hans Verkuil 
>> ---
>>  drivers/media/v4l2-core/v4l2-ctrls.c | 20 
>>  1 file changed, 16 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/media/v4l2-core/v4l2-ctrls.c 
>> b/drivers/media/v4l2-core/v4l2-ctrls.c
>> index b33a8bee82b0..171ab389afdd 100644
>> --- a/drivers/media/v4l2-core/v4l2-ctrls.c
>> +++ b/drivers/media/v4l2-core/v4l2-ctrls.c
>> @@ -2018,13 +2018,18 @@ EXPORT_SYMBOL(v4l2_ctrl_find);
>>  /* Allocate a new v4l2_ctrl_ref and hook it into the handler. */
>>  static int handler_new_ref(struct v4l2_ctrl_handler *hdl,
>> struct v4l2_ctrl *ctrl,
>> -   bool from_other_dev)
>> +   struct v4l2_ctrl_ref **ctrl_ref,
>> +   bool from_other_dev, bool allocate_req)
>>  {
>>  struct v4l2_ctrl_ref *ref;
>>  struct v4l2_ctrl_ref *new_ref;
>>  u32 id = ctrl->id;
>>  u32 class_ctrl = V4L2_CTRL_ID2WHICH(id) | 1;
>>  int bucket = id % hdl->nr_of_buckets;   /* which bucket to use */
>> +unsigned int sz_extra = 0;
> 
> Nitpick: I would name it size_extra_req, to make clear its usage.

OK, makes sense.

Regards,

Hans

> Once renamed:
> 
> Reviewed-by: Mauro Carvalho Chehab 
> 
>> +
>> +if (ctrl_ref)
>> +*ctrl_ref = NULL;
>>  
>>  /*
>>   * Automatically add the control class if it is not yet present and
>> @@ -2038,11 +2043,16 @@ static int handler_new_ref(struct v4l2_ctrl_handler 
>> *hdl,
>>  if (hdl->error)
>>  return hdl->error;
>>  
>> -new_ref = kzalloc(sizeof(*new_ref), GFP_KERNEL);
>> +if (allocate_req)
>> +sz_extra = ctrl->elems * ctrl->elem_size;
>> +new_ref = kzalloc(sizeof(*new_ref) + sz_extra, GFP_KERNEL);
>>  if (!new_ref)
>>  return handler_set_err(hdl, -ENOMEM);
>>  new_ref->ctrl = ctrl;
>>  new_ref->from_other_dev = from_other_dev;
>> +if (sz_extra)
>> +new_ref->p_req.p = _ref[1];
>> +
>>  if (ctrl->handler == hdl) {
>>  /* By default each control starts in a cluster of its own.
>> new_ref->ctrl is basically a cluster array with one
>> @@ -2082,6 +2092,8 @@ static int handler_new_ref(struct v4l2_ctrl_handler 
>> *hdl,
>>  /* Insert the control node in the hash */
>>  new_ref->next = hdl->buckets[bucket];
>>  hdl->buckets[bucket] = new_ref;
>> +if (ctrl_ref)
>> +*ctrl_ref = new_ref;
>>  
>>  unlock:
>>  mutex_unlock(hdl->lock);
>> @@ -2223,7 +2235,7 @@ static struct v4l2_ctrl *v4l2_ctrl_new(struct 
>> v4l2_ctrl_handler *hdl,
>>  ctrl->type_ops->init(ctrl, idx, ctrl->p_new);
>>  }
>>  
>> -if (handler_new_ref(hdl, ctrl, false)) {
>> +if (handler_new_ref(hdl, ctrl, NULL, false, false)) {
>>  kvfree(ctrl);
>>  return NULL;
>>  }
>> @@ -2416,7 +2428,7 @@ int v4l2_ctrl_add_handler(struct v4l2_ctrl_handler 
>> *hdl,
>>  /* Filter any unwanted controls */
>>  if (filter && !filter(ctrl))
>>  continue;
>> -ret = handler_new_ref(hdl, ctrl, from_other_dev);
>> +ret = handler_new_ref(hdl, ctrl, NULL, from_other_dev, false);
>>  if (ret)
>>  break;
>>  }
> 
> 
> 
> Thanks,
> Mauro
> 



Re: [PATCHv17 12/34] v4l2-ctrls: alloc memory for p_req

2018-08-09 Thread Mauro Carvalho Chehab
Em Sat,  4 Aug 2018 14:45:04 +0200
Hans Verkuil  escreveu:

> From: Hans Verkuil 
> 
> To store request data the handler_new_ref() allocates memory
> for it if needed.
> 
> Signed-off-by: Hans Verkuil 
> ---
>  drivers/media/v4l2-core/v4l2-ctrls.c | 20 
>  1 file changed, 16 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/media/v4l2-core/v4l2-ctrls.c 
> b/drivers/media/v4l2-core/v4l2-ctrls.c
> index b33a8bee82b0..171ab389afdd 100644
> --- a/drivers/media/v4l2-core/v4l2-ctrls.c
> +++ b/drivers/media/v4l2-core/v4l2-ctrls.c
> @@ -2018,13 +2018,18 @@ EXPORT_SYMBOL(v4l2_ctrl_find);
>  /* Allocate a new v4l2_ctrl_ref and hook it into the handler. */
>  static int handler_new_ref(struct v4l2_ctrl_handler *hdl,
>  struct v4l2_ctrl *ctrl,
> -bool from_other_dev)
> +struct v4l2_ctrl_ref **ctrl_ref,
> +bool from_other_dev, bool allocate_req)
>  {
>   struct v4l2_ctrl_ref *ref;
>   struct v4l2_ctrl_ref *new_ref;
>   u32 id = ctrl->id;
>   u32 class_ctrl = V4L2_CTRL_ID2WHICH(id) | 1;
>   int bucket = id % hdl->nr_of_buckets;   /* which bucket to use */
> + unsigned int sz_extra = 0;

Nitpick: I would name it size_extra_req, to make clear its usage.

Once renamed:

Reviewed-by: Mauro Carvalho Chehab 

> +
> + if (ctrl_ref)
> + *ctrl_ref = NULL;
>  
>   /*
>* Automatically add the control class if it is not yet present and
> @@ -2038,11 +2043,16 @@ static int handler_new_ref(struct v4l2_ctrl_handler 
> *hdl,
>   if (hdl->error)
>   return hdl->error;
>  
> - new_ref = kzalloc(sizeof(*new_ref), GFP_KERNEL);
> + if (allocate_req)
> + sz_extra = ctrl->elems * ctrl->elem_size;
> + new_ref = kzalloc(sizeof(*new_ref) + sz_extra, GFP_KERNEL);
>   if (!new_ref)
>   return handler_set_err(hdl, -ENOMEM);
>   new_ref->ctrl = ctrl;
>   new_ref->from_other_dev = from_other_dev;
> + if (sz_extra)
> + new_ref->p_req.p = _ref[1];
> +
>   if (ctrl->handler == hdl) {
>   /* By default each control starts in a cluster of its own.
>  new_ref->ctrl is basically a cluster array with one
> @@ -2082,6 +2092,8 @@ static int handler_new_ref(struct v4l2_ctrl_handler 
> *hdl,
>   /* Insert the control node in the hash */
>   new_ref->next = hdl->buckets[bucket];
>   hdl->buckets[bucket] = new_ref;
> + if (ctrl_ref)
> + *ctrl_ref = new_ref;
>  
>  unlock:
>   mutex_unlock(hdl->lock);
> @@ -2223,7 +2235,7 @@ static struct v4l2_ctrl *v4l2_ctrl_new(struct 
> v4l2_ctrl_handler *hdl,
>   ctrl->type_ops->init(ctrl, idx, ctrl->p_new);
>   }
>  
> - if (handler_new_ref(hdl, ctrl, false)) {
> + if (handler_new_ref(hdl, ctrl, NULL, false, false)) {
>   kvfree(ctrl);
>   return NULL;
>   }
> @@ -2416,7 +2428,7 @@ int v4l2_ctrl_add_handler(struct v4l2_ctrl_handler *hdl,
>   /* Filter any unwanted controls */
>   if (filter && !filter(ctrl))
>   continue;
> - ret = handler_new_ref(hdl, ctrl, from_other_dev);
> + ret = handler_new_ref(hdl, ctrl, NULL, from_other_dev, false);
>   if (ret)
>   break;
>   }



Thanks,
Mauro


[PATCHv17 12/34] v4l2-ctrls: alloc memory for p_req

2018-08-04 Thread Hans Verkuil
From: Hans Verkuil 

To store request data the handler_new_ref() allocates memory
for it if needed.

Signed-off-by: Hans Verkuil 
---
 drivers/media/v4l2-core/v4l2-ctrls.c | 20 
 1 file changed, 16 insertions(+), 4 deletions(-)

diff --git a/drivers/media/v4l2-core/v4l2-ctrls.c 
b/drivers/media/v4l2-core/v4l2-ctrls.c
index b33a8bee82b0..171ab389afdd 100644
--- a/drivers/media/v4l2-core/v4l2-ctrls.c
+++ b/drivers/media/v4l2-core/v4l2-ctrls.c
@@ -2018,13 +2018,18 @@ EXPORT_SYMBOL(v4l2_ctrl_find);
 /* Allocate a new v4l2_ctrl_ref and hook it into the handler. */
 static int handler_new_ref(struct v4l2_ctrl_handler *hdl,
   struct v4l2_ctrl *ctrl,
-  bool from_other_dev)
+  struct v4l2_ctrl_ref **ctrl_ref,
+  bool from_other_dev, bool allocate_req)
 {
struct v4l2_ctrl_ref *ref;
struct v4l2_ctrl_ref *new_ref;
u32 id = ctrl->id;
u32 class_ctrl = V4L2_CTRL_ID2WHICH(id) | 1;
int bucket = id % hdl->nr_of_buckets;   /* which bucket to use */
+   unsigned int sz_extra = 0;
+
+   if (ctrl_ref)
+   *ctrl_ref = NULL;
 
/*
 * Automatically add the control class if it is not yet present and
@@ -2038,11 +2043,16 @@ static int handler_new_ref(struct v4l2_ctrl_handler 
*hdl,
if (hdl->error)
return hdl->error;
 
-   new_ref = kzalloc(sizeof(*new_ref), GFP_KERNEL);
+   if (allocate_req)
+   sz_extra = ctrl->elems * ctrl->elem_size;
+   new_ref = kzalloc(sizeof(*new_ref) + sz_extra, GFP_KERNEL);
if (!new_ref)
return handler_set_err(hdl, -ENOMEM);
new_ref->ctrl = ctrl;
new_ref->from_other_dev = from_other_dev;
+   if (sz_extra)
+   new_ref->p_req.p = _ref[1];
+
if (ctrl->handler == hdl) {
/* By default each control starts in a cluster of its own.
   new_ref->ctrl is basically a cluster array with one
@@ -2082,6 +2092,8 @@ static int handler_new_ref(struct v4l2_ctrl_handler *hdl,
/* Insert the control node in the hash */
new_ref->next = hdl->buckets[bucket];
hdl->buckets[bucket] = new_ref;
+   if (ctrl_ref)
+   *ctrl_ref = new_ref;
 
 unlock:
mutex_unlock(hdl->lock);
@@ -2223,7 +2235,7 @@ static struct v4l2_ctrl *v4l2_ctrl_new(struct 
v4l2_ctrl_handler *hdl,
ctrl->type_ops->init(ctrl, idx, ctrl->p_new);
}
 
-   if (handler_new_ref(hdl, ctrl, false)) {
+   if (handler_new_ref(hdl, ctrl, NULL, false, false)) {
kvfree(ctrl);
return NULL;
}
@@ -2416,7 +2428,7 @@ int v4l2_ctrl_add_handler(struct v4l2_ctrl_handler *hdl,
/* Filter any unwanted controls */
if (filter && !filter(ctrl))
continue;
-   ret = handler_new_ref(hdl, ctrl, from_other_dev);
+   ret = handler_new_ref(hdl, ctrl, NULL, from_other_dev, false);
if (ret)
break;
}
-- 
2.18.0