linux-kernel - Re: [PATCH] KVM: selftests: Run dirty_log_perf

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]

Message-ID: <Yv6AiPtRbHv4OSL5@google.com>
Date:   Thu, 18 Aug 2022 18:10:16 +0000
From:   Sean Christopherson <seanjc@...gle.com>
To:     Vipin Sharma <vipinsh@...gle.com>
Cc:     dmatlack@...gle.com, pbonzini@...hat.com, kvm@...r.kernel.org,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH] KVM: selftests: Run dirty_log_perf_test on specific cpus

On Thu, Aug 18, 2022, Vipin Sharma wrote:
> On Wed, Aug 17, 2022 at 2:29 PM Sean Christopherson <seanjc@...gle.com> wrote:
> >
> > On Wed, Aug 17, 2022, Vipin Sharma wrote:
> > > On Wed, Aug 17, 2022 at 10:25 AM Sean Christopherson <seanjc@...gle.com> wrote:
> 
> > > We need error checking here to make sure that the user really wants
> > > cpu 0 and it was not a mistake in typing.
> > > I was thinking of using parse_num API for other places as well instead
> > > of atoi() in dirty_log_perf_test.
> >
> > Yes, definitely.  And maybe give it a name like atoi_paranoid()?
> 
> Lol. Absolutely, if that's what you want!

The goal is to capture that it's effectively atoi(), but with checking.  E.g. most
developers will be familiar with atoi() and won't have to think too hard if they
see e.g. atoi_paranoid().  On the other hand, parse_num() leaves the reader
wondering if it's parsing hex, decimal, octal, etc..., why selftests just doesn't
use atoi(), etc...

> Okay, I will remove -d and only keep -c. I will extend it to support
> pinning the main worker and vcpus. Arguments to -c will be like:
> <main woker lcpu>, <vcpu0's lcpu>, <vcpu1's lcpu>, <vcpu2's lcpu>,...
> Example:
> ./dirty_log_perf_test -v 3 -c 1,20,21,22
> 
> Main worker will run on 1 and 3 vcpus  will run on logical cpus 20, 21 and 22.

I think it makes sense to have the vCPUs be first.  That way vcpu0 => task_map[0],
and we can also extend the option in the future without breaking existing command
lines, e.g. to add more workers and/or redefine the behavior of the "trailing"
numbers to say that all workers are affined to those CPUs (to allow sequestering
the main worker from vCPUs without pinning it to a single CPU).