[PATCH 2/2] dma-buf: clarify dma_fence_add_callback documentation

[Daniel Vetter]

On Wed, Jul 21, 2021 at 11:21:33AM +0200, Christian König wrote:
> That the caller doesn't need to keep a reference is rather
> risky and not defensive at all.
> 
> Especially dma_buf_poll got that horrible wrong, so better
> remove that sentence and also clarify that the callback
> might be called in atomic or interrupt context.
> 
> Signed-off-by: Christian König <christian.koenig at amd.com>

I'm very vary of aspirational interface docs for cross-anything, it just
means everyone does whatever they feel like. I think I get what you aim
for here, but this needs more careful wording.


> ---
>  drivers/dma-buf/dma-fence.c | 13 +++++--------
>  1 file changed, 5 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/dma-buf/dma-fence.c b/drivers/dma-buf/dma-fence.c
> index ce0f5eff575d..1e82ecd443fa 100644
> --- a/drivers/dma-buf/dma-fence.c
> +++ b/drivers/dma-buf/dma-fence.c
> @@ -616,20 +616,17 @@ EXPORT_SYMBOL(dma_fence_enable_sw_signaling);
>   * @cb: the callback to register
>   * @func: the function to call
>   *
> + * Add a software callback to the fence. The caller should keep a reference to
> + * the fence.

Instead of your "The caller should" what about:

It is not required to hold rerence to @fence. But since the fence can
disappear as soon as @cb has returned callers generally must hold their
own reference to prevent issues.


With that or something similar that explains when we must do what and not
vague "should" wording.

Reviewed-by: Daniel Vetter <daniel.vetter at ffwll.ch>

> + *
>   * @cb will be initialized by dma_fence_add_callback(), no initialization
>   * by the caller is required. Any number of callbacks can be registered
>   * to a fence, but a callback can only be registered to one fence at a time.
>   *
> - * Note that the callback can be called from an atomic context.  If
> - * fence is already signaled, this function will return -ENOENT (and
> + * If fence is already signaled, this function will return -ENOENT (and
>   * *not* call the callback).
>   *
> - * Add a software callback to the fence. Same restrictions apply to
> - * refcount as it does to dma_fence_wait(), however the caller doesn't need to
> - * keep a refcount to fence afterward dma_fence_add_callback() has returned:
> - * when software access is enabled, the creator of the fence is required to keep
> - * the fence alive until after it signals with dma_fence_signal(). The callback
> - * itself can be called from irq context.
> + * Note that the callback can be called from an atomic context or irq context.
>   *
>   * Returns 0 in case of success, -ENOENT if the fence is already signaled
>   * and -EINVAL in case of error.
> -- 
> 2.25.1
> 

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch