Re: [PATCH v3 2/4] KVM: selftests: add is_cpu_online() utility function

From: Ricardo Koller <ricarkol@google.com>
To: Sean Christopherson <seanjc@google.com>
Cc: kvm@vger.kernel.org, kvmarm@lists.cs.columbia.edu,
	drjones@redhat.com, maz@kernel.org, pbonzini@redhat.com
Subject: Re: [PATCH v3 2/4] KVM: selftests: add is_cpu_online() utility function
Date: Thu, 24 Mar 2022 10:29:35 -0700	[thread overview]
Message-ID: <Yjyqf5oAxZzC7g7H@google.com> (raw)
In-Reply-To: <YjyoJu0/Saowtrbc@google.com>

On Thu, Mar 24, 2022 at 05:19:34PM +0000, Sean Christopherson wrote:
> On Tue, Mar 22, 2022, Ricardo Koller wrote:
> > Add is_cpu_online() utility function: a wrapper for
> > "/sys/devices/system/cpu/cpu%d/online".
> > 
> > Signed-off-by: Ricardo Koller <ricarkol@google.com>
> > ---
> >  tools/testing/selftests/kvm/include/test_util.h |  2 ++
> >  tools/testing/selftests/kvm/lib/test_util.c     | 16 ++++++++++++++++
> >  2 files changed, 18 insertions(+)
> > 
> > diff --git a/tools/testing/selftests/kvm/include/test_util.h b/tools/testing/selftests/kvm/include/test_util.h
> > index 99e0dcdc923f..14084dc4e152 100644
> > --- a/tools/testing/selftests/kvm/include/test_util.h
> > +++ b/tools/testing/selftests/kvm/include/test_util.h
> > @@ -143,4 +143,6 @@ static inline void *align_ptr_up(void *x, size_t size)
> >  	return (void *)align_up((unsigned long)x, size);
> >  }
> >  
> > +bool is_cpu_online(int pcpu);
> > +
> >  #endif /* SELFTEST_KVM_TEST_UTIL_H */
> > diff --git a/tools/testing/selftests/kvm/lib/test_util.c b/tools/testing/selftests/kvm/lib/test_util.c
> > index 6d23878bbfe1..81950e6b6d10 100644
> > --- a/tools/testing/selftests/kvm/lib/test_util.c
> > +++ b/tools/testing/selftests/kvm/lib/test_util.c
> > @@ -334,3 +334,19 @@ long get_run_delay(void)
> >  
> >  	return val[1];
> >  }
> > +
> > +bool is_cpu_online(int pcpu)
> > +{
> > +	char p[128];
> > +	FILE *fp;
> > +	int ret;
> > +
> > +	snprintf(p, sizeof(p), "/sys/devices/system/cpu/cpu%d/online", pcpu);
> 
> I don't think this is sufficient for the use in patch 03; the CPU could be online
> but disallowed for use by the current task.  I think what you want instead is a
> combination of get_nprocs_conf() + sched_getaffinity() + CPU_ISSET().

Good point. I was just thinking about the more common situation where a
user tries to use a CPU that doesn't even exist. Will fix in v4.

Thanks,
Ricardo