From mboxrd@z Thu Jan 1 00:00:00 1970 Return-path: Received: from lb1-smtp-cloud8.xs4all.net ([194.109.24.21]:52675 "EHLO lb1-smtp-cloud8.xs4all.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754710AbeDWLjZ (ORCPT ); Mon, 23 Apr 2018 07:39:25 -0400 Subject: Re: [RFCv11 PATCH 12/29] v4l2-ctrls: alloc memory for p_req To: Tomasz Figa Cc: Linux Media Mailing List , Hans Verkuil References: <20180409142026.19369-1-hverkuil@xs4all.nl> <20180409142026.19369-13-hverkuil@xs4all.nl> From: Hans Verkuil Message-ID: <52380219-fe21-aa57-140e-9f9ce82de402@xs4all.nl> Date: Mon, 23 Apr 2018 13:39:20 +0200 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit Sender: linux-media-owner@vger.kernel.org List-ID: On 04/10/2018 11:32 AM, Tomasz Figa wrote: > Hi Hans, > > On Mon, Apr 9, 2018 at 11:21 PM Hans Verkuil wrote: > >> 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 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 >