RE: [Q] vb2 userptr: struct vb2_ops::buf_cleanup() is called without buf_init()
Hi Laurent, Thanks for CC. On Tuesday, May 22, 2012 6:22 PM Laurent Pinchart wrote: Hi Guennadi, (CC'ing Pawel and Marek) On Monday 21 May 2012 10:30:19 Guennadi Liakhovetski wrote: Hi A recent report http://thread.gmane.org/gmane.linux.drivers.video-input-infrastructure/47594 has revealed the following asymmetry in how videobuf2 functions: as is also documented in videobuf2-core.h, the user's struct vb2_ops::buf_init() method in the MMAP case is called after allocating the respective buffer, which happens at REQBUFS time, in the USERPTR case it is called after acquiring a new buffer at QBUF time. If the allocation in MMAP case fails, the respective buffer simply doesn't get created. However, if acquiring a new USERPTR buffer at QBUF time fails, the buffer object remains on the queue, but the user-provided .buf_init() method is not called for it. When the queue is destroyed, the user's .buf_cleanup() method is called on an uninitialised buffer. This is exactly the reason for the BUG() in the above referenced report. Therefore my question: is this videobuf2-core behaviour really correct and we should be prepared in .buf_cleanup() to process uninitialised buffers, or should the videobuf2-core be adjusted? From a driver's point of view, it would make sense not to call .buf_cleanup() if .buf_init() hasn't been called. Otherwise each driver would need to check whether the buffer has been initialized, which would lead to code duplication. A new buffer state would help tracking this in the vb2 core. I haven't tried to implement it though, so I might be underestimating the effort. That's definitely a bug in videobuf2. The initial idea was to call buf_cleanup only if buf_init has been called earlier for the given buffer. Like Laurent pointed out a new buffer state might help in resolving all possible cases, especially if you consider that vb2_queue_release might be called almost at any point in the buffer state machine. Best regards -- Marek Szyprowski Samsung Poland RD Center -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Q] vb2 userptr: struct vb2_ops::buf_cleanup() is called without buf_init()
Hi Guennadi, (CC'ing Pawel and Marek) On Monday 21 May 2012 10:30:19 Guennadi Liakhovetski wrote: Hi A recent report http://thread.gmane.org/gmane.linux.drivers.video-input-infrastructure/47594 has revealed the following asymmetry in how videobuf2 functions: as is also documented in videobuf2-core.h, the user's struct vb2_ops::buf_init() method in the MMAP case is called after allocating the respective buffer, which happens at REQBUFS time, in the USERPTR case it is called after acquiring a new buffer at QBUF time. If the allocation in MMAP case fails, the respective buffer simply doesn't get created. However, if acquiring a new USERPTR buffer at QBUF time fails, the buffer object remains on the queue, but the user-provided .buf_init() method is not called for it. When the queue is destroyed, the user's .buf_cleanup() method is called on an uninitialised buffer. This is exactly the reason for the BUG() in the above referenced report. Therefore my question: is this videobuf2-core behaviour really correct and we should be prepared in .buf_cleanup() to process uninitialised buffers, or should the videobuf2-core be adjusted? From a driver's point of view, it would make sense not to call .buf_cleanup() if .buf_init() hasn't been called. Otherwise each driver would need to check whether the buffer has been initialized, which would lead to code duplication. A new buffer state would help tracking this in the vb2 core. I haven't tried to implement it though, so I might be underestimating the effort. -- Regards, Laurent Pinchart -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[Q] vb2 userptr: struct vb2_ops::buf_cleanup() is called without buf_init()
Hi A recent report http://thread.gmane.org/gmane.linux.drivers.video-input-infrastructure/47594 has revealed the following asymmetry in how videobuf2 functions: as is also documented in videobuf2-core.h, the user's struct vb2_ops::buf_init() method in the MMAP case is called after allocating the respective buffer, which happens at REQBUFS time, in the USERPTR case it is called after acquiring a new buffer at QBUF time. If the allocation in MMAP case fails, the respective buffer simply doesn't get created. However, if acquiring a new USERPTR buffer at QBUF time fails, the buffer object remains on the queue, but the user-provided .buf_init() method is not called for it. When the queue is destroyed, the user's .buf_cleanup() method is called on an uninitialised buffer. This is exactly the reason for the BUG() in the above referenced report. Therefore my question: is this videobuf2-core behaviour really correct and we should be prepared in .buf_cleanup() to process uninitialised buffers, or should the videobuf2-core be adjusted? Thanks Guennadi --- Guennadi Liakhovetski, Ph.D. Freelance Open-Source Software Developer http://www.open-technology.de/ -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
