On Thu, Jul 13, 2017 at 08:52:38PM +0200, Radim Krčmář wrote: > 2017-07-13 21:15+0300, Roman Kagan: > > On Thu, Jul 13, 2017 at 06:41:29PM +0200, Radim Krčmář wrote: > > > 2017-07-13 18:38+0200, Radim Krčmář: > > > > 2017-07-13 17:45+0200, Radim Krčmář: > > > > > 2017-07-13 18:29+0300, Roman Kagan: > > > > > > On Fri, Jun 23, 2017 at 12:54:25PM +0200, Paolo Bonzini wrote: > > > > > > > On 22/06/2017 15:51, Roman Kagan wrote: > > > > > > > Looks good, thanks. > > > > > > > > > > > > Are there still any problems with this series? > > > > > > I don't see it in kvm queue, so presumably it wasn't accepted... > > > > > > > > > > No, the problem was on my side. Queing it for the end of this merge > > > > > window. Thanks for the ping. > > > > > > > > And took it out after hitting a bug: we're asking for the VP_INDEX before the > > > > VCPU is in kvm->vcpus[], but the index is its position in that array. > > > > I think we can just use kvm->online_vcpus instead of kvm_vcpu_get_idx(). > > > > > > Ugh, definitely no, we're not under kvm->lock there. > > > Assigning the VP_INDEX in kvm_arch_vcpu_postcreate() would be doable, > > > but adding a new callback before exposing the VCPU fd is probably safer. > > > > But kvm_arch_vcpu_postcreate() *is* the last thing that's called before > > returning the VCPU fd. Isn't it the right place to do it? Or do you > > mean a dedicated kvm_hv_vcpu_postcreate() call rather than open-coding > > it there? > > It is, I just didn't want to guarantee that nothing bad can happen > > 1) after create_vcpu_fd() -- another thread can already access the fd > (getting its number just takes some guess-work) > > 2) after atomic_inc(&kvm->online_vcpus) -- the VCPU's vp_index is still > invalid 0, but someone (get_vcpu_by_vpidx()) might look at it Well, this whole patch is to allow the userspace to set vp_index, under the assumption that nothing in the kernel will use it until the userspace has a chance to do so. This initialization is a fallback for the case when the userspace is too old and doesn't explicitly control vp_index. So the risks you mention are hypthetical only. > I'd prefer to reuse kvm_arch_vcpu_postcreate() if it looks ok to you. > (Adding a new call only after all sane solutions have failed.) Great then, will adjust the patch and respin. Thanks, Roman.
