Linux-NVME Archive on lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] nvme-cli: cleanup comments for telemetry log structure
@ 2019-05-09 15:43 Akinobu Mita
  2019-05-09 16:38 ` Chaitanya Kulkarni
  0 siblings, 1 reply; 3+ messages in thread
From: Akinobu Mita @ 2019-05-09 15:43 UTC (permalink / raw)


This removes three comments for struct nvme_telemetry_log_page_hdr.

- The comment on top of the definision doesn't describe more than the
  struct name itself.

- The rsvd1 field has already been verified.

- The structures with trailing zero size array are commonly used in
  nvme-cli, so we don't need special comment for telemetry_dataarea[0].

Cc: Keith Busch <keith.busch at intel.com>
Cc: Jens Axboe <axboe at fb.com>
Cc: Christoph Hellwig <hch at lst.de>
Cc: Sagi Grimberg <sagi at grimberg.me>
Cc: Kenneth Heitke <kenneth.heitke at intel.com>
Suggested-by: Kenneth Heitke <kenneth.heitke at intel.com>
Signed-off-by: Akinobu Mita <akinobu.mita at gmail.com>
---
 linux/nvme.h | 10 +---------
 1 file changed, 1 insertion(+), 9 deletions(-)

diff --git a/linux/nvme.h b/linux/nvme.h
index f6d7341..2c840b9 100644
--- a/linux/nvme.h
+++ b/linux/nvme.h
@@ -426,9 +426,6 @@ struct nvme_id_nvmset {
 	struct nvme_nvmset_attr_entry	ent[NVME_MAX_NVMSET];
 };
 
-/* Derived from 1.3a Figure 101: Get Log Page ? Telemetry Host
- * -Initiated Log (Log Identifier 07h)
- */
 struct nvme_telemetry_log_page_hdr {
 	__u8    lpi; /* Log page identifier */
 	__u8    rsvd[4];
@@ -436,15 +433,10 @@ struct nvme_telemetry_log_page_hdr {
 	__le16  dalb1; /* Data area 1 last block */
 	__le16  dalb2; /* Data area 2 last block */
 	__le16  dalb3; /* Data area 3 last block */
-	__u8    rsvd1[368]; /* TODO verify */
+	__u8    rsvd1[368];
 	__u8    ctrlavail; /* Controller initiated data avail?*/
 	__u8    ctrldgn; /* Controller initiated telemetry Data Gen # */
 	__u8    rsnident[128];
-	/* We'll have to double fetch so we can get the header,
-	 * parse dalb1->3 determine how much size we need for the
-	 * log then alloc below. Or just do a secondary non-struct
-	 * allocation.
-	 */
 	__u8    telemetry_dataarea[0];
 };
 
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 3+ messages in thread

* [PATCH] nvme-cli: cleanup comments for telemetry log structure
  2019-05-09 16:38 ` Chaitanya Kulkarni
@ 2019-05-09 16:37   ` Keith Busch
  0 siblings, 0 replies; 3+ messages in thread
From: Keith Busch @ 2019-05-09 16:37 UTC (permalink / raw)


On Thu, May 09, 2019@04:38:03PM +0000, Chaitanya Kulkarni wrote:
> Looks good to me.
> Reviewed-by: Chaitanya Kulkarni <chaitanya.kulkarni at wdc.com>
> 
> On 05/09/2019 08:43 AM, Akinobu Mita wrote:
> > This removes three comments for struct nvme_telemetry_log_page_hdr.
> >
> > - The comment on top of the definision doesn't describe more than the
> >    struct name itself.
> >
> > - The rsvd1 field has already been verified.
> >
> > - The structures with trailing zero size array are commonly used in
> >    nvme-cli, so we don't need special comment for telemetry_dataarea[0].

Applied, thanks.

^ permalink raw reply	[flat|nested] 3+ messages in thread

* [PATCH] nvme-cli: cleanup comments for telemetry log structure
  2019-05-09 15:43 [PATCH] nvme-cli: cleanup comments for telemetry log structure Akinobu Mita
@ 2019-05-09 16:38 ` Chaitanya Kulkarni
  2019-05-09 16:37   ` Keith Busch
  0 siblings, 1 reply; 3+ messages in thread
From: Chaitanya Kulkarni @ 2019-05-09 16:38 UTC (permalink / raw)


Looks good to me.
Reviewed-by: Chaitanya Kulkarni <chaitanya.kulkarni at wdc.com>

On 05/09/2019 08:43 AM, Akinobu Mita wrote:
> This removes three comments for struct nvme_telemetry_log_page_hdr.
>
> - The comment on top of the definision doesn't describe more than the
>    struct name itself.
>
> - The rsvd1 field has already been verified.
>
> - The structures with trailing zero size array are commonly used in
>    nvme-cli, so we don't need special comment for telemetry_dataarea[0].
>
> Cc: Keith Busch <keith.busch at intel.com>
> Cc: Jens Axboe <axboe at fb.com>
> Cc: Christoph Hellwig <hch at lst.de>
> Cc: Sagi Grimberg <sagi at grimberg.me>
> Cc: Kenneth Heitke <kenneth.heitke at intel.com>
> Suggested-by: Kenneth Heitke <kenneth.heitke at intel.com>
> Signed-off-by: Akinobu Mita <akinobu.mita at gmail.com>
> ---
>   linux/nvme.h | 10 +---------
>   1 file changed, 1 insertion(+), 9 deletions(-)
>
> diff --git a/linux/nvme.h b/linux/nvme.h
> index f6d7341..2c840b9 100644
> --- a/linux/nvme.h
> +++ b/linux/nvme.h
> @@ -426,9 +426,6 @@ struct nvme_id_nvmset {
>   	struct nvme_nvmset_attr_entry	ent[NVME_MAX_NVMSET];
>   };
>
> -/* Derived from 1.3a Figure 101: Get Log Page ? Telemetry Host
> - * -Initiated Log (Log Identifier 07h)
> - */
>   struct nvme_telemetry_log_page_hdr {
>   	__u8    lpi; /* Log page identifier */
>   	__u8    rsvd[4];
> @@ -436,15 +433,10 @@ struct nvme_telemetry_log_page_hdr {
>   	__le16  dalb1; /* Data area 1 last block */
>   	__le16  dalb2; /* Data area 2 last block */
>   	__le16  dalb3; /* Data area 3 last block */
> -	__u8    rsvd1[368]; /* TODO verify */
> +	__u8    rsvd1[368];
>   	__u8    ctrlavail; /* Controller initiated data avail?*/
>   	__u8    ctrldgn; /* Controller initiated telemetry Data Gen # */
>   	__u8    rsnident[128];
> -	/* We'll have to double fetch so we can get the header,
> -	 * parse dalb1->3 determine how much size we need for the
> -	 * log then alloc below. Or just do a secondary non-struct
> -	 * allocation.
> -	 */
>   	__u8    telemetry_dataarea[0];
>   };
>
>

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2019-05-09 16:38 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2019-05-09 15:43 [PATCH] nvme-cli: cleanup comments for telemetry log structure Akinobu Mita
2019-05-09 16:38 ` Chaitanya Kulkarni
2019-05-09 16:37   ` Keith Busch

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox