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'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.
Best regards,
Tomasz
