[02/15] dma-buf: Switch to inline kerneldoc

Commit Message

Daniel Vetter June 22, 2021, 4:54 p.m. UTC

Also review & update everything while we're at it.

This is prep work to smash a ton of stuff into the kerneldoc for
@resv.

Signed-off-by: Daniel Vetter <[e-mail]>
Cc: Sumit Semwal <[e-mail]>
Cc: "Christian König" <[e-mail]>
Cc: Alex Deucher <[e-mail]>
Cc: Daniel Vetter <[e-mail]>
Cc: Dave Airlie <[e-mail]>
Cc: Nirmoy Das <[e-mail]>
Cc: Deepak R Varma <[e-mail]>
Cc: Chen Li <[e-mail]>
Cc: Kevin Wang <[e-mail]>
Cc: [e-mail]
Cc: [e-mail]
---
 include/linux/dma-buf.h | 107 +++++++++++++++++++++++++++++++---------
 1 file changed, 83 insertions(+), 24 deletions(-)

Comments

Alex Deucher June 22, 2021, 6:24 p.m. UTC | #1

On Tue, Jun 22, 2021 at 12:55 PM Daniel Vetter <[e-mail]> wrote:
>
> Also review & update everything while we're at it.
>
> This is prep work to smash a ton of stuff into the kerneldoc for
> @resv.
>
> Signed-off-by: Daniel Vetter <[e-mail]>
> Cc: Sumit Semwal <[e-mail]>
> Cc: "Christian König" <[e-mail]>
> Cc: Alex Deucher <[e-mail]>
> Cc: Daniel Vetter <[e-mail]>
> Cc: Dave Airlie <[e-mail]>
> Cc: Nirmoy Das <[e-mail]>
> Cc: Deepak R Varma <[e-mail]>
> Cc: Chen Li <[e-mail]>
> Cc: Kevin Wang <[e-mail]>
> Cc: [e-mail]
> Cc: [e-mail]
> ---
>  include/linux/dma-buf.h | 107 +++++++++++++++++++++++++++++++---------
>  1 file changed, 83 insertions(+), 24 deletions(-)
>
> diff --git a/include/linux/dma-buf.h b/include/linux/dma-buf.h
> index 92eec38a03aa..6d18b9e448b9 100644
> --- a/include/linux/dma-buf.h
> +++ b/include/linux/dma-buf.h
> @@ -289,28 +289,6 @@ struct dma_buf_ops {
>
>  /**
>   * struct dma_buf - shared buffer object
> - * @size: size of the buffer; invariant over the lifetime of the buffer.
> - * @file: file pointer used for sharing buffers across, and for refcounting.
> - * @attachments: list of dma_buf_attachment that denotes all devices attached,
> - *               protected by dma_resv lock.
> - * @ops: dma_buf_ops associated with this buffer object.
> - * @lock: used internally to serialize list manipulation, attach/detach and
> - *        vmap/unmap
> - * @vmapping_counter: used internally to refcnt the vmaps
> - * @vmap_ptr: the current vmap ptr if vmapping_counter > 0
> - * @exp_name: name of the exporter; useful for debugging.
> - * @name: userspace-provided name; useful for accounting and debugging,
> - *        protected by @resv.
> - * @name_lock: spinlock to protect name access
> - * @owner: pointer to exporter module; used for refcounting when exporter is a
> - *         kernel module.
> - * @list_node: node for dma_buf accounting and debugging.
> - * @priv: exporter specific private data for this buffer object.
> - * @resv: reservation object linked to this dma-buf
> - * @poll: for userspace poll support
> - * @cb_excl: for userspace poll support
> - * @cb_shared: for userspace poll support
> - * @sysfs_entry: for exposing information about this buffer in sysfs.
>   * The attachment_uid member of @sysfs_entry is protected by dma_resv lock
>   * and is incremented on each attach.
>   *
> @@ -324,24 +302,100 @@ struct dma_buf_ops {
>   * Device DMA access is handled by the separate &struct dma_buf_attachment.
>   */
>  struct dma_buf {
> +       /**
> +        * @size:
> +        *
> +        * Size of the buffer; invariant over the lifetime of the buffer.
> +        */
>         size_t size;
> +
> +       /**
> +        * @file:
> +        *
> +        * File pointer used for sharing buffers across, and for refcounting.
> +        * See dma_buf_get() and dma_buf_put().
> +        */
>         struct file *file;
> +
> +       /**
> +        * @attachments:
> +        *
> +        * List of dma_buf_attachment that denotes all devices attached,
> +        * protected by &dma_resv lock @resv.
> +        */
>         struct list_head attachments;
> +
> +       /** @ops: dma_buf_ops associated with this buffer object. */

For consistency you may want to format this like:
/**
  * @ops:
  *
  * dma_buf_ops associated with this buffer object.
  */

>         const struct dma_buf_ops *ops;
> +
> +       /**
> +        * @lock:
> +        *
> +        * Used internally to serialize list manipulation, attach/detach and
> +        * vmap/unmap. Note that in many cases this is superseeded by
> +        * dma_resv_lock() on @resv.
> +        */
>         struct mutex lock;
> +
> +       /**
> +        * @vmapping_counter:
> +        *
> +        * Used internally to refcnt the vmaps returned by dma_buf_vmap().
> +        * Protected by @lock.
> +        */
>         unsigned vmapping_counter;
> +
> +       /**
> +        * @vmap_ptr:
> +        * The current vmap ptr if @vmapping_counter > 0. Protected by @lock.
> +        */

Same comment as above.

>         struct dma_buf_map vmap_ptr;
> +
> +       /**
> +        * @exp_name:
> +        *
> +        * Name of the exporter; useful for debugging. See the
> +        * DMA_BUF_SET_NAME IOCTL.
> +        */
>         const char *exp_name;
> +
> +       /**
> +        * @name:
> +        *
> +        * Userspace-provided name; useful for accounting and debugging,
> +        * protected by dma_resv_lock() on @resv and @name_lock for read access.
> +        */
>         const char *name;
> +
> +       /** @name_lock: Spinlock to protect name acces for read access. */
>         spinlock_t name_lock;
> +
> +       /**
> +        * @owner:
> +        *
> +        * Pointer to exporter module; used for refcounting when exporter is a
> +        * kernel module.
> +        */
>         struct module *owner;
> +
> +       /** @list_node: node for dma_buf accounting and debugging. */

and here.

>         struct list_head list_node;
> +
> +       /** @priv: exporter specific private data for this buffer object. */

and here.

>         void *priv;
> +
> +       /**
> +        * @resv:
> +        *
> +        * Reservation object linked to this dma-buf.
> +        */
>         struct dma_resv *resv;
>
> -       /* poll support */
> +       /** @poll: for userspace poll support */

here.

>         wait_queue_head_t poll;
>
> +       /** @cb_excl: for userspace poll support */
> +       /** @cb_shared: for userspace poll support */

Here.

Either way,
Reviewed-by: Alex Deucher <[e-mail]>

>         struct dma_buf_poll_cb_t {
>                 struct dma_fence_cb cb;
>                 wait_queue_head_t *poll;
> @@ -349,7 +403,12 @@ struct dma_buf {
>                 __poll_t active;
>         } cb_excl, cb_shared;
>  #ifdef CONFIG_DMABUF_SYSFS_STATS
> -       /* for sysfs stats */
> +       /**
> +        * @sysfs_entry:
> +        *
> +        * For exposing information about this buffer in sysfs. See also
> +        * `DMA-BUF statistics`_ for the uapi this enables.
> +        */
>         struct dma_buf_sysfs_entry {
>                 struct kobject kobj;
>                 struct dma_buf *dmabuf;
> --
> 2.32.0.rc2
>

Daniel Vetter June 22, 2021, 7:21 p.m. UTC | #2

On Tue, Jun 22, 2021 at 9:01 PM Sam Ravnborg <[e-mail]> wrote:
>
> Hi Daniel.
>
> On Tue, Jun 22, 2021 at 06:54:58PM +0200, Daniel Vetter wrote:
> > Also review & update everything while we're at it.
> >
> > This is prep work to smash a ton of stuff into the kerneldoc for
> > @resv.
> >
> > Signed-off-by: Daniel Vetter <[e-mail]>
> > Cc: Sumit Semwal <[e-mail]>
> > Cc: "Christian König" <[e-mail]>
> > Cc: Alex Deucher <[e-mail]>
> > Cc: Daniel Vetter <[e-mail]>
> > Cc: Dave Airlie <[e-mail]>
> > Cc: Nirmoy Das <[e-mail]>
> > Cc: Deepak R Varma <[e-mail]>
> > Cc: Chen Li <[e-mail]>
> > Cc: Kevin Wang <[e-mail]>
> > Cc: [e-mail]
> > Cc: [e-mail]
> > ---
> >  include/linux/dma-buf.h | 107 +++++++++++++++++++++++++++++++---------
> >  1 file changed, 83 insertions(+), 24 deletions(-)
> >
> > diff --git a/include/linux/dma-buf.h b/include/linux/dma-buf.h
> > index 92eec38a03aa..6d18b9e448b9 100644
> > --- a/include/linux/dma-buf.h
> > +++ b/include/linux/dma-buf.h
> > @@ -289,28 +289,6 @@ struct dma_buf_ops {
> >
> >  /**
> >   * struct dma_buf - shared buffer object
> > - * @size: size of the buffer; invariant over the lifetime of the buffer.
> > - * @file: file pointer used for sharing buffers across, and for refcounting.
> > - * @attachments: list of dma_buf_attachment that denotes all devices attached,
> > - *               protected by dma_resv lock.
> > - * @ops: dma_buf_ops associated with this buffer object.
> > - * @lock: used internally to serialize list manipulation, attach/detach and
> > - *        vmap/unmap
> > - * @vmapping_counter: used internally to refcnt the vmaps
> > - * @vmap_ptr: the current vmap ptr if vmapping_counter > 0
> > - * @exp_name: name of the exporter; useful for debugging.
> > - * @name: userspace-provided name; useful for accounting and debugging,
> > - *        protected by @resv.
> > - * @name_lock: spinlock to protect name access
> > - * @owner: pointer to exporter module; used for refcounting when exporter is a
> > - *         kernel module.
> > - * @list_node: node for dma_buf accounting and debugging.
> > - * @priv: exporter specific private data for this buffer object.
> > - * @resv: reservation object linked to this dma-buf
> > - * @poll: for userspace poll support
> > - * @cb_excl: for userspace poll support
> > - * @cb_shared: for userspace poll support
> > - * @sysfs_entry: for exposing information about this buffer in sysfs.
>
> This sentence
> >   * The attachment_uid member of @sysfs_entry is protected by dma_resv lock
> >   * and is incremented on each attach.
> belongs to the paragraph describing sysfs_entry and should be moved too.
> Or maybe reworded and then document all fields in dma_buf_sysfs_entry?

Unfortunately kerneldoc lost the ability to document embedded
structs/unions. At least last time I checked, it's a bit a bikeshed.
So I'd need to pull the entire struct out. I'll just move it since
it's indeed misplaced.

> With this fixed:
> Acked-by: Sam Ravnborg <[e-mail]>

Thanks for taking a look.
-Daniel

Christian König June 23, 2021, 8:32 a.m. UTC | #3

Am 22.06.21 um 18:54 schrieb Daniel Vetter:
> Also review & update everything while we're at it.
>
> This is prep work to smash a ton of stuff into the kerneldoc for
> @resv.
>
> Signed-off-by: Daniel Vetter <[e-mail]>
> Cc: Sumit Semwal <[e-mail]>
> Cc: "Christian König" <[e-mail]>
> Cc: Alex Deucher <[e-mail]>
> Cc: Daniel Vetter <[e-mail]>
> Cc: Dave Airlie <[e-mail]>
> Cc: Nirmoy Das <[e-mail]>
> Cc: Deepak R Varma <[e-mail]>
> Cc: Chen Li <[e-mail]>
> Cc: Kevin Wang <[e-mail]>
> Cc: [e-mail]
> Cc: [e-mail]

Acked-by: Christian König <[e-mail]>

> ---
>   include/linux/dma-buf.h | 107 +++++++++++++++++++++++++++++++---------
>   1 file changed, 83 insertions(+), 24 deletions(-)
>
> diff --git a/include/linux/dma-buf.h b/include/linux/dma-buf.h
> index 92eec38a03aa..6d18b9e448b9 100644
> --- a/include/linux/dma-buf.h
> +++ b/include/linux/dma-buf.h
> @@ -289,28 +289,6 @@ struct dma_buf_ops {
>   
>   /**
>    * struct dma_buf - shared buffer object
> - * @size: size of the buffer; invariant over the lifetime of the buffer.
> - * @file: file pointer used for sharing buffers across, and for refcounting.
> - * @attachments: list of dma_buf_attachment that denotes all devices attached,
> - *               protected by dma_resv lock.
> - * @ops: dma_buf_ops associated with this buffer object.
> - * @lock: used internally to serialize list manipulation, attach/detach and
> - *        vmap/unmap
> - * @vmapping_counter: used internally to refcnt the vmaps
> - * @vmap_ptr: the current vmap ptr if vmapping_counter > 0
> - * @exp_name: name of the exporter; useful for debugging.
> - * @name: userspace-provided name; useful for accounting and debugging,
> - *        protected by @resv.
> - * @name_lock: spinlock to protect name access
> - * @owner: pointer to exporter module; used for refcounting when exporter is a
> - *         kernel module.
> - * @list_node: node for dma_buf accounting and debugging.
> - * @priv: exporter specific private data for this buffer object.
> - * @resv: reservation object linked to this dma-buf
> - * @poll: for userspace poll support
> - * @cb_excl: for userspace poll support
> - * @cb_shared: for userspace poll support
> - * @sysfs_entry: for exposing information about this buffer in sysfs.
>    * The attachment_uid member of @sysfs_entry is protected by dma_resv lock
>    * and is incremented on each attach.
>    *
> @@ -324,24 +302,100 @@ struct dma_buf_ops {
>    * Device DMA access is handled by the separate &struct dma_buf_attachment.
>    */
>   struct dma_buf {
> +	/**
> +	 * @size:
> +	 *
> +	 * Size of the buffer; invariant over the lifetime of the buffer.
> +	 */
>   	size_t size;
> +
> +	/**
> +	 * @file:
> +	 *
> +	 * File pointer used for sharing buffers across, and for refcounting.
> +	 * See dma_buf_get() and dma_buf_put().
> +	 */
>   	struct file *file;
> +
> +	/**
> +	 * @attachments:
> +	 *
> +	 * List of dma_buf_attachment that denotes all devices attached,
> +	 * protected by &dma_resv lock @resv.
> +	 */
>   	struct list_head attachments;
> +
> +	/** @ops: dma_buf_ops associated with this buffer object. */
>   	const struct dma_buf_ops *ops;
> +
> +	/**
> +	 * @lock:
> +	 *
> +	 * Used internally to serialize list manipulation, attach/detach and
> +	 * vmap/unmap. Note that in many cases this is superseeded by
> +	 * dma_resv_lock() on @resv.
> +	 */
>   	struct mutex lock;
> +
> +	/**
> +	 * @vmapping_counter:
> +	 *
> +	 * Used internally to refcnt the vmaps returned by dma_buf_vmap().
> +	 * Protected by @lock.
> +	 */
>   	unsigned vmapping_counter;
> +
> +	/**
> +	 * @vmap_ptr:
> +	 * The current vmap ptr if @vmapping_counter > 0. Protected by @lock.
> +	 */
>   	struct dma_buf_map vmap_ptr;
> +
> +	/**
> +	 * @exp_name:
> +	 *
> +	 * Name of the exporter; useful for debugging. See the
> +	 * DMA_BUF_SET_NAME IOCTL.
> +	 */
>   	const char *exp_name;
> +
> +	/**
> +	 * @name:
> +	 *
> +	 * Userspace-provided name; useful for accounting and debugging,
> +	 * protected by dma_resv_lock() on @resv and @name_lock for read access.
> +	 */
>   	const char *name;
> +
> +	/** @name_lock: Spinlock to protect name acces for read access. */
>   	spinlock_t name_lock;
> +
> +	/**
> +	 * @owner:
> +	 *
> +	 * Pointer to exporter module; used for refcounting when exporter is a
> +	 * kernel module.
> +	 */
>   	struct module *owner;
> +
> +	/** @list_node: node for dma_buf accounting and debugging. */
>   	struct list_head list_node;
> +
> +	/** @priv: exporter specific private data for this buffer object. */
>   	void *priv;
> +
> +	/**
> +	 * @resv:
> +	 *
> +	 * Reservation object linked to this dma-buf.
> +	 */
>   	struct dma_resv *resv;
>   
> -	/* poll support */
> +	/** @poll: for userspace poll support */
>   	wait_queue_head_t poll;
>   
> +	/** @cb_excl: for userspace poll support */
> +	/** @cb_shared: for userspace poll support */
>   	struct dma_buf_poll_cb_t {
>   		struct dma_fence_cb cb;
>   		wait_queue_head_t *poll;
> @@ -349,7 +403,12 @@ struct dma_buf {
>   		__poll_t active;
>   	} cb_excl, cb_shared;
>   #ifdef CONFIG_DMABUF_SYSFS_STATS
> -	/* for sysfs stats */
> +	/**
> +	 * @sysfs_entry:
> +	 *
> +	 * For exposing information about this buffer in sysfs. See also
> +	 * `DMA-BUF statistics`_ for the uapi this enables.
> +	 */
>   	struct dma_buf_sysfs_entry {
>   		struct kobject kobj;
>   		struct dma_buf *dmabuf;

Patch

diff --git a/include/linux/dma-buf.h b/include/linux/dma-buf.h
index 92eec38a03aa..6d18b9e448b9 100644
--- a/include/linux/dma-buf.h
+++ b/include/linux/dma-buf.h
@@ -289,28 +289,6 @@  struct dma_buf_ops {
 
 /**
  * struct dma_buf - shared buffer object
- * @size: size of the buffer; invariant over the lifetime of the buffer.
- * @file: file pointer used for sharing buffers across, and for refcounting.
- * @attachments: list of dma_buf_attachment that denotes all devices attached,
- *               protected by dma_resv lock.
- * @ops: dma_buf_ops associated with this buffer object.
- * @lock: used internally to serialize list manipulation, attach/detach and
- *        vmap/unmap
- * @vmapping_counter: used internally to refcnt the vmaps
- * @vmap_ptr: the current vmap ptr if vmapping_counter > 0
- * @exp_name: name of the exporter; useful for debugging.
- * @name: userspace-provided name; useful for accounting and debugging,
- *        protected by @resv.
- * @name_lock: spinlock to protect name access
- * @owner: pointer to exporter module; used for refcounting when exporter is a
- *         kernel module.
- * @list_node: node for dma_buf accounting and debugging.
- * @priv: exporter specific private data for this buffer object.
- * @resv: reservation object linked to this dma-buf
- * @poll: for userspace poll support
- * @cb_excl: for userspace poll support
- * @cb_shared: for userspace poll support
- * @sysfs_entry: for exposing information about this buffer in sysfs.
  * The attachment_uid member of @sysfs_entry is protected by dma_resv lock
  * and is incremented on each attach.
  *
@@ -324,24 +302,100 @@  struct dma_buf_ops {
  * Device DMA access is handled by the separate &struct dma_buf_attachment.
  */
 struct dma_buf {
+	/**
+	 * @size:
+	 *
+	 * Size of the buffer; invariant over the lifetime of the buffer.
+	 */
 	size_t size;
+
+	/**
+	 * @file:
+	 *
+	 * File pointer used for sharing buffers across, and for refcounting.
+	 * See dma_buf_get() and dma_buf_put().
+	 */
 	struct file *file;
+
+	/**
+	 * @attachments:
+	 *
+	 * List of dma_buf_attachment that denotes all devices attached,
+	 * protected by &dma_resv lock @resv.
+	 */
 	struct list_head attachments;
+
+	/** @ops: dma_buf_ops associated with this buffer object. */
 	const struct dma_buf_ops *ops;
+
+	/**
+	 * @lock:
+	 *
+	 * Used internally to serialize list manipulation, attach/detach and
+	 * vmap/unmap. Note that in many cases this is superseeded by
+	 * dma_resv_lock() on @resv.
+	 */
 	struct mutex lock;
+
+	/**
+	 * @vmapping_counter:
+	 *
+	 * Used internally to refcnt the vmaps returned by dma_buf_vmap().
+	 * Protected by @lock.
+	 */
 	unsigned vmapping_counter;
+
+	/**
+	 * @vmap_ptr:
+	 * The current vmap ptr if @vmapping_counter > 0. Protected by @lock.
+	 */
 	struct dma_buf_map vmap_ptr;
+
+	/**
+	 * @exp_name:
+	 *
+	 * Name of the exporter; useful for debugging. See the
+	 * DMA_BUF_SET_NAME IOCTL.
+	 */
 	const char *exp_name;
+
+	/**
+	 * @name:
+	 *
+	 * Userspace-provided name; useful for accounting and debugging,
+	 * protected by dma_resv_lock() on @resv and @name_lock for read access.
+	 */
 	const char *name;
+
+	/** @name_lock: Spinlock to protect name acces for read access. */
 	spinlock_t name_lock;
+
+	/**
+	 * @owner:
+	 *
+	 * Pointer to exporter module; used for refcounting when exporter is a
+	 * kernel module.
+	 */
 	struct module *owner;
+
+	/** @list_node: node for dma_buf accounting and debugging. */
 	struct list_head list_node;
+
+	/** @priv: exporter specific private data for this buffer object. */
 	void *priv;
+
+	/**
+	 * @resv:
+	 *
+	 * Reservation object linked to this dma-buf.
+	 */
 	struct dma_resv *resv;
 
-	/* poll support */
+	/** @poll: for userspace poll support */
 	wait_queue_head_t poll;
 
+	/** @cb_excl: for userspace poll support */
+	/** @cb_shared: for userspace poll support */
 	struct dma_buf_poll_cb_t {
 		struct dma_fence_cb cb;
 		wait_queue_head_t *poll;
@@ -349,7 +403,12 @@  struct dma_buf {
 		__poll_t active;
 	} cb_excl, cb_shared;
 #ifdef CONFIG_DMABUF_SYSFS_STATS
-	/* for sysfs stats */
+	/**
+	 * @sysfs_entry:
+	 *
+	 * For exposing information about this buffer in sysfs. See also
+	 * `DMA-BUF statistics`_ for the uapi this enables.
+	 */
 	struct dma_buf_sysfs_entry {
 		struct kobject kobj;
 		struct dma_buf *dmabuf;