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