linux-kernel - Re: [RFC][PATCHv2 02/12] videobuf2: handle V4L2 buffer cache flags

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: <2b1d9855-48dd-ecfe-7a7c-5998f76c3cd8@xs4all.nl>
Date:   Wed, 19 Feb 2020 09:25:29 +0100
From:   Hans Verkuil <hverkuil@...all.nl>
To:     Sergey Senozhatsky <senozhatsky@...omium.org>
Cc:     Hans Verkuil <hans.verkuil@...co.com>,
        Tomasz Figa <tfiga@...omium.org>,
        Mauro Carvalho Chehab <mchehab@...nel.org>,
        Kyungmin Park <kyungmin.park@...sung.com>,
        Marek Szyprowski <m.szyprowski@...sung.com>,
        Sakari Ailus <sakari.ailus@....fi>,
        Laurent Pinchart <laurent.pinchart@...asonboard.com>,
        Pawel Osciak <posciak@...omium.org>,
        linux-media@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [RFC][PATCHv2 02/12] videobuf2: handle V4L2 buffer cache flags

On 2/19/20 9:13 AM, Sergey Senozhatsky wrote:
> On (20/02/19 09:07), Hans Verkuil wrote:
> [..]
>>> +static void set_buffer_cache_hints(struct vb2_queue *q,
>>> +				   struct vb2_buffer *vb,
>>> +				   struct v4l2_buffer *b)
>>> +{
>>> +	/*
>>> +	 * DMA exporter should take care of cache syncs, so we can avoid
>>> +	 * explicit ->prepare()/->finish() syncs. For other ->memory types
>>> +	 * we always need ->prepare() or/and ->finish() cache sync.
>>> +	 */
>>> +	if (q->memory == VB2_MEMORY_DMABUF) {
>>> +		vb->need_cache_sync_on_finish = 0;
>>> +		vb->need_cache_sync_on_prepare = 0;
>>> +		return;
>>> +	}
>>> +
>>> +	if (!q->allow_cache_hints)
>>> +		return;
>>> +
>>> +	vb->need_cache_sync_on_prepare = 1;
>>
>> This needs a comment explaining why prepare is set to 1 by default. I remember
>> we discussed this earlier, and the conclusion of that discussion needs to be
>> documented here in a comment.
> 
> Please ignore this patch. There is a follow up which sets _both_
> flags by default. The purpose is to preserve the existing behaviour,
> we can do all sorts of incremental changes (clear flags in more cases,
> etc.) later on. Do you want me to document this in the code?

Yes please!

Regards,

	Hans

> 
> 	-ss
>