Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Fri, 14 Oct 2022 16:34:48 +0000
From:   Sean Christopherson <seanjc@google.com>
To:     "Wang, Wei W" <wei.w.wang@intel.com>
Cc:     Vipin Sharma <vipinsh@google.com>,
        "pbonzini@redhat.com" <pbonzini@redhat.com>,
        "dmatlack@google.com" <dmatlack@google.com>,
        "andrew.jones@linux.dev" <andrew.jones@linux.dev>,
        "kvm@vger.kernel.org" <kvm@vger.kernel.org>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH v5 0/5] dirty_log_perf_test vCPU pinning
Message-ID: <Y0mPqNRSgpArgyS8@google.com>
References: <20221010220538.1154054-1-vipinsh@google.com>
 <DS0PR11MB63735576A8FBF80738FF9B76DC249@DS0PR11MB6373.namprd11.prod.outlook.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <DS0PR11MB63735576A8FBF80738FF9B76DC249@DS0PR11MB6373.namprd11.prod.outlook.com>
Precedence: bulk

On Fri, Oct 14, 2022, Wang, Wei W wrote:
> On Tuesday, October 11, 2022 6:06 AM, Vipin Sharma wrote: 
> > Pin vCPUs to a host physical CPUs (pCPUs) in dirty_log_perf_test and optionally
> > pin the main application thread to a physical cpu if provided. All tests based on
> > perf_test_util framework can take advantage of it if needed.
> > 
> > While at it, I changed atoi() to atoi_paranoid(), atoi_positive,
> > atoi_non_negative() in other tests, sorted command line options alphabetically
> > in dirty_log_perf_test, and added break between -e and -g which was missed in
> > original commit when -e was introduced.
> 
> Just curious why not re-using the existing tools (e.g. taskset) to do the pinning?

IIUC, you're suggesting the test give tasks meaningful names so that the user can
do taskset on the appropriate tasks?  The goal is to ensure vCPUs are pinned before
they do any meaningful work.  I don't see how that can be accomplished with taskset
without some form of hook in the test to effectively pause the test until the user
(or some run script) is ready to continue.

Pinning aside, naming the threads is a great idea!  That would definitely help
debug, e.g. if one vCPU gets stuck or is lagging behind.

> 
> For example, with below changes:
> diff --git a/tools/testing/selftests/kvm/lib/perf_test_util.c b/tools/testing/se                                                                                                             lftests/kvm/lib/perf_test_util.c
> index 9618b37c66f7..aac58d1acb3c 100644
> --- a/tools/testing/selftests/kvm/lib/perf_test_util.c
> +++ b/tools/testing/selftests/kvm/lib/perf_test_util.c
> @@ -264,6 +264,7 @@ void perf_test_start_vcpu_threads(int nr_vcpus,
>                                   void (*vcpu_fn)(struct perf_test_vcpu_args *))
>  {
>         int i;
> +       char vcpu_name[5];
> 
>         vcpu_thread_fn = vcpu_fn;
>         WRITE_ONCE(all_vcpu_threads_running, false);
> @@ -275,6 +276,8 @@ void perf_test_start_vcpu_threads(int nr_vcpus,
>                 WRITE_ONCE(vcpu->running, false);
> 
>                 pthread_create(&vcpu->thread, NULL, vcpu_thread_main, vcpu);
> +               sprintf(vcpu_name, "%s%d", "vcpu", i);
> +               pthread_setname_np(vcpu->thread, vcpu_name);
>         }
> 
> and with top we can get
>     PID USER      PR  NI    VIRT    RES    SHR S  %CPU  %MEM     TIME+ COMMAND
>    4464 root      20   0 4248684   4.0g   1628 R  99.9  26.2   0:50.97 dirty_log_perf_
>    4467 root      20   0 4248684   4.0g   1628 R  99.9  26.2   0:50.93 vcpu0
>    4469 root      20   0 4248684   4.0g   1628 R  99.9  26.2   0:50.93 vcpu2
>    4470 root      20   0 4248684   4.0g   1628 R  99.9  26.2   0:50.94 vcpu3
>    4468 root      20   0 4248684   4.0g   1628 R  99.7  26.2   0:50.93 vcpu1