On 08/09/2018 10:19 PM, Mauro Carvalho Chehab wrote:
> Em Sat, 4 Aug 2018 14:45:04 +0200
> Hans Verkuil <hverk...@xs4all.nl> escreveu:
>
>> From: Hans Verkuil <hans.verk...@cisco.com>
>>
>> To store request data the handler_new_ref() allocates memory
>> for it if needed.
>>
>> Signed-off-by: Hans Verkuil <hans.verk...@cisco.com>
>> ---
>> 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 <mchehab+sams...@kernel.org>
>
>> +
>> + 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 = &new_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
>
