Messages in this thread | | | From | "Wang, Wei W" <> | Subject | RE: [PATCH v1 03/18] KVM: selftests/kvm_util: helper functions for vcpus and threads | Date | Thu, 27 Oct 2022 14:02:29 +0000 |
| |
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).
| |