From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S935556AbcIFPXV convert rfc822-to-8bit (ORCPT ); Tue, 6 Sep 2016 11:23:21 -0400 Received: from fw-tnat.cambridge.arm.com ([217.140.96.140]:56613 "EHLO cam-smtp0.cambridge.arm.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S934171AbcIFPXR (ORCPT ); Tue, 6 Sep 2016 11:23:17 -0400 From: Punit Agrawal To: Christoffer Dall Cc: kvm@vger.kernel.org, Marc Zyngier , Will Deacon , linux-kernel@vger.kernel.org, Steven Rostedt , Ingo Molnar , Paolo Bonzini , kvmarm@lists.cs.columbia.edu, linux-arm-kernel@lists.infradead.org Subject: Re: [RFC v2 PATCH 2/7] KVM: Track the pid of the VM process References: <1473093097-30932-1-git-send-email-punit.agrawal@arm.com> <1473093097-30932-3-git-send-email-punit.agrawal@arm.com> <20160906062252.GA30513@cbox> <87poohjqzk.fsf@e105922-lin.cambridge.arm.com> <20160906102542.GG30513@cbox> <877fapjng0.fsf@e105922-lin.cambridge.arm.com> <20160906112203.GO30513@cbox> Date: Tue, 06 Sep 2016 16:22:17 +0100 In-Reply-To: <20160906112203.GO30513@cbox> (Christoffer Dall's message of "Tue, 6 Sep 2016 13:22:03 +0200") Message-ID: <87poohhx3q.fsf@e105922-lin.cambridge.arm.com> User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/24.5 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8BIT Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Christoffer Dall writes: > On Tue, Sep 06, 2016 at 12:07:59PM +0100, Punit Agrawal wrote: >> Christoffer Dall writes: >> >> > On Tue, Sep 06, 2016 at 10:51:27AM +0100, Punit Agrawal wrote: >> >> Hi Christoffer, >> >> >> >> Christoffer Dall 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 >> >> >> Cc: Paolo Bonzini >> >> >> Cc: "Radim Krčmář" >> >> >> Cc: Christoffer Dall >> >> >> Cc: Marc Zyngier >> >> >> --- >> >> >> 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(¤t->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? >> > When you do get_task_pid(current, PIDTYPE_PID) it actually gets the > kernel view of a PID which is the thead-id from userspace's point of > view, right? That makes sense. It seems to works here because the pid of the first task is also the tgid of the group. And I reckon it's the same assumption being made with debugfs code (more below). I've changed the first argument of the call to get_task_pid to current->group_leader. > > I don't see why this has to be the same as the debugfs code, as there it > makes potentially more sense to thread-specific, but for your case, are > you not targeting the behavior that a user can do "ps aux | grep qemu" > or whatever, and then set tracing for the reported PID (which is > actually a tgid)? The debugfs stats are not thread (vcpu) specific but for the VM. Both values, debugfs and here, are being used to represent the VM to the user. A mismatch in these identifiers will be very confusing. If you agree, I can separately send a patch to address this for VM debugfs directory as well. > > If this is indeed the case, then I don't think the current code supports > this if QEMU was ever changed to create the VM with a different thread > than the tgl. > > That being said, I'm typically wrong when I talk about userspace. > > -Christoffer > _______________________________________________ > kvmarm mailing list > kvmarm@lists.cs.columbia.edu > https://lists.cs.columbia.edu/mailman/listinfo/kvmarm From mboxrd@z Thu Jan 1 00:00:00 1970 From: Punit Agrawal Subject: Re: [RFC v2 PATCH 2/7] KVM: Track the pid of the VM process Date: Tue, 06 Sep 2016 16:22:17 +0100 Message-ID: <87poohhx3q.fsf@e105922-lin.cambridge.arm.com> References: <1473093097-30932-1-git-send-email-punit.agrawal@arm.com> <1473093097-30932-3-git-send-email-punit.agrawal@arm.com> <20160906062252.GA30513@cbox> <87poohjqzk.fsf@e105922-lin.cambridge.arm.com> <20160906102542.GG30513@cbox> <877fapjng0.fsf@e105922-lin.cambridge.arm.com> <20160906112203.GO30513@cbox> Mime-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: base64 Cc: kvm@vger.kernel.org, Marc Zyngier , Will Deacon , linux-kernel@vger.kernel.org, Steven Rostedt , Ingo Molnar , Paolo Bonzini , kvmarm@lists.cs.columbia.edu, linux-arm-kernel@lists.infradead.org To: Christoffer Dall Return-path: In-Reply-To: <20160906112203.GO30513@cbox> (Christoffer Dall's message of "Tue, 6 Sep 2016 13:22:03 +0200") List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: kvmarm-bounces@lists.cs.columbia.edu Sender: kvmarm-bounces@lists.cs.columbia.edu List-Id: kvm.vger.kernel.org Q2hyaXN0b2ZmZXIgRGFsbCA8Y2hyaXN0b2ZmZXIuZGFsbEBsaW5hcm8ub3JnPiB3cml0ZXM6Cgo+ IE9uIFR1ZSwgU2VwIDA2LCAyMDE2IGF0IDEyOjA3OjU5UE0gKzAxMDAsIFB1bml0IEFncmF3YWwg d3JvdGU6Cj4+IENocmlzdG9mZmVyIERhbGwgPGNocmlzdG9mZmVyLmRhbGxAbGluYXJvLm9yZz4g d3JpdGVzOgo+PiAKPj4gPiBPbiBUdWUsIFNlcCAwNiwgMjAxNiBhdCAxMDo1MToyN0FNICswMTAw LCBQdW5pdCBBZ3Jhd2FsIHdyb3RlOgo+PiA+PiBIaSBDaHJpc3RvZmZlciwKPj4gPj4gCj4+ID4+ IENocmlzdG9mZmVyIERhbGwgPGNocmlzdG9mZmVyLmRhbGxAbGluYXJvLm9yZz4gd3JpdGVzOgo+ PiA+PiAKPj4gPj4gPiBPbiBNb24sIFNlcCAwNSwgMjAxNiBhdCAwNTozMTozMlBNICswMTAwLCBQ dW5pdCBBZ3Jhd2FsIHdyb3RlOgo+PiA+PiA+PiBVc2Vyc3BhY2UgdG9vbHMgc3VjaCBhcyBwZXJm IGNhbiBiZSB1c2VkIHRvIHByb2ZpbGUgaW5kaXZpZHVhbAo+PiA+PiA+PiBwcm9jZXNzZXMuCj4+ ID4+ID4+IAo+PiA+PiA+PiBUcmFjayB0aGUgUElEIG9mIHRoZSB2aXJ0dWFsIG1hY2hpbmUgcHJv Y2VzcyB0byBtYXRjaCBwcm9maWxpbmcgcmVxdWVzdHMKPj4gPj4gPj4gdGFyZ2V0ZWQgYXQgaXQu IFRoaXMgY2FuIGJlIHVzZWQgdG8gdGFrZSBhcHByb3ByaWF0ZSBhY3Rpb24gdG8gZW5hYmxlCj4+ ID4+ID4+IHRoZSByZXF1ZXN0ZWQgcHJvZmlsaW5nIG9wZXJhdGlvbnMgZm9yIHRoZSBWTXMgb2Yg aW50ZXJlc3QuCj4+ID4+ID4+IAo+PiA+PiA+PiBTaWduZWQtb2ZmLWJ5OiBQdW5pdCBBZ3Jhd2Fs IDxwdW5pdC5hZ3Jhd2FsQGFybS5jb20+Cj4+ID4+ID4+IENjOiBQYW9sbyBCb256aW5pIDxwYm9u emluaUByZWRoYXQuY29tPgo+PiA+PiA+PiBDYzogIlJhZGltIEtyxI1tw6HFmSIgPHJrcmNtYXJA cmVkaGF0LmNvbT4KPj4gPj4gPj4gQ2M6IENocmlzdG9mZmVyIERhbGwgPGNocmlzdG9mZmVyLmRh bGxAbGluYXJvLm9yZz4KPj4gPj4gPj4gQ2M6IE1hcmMgWnluZ2llciA8bWFyYy56eW5naWVyQGFy bS5jb20+Cj4+ID4+ID4+IC0tLQo+PiA+PiA+PiAgaW5jbHVkZS9saW51eC9rdm1faG9zdC5oIHwg MSArCj4+ID4+ID4+ICB2aXJ0L2t2bS9rdm1fbWFpbi5jICAgICAgfCAyICsrCj4+ID4+ID4+ICAy IGZpbGVzIGNoYW5nZWQsIDMgaW5zZXJ0aW9ucygrKQo+PiA+PiA+PiAKPj4gPj4gPj4gZGlmZiAt LWdpdCBhL2luY2x1ZGUvbGludXgva3ZtX2hvc3QuaCBiL2luY2x1ZGUvbGludXgva3ZtX2hvc3Qu aAo+PiA+PiA+PiBpbmRleCA5YzI4YjRkLi43YzQyYzk0IDEwMDY0NAo+PiA+PiA+PiAtLS0gYS9p bmNsdWRlL2xpbnV4L2t2bV9ob3N0LmgKPj4gPj4gPj4gKysrIGIvaW5jbHVkZS9saW51eC9rdm1f aG9zdC5oCj4+ID4+ID4+IEBAIC0zNzQsNiArMzc0LDcgQEAgc3RydWN0IGt2bV9tZW1zbG90cyB7 Cj4+ID4+ID4+ICBzdHJ1Y3Qga3ZtIHsKPj4gPj4gPj4gIAlzcGlubG9ja190IG1tdV9sb2NrOwo+ PiA+PiA+PiAgCXN0cnVjdCBtdXRleCBzbG90c19sb2NrOwo+PiA+PiA+PiArCXN0cnVjdCBwaWQg KnBpZDsKPj4gPj4gPj4gIAlzdHJ1Y3QgbW1fc3RydWN0ICptbTsgLyogdXNlcnNwYWNlIHRpZWQg dG8gdGhpcyB2bSAqLwo+PiA+PiA+PiAgCXN0cnVjdCBrdm1fbWVtc2xvdHMgKm1lbXNsb3RzW0tW TV9BRERSRVNTX1NQQUNFX05VTV07Cj4+ID4+ID4+ICAJc3RydWN0IHNyY3Vfc3RydWN0IHNyY3U7 Cj4+ID4+ID4+IGRpZmYgLS1naXQgYS92aXJ0L2t2bS9rdm1fbWFpbi5jIGIvdmlydC9rdm0va3Zt X21haW4uYwo+PiA+PiA+PiBpbmRleCAxOTUwNzgyLi5hYjI1MzVhIDEwMDY0NAo+PiA+PiA+PiAt LS0gYS92aXJ0L2t2bS9rdm1fbWFpbi5jCj4+ID4+ID4+ICsrKyBiL3ZpcnQva3ZtL2t2bV9tYWlu LmMKPj4gPj4gPj4gQEAgLTYxMyw2ICs2MTMsNyBAQCBzdGF0aWMgc3RydWN0IGt2bSAqa3ZtX2Ny ZWF0ZV92bSh1bnNpZ25lZCBsb25nIHR5cGUpCj4+ID4+ID4+ICAJc3Bpbl9sb2NrX2luaXQoJmt2 bS0+bW11X2xvY2spOwo+PiA+PiA+PiAgCWF0b21pY19pbmMoJmN1cnJlbnQtPm1tLT5tbV9jb3Vu dCk7Cj4+ID4+ID4+ICAJa3ZtLT5tbSA9IGN1cnJlbnQtPm1tOwo+PiA+PiA+PiArCWt2bS0+cGlk ID0gZ2V0X3Rhc2tfcGlkKGN1cnJlbnQsIFBJRFRZUEVfUElEKTsKPj4gPj4gPgo+PiA+PiA+IEhv dyBkb29lcyB0aGlzIGRlYWwgd2l0aCB0aHJlYWRpbmc/ICBJcyB0aGUgaWRlYSB0aGF0IHRoZSB1 c2VyIGJ5Cj4+ID4+ID4gc3BlY2lmeWluZyB0aGUgbWFpbiB0aHJlYWQncyBwaWQgd2lsbCBlbmFi bGUgdHJhcHBpbmcgZm9yIGFsbCB2Y3B1Cj4+ID4+ID4gdGhyZWFkcyBiZWxvbmdpbmcgdG8gdGhh dCBWTT8KPj4gPj4gCj4+ID4+IFllcyB0aGF0J3MgY29ycmVjdCAtIHNwZWNpZnlpbmcgdGhlIG1h aW4gdGhyZWFkIFBJRCB3aWxsIGVuYWJsZSB0cmFwcGluZwo+PiA+PiBmb3IgdGhlIFZNIChhbGwg dmNwdXMpLgo+PiA+PiAKPj4gPj4gSSBhbSBoYXBweSB0byBtb3ZlIHRvIGEgbW9yZSBzdWl0YWJs ZSBpZGVudGlmaWVyIGlmIGF2YWlsYWJsZS4KPj4gPj4gCj4+ID4KPj4gPiBXaGF0IGlzIHRoZSAn bWFpbiB0aHJlYWQnID8KPj4gPgo+PiA+IERvZXMgc29tZXRoaW5nIG1hbmRhdGUgdGhhdCB0aGUg Vk0gaXMgY3JlYXRlZCBieSB0aGUgdGhyZWFkIGdyb3VwCj4+ID4gbGVhZGVyPyAgSWYgbm90LCBp cyBpdCBub3QgYSBiaXQgc3RyYW5nZSBmcm9tIGEgdXNlciBwZXJzcGVjdGl2ZSwgdGhhdAo+PiA+ IHlvdSBoYXZlIHRvIGZpbmQgdGhlIHNwZWNpZmljIHN1YnRocmVhZCBwaWQgdGhhdCBjcmVhdGVk IHRoZSB2bSB0bwo+PiA+IGVuYWJsZSB0aGlzIHRyYWNpbmcgZm9yIGFsbCB2Y3B1IHRocmVhZHMg YW5kIHRoYXQgdGhlIHRnaWQgZG9lc24ndCB3b3JrCj4+ID4gaW4gdGhpcyBjYXNlPwo+PiAKPj4g TGV0IG1lIGNvcnJlY3QgbXkgdGVybWlub2xvZ3kgdXNhZ2UgLSB0aGUgdmFsdWUgcmVjb3JkZWQg YWJvdmUgKGFuZCB1c2VkCj4+IHRvIGlkZW50aWZ5IHRoZSBWTSkgc2hvdWxkIGJlIHRoZSB0Z2lk LiBJdCBpcyBjb25mdXNpbmcgYmVjYXVzZSAncHMnCj4+IHJlcG9ydHMgaXQgYXMgcGlkLgo+PiAK Pj4gSSBwaWNrZWQgdGhlIHZhbHVlIGFzIGV4aXN0aW5nIEtWTSBjb2RlIGFscmVhZHkgdXNlcyB0 aGUgUElEIG9mIHRoZQo+PiBjcmVhdGluZyB0YXNrIChzZWUga3ZtX2NyZWF0ZV92bV9kZWJ1Z2Zz KSB0byBleHBvcnQgVk0gc3RhdGlzdGljcyBpbgo+PiBkZWJ1Z2ZzLgo+PiAKPj4gSWYgSSd2ZSBn b3QgdGhpcyB3cm9uZywgdGhlbiBrdm1fY3JlYXRlX3ZtX2RlYnVnZnMgYWxzbyBsaWtlbHkgbmVl ZHMgYW4KPj4gdXBkYXRlLgo+PiAKPj4gV2hhdCBkbyB5b3UgdGhpbms/Cj4+IAo+IFdoZW4geW91 IGRvIGdldF90YXNrX3BpZChjdXJyZW50LCBQSURUWVBFX1BJRCkgaXQgYWN0dWFsbHkgZ2V0cyB0 aGUKPiBrZXJuZWwgdmlldyBvZiBhIFBJRCB3aGljaCBpcyB0aGUgdGhlYWQtaWQgZnJvbSB1c2Vy c3BhY2UncyBwb2ludCBvZgo+IHZpZXcsIHJpZ2h0PwoKVGhhdCBtYWtlcyBzZW5zZS4gSXQgc2Vl bXMgdG8gd29ya3MgaGVyZSBiZWNhdXNlIHRoZSBwaWQgb2YgdGhlIGZpcnN0CnRhc2sgaXMgYWxz byB0aGUgdGdpZCBvZiB0aGUgZ3JvdXAuIEFuZCBJIHJlY2tvbiBpdCdzIHRoZSBzYW1lCmFzc3Vt cHRpb24gYmVpbmcgbWFkZSB3aXRoIGRlYnVnZnMgY29kZSAobW9yZSBiZWxvdykuCgpJJ3ZlIGNo YW5nZWQgdGhlIGZpcnN0IGFyZ3VtZW50IG9mIHRoZSBjYWxsIHRvIGdldF90YXNrX3BpZCB0bwpj dXJyZW50LT5ncm91cF9sZWFkZXIuCgo+Cj4gSSBkb24ndCBzZWUgd2h5IHRoaXMgaGFzIHRvIGJl IHRoZSBzYW1lIGFzIHRoZSBkZWJ1Z2ZzIGNvZGUsIGFzIHRoZXJlIGl0Cj4gbWFrZXMgcG90ZW50 aWFsbHkgbW9yZSBzZW5zZSB0byB0aHJlYWQtc3BlY2lmaWMsIGJ1dCBmb3IgeW91ciBjYXNlLCBh cmUKPiB5b3Ugbm90IHRhcmdldGluZyB0aGUgYmVoYXZpb3IgdGhhdCBhIHVzZXIgY2FuIGRvICJw cyBhdXggfCBncmVwIHFlbXUiCj4gb3Igd2hhdGV2ZXIsIGFuZCB0aGVuIHNldCB0cmFjaW5nIGZv ciB0aGUgcmVwb3J0ZWQgUElEICh3aGljaCBpcwo+IGFjdHVhbGx5IGEgdGdpZCk/CgpUaGUgZGVi dWdmcyBzdGF0cyBhcmUgbm90IHRocmVhZCAodmNwdSkgc3BlY2lmaWMgYnV0IGZvciB0aGUgVk0u CgpCb3RoIHZhbHVlcywgZGVidWdmcyBhbmQgaGVyZSwgYXJlIGJlaW5nIHVzZWQgdG8gcmVwcmVz ZW50IHRoZSBWTSB0byB0aGUKdXNlci4gQSBtaXNtYXRjaCBpbiB0aGVzZSBpZGVudGlmaWVycyB3 aWxsIGJlIHZlcnkgY29uZnVzaW5nLgoKSWYgeW91IGFncmVlLCBJIGNhbiBzZXBhcmF0ZWx5IHNl bmQgYSBwYXRjaCB0byBhZGRyZXNzIHRoaXMgZm9yIFZNCmRlYnVnZnMgZGlyZWN0b3J5IGFzIHdl bGwuCgo+Cj4gSWYgdGhpcyBpcyBpbmRlZWQgdGhlIGNhc2UsIHRoZW4gSSBkb24ndCB0aGluayB0 aGUgY3VycmVudCBjb2RlIHN1cHBvcnRzCj4gdGhpcyBpZiBRRU1VIHdhcyBldmVyIGNoYW5nZWQg dG8gY3JlYXRlIHRoZSBWTSB3aXRoIGEgZGlmZmVyZW50IHRocmVhZAo+IHRoYW4gdGhlIHRnbC4K Pgo+IFRoYXQgYmVpbmcgc2FpZCwgSSdtIHR5cGljYWxseSB3cm9uZyB3aGVuIEkgdGFsayBhYm91 dCB1c2Vyc3BhY2UuCj4KPiAtQ2hyaXN0b2ZmZXIKPiBfX19fX19fX19fX19fX19fX19fX19fX19f X19fX19fX19fX19fX19fX19fX19fXwo+IGt2bWFybSBtYWlsaW5nIGxpc3QKPiBrdm1hcm1AbGlz dHMuY3MuY29sdW1iaWEuZWR1Cj4gaHR0cHM6Ly9saXN0cy5jcy5jb2x1bWJpYS5lZHUvbWFpbG1h bi9saXN0aW5mby9rdm1hcm0KX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19f X19fX19fX18Ka3ZtYXJtIG1haWxpbmcgbGlzdAprdm1hcm1AbGlzdHMuY3MuY29sdW1iaWEuZWR1 Cmh0dHBzOi8vbGlzdHMuY3MuY29sdW1iaWEuZWR1L21haWxtYW4vbGlzdGluZm8va3ZtYXJtCg== From mboxrd@z Thu Jan 1 00:00:00 1970 From: punit.agrawal@arm.com (Punit Agrawal) Date: Tue, 06 Sep 2016 16:22:17 +0100 Subject: [RFC v2 PATCH 2/7] KVM: Track the pid of the VM process In-Reply-To: <20160906112203.GO30513@cbox> (Christoffer Dall's message of "Tue, 6 Sep 2016 13:22:03 +0200") References: <1473093097-30932-1-git-send-email-punit.agrawal@arm.com> <1473093097-30932-3-git-send-email-punit.agrawal@arm.com> <20160906062252.GA30513@cbox> <87poohjqzk.fsf@e105922-lin.cambridge.arm.com> <20160906102542.GG30513@cbox> <877fapjng0.fsf@e105922-lin.cambridge.arm.com> <20160906112203.GO30513@cbox> Message-ID: <87poohhx3q.fsf@e105922-lin.cambridge.arm.com> To: linux-arm-kernel@lists.infradead.org List-Id: linux-arm-kernel.lists.infradead.org Christoffer Dall writes: > On Tue, Sep 06, 2016 at 12:07:59PM +0100, Punit Agrawal wrote: >> Christoffer Dall writes: >> >> > On Tue, Sep 06, 2016 at 10:51:27AM +0100, Punit Agrawal wrote: >> >> Hi Christoffer, >> >> >> >> Christoffer Dall 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 >> >> >> Cc: Paolo Bonzini >> >> >> Cc: "Radim Kr?m??" >> >> >> Cc: Christoffer Dall >> >> >> Cc: Marc Zyngier >> >> >> --- >> >> >> 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(¤t->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? >> > When you do get_task_pid(current, PIDTYPE_PID) it actually gets the > kernel view of a PID which is the thead-id from userspace's point of > view, right? That makes sense. It seems to works here because the pid of the first task is also the tgid of the group. And I reckon it's the same assumption being made with debugfs code (more below). I've changed the first argument of the call to get_task_pid to current->group_leader. > > I don't see why this has to be the same as the debugfs code, as there it > makes potentially more sense to thread-specific, but for your case, are > you not targeting the behavior that a user can do "ps aux | grep qemu" > or whatever, and then set tracing for the reported PID (which is > actually a tgid)? The debugfs stats are not thread (vcpu) specific but for the VM. Both values, debugfs and here, are being used to represent the VM to the user. A mismatch in these identifiers will be very confusing. If you agree, I can separately send a patch to address this for VM debugfs directory as well. > > If this is indeed the case, then I don't think the current code supports > this if QEMU was ever changed to create the VM with a different thread > than the tgl. > > That being said, I'm typically wrong when I talk about userspace. > > -Christoffer > _______________________________________________ > kvmarm mailing list > kvmarm at lists.cs.columbia.edu > https://lists.cs.columbia.edu/mailman/listinfo/kvmarm