linux-kernel - Re: [RFC v5 07/11] [media] vb2: add in-fence support to QBUF

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 for Android: free password hash cracker in your pocket

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

Message-ID: <20171117114751.2dc10542@vento.lan>
Date:   Fri, 17 Nov 2017 11:47:51 -0200
From:   Mauro Carvalho Chehab <mchehab@....samsung.com>
To:     Gustavo Padovan <gustavo@...ovan.org>
Cc:     linux-media@...r.kernel.org, Hans Verkuil <hverkuil@...all.nl>,
        Shuah Khan <shuahkh@....samsung.com>,
        Pawel Osciak <pawel@...iak.com>,
        Alexandre Courbot <acourbot@...omium.org>,
        Sakari Ailus <sakari.ailus@....fi>,
        Brian Starkey <brian.starkey@....com>,
        Thierry Escande <thierry.escande@...labora.com>,
        linux-kernel@...r.kernel.org,
        Gustavo Padovan <gustavo.padovan@...labora.com>
Subject: Re: [RFC v5 07/11] [media] vb2: add in-fence support to QBUF

Em Fri, 17 Nov 2017 11:12:48 -0200
Gustavo Padovan <gustavo@...ovan.org> escreveu:

> > >  	/*
> > >  	 * If streamon has been called, and we haven't yet called
> > >  	 * start_streaming() since not enough buffers were queued, and
> > >  	 * we now have reached the minimum number of queued buffers,
> > >  	 * then we can finally call start_streaming().
> > > -	 *
> > > -	 * If already streaming, give the buffer to driver for processing.
> > > -	 * If not, the buffer will be given to driver on next streamon.
> > >  	 */
> > >  	if (q->streaming && !q->start_streaming_called &&
> > > -	    q->queued_count >= q->min_buffers_needed) {
> > > +	    __get_num_ready_buffers(q) >= q->min_buffers_needed) {  
> > 
> > I guess the case where fences is not used is not covered here.
> > 
> > You probably should add a check at __get_num_ready_buffers(q)
> > as well, making it just return q->queued_count if fences isn't
> > used.  
> 
> We can't know that beforehand, some buffer ahead may have a fence,
> but there is already a check for !fence for each buffer. If none of
> them have fences the return will be equal to q->queued_count.

Hmm... are we willing to support the case where just some
buffers have fences? Why?

In any case, we should add a notice at the documentation telling
about what happens if not all buffers have fences, and if fences
are required to be setup before starting streaming, or if it is
possible to dynamically change the fances behavior while streaming.

-- 
Thanks,
Mauro