Re: [PATCH v3 4/5] Sysfs: Export VMCSINFO via sysfs

2012-07-03 Thread Yanfei Zhang 
于 2012年06月29日 10:58, Greg KH 写道:
 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:
 于 2012年06月28日 03:22, Greg KH 写道:
 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 zhangyan...@cn.fujitsu.com
 ---
  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 linux/async.h
  #include linux/pm_runtime.h
  #include linux/netdevice.h
 +#include asm/vmcsinfo.h

 Did you just break the build on all other arches?  Not nice.

 @@ -1038,6 +1039,11 @@ int device_add(struct device *dev)
   error = dpm_sysfs_add(dev);
   if (error)
   goto DPMError;
 +#if defined(CONFIG_KVM_INTEL) || defined(CONFIG_KVM_INTEL_MODULE)
 + error = 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
  
 +#if defined(CONFIG_KVM_INTEL) || defined(CONFIG_KVM_INTEL_MODULE)
 +#include asm/vmcsinfo.h
 +#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.
 
 Ok, as others have rightly pointed out, this wasn't the most helpful
 review comment, sorry about that.
 
 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.
 
 But, for this, I would question why you even want / need to do this in
 the drivers/base/core/ file in the first place.  Shouldn't it be in some
 arch or cpu specific file instead that already handles the cpu files?
 
 thanks,
 
 greg k-h
 

Many thanks. I have moved the code to my vmcsinfo_intel module.
Thanks again for your helpful comment.

Thanks
Zhang Yanfei
--
To unsubscribe from this list: send the line unsubscribe kvm in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v3 4/5] Sysfs: Export VMCSINFO via sysfs

2012-06-28 Thread Yanfei Zhang 
于 2012年06月28日 03:22, Greg KH 写道:
 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 zhangyan...@cn.fujitsu.com
 ---
  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 linux/async.h
  #include linux/pm_runtime.h
  #include linux/netdevice.h
 +#include asm/vmcsinfo.h
 
 Did you just break the build on all other arches?  Not nice.
 
 @@ -1038,6 +1039,11 @@ int device_add(struct device *dev)
  error = dpm_sysfs_add(dev);
  if (error)
  goto DPMError;
 +#if defined(CONFIG_KVM_INTEL) || defined(CONFIG_KVM_INTEL_MODULE)
 +error = 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
 
+#if defined(CONFIG_KVM_INTEL) || defined(CONFIG_KVM_INTEL_MODULE)
+#include asm/vmcsinfo.h
+#else
+static inline int vmcs_sysfs_add(struct device *dev) { return 0; }
+static inline void vmcs_sysfs_remove(struct device *dev) { }
+#endif
+
 #ifdef CONFIG_SYSFS_DEPRECATED
 #ifdef CONFIG_SYSFS_DEPRECATED_V2
 long sysfs_deprecated = 1;
@@ -1038,6 +1045,9 @@ int device_add(struct device *dev)
error = dpm_sysfs_add(dev);
if (error)
goto DPMError;
+   error = vmcs_sysfs_add(dev);
+   if (error)
+   goto VMCSError;
device_pm_add(dev);
 
/* Notify clients of device addition.  This call must come
@@ -1069,6 +1079,8 @@ int device_add(struct device *dev)
 done:
put_device(dev);
return error;
+ VMCSError:
+   dpm_sysfs_remove(dev);
  DPMError:
bus_remove_device(dev);
  BusError:
@@ -1171,6 +1183,7 @@ void device_del(struct device *dev)
blocking_notifier_call_chain(dev-bus-p-bus_notifier,
 BUS_NOTIFY_DEL_DEVICE, dev);
device_pm_remove(dev);
+   vmcs_sysfs_remove(dev);
dpm_sysfs_remove(dev);
if (parent)
klist_del(dev-p-knode_parent);
-- 
1.7.1

--
To unsubscribe from this list: send the line unsubscribe kvm in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v3 4/5] Sysfs: Export VMCSINFO via sysfs

2012-06-28 Thread Greg KH 
On Thu, Jun 28, 2012 at 05:54:30PM +0800, Yanfei Zhang wrote:
 于 2012年06月28日 03:22, Greg KH 写道:
  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 zhangyan...@cn.fujitsu.com
  ---
   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 linux/async.h
   #include linux/pm_runtime.h
   #include linux/netdevice.h
  +#include asm/vmcsinfo.h
  
  Did you just break the build on all other arches?  Not nice.
  
  @@ -1038,6 +1039,11 @@ int device_add(struct device *dev)
 error = dpm_sysfs_add(dev);
 if (error)
 goto DPMError;
  +#if defined(CONFIG_KVM_INTEL) || defined(CONFIG_KVM_INTEL_MODULE)
  +  error = 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
  
 +#if defined(CONFIG_KVM_INTEL) || defined(CONFIG_KVM_INTEL_MODULE)
 +#include asm/vmcsinfo.h
 +#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.

greg k-h
--
To unsubscribe from this list: send the line unsubscribe kvm in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v3 4/5] Sysfs: Export VMCSINFO via sysfs

2012-06-28 Thread Greg KH 
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:
  于 2012年06月28日 03:22, Greg KH 写道:
   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 zhangyan...@cn.fujitsu.com
   ---
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 linux/async.h
#include linux/pm_runtime.h
#include linux/netdevice.h
   +#include asm/vmcsinfo.h
   
   Did you just break the build on all other arches?  Not nice.
   
   @@ -1038,6 +1039,11 @@ int device_add(struct device *dev)
error = dpm_sysfs_add(dev);
if (error)
goto DPMError;
   +#if defined(CONFIG_KVM_INTEL) || defined(CONFIG_KVM_INTEL_MODULE)
   +error = 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
   
  +#if defined(CONFIG_KVM_INTEL) || defined(CONFIG_KVM_INTEL_MODULE)
  +#include asm/vmcsinfo.h
  +#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.

Ok, as others have rightly pointed out, this wasn't the most helpful
review comment, sorry about that.

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.

But, for this, I would question why you even want / need to do this in
the drivers/base/core/ file in the first place.  Shouldn't it be in some
arch or cpu specific file instead that already handles the cpu files?

thanks,

greg k-h
--
To unsubscribe from this list: send the line unsubscribe kvm in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v3 4/5] Sysfs: Export VMCSINFO via sysfs

2012-06-27 Thread Greg KH 
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 zhangyan...@cn.fujitsu.com
 ---
  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 linux/async.h
  #include linux/pm_runtime.h
  #include linux/netdevice.h
 +#include asm/vmcsinfo.h

Did you just break the build on all other arches?  Not nice.

 @@ -1038,6 +1039,11 @@ int device_add(struct device *dev)
   error = dpm_sysfs_add(dev);
   if (error)
   goto DPMError;
 +#if defined(CONFIG_KVM_INTEL) || defined(CONFIG_KVM_INTEL_MODULE)
 + error = 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
--
To unsubscribe from this list: send the line unsubscribe kvm in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html