On 04/10/2018 11:32 AM, Tomasz Figa wrote:
> Hi Hans,
>
> On Mon, Apr 9, 2018 at 11:21 PM Hans Verkuil <hverkuil@xxxxxxxxx> wrote:
>
>> From: Hans Verkuil <hans.verkuil@xxxxxxxxx>
>
>> To store request data the handler_new_ref() allocates memory
>> for it if needed.
>
>> Signed-off-by: Hans Verkuil <hans.verkuil@xxxxxxxxx>
>> ---
>> 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 d09f49530d9e..3c1b00baa8d0 100644
>> --- a/drivers/media/v4l2-core/v4l2-ctrls.c
>> +++ b/drivers/media/v4l2-core/v4l2-ctrls.c
>> @@ -1997,13 +1997,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
>> @@ -2017,11 +2022,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);
>
> I think we might want to use kvzalloc() here to cover allocation of big
> elements and/or large arrays, without order>0 allocations killing the
> systems.
I don't want to touch that now. Perhaps something for the future.
>
> I'm actually also wondering if it wouldn't be cleaner to just allocate the
> extra data separately, since we store a pointer in new_ref->p_req.p anyway.
It would give too much overhead since sz_extra is almost always 4. Controls that
need more space are very rare. Calling kzalloc again for just 4 bytes is overkill.
Regards,
Hans
>
> Best regards,
> Tomasz
>
