kvm.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Paolo Bonzini <pbonzini@redhat.com>
To: Alexander Graf <graf@amazon.de>,
	milanpa@amazon.com, Milan Pandurov <milanpa@amazon.de>,
	kvm@vger.kernel.org
Cc: rkrcmar@redhat.com, borntraeger@de.ibm.com
Subject: Re: [PATCH 2/2] kvm: Add ioctl for gathering debug counters
Date: Thu, 23 Jan 2020 13:08:15 +0100	[thread overview]
Message-ID: <7e6093f1-1d80-8278-c843-b4425ce098bf@redhat.com> (raw)
In-Reply-To: <7f206904-be2b-7901-1a88-37ed033b4de3@amazon.de>

On 21/01/20 16:38, Alexander Graf wrote:
>>>> ONE_REG would force us to define constants for each counter, and would
>>>> make it hard to retire them.  I don't like this.
>>>
>>> Why does it make it hard to retire them? We would just return -EINVAL
>>> on retrieval, like we do for any other non-supported ONE_REG.
>>>
>>> It's the same as a file not existing in debugfs/statfs. Or an entry
>>> in the array of this patch to disappear.

The devil is in the details.  For example, would you retire uapi/
constants and cause programs to fail compilation?  Or do you keep the
obsolete constants forever?  Also, fixing the mapping from ONE_REG
number to stat would mean a switch statement (or loop of some kind---a
switch statement is basically an unrolled binary search) to access the
stats.  Instead returning the id in KVM_GET_SUPPORTED_DEBUGFS_STAT would
simplify returning the stats to a simple copy_to_user.

Of course, some of the complexity would be punted to userspace.  But
userspace is much closer to the humans that ultimately look at the
stats, so the question is: does userspace really care about knowing
which stat is which, or do they just care about having a name that will
ultimately be consumed by humans down the pipe?  If the latter (which is
also my gut feeling), that is also a point against ONE_REG.

> It's not a problem of exposing the type information - we have that today
> by implicitly saying "every counter is 64bit".
> 
> The thing I'm worried about is that we keep inventing these special
> purpose interfaces that really do nothing but transfer numbers in one
> way or another. ONE_REG's purpose was to unify them. Debug counters
> really are the same story.

See above: I am not sure they are the same story because their consumers
might be very different from registers.  Registers are generally
consumed by programs (to migrate VMs, for example) and only occasionally
by humans, while stats are meant to be consumed by humans.  We may
disagree on whether this justifies a completely different API...

Paolo


  reply	other threads:[~2020-01-23 12:08 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-01-15 13:43 [PATCH 2/2] kvm: Add ioctl for gathering debug counters Milan Pandurov
2020-01-15 14:04 ` Alexander Graf
2020-01-15 14:43   ` milanpa
2020-01-15 14:59     ` Alexander Graf
2020-01-17 23:38       ` Paolo Bonzini
2020-01-20 17:53         ` Alexander Graf
2020-01-20 18:57           ` milanpa
2020-01-21 15:38             ` Alexander Graf
2020-01-23 12:08               ` Paolo Bonzini [this message]
2020-01-23 12:32                 ` Alexander Graf
2020-01-23 14:19                   ` Paolo Bonzini
2020-01-23 14:45                     ` Alexander Graf
2020-01-23 14:50                       ` Paolo Bonzini
2020-01-23 14:58                         ` Alexander Graf
2020-01-23 15:05                           ` Paolo Bonzini
2020-01-23 15:27                             ` milanpa
2020-01-23 16:15                               ` Paolo Bonzini
2020-01-23 18:31                                 ` milanpa

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=7e6093f1-1d80-8278-c843-b4425ce098bf@redhat.com \
    --to=pbonzini@redhat.com \
    --cc=borntraeger@de.ibm.com \
    --cc=graf@amazon.de \
    --cc=kvm@vger.kernel.org \
    --cc=milanpa@amazon.com \
    --cc=milanpa@amazon.de \
    --cc=rkrcmar@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body. 
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).