All of lore.kernel.org
 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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.