Re: [PATCHv13 16/28] videobuf2-core: embed media_request_object

[Mauro Carvalho Chehab <mchehab+samsung@kernel.org>]

Em Thu,  3 May 2018 16:53:06 +0200
Hans Verkuil <hverkuil@xs4all.nl> escreveu:

> From: Hans Verkuil <hans.verkuil@cisco.com>
> 
> Make vb2_buffer a request object.
> 
> Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
> ---
>  include/media/videobuf2-core.h | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/include/media/videobuf2-core.h b/include/media/videobuf2-core.h
> index 224c4820a044..3d54654c3cd4 100644
> --- a/include/media/videobuf2-core.h
> +++ b/include/media/videobuf2-core.h
> @@ -17,6 +17,7 @@
>  #include <linux/poll.h>
>  #include <linux/dma-buf.h>
>  #include <linux/bitops.h>
> +#include <media/media-request.h>
>  
>  #define VB2_MAX_FRAME	(32)
>  #define VB2_MAX_PLANES	(8)
> @@ -238,6 +239,7 @@ struct vb2_queue;
>   * @num_planes:		number of planes in the buffer
>   *			on an internal driver queue.
>   * @timestamp:		frame timestamp in ns.
> + * @req_obj:		used to bind this buffer to a request
>   */
>  struct vb2_buffer {
>  	struct vb2_queue	*vb2_queue;
> @@ -246,6 +248,7 @@ struct vb2_buffer {
>  	unsigned int		memory;
>  	unsigned int		num_planes;
>  	u64			timestamp;
> +	struct media_request_object	req_obj;
>  
>  	/* private: internal use only
>  	 *

Hmm... this has a side effect of embedding a kref at struct vb2_buffer.
One struct can have just one kref.

I guess this is likely ok, but this is a big struct. I don't like
the idea of having a hidden kref indirectly embedded there, as the
lifetime of this struct will now be controlled outside vb2, with
looks weird.

Thanks,
Mauro