Skip to contentSkip to conversation

memory-hotplug: add 0x prefix to HEX block_size_bytes

PATCH In review Wu Fengguang 5 participants 8 messages Jan 14, 2010 – Jan 20, 2010

Subsystems (3)
Memory management Driver core Memory hot(un)plug
lore
Share & tools
lore

Wu Fengguang

author
#0 lore raw

PATCH 1/1 memory-hotplug: add 0x prefix to HEX block_size_bytes

CC Andi Kleen <[email protected]>
Signed-off-by Wu Fengguang <[email protected]>
---
 drivers/base/memory.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/base/memory.c +1 -1
@@ -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 [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>

author
#1 lore raw

PATCH 1/1 sysdev: fix prototype for memory_sysdev_class show/store functions

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 <[email protected]>
CC Greg Kroah-Hartman <[email protected]>
CC "Zheng, Shaohui" <[email protected]>
Signed-off-by Wu Fengguang <[email protected]>
---
 drivers/base/memory.c |   32 ++++++++++++++++++++------------
 1 file changed, 20 insertions(+), 12 deletions(-)
drivers/base/memory.c +20 -12
@@ -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 [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>

#2 lore raw

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

Quoted 14 lines · 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
Quoted 5 lines

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 [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>

author
#3 lore raw

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

Quoted 2 lines

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

I guess yes - more than one.

Quoted 14 lines

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 <[email protected]>
Date:   Tue Jan 5 12:48:00 2010 +0100

sysdev: Convert cpu driver sysdev class attributes

commit c314abcfb978695949f9314ea9f1b6eea5dbd225

Author: Andi Kleen <[email protected]>
Date:   Tue Jan 5 12:47:59 2010 +0100

sysdev: Convert node driver class attributes to be data driven

commit c29af9636774ea95c795b2ddb71e7dc6e50861bf

Author: Andi Kleen <[email protected]>
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 [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>

#4 lore raw

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

Quoted 4 lines · Wu Fengguang
Quoted 2 lines

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 [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>

maintainer
#5 lore raw
In reply to Wu Fengguang
Quoted 9 lines · On Thu, 14 Jan 2010 19:59:56 +0800 Wu Fengguang <[email protected]> wrote:
CC Andi Kleen <[email protected]>
Signed-off-by Wu Fengguang <[email protected]>
---
 drivers/base/memory.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

drivers/base/memory.c static SYSDEV_ATTR(removable, 0444, show · line 317

Quoted 8 lines
drivers/base/memory.c +1 -1
@@ -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 [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>

author
#6 lore raw
Quoted 7 lines

drivers/base/memory.c static SYSDEV_ATTR(removable, 0444, show · line 314

Quoted 2 lines
drivers/base/memory.c +1 -1
-	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 [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>

#7 lore raw
Quoted 8 lines
Quoted 7 lines

drivers/base/memory.c static SYSDEV_ATTR(removable, 0444, show · line 314

Quoted 2 lines
drivers/base/memory.c +1 -1
-	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.

Quoted 8 lines

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 [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>