Re: [PATCHv3 1/5] videobuf2-core: don't call memop 'finish' when queueing

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Hans,

On Tue, May 22, 2018 at 10:14:47AM +0200, Hans Verkuil wrote:
> From: Hans Verkuil <hans.verkuil@xxxxxxxxx>
> 
> When a buffer is queued or requeued in vb2_buffer_done, then don't
> call the finish memop. In this case the buffer is only returned to vb2,
> not to userspace.
> 
> Calling 'finish' here will cause an unbalance when the queue is
> canceled, since the core will call the same memop again.
> 
> Signed-off-by: Hans Verkuil <hans.verkuil@xxxxxxxxx>
> ---
>  drivers/media/common/videobuf2/videobuf2-core.c | 9 ++++++---
>  1 file changed, 6 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/media/common/videobuf2/videobuf2-core.c b/drivers/media/common/videobuf2/videobuf2-core.c
> index d3f7bb33a54d..f32ec7342ef0 100644
> --- a/drivers/media/common/videobuf2/videobuf2-core.c
> +++ b/drivers/media/common/videobuf2/videobuf2-core.c
> @@ -916,9 +916,12 @@ void vb2_buffer_done(struct vb2_buffer *vb, enum vb2_buffer_state state)
>  	dprintk(4, "done processing on buffer %d, state: %d\n",
>  			vb->index, state);
>  
> -	/* sync buffers */
> -	for (plane = 0; plane < vb->num_planes; ++plane)
> -		call_void_memop(vb, finish, vb->planes[plane].mem_priv);
> +	if (state != VB2_BUF_STATE_QUEUED &&
> +	    state != VB2_BUF_STATE_REQUEUEING) {
> +		/* sync buffers */
> +		for (plane = 0; plane < vb->num_planes; ++plane)
> +			call_void_memop(vb, finish, vb->planes[plane].mem_priv);
> +	}
>  
>  	spin_lock_irqsave(&q->done_lock, flags);
>  	if (state == VB2_BUF_STATE_QUEUED ||

How long do you think this problem has existed? Would this be worth fixing
in the stable series as well?


Could it be related to either of these two:

commit 03703ed1debf777ea845aa9b50ba2e80a5e7dd3c
Author: Sakari Ailus <sakari.ailus@xxxxxxxxxxxxxxx>
Date:   Fri Feb 2 05:08:59 2018 -0500

    media: vb2: core: Finish buffers at the end of the stream
    
    If buffers were prepared or queued and the buffers were released without
    starting the queue, the finish mem op (corresponding to the prepare mem
    op) was never called to the buffers.
    
    Before commit a136f59c0a1f there was no need to do this as in such a case
    the prepare mem op had not been called yet. Address the problem by
    explicitly calling finish mem op when the queue is stopped if the buffer
    is in either prepared or queued state.
    
    Fixes: a136f59c0a1f ("[media] vb2: Move buffer cache synchronisation to prepare from queue")
    
    Cc: stable@xxxxxxxxxxxxxxx # for v4.13 and up
    Signed-off-by: Sakari Ailus <sakari.ailus@xxxxxxxxxxxxxxx>
    Tested-by: Devin Heitmueller <dheitmueller@xxxxxxxxxxxxxx>
    Signed-off-by: Hans Verkuil <hans.verkuil@xxxxxxxxx>
    Signed-off-by: Mauro Carvalho Chehab <mchehab@xxxxxxxxxxxxxxxx>

commit a136f59c0a1f1b09eb203951975e3fc5e8d3e722
Author: Sakari Ailus <sakari.ailus@xxxxxxxxxxxxxxx>
Date:   Wed May 31 11:17:26 2017 -0300

    [media] vb2: Move buffer cache synchronisation to prepare from queue
    
    The buffer cache should be synchronised in buffer preparation, not when
    the buffer is queued to the device. Fix this.
    
    Mmap buffers do not need cache synchronisation since they are always
    coherent.
    
    Signed-off-by: Sakari Ailus <sakari.ailus@xxxxxxxxxxxxxxx>
    Acked-by: Hans Verkuil <hans.verkuil@xxxxxxxxx>
    Signed-off-by: Mauro Carvalho Chehab <mchehab@xxxxxxxxxxxxxxxx>

-- 
Kind regards,

Sakari Ailus
e-mail: sakari.ailus@xxxxxx



[Index of Archives]     [Linux Input]     [Video for Linux]     [Gstreamer Embedded]     [Mplayer Users]     [Linux USB Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Yosemite Backpacking]

  Powered by Linux