From mboxrd@z Thu Jan  1 00:00:00 1970
From: Greg KH <gregkh@linuxfoundation.org>
Date: Thu, 17 Jun 2021 05:56:41 +0000
Subject: Re: [PATCH v10 3/5] KVM: stats: Add documentation for binary statistics interface
Message-Id: <YMrkGZzPrt0jA1iP@kroah.com>
List-Id: <kvm-ppc.vger.kernel.org>
References: <20210617044146.2667540-1-jingzhangos@google.com>
 <20210617044146.2667540-4-jingzhangos@google.com>
In-Reply-To: <20210617044146.2667540-4-jingzhangos@google.com>
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
To: Jing Zhang <jingzhangos@google.com>
Cc: KVM <kvm@vger.kernel.org>, KVMARM <kvmarm@lists.cs.columbia.edu>, LinuxMIPS <linux-mips@vger.kernel.org>, KVMPPC <kvm-ppc@vger.kernel.org>, LinuxS390 <linux-s390@vger.kernel.org>, Linuxkselftest <linux-kselftest@vger.kernel.org>, Paolo Bonzini <pbonzini@redhat.com>, Marc Zyngier <maz@kernel.org>, James Morse <james.morse@arm.com>, Julien Thierry <julien.thierry.kdev@gmail.com>, Suzuki K Poulose <suzuki.poulose@arm.com>, Will Deacon <will@kernel.org>, Huacai Chen <chenhuacai@kernel.org>, Aleksandar Markovic <aleksandar.qemu.devel@gmail.com>, Thomas Bogendoerfer <tsbogend@alpha.franken.de>, Paul Mackerras <paulus@ozlabs.org>, Christian Borntraeger <borntraeger@de.ibm.com>, Janosch Frank <frankja@linux.ibm.com>, David Hildenbrand <david@redhat.com>, Cornelia Huck <cohuck@redhat.com>, Claudio Imbrenda <imbrenda@linux.ibm.com>, Sean Christopherson <seanjc@google.com>, Vitaly Kuznetsov <vkuznets@redhat.com>, Jim Mattson <jmattson@google.com>, Peter Shier <pshier@google.com>, Oliver Upton <oupton@google.com>, David Rientjes <rientjes@google.com>, Emanuele Giuseppe Esposito <eesposit@redhat.com>, David Matlack <dmatlack@google.com>, Ricardo Koller <ricarkol@google.com>, Krish Sadhukhan <krish.sadhukhan@oracle.com>, Fuad Tabba <tabba@google.com>

On Thu, Jun 17, 2021 at 04:41:44AM +0000, Jing Zhang wrote:
> +	struct kvm_stats_desc {
> +		__u32 flags;
> +		__s16 exponent;
> +		__u16 size;
> +		__u32 offset;
> +		__u32 unused;
> +		char name[0];
> +	};

<snip>

> +The ``unused`` fields are reserved for future support for other types of
> +statistics data, like log/linear histogram.

you HAVE to set unused to 0 for now, otherwise userspace does not know
it is unused, right?  And then, really it is "used", so why not just say
that now?  It's tricky, but you have to get this right now otherwise you
can never use it in the future.

> +The ``name`` field points to the name string of the statistics data. The name

It is not a pointer, it is the data itself.

> +string starts at the end of ``struct kvm_stats_desc``.
> +The maximum length (including trailing '\0') is indicated by ``name_size``
> +in ``struct kvm_stats_header``.

I thought we were replacing [0] arrays with [], are you sure you should
be declaring this as [0]?  Same for all structures in this document (and
code).

thanks,

greg k-h