Re: [PATCH 01/15] iio: buffer: let the block keep its DMA fence

["Nuno Sá" <nuno.sa@analog.com>]

On Tue, Oct 27, 2026 at 05:19:05PM +0100, Greg Kroah-Hartman wrote:
> From: Josef Bacik <josef@toxicpanda.com>
> 
> iio_buffer_enqueue_dmabuf() hands a plain pointer to the fence to the
> buffer implementation, which keeps it in block->fence, and since the fix
> for the fence leak it drops the initial reference right after
> dma_resv_add_fence(). The only reference left is the one of the
> reservation object.
> 
> That one does not last until the fence is signalled. dma_resv_add_fence()
> replaces and puts a fence as soon as a later fence of the same context and
> the same usage is added. All the fences of an attachment share a context,
> and an enqueue on an output buffer only waits for the writers, not for the
> read fence of the enqueue before it. A second IIO_BUFFER_DMABUF_ENQUEUE of
> a buffer which is still queued is refused by iio_dma_can_enqueue_block(),
> but by then its fence has taken the place of the first one. When the DMA
> is done, iio_buffer_signal_dmabuf_done() signals a fence which has been
> freed and schedules the work item in it.
> 
> The leak came from the reference which iio_buffer_signal_dmabuf_done()
> takes and iio_buffer_cleanup() drops: with it nobody dropped the initial
> one. Do not take it, and leave the initial reference to the block until
> the cleanup.
> 
> Fixes: a09399935508 ("iio: buffer: Fix DMA fence leak in iio_buffer_enqueue_dmabuf()")
> Cc: stable <stable@kernel.org>
> Cc: David Lechner <dlechner@baylibre.com>
> Cc: "Nuno Sá" <nuno.sa@analog.com>
> Cc: Andy Shevchenko <andy@kernel.org>
> Cc: "Benoît Monin" <benoit.monin@bootlin.com>
> Cc: Paul Cercueil <paul@crapouillou.net>
> Signed-off-by: Josef Bacik <josef@toxicpanda.com>
> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> ---

Patch looks good to me! Maybe Paul remembers why all of that refcouting
dance was needed. Like this, it feels more natural:

Reviewed-by: Nuno Sá <nuno.sa@analog.com>

>  drivers/iio/industrialio-buffer.c | 15 +++++++--------
>  1 file changed, 7 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/iio/industrialio-buffer.c b/drivers/iio/industrialio-buffer.c
> index 4294101810fd..a2386dba497c 100644
> --- a/drivers/iio/industrialio-buffer.c
> +++ b/drivers/iio/industrialio-buffer.c
> @@ -1931,9 +1931,14 @@ static int iio_buffer_enqueue_dmabuf(struct iio_dev_buffer_pair *ib,
>  	if (ret)
>  		goto err_queue_unlock;
>  
> +	/*
> +	 * The reservation object takes a reference of its own, and drops it as
> +	 * soon as a later fence of this attachment takes the place of this
> +	 * one. The initial reference stays with the block, which points to the
> +	 * fence until iio_buffer_cleanup() has run.
> +	 */
>  	dma_resv_add_fence(dmabuf->resv, &fence->base,
>  			   dma_to_ram ? DMA_RESV_USAGE_WRITE : DMA_RESV_USAGE_READ);
> -	dma_fence_put(&fence->base);
>  	dma_resv_unlock(dmabuf->resv);
>  
>  	cookie = dma_fence_begin_signalling();
> @@ -1989,18 +1994,12 @@ void iio_buffer_signal_dmabuf_done(struct dma_fence *fence, int ret)
>  		container_of(fence, struct iio_dma_fence, base);
>  	bool cookie = dma_fence_begin_signalling();
>  
> -	/*
> -	 * Get a reference to the fence, so that it's not freed as soon as
> -	 * it's signaled.
> -	 */
> -	dma_fence_get(fence);
> -
>  	fence->error = ret;
>  	dma_fence_signal(fence);
>  	dma_fence_end_signalling(cookie);
>  
>  	/*
> -	 * The fence will be unref'd in iio_buffer_cleanup.
> +	 * The reference of the block will be dropped in iio_buffer_cleanup.
>  	 * It can't be done here, as the unref functions might try to lock the
>  	 * resv object, which can deadlock.
>  	 */
> 
> -- 
> 2.56.0
>