Em Tue, 10 Apr 2018 09:32:00 +0000
Tomasz Figa <tfiga@xxxxxxxxxx> escreveu:
> 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)
You don't need a flag here. Instead, it could simply be:
if (ctrl_ref)
> > + 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.
Yeah, me too.
>
> Best regards,
> Tomasz
Thanks,
Mauro
