linux-doc.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Sean Christopherson <seanjc@google.com>
To: Jinrong Liang <ljr.kernel@gmail.com>
Cc: Like Xu <like.xu.linux@gmail.com>,
	Paolo Bonzini <pbonzini@redhat.com>,
	Jonathan Corbet <corbet@lwn.net>, Shuah Khan <shuah@kernel.org>,
	Aaron Lewis <aaronlewis@google.com>,
	David Matlack <dmatlack@google.com>,
	Vishal Annapurve <vannapurve@google.com>,
	Wanpeng Li <wanpengli@tencent.com>,
	Bagas Sanjaya <bagasdotme@gmail.com>,
	Jinrong Liang <cloudliang@tencent.com>,
	linux-kselftest@vger.kernel.org, linux-doc@vger.kernel.org,
	kvm@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH v2 1/7] KVM: selftests: Replace int with uint32_t for nevents
Date: Thu, 25 May 2023 09:23:03 -0700	[thread overview]
Message-ID: <ZG+LZ4rATc8elt38@google.com> (raw)
In-Reply-To: <20230420104622.12504-2-ljrcore@126.com>

On Thu, Apr 20, 2023, Jinrong Liang wrote:
> From: Jinrong Liang <cloudliang@tencent.com>
> 
> From: Jinrong Liang <cloudliang@tencent.com>
> 
> Defined as type __u32, the nevents field in kvm_pmu_event_filter
> can only accept positive values within a specific range. Therefore,
> replacing int with uint32_t for nevents ensures consistency and
> readability in the code.

Not really.  It fixes one type of inconsistency that is fairly common (userspace
passing an integer count to the kernel), and replaces it with a different type
of inconsistency (signed iterator comparing against an unsigned count).  There's
already one of those in remove_event(), but I'd rather not create more.

Passing an unsigned int to track what *should* be a small-ish, postive integer
can also make it more difficult to detect bugs, e.g. assertions like this won't
work:

	TEST_ASSERT(nevents >= 0);

If this code were being written from scratch then I wouldn't object to using
uint32_t everywhere, but I don't see the point of trying to retroactively change
the code.

  reply	other threads:[~2023-05-25 16:23 UTC|newest]

Thread overview: 19+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-04-20 10:46 [PATCH v2 0/7] KVM: selftests: Add tests for pmu event filter Jinrong Liang
2023-04-20 10:46 ` [PATCH v2 1/7] KVM: selftests: Replace int with uint32_t for nevents Jinrong Liang
2023-05-25 16:23   ` Sean Christopherson [this message]
2023-04-20 10:46 ` [PATCH v2 2/7] KVM: selftests: Apply create_pmu_event_filter() to fixed ctrs Jinrong Liang
2023-05-25 17:44   ` Sean Christopherson
2023-04-20 10:46 ` [PATCH v2 3/7] KVM: selftests: Test unavailable event filters are rejected Jinrong Liang
2023-05-25 17:46   ` Sean Christopherson
2023-04-20 10:46 ` [PATCH v2 4/7] KVM: x86/pmu: Add documentation for fixed ctr on PMU filter Jinrong Liang
2023-05-25 17:56   ` Sean Christopherson
2023-04-20 10:46 ` [PATCH v2 5/7] KVM: selftests: Check if pmu_event_filter meets expectations on fixed ctrs Jinrong Liang
2023-05-25 18:11   ` Sean Christopherson
2023-04-20 10:46 ` [PATCH v2 6/7] KVM: selftests: Check gp event filters without affecting fixed event filters Jinrong Liang
2023-05-25 18:12   ` Sean Christopherson
2023-04-20 10:46 ` [PATCH v2 7/7] KVM: selftests: Test pmu event filter with incompatible kvm_pmu_event_filter Jinrong Liang
2023-05-24 23:50   ` Sean Christopherson
2023-05-25  2:19     ` Jinrong Liang
2023-05-25 15:55       ` Sean Christopherson
2023-05-22  3:33 ` [PATCH v2 0/7] KVM: selftests: Add tests for pmu event filter Jinrong Liang
2023-05-22 15:02   ` Sean Christopherson

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=ZG+LZ4rATc8elt38@google.com \
    --to=seanjc@google.com \
    --cc=aaronlewis@google.com \
    --cc=bagasdotme@gmail.com \
    --cc=cloudliang@tencent.com \
    --cc=corbet@lwn.net \
    --cc=dmatlack@google.com \
    --cc=kvm@vger.kernel.org \
    --cc=like.xu.linux@gmail.com \
    --cc=linux-doc@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-kselftest@vger.kernel.org \
    --cc=ljr.kernel@gmail.com \
    --cc=pbonzini@redhat.com \
    --cc=shuah@kernel.org \
    --cc=vannapurve@google.com \
    --cc=wanpengli@tencent.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).