[PATCH] memory-hotplug: add 0x prefix to HEX block_size_bytes

[PATCH] memory-hotplug: add 0x prefix to HEX block_size_bytes

[Wu Fengguang]

CC: Andi Kleen <andi@firstfloor.org> 
Signed-off-by: Wu Fengguang <fengguang.wu@intel.com>
---
 drivers/base/memory.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

--- linux-mm.orig/drivers/base/memory.c	2010-01-14 19:55:40.000000000 +0800
+++ linux-mm/drivers/base/memory.c	2010-01-14 19:55:47.000000000 +0800
@@ -311,7 +311,7 @@ static SYSDEV_ATTR(removable, 0444, show
 static ssize_t
 print_block_size(struct class *class, char *buf)
 {
-	return sprintf(buf, "%lx\n", (unsigned long)PAGES_PER_SECTION * PAGE_SIZE);
+	return sprintf(buf, "%#lx\n", (unsigned long)PAGES_PER_SECTION * PAGE_SIZE);
 }
 
 static CLASS_ATTR(block_size_bytes, 0444, print_block_size, NULL);

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

[PATCH] sysdev: fix prototype for memory_sysdev_class show/store functions

[Wu Fengguang]

The function prototype mismatches in call stack:

                [<ffffffff81494268>] print_block_size+0x58/0x60
                [<ffffffff81487e3f>] sysdev_class_show+0x1f/0x30
                [<ffffffff811d629b>] sysfs_read_file+0xcb/0x1f0
                [<ffffffff81176328>] vfs_read+0xc8/0x180

Due to prototype mismatch, print_block_size() will sprintf() into
*attribute instead of *buf, hence user space will read the initial
zeros from *buf:
	$ hexdump /sys/devices/system/memory/block_size_bytes
	0000000 0000 0000 0000 0000
	0000008

After patch:
	cat /sys/devices/system/memory/block_size_bytes
	0x8000000

This complements commits c29af9636 and 4a0b2b4dbe.

CC: Andi Kleen <andi@firstfloor.org> 
CC: Greg Kroah-Hartman <gregkh@suse.de> 
CC: "Zheng, Shaohui" <shaohui.zheng@intel.com>
Signed-off-by: Wu Fengguang <fengguang.wu@intel.com>
---
 drivers/base/memory.c |   32 ++++++++++++++++++++------------
 1 file changed, 20 insertions(+), 12 deletions(-)

--- linux-mm.orig/drivers/base/memory.c	2010-01-14 20:03:00.000000000 +0800
+++ linux-mm/drivers/base/memory.c	2010-01-14 20:03:01.000000000 +0800
@@ -309,17 +309,19 @@ static SYSDEV_ATTR(removable, 0444, show
  * Block size attribute stuff
  */
 static ssize_t
-print_block_size(struct class *class, char *buf)
+print_block_size(struct sysdev_class *class,
+		 struct sysdev_class_attribute *class_attr,
+		 char *buf)
 {
 	return sprintf(buf, "%#lx\n", (unsigned long)PAGES_PER_SECTION * PAGE_SIZE);
 }
 
-static CLASS_ATTR(block_size_bytes, 0444, print_block_size, NULL);
+static SYSDEV_CLASS_ATTR(block_size_bytes, 0444, print_block_size, NULL);
 
 static int block_size_init(void)
 {
 	return sysfs_create_file(&memory_sysdev_class.kset.kobj,
-				&class_attr_block_size_bytes.attr);
+				&attr_block_size_bytes.attr);
 }
 
 /*
@@ -330,7 +332,9 @@ static int block_size_init(void)
  */
 #ifdef CONFIG_ARCH_MEMORY_PROBE
 static ssize_t
-memory_probe_store(struct class *class, const char *buf, size_t count)
+memory_probe_store(struct sysdev_class *class,
+		   struct sysdev_class_attribute *class_attr,
+		   const char *buf, size_t count)
 {
 	u64 phys_addr;
 	int nid;
@@ -346,12 +350,12 @@ memory_probe_store(struct class *class, 
 
 	return count;
 }
-static CLASS_ATTR(probe, S_IWUSR, NULL, memory_probe_store);
+static SYSDEV_CLASS_ATTR(probe, S_IWUSR, NULL, memory_probe_store);
 
 static int memory_probe_init(void)
 {
 	return sysfs_create_file(&memory_sysdev_class.kset.kobj,
-				&class_attr_probe.attr);
+				&attr_probe.attr);
 }
 #else
 static inline int memory_probe_init(void)
@@ -367,7 +371,9 @@ static inline int memory_probe_init(void
 
 /* Soft offline a page */
 static ssize_t
-store_soft_offline_page(struct class *class, const char *buf, size_t count)
+store_soft_offline_page(struct sysdev_class *class,
+			struct sysdev_class_attribute *class_attr,
+			const char *buf, size_t count)
 {
 	int ret;
 	u64 pfn;
@@ -384,7 +390,9 @@ store_soft_offline_page(struct class *cl
 
 /* Forcibly offline a page, including killing processes. */
 static ssize_t
-store_hard_offline_page(struct class *class, const char *buf, size_t count)
+store_hard_offline_page(struct sysdev_class *class,
+			struct sysdev_class_attribute *class_attr,
+			const char *buf, size_t count)
 {
 	int ret;
 	u64 pfn;
@@ -397,18 +405,18 @@ store_hard_offline_page(struct class *cl
 	return ret ? ret : count;
 }
 
-static CLASS_ATTR(soft_offline_page, 0644, NULL, store_soft_offline_page);
-static CLASS_ATTR(hard_offline_page, 0644, NULL, store_hard_offline_page);
+static SYSDEV_CLASS_ATTR(soft_offline_page, 0644, NULL, store_soft_offline_page);
+static SYSDEV_CLASS_ATTR(hard_offline_page, 0644, NULL, store_hard_offline_page);
 
 static __init int memory_fail_init(void)
 {
 	int err;
 
 	err = sysfs_create_file(&memory_sysdev_class.kset.kobj,
-				&class_attr_soft_offline_page.attr);
+				&attr_soft_offline_page.attr);
 	if (!err)
 		err = sysfs_create_file(&memory_sysdev_class.kset.kobj,
-				&class_attr_hard_offline_page.attr);
+				&attr_hard_offline_page.attr);
 	return err;
 }
 #else

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH] sysdev: fix prototype for memory_sysdev_class show/store functions

[Andi Kleen]

On Thu, Jan 14, 2010 at 08:04:19PM +0800, Wu Fengguang wrote:
> The function prototype mismatches in call stack:
> 
>                 [<ffffffff81494268>] print_block_size+0x58/0x60
>                 [<ffffffff81487e3f>] sysdev_class_show+0x1f/0x30
>                 [<ffffffff811d629b>] sysfs_read_file+0xcb/0x1f0
>                 [<ffffffff81176328>] vfs_read+0xc8/0x180
> 
> Due to prototype mismatch, print_block_size() will sprintf() into
> *attribute instead of *buf, hence user space will read the initial
> zeros from *buf:
> 	$ hexdump /sys/devices/system/memory/block_size_bytes
> 	0000000 0000 0000 0000 0000
> 	0000008
> 
> After patch:
> 	cat /sys/devices/system/memory/block_size_bytes
> 	0x8000000
> 
> This complements commits c29af9636 and 4a0b2b4dbe.

Hmm, this was already fixed in my patch in the original series

SYSFS: Fix type of sysdev class attribute in memory driver

This attribute is really a sysdev_class attribute, not a plain class attribute.

They are identical in layout currently, but this might not always be 
the case.

And with the final patches they were identical in layout again anyways.

I don't know why Greg didn't merge that one. Greg, did you forget
some patches?

For the record the full series was:

SYSFS: Pass attribute in sysdev_class attributes show/store
SYSFS: Convert node driver class attributes to be data driven
SYSDEV: Convert cpu driver sysdev class attributes 
SYSFS: Add sysfs_add/remove_files utility functions
SYSFS: Add attribute array to sysdev classes
SYSDEV: Convert node driver 
SYSDEV: Use sysdev_class attribute arrays in node driver
SYSFS: Add sysdev_create/remove_files
SYSFS: Fix type of sysdev class attribute in memory driver
SYSDEV: Add attribute argument to class_attribute show/store
SYSFS: Add class_attr_string for simple read-only string
SYSFS: Convert some drivers to CLASS_ATTR_STRING

-Andi

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH] sysdev: fix prototype for memory_sysdev_class show/store functions

[Wu Fengguang]

> I don't know why Greg didn't merge that one. Greg, did you forget
> some patches?

I guess yes - more than one.
 
> For the record the full series was:
> 
> SYSFS: Pass attribute in sysdev_class attributes show/store
> SYSFS: Convert node driver class attributes to be data driven
> SYSDEV: Convert cpu driver sysdev class attributes 
> SYSFS: Add sysfs_add/remove_files utility functions
> SYSFS: Add attribute array to sysdev classes
> SYSDEV: Convert node driver 
> SYSDEV: Use sysdev_class attribute arrays in node driver
> SYSFS: Add sysdev_create/remove_files
> SYSFS: Fix type of sysdev class attribute in memory driver
> SYSDEV: Add attribute argument to class_attribute show/store
> SYSFS: Add class_attr_string for simple read-only string
> SYSFS: Convert some drivers to CLASS_ATTR_STRING
 
Only the first 3 of them reach linux-next-20100114:

commit 86950fd010b72acf3c7d4ae8c1df440fef09c2b5
Author: Andi Kleen <andi@firstfloor.org>
Date:   Tue Jan 5 12:48:00 2010 +0100

    sysdev: Convert cpu driver sysdev class attributes
    
commit c314abcfb978695949f9314ea9f1b6eea5dbd225
Author: Andi Kleen <andi@firstfloor.org>
Date:   Tue Jan 5 12:47:59 2010 +0100

    sysdev: Convert node driver class attributes to be data driven
    
commit c29af9636774ea95c795b2ddb71e7dc6e50861bf
Author: Andi Kleen <andi@firstfloor.org>
Date:   Tue Jan 5 12:47:58 2010 +0100

    sysdev: Pass attribute in sysdev_class attributes show/store


Others may need resubmission.

Thanks,
Fengguang

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH] sysdev: fix prototype for memory_sysdev_class show/store functions

[Greg KH]

On Mon, Jan 18, 2010 at 09:07:02PM +0800, Wu Fengguang wrote:
> > I don't know why Greg didn't merge that one. Greg, did you forget
> > some patches?
> 
> I guess yes - more than one.

They are all now in my tree.

thanks,

greg k-h

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH] memory-hotplug: add 0x prefix to HEX block_size_bytes

[Andrew Morton]

On Thu, 14 Jan 2010 19:59:56 +0800
Wu Fengguang <fengguang.wu@intel.com> wrote:

> CC: Andi Kleen <andi@firstfloor.org> 
> Signed-off-by: Wu Fengguang <fengguang.wu@intel.com>
> ---
>  drivers/base/memory.c |    2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> --- linux-mm.orig/drivers/base/memory.c	2010-01-14 19:55:40.000000000 +0800
> +++ linux-mm/drivers/base/memory.c	2010-01-14 19:55:47.000000000 +0800
> @@ -311,7 +311,7 @@ static SYSDEV_ATTR(removable, 0444, show
>  static ssize_t
>  print_block_size(struct class *class, char *buf)
>  {
> -	return sprintf(buf, "%lx\n", (unsigned long)PAGES_PER_SECTION * PAGE_SIZE);
> +	return sprintf(buf, "%#lx\n", (unsigned long)PAGES_PER_SECTION * PAGE_SIZE);
>  }
>  
>  static CLASS_ATTR(block_size_bytes, 0444, print_block_size, NULL);

crappy changelog!

Why this change?  Perhaps showing us an example of the before-and-after
output would help us see what is being fixed, and why.

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH] memory-hotplug: add 0x prefix to HEX block_size_bytes

[Wu Fengguang]

> > -	return sprintf(buf, "%lx\n", (unsigned long)PAGES_PER_SECTION * PAGE_SIZE);
> > +	return sprintf(buf, "%#lx\n", (unsigned long)PAGES_PER_SECTION * PAGE_SIZE);
 
> crappy changelog!
> 
> Why this change?  Perhaps showing us an example of the before-and-after
> output would help us see what is being fixed, and why.

Sorry for being late (some SMTP problem).

                # cat /sys/devices/system/memory/block_size_bytes
before patch:   8000000
after  patch:   0x8000000

This is a good fix because someone is very likely to mistake 8000000
as a decimal number. 0x8000000 looks much better.

Thanks,
Fengguang

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH] memory-hotplug: add 0x prefix to HEX block_size_bytes

[KOSAKI Motohiro]

> > > -	return sprintf(buf, "%lx\n", (unsigned long)PAGES_PER_SECTION * PAGE_SIZE);
> > > +	return sprintf(buf, "%#lx\n", (unsigned long)PAGES_PER_SECTION * PAGE_SIZE);
>  
> > crappy changelog!
> > 
> > Why this change?  Perhaps showing us an example of the before-and-after
> > output would help us see what is being fixed, and why.
> 
> Sorry for being late (some SMTP problem).
> 
>                 # cat /sys/devices/system/memory/block_size_bytes
> before patch:   8000000
> after  patch:   0x8000000
> 
> This is a good fix because someone is very likely to mistake 8000000
> as a decimal number. 0x8000000 looks much better.

I'm sorry. NAK.

print_block_size() was introduced at 2005. So, we can't assume any
programs don't use it.

Your patch is good, but too late...



--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>