linux-kernel - Re: [PATCH 1/4] media: dw100: Implement V4L2 requests support

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20260106154517.GD26157@pendragon.ideasonboard.com>
Date: Tue, 6 Jan 2026 17:45:17 +0200
From: Laurent Pinchart <laurent.pinchart@...asonboard.com>
To: Nicolas Dufresne <nicolas@...fresne.ca>
Cc: Stefan Klug <stefan.klug@...asonboard.com>,
	Xavier Roumegue <xavier.roumegue@....nxp.com>,
	Mauro Carvalho Chehab <mchehab@...nel.org>,
	Sebastian Andrzej Siewior <bigeasy@...utronix.de>,
	Clark Williams <clrkwllms@...nel.org>,
	Steven Rostedt <rostedt@...dmis.org>, linux-media@...r.kernel.org,
	linux-kernel@...r.kernel.org, linux-rt-devel@...ts.linux.dev,
	Hans Verkuil <hans@...erkuil.nl>
Subject: Re: [PATCH 1/4] media: dw100: Implement V4L2 requests support

On Tue, Jan 06, 2026 at 10:41:56AM -0500, Nicolas Dufresne wrote:
> Hi Laurent,
> 
> Le mardi 06 janvier 2026 à 16:53 +0200, Laurent Pinchart a écrit :
> > CC'ing Hans Verkuil for two questions below.
> > > 
> 
> [...]
> 
> > > The docs say "a buffer that was never queued to the driver but is
> > > associated with a queued request was canceled..." So to my understanding
> > > the only purpose of this function is to mark all controls in the request
> > > as being handled, so that the request can be finished.
> > 
> > That doesn't explain why it should be done twice per request. Hans,
> > could you clarify this ?
> 
> I explained it in another thread, it is only called if device_run() is not going
> to be called, so only once. vb2 does not have access to the the control handler,
> so it can't call the v4l2_ctrl_request_complete() fonction directly.

But the function is called per queue. If a buffer has been queued on
both the capture and the output queue for a request, won't the operation
be called twice with the same request ?

> [...]
> 
> > > Nicolas, if I go you right you mean that one might be tempted to do
> > > 
> > > v4l2_ctrl_request_setup()
> > > v4l2_m2m_buf_done(src)
> > > v4l2_m2m_buf_done(dst)
> > > v4l2_ctrl_request_complete()
> > > 
> > > which feels natural from the names of the functions, but the
> > > v4l2_m2m_buf_done(src) might complete the request if it has no
> > > associated controls and therefore the later v4l2_ctrl_request_complete()
> > > would fail...
> > > 
> > > I see that the usage of v4l2_m2m_buf_done_and_job_finish() is more
> > > compact and will use that in v2. For the sake of understanding: The only
> > > functional issue with my code is that v4l2_m2m_buf_done(src_buf) is
> > > called before v4l2_m2m_buf_done(dest_buf), right?
> > 
> > Is that an issue, why would the destination buffer need to be completed
> > first ?
> 
> The VB2 media_request_object is being removed from the request once the OUTPUT
> (src) buffer is marked done. If this was the last media_request_object, the
> request will move to completed state, which will signal its FD.

I'll reply to this separately, I need to read the code first.

> If you do that before you mark the CAPTURE (dst) buffer as done, an application
> that uses non blocking IOs may endup calling DQBUF(dst) too soon, which will
> return EBUSY. Since we really want the request FD to be used as the only
> synchronisation point, we made the rule that the request FD must be signalled
> last. Since its error prone, and its not illegal to synchronise on the device
> read/write/pri state, we made a condensed helper for it.
> 
> Alternatively, the manual request completion is being added for cases this
> implicit request completion does not work, or when it makes everything too
> complicated to adhere to this rule. This is the case for dual-stage video
> decoders (MTK/RPi).
> 
> [...]

-- 
Regards,

Laurent Pinchart