linux-kernel - Re: [PATCH AUTOSEL 4.19 2/4] media: vb2: use lock if wait

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: <49863857-f7b4-40c7-ae0c-3ba2bebe6200@xs4all.nl>
Date: Mon, 25 Nov 2024 08:33:06 +0100
From: Hans Verkuil <hverkuil-cisco@...all.nl>
To: Sasha Levin <sashal@...nel.org>, linux-kernel@...r.kernel.org,
 stable@...r.kernel.org
Cc: Mauro Carvalho Chehab <mchehab+huawei@...nel.org>, tfiga@...omium.org,
 m.szyprowski@...sung.com, mchehab@...nel.org, linux-media@...r.kernel.org
Subject: Re: [PATCH AUTOSEL 4.19 2/4] media: vb2: use lock if
 wait_prepare/finish are NULL

Hi Sasha,

This shouldn't be backported. It is not a fix, and it relies on a number of other
changes for this to work anyway, and none of those other changes are backported.

So please drop this patch.

Thank you,

	Hans

On 24/11/2024 13:58, Sasha Levin wrote:
> From: Hans Verkuil <hverkuil-cisco@...all.nl>
> 
> [ Upstream commit 88785982a19daa765e30ab3a605680202cfaee4e ]
> 
> If the wait_prepare or wait_finish callback is set, then call it.
> If it is NULL and the queue lock pointer is not NULL, then just
> unlock/lock that mutex.
> 
> This allows simplifying drivers by dropping the wait_prepare and
> wait_finish ops (and eventually the vb2_ops_wait_prepare/finish helpers).
> 
> Signed-off-by: Hans Verkuil <hverkuil-cisco@...all.nl>
> Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@...nel.org>
> Signed-off-by: Sasha Levin <sashal@...nel.org>
> ---
>  drivers/media/common/videobuf2/videobuf2-core.c | 13 ++++++++++---
>  1 file changed, 10 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/media/common/videobuf2/videobuf2-core.c b/drivers/media/common/videobuf2/videobuf2-core.c
> index 98719aa986bb9..e439831f6df46 100644
> --- a/drivers/media/common/videobuf2/videobuf2-core.c
> +++ b/drivers/media/common/videobuf2/videobuf2-core.c
> @@ -1528,7 +1528,10 @@ static int __vb2_wait_for_done_vb(struct vb2_queue *q, int nonblocking)
>  		 * become ready or for streamoff. Driver's lock is released to
>  		 * allow streamoff or qbuf to be called while waiting.
>  		 */
> -		call_void_qop(q, wait_prepare, q);
> +		if (q->ops->wait_prepare)
> +			call_void_qop(q, wait_prepare, q);
> +		else if (q->lock)
> +			mutex_unlock(q->lock);
>  
>  		/*
>  		 * All locks have been released, it is safe to sleep now.
> @@ -1538,12 +1541,16 @@ static int __vb2_wait_for_done_vb(struct vb2_queue *q, int nonblocking)
>  				!list_empty(&q->done_list) || !q->streaming ||
>  				q->error);
>  
> +		if (q->ops->wait_finish)
> +			call_void_qop(q, wait_finish, q);
> +		else if (q->lock)
> +			mutex_lock(q->lock);
> +
> +		q->waiting_in_dqbuf = 0;
>  		/*
>  		 * We need to reevaluate both conditions again after reacquiring
>  		 * the locks or return an error if one occurred.
>  		 */
> -		call_void_qop(q, wait_finish, q);
> -		q->waiting_in_dqbuf = 0;
>  		if (ret) {
>  			dprintk(1, "sleep was interrupted\n");
>  			return ret;