[PATCH v2 1/3] blk-mq.c: Add documention of function blk_mq_init_queue

[PATCH v2 1/3] blk-mq.c: Add documention of function blk_mq_init_queue

[Marcos Paulo de Souza]

Reviewed-by: Chaitanya Kulkarni <chaitanya.kulkarni@wdc.com>
Signed-off-by: Marcos Paulo de Souza <marcos.souza.org@gmail.com>
---

 No changes from v1.

 block/blk-mq.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/block/blk-mq.c b/block/blk-mq.c
index 9516304a38ee..4a8277a54c03 100644
--- a/block/blk-mq.c
+++ b/block/blk-mq.c
@@ -2650,6 +2650,15 @@ void blk_mq_release(struct request_queue *q)
 	blk_mq_sysfs_deinit(q);
 }
 
+/**
+ * blk_mq_init_queue - Create a new request queue associating a blk_mq_tag_set
+ * @set: tag_set to be associated with the request queue
+ *
+ * Description:
+ * This function creates a new request queue, associating @set with it.
+ *
+ * Returns a new request queue on success, or ERR_PTR() on failure.
+ */
 struct request_queue *blk_mq_init_queue(struct blk_mq_tag_set *set)
 {
 	struct request_queue *uninit_q, *q;
-- 
2.16.4

[PATCH v2 2/3] blk-mq.c: Rework documention of blk_mq_init_sq_queue function

[Marcos Paulo de Souza]

Describing better what the function does, what the arguments are meant for, and
what SQ stands for.

Signed-off-by: Marcos Paulo de Souza <marcos.souza.org@gmail.com>
---

 Changes from v1:
 * Change "SQ" to "only one hw_queue (SQ)" in the function signature
 	documentation (suggested by Chaitanya)

 block/blk-mq.c | 17 ++++++++++++++---
 1 file changed, 14 insertions(+), 3 deletions(-)

diff --git a/block/blk-mq.c b/block/blk-mq.c
index 4a8277a54c03..3f13cfb47cf0 100644
--- a/block/blk-mq.c
+++ b/block/blk-mq.c
@@ -2675,9 +2675,20 @@ struct request_queue *blk_mq_init_queue(struct blk_mq_tag_set *set)
 }
 EXPORT_SYMBOL(blk_mq_init_queue);
 
-/*
- * Helper for setting up a queue with mq ops, given queue depth, and
- * the passed in mq ops flags.
+/**
+ * blk_mq_init_sq_queue - Create a new request queue with only one hw_queue (SQ)
+ * @set: tag set to be associated by the newly created request queue
+ * @ops: operations to be associated with @set
+ * @queue_depth: number of tags of @set
+ * @set_flags: flags of @set
+ *
+ * Description:
+ * @set is initialized using @ops, @queue_depth, @set_flags, and using only one
+ * hw_queue (SQ). Later on blk_mq_alloc_tag_set() is called to validate and/or
+ * adjust values of the tag set. blk_mq_init_queue() is called passing @set as
+ * argument, returning a new request queue with @set associated.
+ *
+ * Returns the newly created request queue on success, or ERR_PTR() on failure.
  */
 struct request_queue *blk_mq_init_sq_queue(struct blk_mq_tag_set *set,
 					   const struct blk_mq_ops *ops,
-- 
2.16.4

Re: [PATCH v2 2/3] blk-mq.c: Rework documention of blk_mq_init_sq_queue function

[Chaitanya Kulkarni]

Looks good.

Reviewed-by: Chaitanya Kulkarni <chaitanya.kulkarni@wdc.com>

On 4/15/19, 8:28 PM, "linux-block-owner@vger.kernel.org on behalf of Marcos Paulo de Souza" <linux-block-owner@vger.kernel.org on behalf of marcos.souza.org@gmail.com> wrote:

    Describing better what the function does, what the arguments are meant for, and
    what SQ stands for.
    
    Signed-off-by: Marcos Paulo de Souza <marcos.souza.org@gmail.com>
    ---
    
     Changes from v1:
     * Change "SQ" to "only one hw_queue (SQ)" in the function signature
     	documentation (suggested by Chaitanya)
    
     block/blk-mq.c | 17 ++++++++++++++---
     1 file changed, 14 insertions(+), 3 deletions(-)
    
    diff --git a/block/blk-mq.c b/block/blk-mq.c
    index 4a8277a54c03..3f13cfb47cf0 100644
    --- a/block/blk-mq.c
    +++ b/block/blk-mq.c
    @@ -2675,9 +2675,20 @@ struct request_queue *blk_mq_init_queue(struct blk_mq_tag_set *set)
     }
     EXPORT_SYMBOL(blk_mq_init_queue);
     
    -/*
    - * Helper for setting up a queue with mq ops, given queue depth, and
    - * the passed in mq ops flags.
    +/**
    + * blk_mq_init_sq_queue - Create a new request queue with only one hw_queue (SQ)
    + * @set: tag set to be associated by the newly created request queue
    + * @ops: operations to be associated with @set
    + * @queue_depth: number of tags of @set
    + * @set_flags: flags of @set
    + *
    + * Description:
    + * @set is initialized using @ops, @queue_depth, @set_flags, and using only one
    + * hw_queue (SQ). Later on blk_mq_alloc_tag_set() is called to validate and/or
    + * adjust values of the tag set. blk_mq_init_queue() is called passing @set as
    + * argument, returning a new request queue with @set associated.
    + *
    + * Returns the newly created request queue on success, or ERR_PTR() on failure.
      */
     struct request_queue *blk_mq_init_sq_queue(struct blk_mq_tag_set *set,
     					   const struct blk_mq_ops *ops,
    -- 
    2.16.4
    
    

Re: [PATCH v2 2/3] blk-mq.c: Rework documention of blk_mq_init_sq_queue function

[Bart Van Assche]

On Tue, 2019-04-16 at 00:28 -0300, Marcos Paulo de Souza wrote:
>  * @set_flags: flags of @set

The above does not look very helpful to me ...

Bart.

[PATCH v2 3/3] Documentation: kernel-api.rst: Add block/blk-mq.c

[Marcos Paulo de Souza]

Now documented functions of blk-mq.c will appear in the official kernel
documentation.

Signed-off-by: Marcos Paulo de Souza <marcos.souza.org@gmail.com>
---

 No changes from v1.

 Documentation/core-api/kernel-api.rst | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/Documentation/core-api/kernel-api.rst b/Documentation/core-api/kernel-api.rst
index 71f5d2fe39b7..cc323480ce0e 100644
--- a/Documentation/core-api/kernel-api.rst
+++ b/Documentation/core-api/kernel-api.rst
@@ -276,6 +276,12 @@ Block Devices
 .. kernel-doc:: block/blk-map.c
    :export:
 
+.. kernel-doc:: block/blk-mq.c
+   :export:
+
+.. kernel-doc:: block/blk-mq.c
+   :internal:
+
 .. kernel-doc:: block/blk-sysfs.c
    :internal:
 
-- 
2.16.4

Re: [PATCH v2 3/3] Documentation: kernel-api.rst: Add block/blk-mq.c

[Chaitanya Kulkarni]

Looks good.

Reviewed-by: Chaitanya Kulkarni <chaitanya.kulkarni@wdc.com>

On 4/15/19, 8:28 PM, "Marcos Paulo de Souza" <marcos.souza.org@gmail.com> wrote:

    Now documented functions of blk-mq.c will appear in the official kernel
    documentation.
    
    Signed-off-by: Marcos Paulo de Souza <marcos.souza.org@gmail.com>
    ---
    
     No changes from v1.
    
     Documentation/core-api/kernel-api.rst | 6 ++++++
     1 file changed, 6 insertions(+)
    
    diff --git a/Documentation/core-api/kernel-api.rst b/Documentation/core-api/kernel-api.rst
    index 71f5d2fe39b7..cc323480ce0e 100644
    --- a/Documentation/core-api/kernel-api.rst
    +++ b/Documentation/core-api/kernel-api.rst
    @@ -276,6 +276,12 @@ Block Devices
     .. kernel-doc:: block/blk-map.c
        :export:
     
    +.. kernel-doc:: block/blk-mq.c
    +   :export:
    +
    +.. kernel-doc:: block/blk-mq.c
    +   :internal:
    +
     .. kernel-doc:: block/blk-sysfs.c
        :internal:
     
    -- 
    2.16.4
    
    

Re: [PATCH v2 1/3] blk-mq.c: Add documention of function blk_mq_init_queue

[Bart Van Assche]

On Tue, 2019-04-16 at 00:27 -0300, Marcos Paulo de Souza wrote:
> Reviewed-by: Chaitanya Kulkarni <chaitanya.kulkarni@wdc.com>
> Signed-off-by: Marcos Paulo de Souza <marcos.souza.org@gmail.com>
> ---
> 
>  No changes from v1.
> 
>  block/blk-mq.c | 9 ﯯ뻻
>  1 file changed, 9 insertions()

Please always include a cover letter with a patch series that gives an
overview of the purpose of the patch series.

Thanks,

Bart.

Re: [PATCH v2 1/3] blk-mq.c: Add documention of function blk_mq_init_queue

[Marcos Paulo de Souza]

gentle ping v2.

On Tue, Apr 16, 2019 at 12:27:59AM -0300, Marcos Paulo de Souza wrote:
> Reviewed-by: Chaitanya Kulkarni <chaitanya.kulkarni@wdc.com>
> Signed-off-by: Marcos Paulo de Souza <marcos.souza.org@gmail.com>
> ---
> 
>  No changes from v1.
> 
>  block/blk-mq.c | 9 +++++++++
>  1 file changed, 9 insertions(+)
> 
> diff --git a/block/blk-mq.c b/block/blk-mq.c
> index 9516304a38ee..4a8277a54c03 100644
> --- a/block/blk-mq.c
> +++ b/block/blk-mq.c
> @@ -2650,6 +2650,15 @@ void blk_mq_release(struct request_queue *q)
>  	blk_mq_sysfs_deinit(q);
>  }
>  
> +/**
> + * blk_mq_init_queue - Create a new request queue associating a blk_mq_tag_set
> + * @set: tag_set to be associated with the request queue
> + *
> + * Description:
> + * This function creates a new request queue, associating @set with it.
> + *
> + * Returns a new request queue on success, or ERR_PTR() on failure.
> + */
>  struct request_queue *blk_mq_init_queue(struct blk_mq_tag_set *set)
>  {
>  	struct request_queue *uninit_q, *q;
> -- 
> 2.16.4
> 

-- 
Thanks,
Marcos

Re: [PATCH v2 1/3] blk-mq.c: Add documention of function blk_mq_init_queue

[Jens Axboe]

On 5/1/19 8:04 PM, Marcos Paulo de Souza wrote:
> gentle ping v2.

It's not that I hate the series, I just don't see it adding any real
value. It should already be quite clear what the functions do, by name,
and the comments don't really add to that.

-- 
Jens Axboe

Re: [PATCH v2 1/3] blk-mq.c: Add documention of function blk_mq_init_queue

[Marcos Paulo de Souza]

On Thu, May 02, 2019 at 02:37:56PM -0600, Jens Axboe wrote:
> On 5/1/19 8:04 PM, Marcos Paulo de Souza wrote:
> > gentle ping v2.
> 
> It's not that I hate the series, I just don't see it adding any real
> value. It should already be quite clear what the functions do, by name,
> and the comments don't really add to that.

My first goal was to make someone to first read the function comment without
going into the details of the code, and at the same time, by adding these
documentations, having these functions being listed at
https://www.kernel.org/doc/html/latest/.

So, in IMHO, they add a fast way, for someone who is trying to understand how
things work, to understand exactly what the function does in some seconds, than
reading the functions code.

In you're now happy with these comments, can you point how can I improve them?

Thanks in advance,
Marcos

> 
> -- 
> Jens Axboe
> 

-- 
Thanks,
Marcos