From mboxrd@z Thu Jan 1 00:00:00 1970 Return-path: Received: from [222.73.24.84] (helo=song.cn.fujitsu.com) by merlin.infradead.org with esmtp (Exim 4.76 #1 (Red Hat Linux)) id 1Slz62-0007Nz-4y for kexec@lists.infradead.org; Tue, 03 Jul 2012 09:08:41 +0000 Message-ID: <4FF2B5CF.2000707@cn.fujitsu.com> Date: Tue, 03 Jul 2012 17:05:19 +0800 From: Yanfei Zhang MIME-Version: 1.0 Subject: Re: [PATCH v3 4/5] Sysfs: Export VMCSINFO via sysfs References: <4FEAC945.50700@cn.fujitsu.com> <4FEACA5E.4090009@cn.fujitsu.com> <20120627192236.GB1965@kroah.com> <4FEC29D6.5020109@cn.fujitsu.com> <20120628113738.GA5499@kroah.com> <20120629025848.GC8074@kroah.com> In-Reply-To: <20120629025848.GC8074@kroah.com> List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: base64 Sender: kexec-bounces@lists.infradead.org Errors-To: kexec-bounces+dwmw2=infradead.org@lists.infradead.org To: Greg KH Cc: dzickus@redhat.com, luto@mit.edu, kvm@vger.kernel.org, Joerg Roedel , mtosatti@redhat.com, kexec@lists.infradead.org, linux-kernel@vger.kernel.org, paul.gortmaker@windriver.com, ludwig.nussel@suse.de, Avi Kivity , ebiederm@xmission.com 5LqOIDIwMTLlubQwNuaciDI55pelIDEwOjU4LCBHcmVnIEtIIOWGmemBkzoKPiBPbiBUaHUsIEp1 biAyOCwgMjAxMiBhdCAwNDozNzozOEFNIC0wNzAwLCBHcmVnIEtIIHdyb3RlOgo+PiBPbiBUaHUs IEp1biAyOCwgMjAxMiBhdCAwNTo1NDozMFBNICswODAwLCBZYW5mZWkgWmhhbmcgd3JvdGU6Cj4+ PiDkuo4gMjAxMuW5tDA25pyIMjjml6UgMDM6MjIsIEdyZWcgS0gg5YaZ6YGTOgo+Pj4+IE9uIFdl ZCwgSnVuIDI3LCAyMDEyIGF0IDA0OjU0OjU0UE0gKzA4MDAsIFlhbmZlaSBaaGFuZyB3cm90ZToK Pj4+Pj4gVGhpcyBwYXRjaCBleHBvcnQgb2Zmc2V0cyBvZiBmaWVsZHMgdmlhIC9zeXMvZGV2aWNl cy9jcHUvdm1jcy8uCj4+Pj4+IEluZGl2aWR1YWwgb2Zmc2V0cyBhcmUgY29udGFpbmVkIGluIHN1 YmZpbGVzIG5hbWVkIGJ5IHRoZSBmaWxlZCdzCj4+Pj4+IGVuY29kaW5nLCBlLmcuOiAvc3lzL2Rl dmljZXMvY3B1L3ZtY3MvMDgwMAo+Pj4+Pgo+Pj4+PiBTaWduZWQtb2ZmLWJ5OiB6aGFuZ3lhbmZl aSA8emhhbmd5YW5mZWlAY24uZnVqaXRzdS5jb20+Cj4+Pj4+IC0tLQo+Pj4+PiAgZHJpdmVycy9i YXNlL2NvcmUuYyB8ICAgMTMgKysrKysrKysrKysrKwo+Pj4+PiAgMSBmaWxlcyBjaGFuZ2VkLCAx MyBpbnNlcnRpb25zKCspLCAwIGRlbGV0aW9ucygtKQo+Pj4+Pgo+Pj4+PiBkaWZmIC0tZ2l0IGEv ZHJpdmVycy9iYXNlL2NvcmUuYyBiL2RyaXZlcnMvYmFzZS9jb3JlLmMKPj4+Pj4gaW5kZXggMzQ2 YmU4Yi4uZGQwNWVlNyAxMDA2NDQKPj4+Pj4gLS0tIGEvZHJpdmVycy9iYXNlL2NvcmUuYwo+Pj4+ PiArKysgYi9kcml2ZXJzL2Jhc2UvY29yZS5jCj4+Pj4+IEBAIC0yNiw2ICsyNiw3IEBACj4+Pj4+ ICAjaW5jbHVkZSA8bGludXgvYXN5bmMuaD4KPj4+Pj4gICNpbmNsdWRlIDxsaW51eC9wbV9ydW50 aW1lLmg+Cj4+Pj4+ICAjaW5jbHVkZSA8bGludXgvbmV0ZGV2aWNlLmg+Cj4+Pj4+ICsjaW5jbHVk ZSA8YXNtL3ZtY3NpbmZvLmg+Cj4+Pj4KPj4+PiBEaWQgeW91IGp1c3QgYnJlYWsgdGhlIGJ1aWxk IG9uIGFsbCBvdGhlciBhcmNoZXM/ICBOb3QgbmljZS4KPj4+Pgo+Pj4+PiBAQCAtMTAzOCw2ICsx MDM5LDExIEBAIGludCBkZXZpY2VfYWRkKHN0cnVjdCBkZXZpY2UgKmRldikKPj4+Pj4gIAllcnJv ciA9IGRwbV9zeXNmc19hZGQoZGV2KTsKPj4+Pj4gIAlpZiAoZXJyb3IpCj4+Pj4+ICAJCWdvdG8g RFBNRXJyb3I7Cj4+Pj4+ICsjaWYgZGVmaW5lZChDT05GSUdfS1ZNX0lOVEVMKSB8fCBkZWZpbmVk KENPTkZJR19LVk1fSU5URUxfTU9EVUxFKQo+Pj4+PiArCWVycm9yID0gdm1jc19zeXNmc19hZGQo ZGV2KTsKPj4+Pj4gKwlpZiAoZXJyb3IpCj4+Pj4+ICsJCWdvdG8gVk1DU0Vycm9yOwo+Pj4+PiAr I2VuZGlmCj4+Pj4KPj4+PiBPaCBteSBubywgdGhhdCdzIG5vIHdheSB0byBldmVyIGRvIHRoaXMs IHlvdSBrbm93IGJldHRlciB0aGFuIHRoYXQsCj4+Pj4gcGxlYXNlIGZpeC4KPj4+Pgo+Pj4+IGdy ZWcgay1oCj4+Pj4KPj4+Cj4+PiBTb3JyeSBmb3IgbXkgdGhvdWdodGxlc3MsIEhlcmUgaXMgdGhl IG5ldyBwYXRjaC4KPj4+Cj4+PiAtLS0KPj4+ICBkcml2ZXJzL2Jhc2UvY29yZS5jIHwgICAxMyAr KysrKysrKysrKysrCj4+PiAgMSBmaWxlcyBjaGFuZ2VkLCAxMyBpbnNlcnRpb25zKCspLCAwIGRl bGV0aW9ucygtKQo+Pj4KPj4+IGRpZmYgLS1naXQgYS9kcml2ZXJzL2Jhc2UvY29yZS5jIGIvZHJp dmVycy9iYXNlL2NvcmUuYwo+Pj4gaW5kZXggMzQ2YmU4Yi4uN2I1MjY2YSAxMDA2NDQKPj4+IC0t LSBhL2RyaXZlcnMvYmFzZS9jb3JlLmMKPj4+ICsrKyBiL2RyaXZlcnMvYmFzZS9jb3JlLmMKPj4+ IEBAIC0zMCw2ICszMCwxMyBAQAo+Pj4gICNpbmNsdWRlICJiYXNlLmgiCj4+PiAgI2luY2x1ZGUg InBvd2VyL3Bvd2VyLmgiCj4+PiAgCj4+PiArI2lmIGRlZmluZWQoQ09ORklHX0tWTV9JTlRFTCkg fHwgZGVmaW5lZChDT05GSUdfS1ZNX0lOVEVMX01PRFVMRSkKPj4+ICsjaW5jbHVkZSA8YXNtL3Zt Y3NpbmZvLmg+Cj4+PiArI2Vsc2UKPj4+ICtzdGF0aWMgaW5saW5lIGludCB2bWNzX3N5c2ZzX2Fk ZChzdHJ1Y3QgZGV2aWNlICpkZXYpIHsgcmV0dXJuIDA7IH0KPj4+ICtzdGF0aWMgaW5saW5lIHZv aWQgdm1jc19zeXNmc19yZW1vdmUoc3RydWN0IGRldmljZSAqZGV2KSB7IH0KPj4+ICsjZW5kaWYK Pj4KPj4ge3NpZ2h9ICBObywgYWdhaW4sIHlvdSBrbm93IGJldHRlciwgZG9uJ3QgZG8gdGhpcy4K PiAKPiBPaywgYXMgb3RoZXJzIGhhdmUgcmlnaHRseSBwb2ludGVkIG91dCwgdGhpcyB3YXNuJ3Qg dGhlIG1vc3QgaGVscGZ1bAo+IHJldmlldyBjb21tZW50LCBzb3JyeSBhYm91dCB0aGF0Lgo+IAo+ IEluIExpbnV4LCB3ZSBkb24ndCBwdXQgaWZkZWZzIGluIC5jIGZpbGVzLCB3ZSBwdXQgdGhlbSBp biAuaCBmaWxlcy4gIFNlZQo+IG1hbnkgZXhhbXBsZXMgb2YgdGhpcyBhbGwgb3ZlciB0aGUgcGxh Y2UuICBUaGF0J3MgbXkgbWFpbiBjb21wbGFpbnRzIHRoZQo+IHBhc3QgdHdvIHRpbWVzIG9mIHRo aXMgcGF0Y2guCj4gCj4gQnV0LCBmb3IgdGhpcywgSSB3b3VsZCBxdWVzdGlvbiB3aHkgeW91IGV2 ZW4gd2FudCAvIG5lZWQgdG8gZG8gdGhpcyBpbgo+IHRoZSBkcml2ZXJzL2Jhc2UvY29yZS8gZmls ZSBpbiB0aGUgZmlyc3QgcGxhY2UuICBTaG91bGRuJ3QgaXQgYmUgaW4gc29tZQo+IGFyY2ggb3Ig Y3B1IHNwZWNpZmljIGZpbGUgaW5zdGVhZCB0aGF0IGFscmVhZHkgaGFuZGxlcyB0aGUgY3B1IGZp bGVzPwo+IAo+IHRoYW5rcywKPiAKPiBncmVnIGstaAo+IAoKTWFueSB0aGFua3MuIEkgaGF2ZSBt b3ZlZCB0aGUgY29kZSB0byBteSB2bWNzaW5mb19pbnRlbCBtb2R1bGUuClRoYW5rcyBhZ2FpbiBm b3IgeW91ciBoZWxwZnVsIGNvbW1lbnQuCgpUaGFua3MKWmhhbmcgWWFuZmVpCgpfX19fX19fX19f X19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fXwprZXhlYyBtYWlsaW5nIGxpc3QK a2V4ZWNAbGlzdHMuaW5mcmFkZWFkLm9yZwpodHRwOi8vbGlzdHMuaW5mcmFkZWFkLm9yZy9tYWls bWFuL2xpc3RpbmZvL2tleGVjCg== From mboxrd@z Thu Jan 1 00:00:00 1970 From: Yanfei Zhang Subject: Re: [PATCH v3 4/5] Sysfs: Export VMCSINFO via sysfs Date: Tue, 03 Jul 2012 17:05:19 +0800 Message-ID: <4FF2B5CF.2000707@cn.fujitsu.com> References: <4FEAC945.50700@cn.fujitsu.com> <4FEACA5E.4090009@cn.fujitsu.com> <20120627192236.GB1965@kroah.com> <4FEC29D6.5020109@cn.fujitsu.com> <20120628113738.GA5499@kroah.com> <20120629025848.GC8074@kroah.com> Mime-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: QUOTED-PRINTABLE Cc: Avi Kivity , mtosatti@redhat.com, ebiederm@xmission.com, luto@mit.edu, Joerg Roedel , dzickus@redhat.com, paul.gortmaker@windriver.com, ludwig.nussel@suse.de, linux-kernel@vger.kernel.org, kvm@vger.kernel.org, kexec@lists.infradead.org To: Greg KH Return-path: In-Reply-To: <20120629025848.GC8074@kroah.com> Sender: linux-kernel-owner@vger.kernel.org List-Id: kvm.vger.kernel.org =E4=BA=8E 2012=E5=B9=B406=E6=9C=8829=E6=97=A5 10:58, Greg KH =E5=86=99=E9= =81=93: > On Thu, Jun 28, 2012 at 04:37:38AM -0700, Greg KH wrote: >> On Thu, Jun 28, 2012 at 05:54:30PM +0800, Yanfei Zhang wrote: >>> =E4=BA=8E 2012=E5=B9=B406=E6=9C=8828=E6=97=A5 03:22, Greg KH =E5=86= =99=E9=81=93: >>>> On Wed, Jun 27, 2012 at 04:54:54PM +0800, Yanfei Zhang wrote: >>>>> This patch export offsets of fields via /sys/devices/cpu/vmcs/. >>>>> Individual offsets are contained in subfiles named by the filed's >>>>> encoding, e.g.: /sys/devices/cpu/vmcs/0800 >>>>> >>>>> Signed-off-by: zhangyanfei >>>>> --- >>>>> drivers/base/core.c | 13 +++++++++++++ >>>>> 1 files changed, 13 insertions(+), 0 deletions(-) >>>>> >>>>> diff --git a/drivers/base/core.c b/drivers/base/core.c >>>>> index 346be8b..dd05ee7 100644 >>>>> --- a/drivers/base/core.c >>>>> +++ b/drivers/base/core.c >>>>> @@ -26,6 +26,7 @@ >>>>> #include >>>>> #include >>>>> #include >>>>> +#include >>>> >>>> Did you just break the build on all other arches? Not nice. >>>> >>>>> @@ -1038,6 +1039,11 @@ int device_add(struct device *dev) >>>>> error =3D dpm_sysfs_add(dev); >>>>> if (error) >>>>> goto DPMError; >>>>> +#if defined(CONFIG_KVM_INTEL) || defined(CONFIG_KVM_INTEL_MODULE= ) >>>>> + error =3D vmcs_sysfs_add(dev); >>>>> + if (error) >>>>> + goto VMCSError; >>>>> +#endif >>>> >>>> Oh my no, that's no way to ever do this, you know better than that= , >>>> please fix. >>>> >>>> greg k-h >>>> >>> >>> Sorry for my thoughtless, Here is the new patch. >>> >>> --- >>> drivers/base/core.c | 13 +++++++++++++ >>> 1 files changed, 13 insertions(+), 0 deletions(-) >>> >>> diff --git a/drivers/base/core.c b/drivers/base/core.c >>> index 346be8b..7b5266a 100644 >>> --- a/drivers/base/core.c >>> +++ b/drivers/base/core.c >>> @@ -30,6 +30,13 @@ >>> #include "base.h" >>> #include "power/power.h" >>> =20 >>> +#if defined(CONFIG_KVM_INTEL) || defined(CONFIG_KVM_INTEL_MODULE) >>> +#include >>> +#else >>> +static inline int vmcs_sysfs_add(struct device *dev) { return 0; } >>> +static inline void vmcs_sysfs_remove(struct device *dev) { } >>> +#endif >> >> {sigh} No, again, you know better, don't do this. >=20 > Ok, as others have rightly pointed out, this wasn't the most helpful > review comment, sorry about that. >=20 > In Linux, we don't put ifdefs in .c files, we put them in .h files. = See > many examples of this all over the place. That's my main complaints = the > past two times of this patch. >=20 > But, for this, I would question why you even want / need to do this i= n > the drivers/base/core/ file in the first place. Shouldn't it be in s= ome > arch or cpu specific file instead that already handles the cpu files? >=20 > thanks, >=20 > greg k-h >=20 Many thanks. I have moved the code to my vmcsinfo_intel module. Thanks again for your helpful comment. Thanks Zhang Yanfei