Hi Ezequiel, On 5/29/18, Ezequiel Garcia <ezequiel@xxxxxxxxxxxxx> wrote: > On Fri, 2018-05-25 at 12:11 +0530, sathyam panda wrote: >> Hello, >> >> On 5/21/18, Ezequiel Garcia <ezequiel@xxxxxxxxxxxxx> wrote: >> > The in-fence implementation involves having a per-buffer fence >> > callback, >> > that triggers on the fence signal. The fence callback is called >> > asynchronously >> > and needs a valid reference to the associated ideobuf2 buffer. >> > >> > Allow this by making the vb2_buffer refcounted, so it can be passed >> > to other contexts. >> > >> >> -Is it really required, because when a queued buffer with an in_fence >> is deallocated, firstly queue is cancelled. >> -And __vb2_dqbuf is called which calls dma_fence_remove_callback. >> -So if fence callback has been called -__vb2_dqbuf will wait to >> acquire fence lock. >> -So during execution of fence callback, buffers and queue are still >> valid. >> -And if __vb2_dqbuf remove callback first ,then dma_fence_signal will >> wait for lock >> - so there won't be any fence callback to call for that buffer when >> dma_fence_signal resumes. >> > > Hi Sathyam, > > Thanks for your review! The refcount is definitely required, > as the fence callback only schedules a workqueue, which is > completely asynchronous with respect to the rest of the > ioctls. > > In particular, the workqueue is not synchronized with > vb2_core_queue_release. > > Also, another subtle detail, dma_fence_remove_callback > can fail to remove the callback. > > Thanks, > Eze > - Sorry, I didn't pay attention to the use of workqueue, thanks for bringing that into my notice. Now it make sense. - I have a doubt about the queue since many driver specific structures have queues which they release with kfree() after calling vb2_queue_release(). - And since vb2_core_queue_release decrements the buffer refcount only once if fence is already signalled, so buffer is still valid, but queue isn't as the memory is deallocated. - So is it safe to access queue in the __qbuf_work. Please correct me if I am wrong. Regards, Sathyam
