linux-kernel - Re: [PATCH v1 03/18] KVM: selftests/kvm_util: helper functions for vcpus and threads

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Thu, 27 Oct 2022 14:54:55 +0000
From:   Sean Christopherson <seanjc@...gle.com>
To:     "Wang, Wei W" <wei.w.wang@...el.com>
Cc:     "pbonzini@...hat.com" <pbonzini@...hat.com>,
        "dmatlack@...gle.com" <dmatlack@...gle.com>,
        "vipinsh@...gle.com" <vipinsh@...gle.com>,
        "ajones@...tanamicro.com" <ajones@...tanamicro.com>,
        "eric.auger@...hat.com" <eric.auger@...hat.com>,
        "kvm@...r.kernel.org" <kvm@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v1 03/18] KVM: selftests/kvm_util: helper functions for
 vcpus and threads

On Thu, Oct 27, 2022, Wang, Wei W wrote:
> On Thursday, October 27, 2022 8:10 AM, Sean Christopherson wrote:
> > > +void vm_vcpu_threads_create(struct kvm_vm *vm,
> > > +		void *(*start_routine)(void *), uint32_t private_data_size)
> > 
> > I vote (very strongly) to not deal with allocating private data.  The private data
> > isn't strictly related to threads, and the vast majority of callers don't need private
> > data, i.e. the param is dead weight in most cases.
> > 
> > And unless I'm missing something, it's trivial to move to a separate helper,
> > though honestly even that seems like overkill.
> > 
> > Wait, looking further, it already is a separate helper...  Forcing a bunch of
> > callers to specify '0' just to eliminate one function call in a handful of cases is not
> > a good tradeoff.
> 
> The intention was to do the allocation within one vm_for_each_vcpu()
> iteration when possible. Just a micro-optimization, but no problem, we can keep
> them separate if that looks better (simpler).

Keep them separate, that level of optimization is not something that's ever going
to be noticeable.

I don't want to say that performance is an afterthought for KVM selftests, but in
common code it's definitely way down the list of priorities because even the most
naive implementation for things like configuring vCPUs is going to have a runtime
measured in milliseconds.