lkml.org 
[lkml]   [2022]   [Oct]   [27]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
From
SubjectRE: [PATCH v1 03/18] KVM: selftests/kvm_util: helper functions for vcpus and threads
Date
On Thursday, October 27, 2022 8:10 AM, Sean Christopherson wrote:
> On Mon, Oct 24, 2022, Wei Wang wrote:
> > @@ -14,4 +15,23 @@
> > for (i = 0, vcpu = vm->vcpus[0]; \
> > vcpu && i < KVM_MAX_VCPUS; vcpu = vm->vcpus[++i])
> >
> > +void __pthread_create_with_name(pthread_t *thread, const
> > +pthread_attr_t *attr,
>
> Can these return pthread_t instead of taking them as a param and have a "void"
> return? I'm pretty sure pthread_t is an integer type in most implementations,
> i.e. can be cheaply copied by value.

Yes, sounds good.

>
> > + void *(*start_routine)(void *), void *arg, char *name);
>
> Add a typedef for the payload, both to make it harder to screw up, and to make
> the code more readable. Does pthread really not provide one already?

You meant typedef for start_routine? I searched throughout pthread.h, and didn't find it.
Maybe we could create one here.

> > +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).

\
 
 \ /
  Last update: 2022-10-27 16:03    [W:0.071 / U:0.028 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site