Re: [RFC v2 PATCH 2/7] KVM: Track the pid of the VM process

[Punit Agrawal <punit.agrawal@arm.com>]

Christoffer Dall <christoffer.dall@linaro.org> writes:

> On Tue, Sep 06, 2016 at 10:51:27AM +0100, Punit Agrawal wrote:
>> Hi Christoffer,
>> 
>> Christoffer Dall <christoffer.dall@linaro.org> writes:
>> 
>> > On Mon, Sep 05, 2016 at 05:31:32PM +0100, Punit Agrawal wrote:
>> >> Userspace tools such as perf can be used to profile individual
>> >> processes.
>> >> 
>> >> Track the PID of the virtual machine process to match profiling requests
>> >> targeted at it. This can be used to take appropriate action to enable
>> >> the requested profiling operations for the VMs of interest.
>> >> 
>> >> Signed-off-by: Punit Agrawal <punit.agrawal@arm.com>
>> >> Cc: Paolo Bonzini <pbonzini@redhat.com>
>> >> Cc: "Radim Krčmář" <rkrcmar@redhat.com>
>> >> Cc: Christoffer Dall <christoffer.dall@linaro.org>
>> >> Cc: Marc Zyngier <marc.zyngier@arm.com>
>> >> ---
>> >>  include/linux/kvm_host.h | 1 +
>> >>  virt/kvm/kvm_main.c      | 2 ++
>> >>  2 files changed, 3 insertions(+)
>> >> 
>> >> diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h
>> >> index 9c28b4d..7c42c94 100644
>> >> --- a/include/linux/kvm_host.h
>> >> +++ b/include/linux/kvm_host.h
>> >> @@ -374,6 +374,7 @@ struct kvm_memslots {
>> >>  struct kvm {
>> >>  	spinlock_t mmu_lock;
>> >>  	struct mutex slots_lock;
>> >> +	struct pid *pid;
>> >>  	struct mm_struct *mm; /* userspace tied to this vm */
>> >>  	struct kvm_memslots *memslots[KVM_ADDRESS_SPACE_NUM];
>> >>  	struct srcu_struct srcu;
>> >> diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
>> >> index 1950782..ab2535a 100644
>> >> --- a/virt/kvm/kvm_main.c
>> >> +++ b/virt/kvm/kvm_main.c
>> >> @@ -613,6 +613,7 @@ static struct kvm *kvm_create_vm(unsigned long type)
>> >>  	spin_lock_init(&kvm->mmu_lock);
>> >>  	atomic_inc(&current->mm->mm_count);
>> >>  	kvm->mm = current->mm;
>> >> +	kvm->pid = get_task_pid(current, PIDTYPE_PID);
>> >
>> > How dooes this deal with threading?  Is the idea that the user by
>> > specifying the main thread's pid will enable trapping for all vcpu
>> > threads belonging to that VM?
>> 
>> Yes that's correct - specifying the main thread PID will enable trapping
>> for the VM (all vcpus).
>> 
>> I am happy to move to a more suitable identifier if available.
>> 
>
> What is the 'main thread' ?
>
> Does something mandate that the VM is created by the thread group
> leader?  If not, is it not a bit strange from a user perspective, that
> you have to find the specific subthread pid that created the vm to
> enable this tracing for all vcpu threads and that the tgid doesn't work
> in this case?

Let me correct my terminology usage - the value recorded above (and used
to identify the VM) should be the tgid. It is confusing because 'ps'
reports it as pid.

I picked the value as existing KVM code already uses the PID of the
creating task (see kvm_create_vm_debugfs) to export VM statistics in
debugfs.

If I've got this wrong, then kvm_create_vm_debugfs also likely needs an
update.

What do you think?

>
> Thanks,
> -Christoffer
> _______________________________________________
> kvmarm mailing list
> kvmarm@lists.cs.columbia.edu
> https://lists.cs.columbia.edu/mailman/listinfo/kvmarm