Re: [RFC PATCH 3/6] dma-buf: Add ranged mapping invalidation

[Christian König]

On 10/5/26 11:55, Fred Griffoul wrote:
> From: Fred Griffoul <fgriffo@xxxxxxxxxxxx>
> 
> dma_buf_invalidate_mappings() tells every importer that the whole
> buffer changed. An exporter that changes one part of its memory cannot
> say which bytes changed, so importers throw away mappings that are
> still valid.
> 
> Add an exporter helper that invalidates a byte range, and an importer
> callback that receives it. The callback means that the address, the
> attributes or the backing of the range changed. If part of the range is
> no longer backed, get_phys() returns -ENOENT for it. Importers must stop
> using their old answer before the callback returns. Importers that do
> not implement the callback still receive a whole-buffer invalidation.

Yeah that is exactly one of the reasons why we don't allow that.

Clear NAK to the whole approach. See the reply to patch #2 for a detailed description.

Regards,
Christian.

> 
> Signed-off-by: Fred Griffoul <fgriffo@xxxxxxxxxxxx>
> ---
>  drivers/dma-buf/dma-buf.c | 30 ++++++++++++++++++++++++++++++
>  include/linux/dma-buf.h   | 16 ++++++++++++++++
>  2 files changed, 46 insertions(+)
> 
> diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-buf.c
> index 66b85d53ed22..e7010163eb2f 100644
> --- a/drivers/dma-buf/dma-buf.c
> +++ b/drivers/dma-buf/dma-buf.c
> @@ -1389,6 +1389,36 @@ void dma_buf_invalidate_mappings(struct dma_buf *dmabuf)
>  }
>  EXPORT_SYMBOL_NS_GPL(dma_buf_invalidate_mappings, "DMA_BUF");
> 
> +/**
> + * dma_buf_invalidate_mappings_range - notify attachments that a range changed
> + * @dmabuf: buffer whose layout changed
> + * @offset: first changed byte
> + * @length: number of changed bytes
> + *
> + * Importers with a ranged callback stop using their old mappings of the range
> + * before returning. Other importers receive the existing whole-buffer
> + * callback, which is correct but coarser. The reservation lock must be held.
> + */
> +void dma_buf_invalidate_mappings_range(struct dma_buf *dmabuf,
> +                                      unsigned long offset,
> +                                      unsigned long length)
> +{
> +       struct dma_buf_attachment *attach;
> +
> +       dma_resv_assert_held(dmabuf->resv);
> +       list_for_each_entry(attach, &dmabuf->attachments, node) {
> +               const struct dma_buf_attach_ops *ops = attach->importer_ops;
> +
> +               if (!ops)
> +                       continue;
> +               if (ops->invalidate_mappings_range)
> +                       ops->invalidate_mappings_range(attach, offset, length);
> +               else if (ops->invalidate_mappings)
> +                       ops->invalidate_mappings(attach);
> +       }
> +}
> +EXPORT_SYMBOL_NS_GPL(dma_buf_invalidate_mappings_range, "DMA_BUF");
> +
>  /**
>   * dma_buf_get_phys - describe the run that starts at an offset
>   * @attach: attachment to query
> diff --git a/include/linux/dma-buf.h b/include/linux/dma-buf.h
> index b223962e20c2..55c3fe60a0ba 100644
> --- a/include/linux/dma-buf.h
> +++ b/include/linux/dma-buf.h
> @@ -485,6 +485,19 @@ struct dma_buf_attach_ops {
>          * required behavior.
>          */
>         void (*invalidate_mappings)(struct dma_buf_attachment *attach);
> +
> +       /**
> +        * @invalidate_mappings_range: [optional] a byte range changed
> +        *
> +        * The exporter changed the address, attributes or backing of
> +        * [@offset, @offset + @length). The importer must stop using its old
> +        * answer for that range before returning.
> +        * Importers without this callback receive @invalidate_mappings for
> +        * the whole buffer instead.
> +        */
> +       void (*invalidate_mappings_range)(struct dma_buf_attachment *attach,
> +                                         unsigned long offset,
> +                                         unsigned long length);
>  };
> 
>  /**
> @@ -600,6 +613,9 @@ struct sg_table *dma_buf_map_attachment(struct dma_buf_attachment *,
>  void dma_buf_unmap_attachment(struct dma_buf_attachment *, struct sg_table *,
>                                 enum dma_data_direction);
>  void dma_buf_invalidate_mappings(struct dma_buf *dma_buf);
> +void dma_buf_invalidate_mappings_range(struct dma_buf *dma_buf,
> +                                      unsigned long offset,
> +                                      unsigned long length);
>  bool dma_buf_attach_revocable(struct dma_buf_attachment *attach);
>  /* bits 0-7: memory type (a value, not flags) */
>  #define DMA_BUF_PHYS_ATTR_TYPE_MASK    GENMASK(7, 0)
> --
> 2.47.3
>