Re: [RFC PATCH] memory driver: make phys_index/end_phys_index reflect the start/end section number
On 04/09/2014 11:17 PM, Li Zhong wrote: > On Wed, 2014-04-09 at 12:39 -0500, Nathan Fontenot wrote: >> On 04/08/2014 02:47 PM, Dave Hansen wrote: >>> >>> That document really needs to be updated to stop referring to sections >>> (at least in the descriptions of the user interface). We can not change >>> the units of phys_index/end_phys_index without also changing >>> block_size_bytes. >>> >> >> Here is a first pass at updating the documentation. >> >> I have tried to update the documentation to refer to memory blocks instead >> of memory sections where appropriate and added a paragraph to explain >> that memory blocks are mode of memory sections. >> >> Thoughts? > > If we all agree to hide the information about sections, then I think we > also need to update the section id's used for phys_index/end_phys_index, > something like following on top of yours? > > -- > diff --git a/Documentation/memory-hotplug.txt > b/Documentation/memory-hotplug.txt > index 92d15e2..9fbb025 100644 > --- a/Documentation/memory-hotplug.txt > +++ b/Documentation/memory-hotplug.txt > @@ -138,10 +138,7 @@ is described under /sys/devices/system/memory as > /sys/devices/system/memory/memoryXXX > (XXX is the memory block id.) > > -Now, XXX is defined as (start_address_of_section / section_size) of the first > -section contained in the memory block. The files 'phys_index' and > -'end_phys_index' under each directory report the beginning and end section > id's > -for the memory block covered by the sysfs directory. It is expected that all > +For the memory block covered by the sysfs directory. It is expected that all > memory sections in this range are present and no memory holes exist in the > range. Currently there is no way to determine if there is a memory hole, but > the existence of one should not affect the hotplug capabilities of the memory > @@ -155,16 +152,14 @@ This device covers address range [0x1 ... > 0x14000) > Under each memory block, you can see 4 or 5 files, the end_phys_index file > being a recent addition and not present on older kernels. > > -/sys/devices/system/memory/memoryXXX/start_phys_index > +/sys/devices/system/memory/memoryXXX/phys_index > /sys/devices/system/memory/memoryXXX/end_phys_index > /sys/devices/system/memory/memoryXXX/phys_device > /sys/devices/system/memory/memoryXXX/state > /sys/devices/system/memory/memoryXXX/removable > > -'phys_index' : read-only and contains section id of the first section > - in the memory block, same as XXX. > -'end_phys_index' : read-only and contains section id of the last section > - in the memory block. > +'phys_index' : read-only and contains memory block id, same as XXX. > +'end_phys_index' : read-only and contains memory block id, same as XXX. > 'state' : read-write > at read: contains online/offline state of memory. > at write: user can specify "online_kernel", > -- > > Not sure whether it is proper to remove end_phys_index, too? If we are going to leave the code as it is today such that the start_phys_index and end_phys_index files both contain the same value I don't see why we should not do this. Li Zhong, unless anyone has objections, can you submit a patch to update the files in sysfs and the documentation? -Nathan > > Thanks, > Zhong > > > > >> >> -Nathan >> --- >> Documentation/memory-hotplug.txt | 113 >> --- >> 1 file changed, 59 insertions(+), 54 deletions(-) >> >> Index: linux/Documentation/memory-hotplug.txt >> === >> --- linux.orig/Documentation/memory-hotplug.txt >> +++ linux/Documentation/memory-hotplug.txt >> @@ -88,16 +88,21 @@ phase by hand. >> >> 1.3. Unit of Memory online/offline operation >> >> -Memory hotplug uses SPARSEMEM memory model. SPARSEMEM divides the whole >> memory >> -into chunks of the same size. The chunk is called a "section". The size of >> -a section is architecture dependent. For example, power uses 16MiB, ia64 >> uses >> -1GiB. The unit of online/offline operation is "one section". (see Section >> 3.) >> +Memory hotplug uses SPARSEMEM memory model which allows memory to be divided >> +into chunks of the same size. These chunks are called "sections". The size >> of >> +a memory section is architecture dependent. For example, power uses 16MiB, >> ia64 >> +uses 1GiB. >> + >> +Memory sections are combined into chunks referred to as "memory blocks". The >> +size of a memory block is architecture dependent and represents the logical >> +unit upon which memory online/offline operations are to be performed. The >> +default size of a memory block is the same as memory section size unless an >> +architecture specifies otherwise. (see Section 3.) >> >> -To determine the size of sections, please read this file: >> +To determine the size (in bytes) of a memory block please read
Re: [RFC PATCH] memory driver: make phys_index/end_phys_index reflect the start/end section number
On 04/09/2014 11:17 PM, Li Zhong wrote: On Wed, 2014-04-09 at 12:39 -0500, Nathan Fontenot wrote: On 04/08/2014 02:47 PM, Dave Hansen wrote: That document really needs to be updated to stop referring to sections (at least in the descriptions of the user interface). We can not change the units of phys_index/end_phys_index without also changing block_size_bytes. Here is a first pass at updating the documentation. I have tried to update the documentation to refer to memory blocks instead of memory sections where appropriate and added a paragraph to explain that memory blocks are mode of memory sections. Thoughts? If we all agree to hide the information about sections, then I think we also need to update the section id's used for phys_index/end_phys_index, something like following on top of yours? -- diff --git a/Documentation/memory-hotplug.txt b/Documentation/memory-hotplug.txt index 92d15e2..9fbb025 100644 --- a/Documentation/memory-hotplug.txt +++ b/Documentation/memory-hotplug.txt @@ -138,10 +138,7 @@ is described under /sys/devices/system/memory as /sys/devices/system/memory/memoryXXX (XXX is the memory block id.) -Now, XXX is defined as (start_address_of_section / section_size) of the first -section contained in the memory block. The files 'phys_index' and -'end_phys_index' under each directory report the beginning and end section id's -for the memory block covered by the sysfs directory. It is expected that all +For the memory block covered by the sysfs directory. It is expected that all memory sections in this range are present and no memory holes exist in the range. Currently there is no way to determine if there is a memory hole, but the existence of one should not affect the hotplug capabilities of the memory @@ -155,16 +152,14 @@ This device covers address range [0x1 ... 0x14000) Under each memory block, you can see 4 or 5 files, the end_phys_index file being a recent addition and not present on older kernels. -/sys/devices/system/memory/memoryXXX/start_phys_index +/sys/devices/system/memory/memoryXXX/phys_index /sys/devices/system/memory/memoryXXX/end_phys_index /sys/devices/system/memory/memoryXXX/phys_device /sys/devices/system/memory/memoryXXX/state /sys/devices/system/memory/memoryXXX/removable -'phys_index' : read-only and contains section id of the first section - in the memory block, same as XXX. -'end_phys_index' : read-only and contains section id of the last section - in the memory block. +'phys_index' : read-only and contains memory block id, same as XXX. +'end_phys_index' : read-only and contains memory block id, same as XXX. 'state' : read-write at read: contains online/offline state of memory. at write: user can specify online_kernel, -- Not sure whether it is proper to remove end_phys_index, too? If we are going to leave the code as it is today such that the start_phys_index and end_phys_index files both contain the same value I don't see why we should not do this. Li Zhong, unless anyone has objections, can you submit a patch to update the files in sysfs and the documentation? -Nathan Thanks, Zhong -Nathan --- Documentation/memory-hotplug.txt | 113 --- 1 file changed, 59 insertions(+), 54 deletions(-) Index: linux/Documentation/memory-hotplug.txt === --- linux.orig/Documentation/memory-hotplug.txt +++ linux/Documentation/memory-hotplug.txt @@ -88,16 +88,21 @@ phase by hand. 1.3. Unit of Memory online/offline operation -Memory hotplug uses SPARSEMEM memory model. SPARSEMEM divides the whole memory -into chunks of the same size. The chunk is called a section. The size of -a section is architecture dependent. For example, power uses 16MiB, ia64 uses -1GiB. The unit of online/offline operation is one section. (see Section 3.) +Memory hotplug uses SPARSEMEM memory model which allows memory to be divided +into chunks of the same size. These chunks are called sections. The size of +a memory section is architecture dependent. For example, power uses 16MiB, ia64 +uses 1GiB. + +Memory sections are combined into chunks referred to as memory blocks. The +size of a memory block is architecture dependent and represents the logical +unit upon which memory online/offline operations are to be performed. The +default size of a memory block is the same as memory section size unless an +architecture specifies otherwise. (see Section 3.) -To determine the size of sections, please read this file: +To determine the size (in bytes) of a memory block please read this file: /sys/devices/system/memory/block_size_bytes -This file shows the size of sections in byte. --- 2. Kernel Configuration @@ -123,14
Re: [RFC PATCH] memory driver: make phys_index/end_phys_index reflect the start/end section number
On Wed, 2014-04-09 at 12:39 -0500, Nathan Fontenot wrote: > On 04/08/2014 02:47 PM, Dave Hansen wrote: > > > > That document really needs to be updated to stop referring to sections > > (at least in the descriptions of the user interface). We can not change > > the units of phys_index/end_phys_index without also changing > > block_size_bytes. > > > > Here is a first pass at updating the documentation. > > I have tried to update the documentation to refer to memory blocks instead > of memory sections where appropriate and added a paragraph to explain > that memory blocks are mode of memory sections. > > Thoughts? If we all agree to hide the information about sections, then I think we also need to update the section id's used for phys_index/end_phys_index, something like following on top of yours? -- diff --git a/Documentation/memory-hotplug.txt b/Documentation/memory-hotplug.txt index 92d15e2..9fbb025 100644 --- a/Documentation/memory-hotplug.txt +++ b/Documentation/memory-hotplug.txt @@ -138,10 +138,7 @@ is described under /sys/devices/system/memory as /sys/devices/system/memory/memoryXXX (XXX is the memory block id.) -Now, XXX is defined as (start_address_of_section / section_size) of the first -section contained in the memory block. The files 'phys_index' and -'end_phys_index' under each directory report the beginning and end section id's -for the memory block covered by the sysfs directory. It is expected that all +For the memory block covered by the sysfs directory. It is expected that all memory sections in this range are present and no memory holes exist in the range. Currently there is no way to determine if there is a memory hole, but the existence of one should not affect the hotplug capabilities of the memory @@ -155,16 +152,14 @@ This device covers address range [0x1 ... 0x14000) Under each memory block, you can see 4 or 5 files, the end_phys_index file being a recent addition and not present on older kernels. -/sys/devices/system/memory/memoryXXX/start_phys_index +/sys/devices/system/memory/memoryXXX/phys_index /sys/devices/system/memory/memoryXXX/end_phys_index /sys/devices/system/memory/memoryXXX/phys_device /sys/devices/system/memory/memoryXXX/state /sys/devices/system/memory/memoryXXX/removable -'phys_index' : read-only and contains section id of the first section - in the memory block, same as XXX. -'end_phys_index' : read-only and contains section id of the last section - in the memory block. +'phys_index' : read-only and contains memory block id, same as XXX. +'end_phys_index' : read-only and contains memory block id, same as XXX. 'state' : read-write at read: contains online/offline state of memory. at write: user can specify "online_kernel", -- Not sure whether it is proper to remove end_phys_index, too? Thanks, Zhong > > -Nathan > --- > Documentation/memory-hotplug.txt | 113 > --- > 1 file changed, 59 insertions(+), 54 deletions(-) > > Index: linux/Documentation/memory-hotplug.txt > === > --- linux.orig/Documentation/memory-hotplug.txt > +++ linux/Documentation/memory-hotplug.txt > @@ -88,16 +88,21 @@ phase by hand. > > 1.3. Unit of Memory online/offline operation > > -Memory hotplug uses SPARSEMEM memory model. SPARSEMEM divides the whole > memory > -into chunks of the same size. The chunk is called a "section". The size of > -a section is architecture dependent. For example, power uses 16MiB, ia64 uses > -1GiB. The unit of online/offline operation is "one section". (see Section 3.) > +Memory hotplug uses SPARSEMEM memory model which allows memory to be divided > +into chunks of the same size. These chunks are called "sections". The size of > +a memory section is architecture dependent. For example, power uses 16MiB, > ia64 > +uses 1GiB. > + > +Memory sections are combined into chunks referred to as "memory blocks". The > +size of a memory block is architecture dependent and represents the logical > +unit upon which memory online/offline operations are to be performed. The > +default size of a memory block is the same as memory section size unless an > +architecture specifies otherwise. (see Section 3.) > > -To determine the size of sections, please read this file: > +To determine the size (in bytes) of a memory block please read this file: > > /sys/devices/system/memory/block_size_bytes > > -This file shows the size of sections in byte. > > --- > 2. Kernel Configuration > @@ -123,14 +128,15 @@ config options. > (CONFIG_ACPI_CONTAINER). > This option can be kernel module too. > > + > > -4 sysfs files for memory hotplug > +3 sysfs files for memory hotplug > > -All sections have their device information in
Re: [RFC PATCH] memory driver: make phys_index/end_phys_index reflect the start/end section number
On 04/10/2014 11:14 AM, Li Zhong wrote: > On Wed, 2014-04-09 at 08:49 -0700, Dave Hansen wrote: >> On 04/09/2014 02:20 AM, Li Zhong wrote: >>> Or do you mean we don't need to expose any information related to >>> SECTION to userspace? >> >> Right, we don't need to expose sections themselves to userspace. Do we? >> > OK, I agree with that. > > Yanfei, I recall you once expressed your preference for section > numbers? Hmmm Looking at the git log: commit d33601644cd3b09afb2edd9474517edc441c8fad Author: Nathan Fontenot Date: Thu Jan 20 10:44:29 2011 -0600 memory hotplug: Update phys_index to [start|end]_section_nr Update the 'phys_index' property of a the memory_block struct to be called start_section_nr, and add a end_section_nr property. The data tracked here is the same but the updated naming is more in line with what is stored here, namely the first and last section number that the memory block spans. The names presented to userspace remain the same, phys_index for start_section_nr and end_phys_index for end_section_nr, to avoid breaking anything in userspace. This also updates the node sysfs code to be aware of the new capability for a memory block to contain multiple memory sections and be aware of the memory block structure name changes (start_section_nr). This requires an additional parameter to unregister_mem_sect_under_nodes so that we know which memory section of the memory block to unregister. Signed-off-by: Nathan Fontenot Reviewed-by: Robin Holt Reviewed-by: KAMEZAWA Hiroyuki Signed-off-by: Greg Kroah-Hartman So obviously, Nathan added the end_phys_index sysfile to present the last section number of a memory block (for end_section_nr), but what he did in the patch seems not matching the log. So what is the motivation of adding this 'end_phys_index' file here? Confused. -- Thanks. Zhang Yanfei -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC PATCH] memory driver: make phys_index/end_phys_index reflect the start/end section number
On Wed, 2014-04-09 at 08:49 -0700, Dave Hansen wrote: > On 04/09/2014 02:20 AM, Li Zhong wrote: > > Or do you mean we don't need to expose any information related to > > SECTION to userspace? > > Right, we don't need to expose sections themselves to userspace. Do we? > OK, I agree with that. Yanfei, I recall you once expressed your preference for section numbers? Thanks, Zhong -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC PATCH] memory driver: make phys_index/end_phys_index reflect the start/end section number
On 04/10/2014 01:39 AM, Nathan Fontenot wrote: > On 04/08/2014 02:47 PM, Dave Hansen wrote: >> >> That document really needs to be updated to stop referring to sections >> (at least in the descriptions of the user interface). We can not change >> the units of phys_index/end_phys_index without also changing >> block_size_bytes. >> > > Here is a first pass at updating the documentation. > > I have tried to update the documentation to refer to memory blocks instead > of memory sections where appropriate and added a paragraph to explain > that memory blocks are mode of memory sections. > > Thoughts? I think the change is basically ok. So Reviewed-by: Zhang Yanfei Only one nitpick below. > > -Nathan > --- > Documentation/memory-hotplug.txt | 113 > --- > 1 file changed, 59 insertions(+), 54 deletions(-) > > Index: linux/Documentation/memory-hotplug.txt > === > --- linux.orig/Documentation/memory-hotplug.txt > +++ linux/Documentation/memory-hotplug.txt > @@ -88,16 +88,21 @@ phase by hand. > > 1.3. Unit of Memory online/offline operation > > -Memory hotplug uses SPARSEMEM memory model. SPARSEMEM divides the whole > memory > -into chunks of the same size. The chunk is called a "section". The size of > -a section is architecture dependent. For example, power uses 16MiB, ia64 uses > -1GiB. The unit of online/offline operation is "one section". (see Section 3.) > +Memory hotplug uses SPARSEMEM memory model which allows memory to be divided > +into chunks of the same size. These chunks are called "sections". The size of > +a memory section is architecture dependent. For example, power uses 16MiB, > ia64 > +uses 1GiB. > + > +Memory sections are combined into chunks referred to as "memory blocks". The > +size of a memory block is architecture dependent and represents the logical > +unit upon which memory online/offline operations are to be performed. The > +default size of a memory block is the same as memory section size unless an > +architecture specifies otherwise. (see Section 3.) > > -To determine the size of sections, please read this file: > +To determine the size (in bytes) of a memory block please read this file: > > /sys/devices/system/memory/block_size_bytes > > -This file shows the size of sections in byte. > > --- > 2. Kernel Configuration > @@ -123,14 +128,15 @@ config options. > (CONFIG_ACPI_CONTAINER). > This option can be kernel module too. > > + > > -4 sysfs files for memory hotplug > +3 sysfs files for memory hotplug > > -All sections have their device information in sysfs. Each section is part of > -a memory block under /sys/devices/system/memory as > +All memory blocks have their device information in sysfs. Each memory block > +is described under /sys/devices/system/memory as > > /sys/devices/system/memory/memoryXXX > -(XXX is the section id.) > +(XXX is the memory block id.) > > Now, XXX is defined as (start_address_of_section / section_size) of the first > section contained in the memory block. The files 'phys_index' and > @@ -141,13 +147,13 @@ range. Currently there is no way to dete > the existence of one should not affect the hotplug capabilities of the memory > block. > > -For example, assume 1GiB section size. A device for a memory starting at > +For example, assume 1GiB memory block size. A device for a memory starting at > 0x1 is /sys/device/system/memory/memory4 > (0x1 / 1Gib = 4) > This device covers address range [0x1 ... 0x14000) > > -Under each section, you can see 4 or 5 files, the end_phys_index file being > -a recent addition and not present on older kernels. > +Under each memory block, you can see 4 or 5 files, the end_phys_index file > +being a recent addition and not present on older kernels. > > /sys/devices/system/memory/memoryXXX/start_phys_index > /sys/devices/system/memory/memoryXXX/end_phys_index > @@ -185,6 +191,7 @@ For example: > A backlink will also be created: > /sys/devices/system/memory/memory9/node0 -> ../../node/node0 > > + > > 4. Physical memory hot-add phase > > @@ -227,11 +234,10 @@ You can tell the physical address of new > > % echo start_address_of_new_memory > /sys/devices/system/memory/probe > > -Then, [start_address_of_new_memory, start_address_of_new_memory + > section_size) > -memory range is hot-added. In this case, hotplug script is not called (in > -current implementation). You'll have to online memory by yourself. > -Please see "How to online memory" in this text. > - > +Then, [start_address_of_new_memory, start_address_of_new_memory + > +memory_block_size] memory range is hot-added. In this case, hotplug script is > +not called (in current implementation). You'll have to online
Re: [RFC PATCH] memory driver: make phys_index/end_phys_index reflect the start/end section number
On 04/08/2014 02:47 PM, Dave Hansen wrote: > > That document really needs to be updated to stop referring to sections > (at least in the descriptions of the user interface). We can not change > the units of phys_index/end_phys_index without also changing > block_size_bytes. > Here is a first pass at updating the documentation. I have tried to update the documentation to refer to memory blocks instead of memory sections where appropriate and added a paragraph to explain that memory blocks are mode of memory sections. Thoughts? -Nathan --- Documentation/memory-hotplug.txt | 113 --- 1 file changed, 59 insertions(+), 54 deletions(-) Index: linux/Documentation/memory-hotplug.txt === --- linux.orig/Documentation/memory-hotplug.txt +++ linux/Documentation/memory-hotplug.txt @@ -88,16 +88,21 @@ phase by hand. 1.3. Unit of Memory online/offline operation -Memory hotplug uses SPARSEMEM memory model. SPARSEMEM divides the whole memory -into chunks of the same size. The chunk is called a "section". The size of -a section is architecture dependent. For example, power uses 16MiB, ia64 uses -1GiB. The unit of online/offline operation is "one section". (see Section 3.) +Memory hotplug uses SPARSEMEM memory model which allows memory to be divided +into chunks of the same size. These chunks are called "sections". The size of +a memory section is architecture dependent. For example, power uses 16MiB, ia64 +uses 1GiB. + +Memory sections are combined into chunks referred to as "memory blocks". The +size of a memory block is architecture dependent and represents the logical +unit upon which memory online/offline operations are to be performed. The +default size of a memory block is the same as memory section size unless an +architecture specifies otherwise. (see Section 3.) -To determine the size of sections, please read this file: +To determine the size (in bytes) of a memory block please read this file: /sys/devices/system/memory/block_size_bytes -This file shows the size of sections in byte. --- 2. Kernel Configuration @@ -123,14 +128,15 @@ config options. (CONFIG_ACPI_CONTAINER). This option can be kernel module too. + -4 sysfs files for memory hotplug +3 sysfs files for memory hotplug -All sections have their device information in sysfs. Each section is part of -a memory block under /sys/devices/system/memory as +All memory blocks have their device information in sysfs. Each memory block +is described under /sys/devices/system/memory as /sys/devices/system/memory/memoryXXX -(XXX is the section id.) +(XXX is the memory block id.) Now, XXX is defined as (start_address_of_section / section_size) of the first section contained in the memory block. The files 'phys_index' and @@ -141,13 +147,13 @@ range. Currently there is no way to dete the existence of one should not affect the hotplug capabilities of the memory block. -For example, assume 1GiB section size. A device for a memory starting at +For example, assume 1GiB memory block size. A device for a memory starting at 0x1 is /sys/device/system/memory/memory4 (0x1 / 1Gib = 4) This device covers address range [0x1 ... 0x14000) -Under each section, you can see 4 or 5 files, the end_phys_index file being -a recent addition and not present on older kernels. +Under each memory block, you can see 4 or 5 files, the end_phys_index file +being a recent addition and not present on older kernels. /sys/devices/system/memory/memoryXXX/start_phys_index /sys/devices/system/memory/memoryXXX/end_phys_index @@ -185,6 +191,7 @@ For example: A backlink will also be created: /sys/devices/system/memory/memory9/node0 -> ../../node/node0 + 4. Physical memory hot-add phase @@ -227,11 +234,10 @@ You can tell the physical address of new % echo start_address_of_new_memory > /sys/devices/system/memory/probe -Then, [start_address_of_new_memory, start_address_of_new_memory + section_size) -memory range is hot-added. In this case, hotplug script is not called (in -current implementation). You'll have to online memory by yourself. -Please see "How to online memory" in this text. - +Then, [start_address_of_new_memory, start_address_of_new_memory + +memory_block_size] memory range is hot-added. In this case, hotplug script is +not called (in current implementation). You'll have to online memory by +yourself. Please see "How to online memory" in this text. -- @@ -240,36 +246,36 @@ Please see "How to online memory" in thi 5.1. State of memory -To see (online/offline) state of memory section, read 'state' file. +To see (online/offline) state of a memory block, read 'state' file. % cat
Re: [RFC PATCH] memory driver: make phys_index/end_phys_index reflect the start/end section number
On 04/08/2014 02:47 PM, Dave Hansen wrote: > On 04/08/2014 11:23 AM, Nathan Fontenot wrote: >> On 04/08/2014 11:13 AM, Dave Hansen wrote: >>> On 04/08/2014 01:27 AM, Li Zhong wrote: If Dave and others don't have further objections, it seems this small userspace incompatibility could be accepted by most of us, and I don't need to make a version 2. >>> >>> Let me ask another question then. What are the units of >>> phys_index/end_phys_index? How do we expose those units to userspace? >>> >> >> The documentation for these files just states that the files contain >> the first and last section id of memory in the memory block for >> phys_index and end_phys_index respectively. >> >> I'm not sure the values have ever been units of anything, at least not >> that I remember. > > > > There are two units. SECTION_SIZE, which is completely internal to the > kernel, and block_size_bytes which used to be the same as SECTION_SIZE, > but is not now. Which one of those two is phys_index/end_phys_index in, > and if it is in terms of SECTION_SIZE like this patch proposes, how do > we tell userspace how large SECTION_SIZE is? > > block_size_bytes is supposed to tell you how large the sections are. In > the case where we lumped a bunch of sections together, we also bumped up > block_size_bytes. That's why we currently divide the *ACTUAL* section > number in phys_index/end_phys_index by block_size_bytes. > > That document really needs to be updated to stop referring to sections > (at least in the descriptions of the user interface). We can not change > the units of phys_index/end_phys_index without also changing > block_size_bytes. > Re-reading the documentation. You're correct, it needs help. -Nathan -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC PATCH] memory driver: make phys_index/end_phys_index reflect the start/end section number
On 04/09/2014 10:49 AM, Dave Hansen wrote: > On 04/09/2014 02:20 AM, Li Zhong wrote: >> Or do you mean we don't need to expose any information related to >> SECTION to userspace? > > Right, we don't need to expose sections themselves to userspace. Do we? > No. the layout in sysfs is based in block_size_bytes so I do not see any need to expose sections to userspace. -Nathan -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC PATCH] memory driver: make phys_index/end_phys_index reflect the start/end section number
On 04/09/2014 02:20 AM, Li Zhong wrote: > Or do you mean we don't need to expose any information related to > SECTION to userspace? Right, we don't need to expose sections themselves to userspace. Do we? -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC PATCH] memory driver: make phys_index/end_phys_index reflect the start/end section number
On Tue, 2014-04-08 at 12:47 -0700, Dave Hansen wrote: > On 04/08/2014 11:23 AM, Nathan Fontenot wrote: > > On 04/08/2014 11:13 AM, Dave Hansen wrote: > >> On 04/08/2014 01:27 AM, Li Zhong wrote: > >>> If Dave and others don't have further objections, it seems this small > >>> userspace incompatibility could be accepted by most of us, and I don't > >>> need to make a version 2. > >> > >> Let me ask another question then. What are the units of > >> phys_index/end_phys_index? How do we expose those units to userspace? > >> > > > > The documentation for these files just states that the files contain > > the first and last section id of memory in the memory block for > > phys_index and end_phys_index respectively. > > > > I'm not sure the values have ever been units of anything, at least not > > that I remember. > > > > There are two units. SECTION_SIZE, which is completely internal to the > kernel, and block_size_bytes which used to be the same as SECTION_SIZE, > but is not now. Which one of those two is phys_index/end_phys_index in, > and if it is in terms of SECTION_SIZE like this patch proposes, how do > we tell userspace how large SECTION_SIZE is? With this patch, I think we could still tell how large SECTION_SIZE is. block_size_bytes tells us the size of the block, and end_phys_index-phys_index+1, tells us the numbers of sections in the block, and then we could know the SECTION_SIZE by a division. Not that obvious, but if needed, we could easily add a file telling us section_size or sections_per_block. > > block_size_bytes is supposed to tell you how large the sections are. In > the case where we lumped a bunch of sections together, we also bumped up > block_size_bytes. That's why we currently divide the *ACTUAL* section > number in phys_index/end_phys_index by block_size_bytes. > > That document really needs to be updated to stop referring to sections > (at least in the descriptions of the user interface). We can not change > the units of phys_index/end_phys_index without also changing > block_size_bytes. How about adding a new section_size_bytes together with this patch? Or do you mean we don't need to expose any information related to SECTION to userspace? Thanks, Zhong -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC PATCH] memory driver: make phys_index/end_phys_index reflect the start/end section number
On Tue, 2014-04-08 at 12:47 -0700, Dave Hansen wrote: On 04/08/2014 11:23 AM, Nathan Fontenot wrote: On 04/08/2014 11:13 AM, Dave Hansen wrote: On 04/08/2014 01:27 AM, Li Zhong wrote: If Dave and others don't have further objections, it seems this small userspace incompatibility could be accepted by most of us, and I don't need to make a version 2. Let me ask another question then. What are the units of phys_index/end_phys_index? How do we expose those units to userspace? The documentation for these files just states that the files contain the first and last section id of memory in the memory block for phys_index and end_phys_index respectively. I'm not sure the values have ever been units of anything, at least not that I remember. sigh There are two units. SECTION_SIZE, which is completely internal to the kernel, and block_size_bytes which used to be the same as SECTION_SIZE, but is not now. Which one of those two is phys_index/end_phys_index in, and if it is in terms of SECTION_SIZE like this patch proposes, how do we tell userspace how large SECTION_SIZE is? With this patch, I think we could still tell how large SECTION_SIZE is. block_size_bytes tells us the size of the block, and end_phys_index-phys_index+1, tells us the numbers of sections in the block, and then we could know the SECTION_SIZE by a division. Not that obvious, but if needed, we could easily add a file telling us section_size or sections_per_block. block_size_bytes is supposed to tell you how large the sections are. In the case where we lumped a bunch of sections together, we also bumped up block_size_bytes. That's why we currently divide the *ACTUAL* section number in phys_index/end_phys_index by block_size_bytes. That document really needs to be updated to stop referring to sections (at least in the descriptions of the user interface). We can not change the units of phys_index/end_phys_index without also changing block_size_bytes. How about adding a new section_size_bytes together with this patch? Or do you mean we don't need to expose any information related to SECTION to userspace? Thanks, Zhong -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC PATCH] memory driver: make phys_index/end_phys_index reflect the start/end section number
On 04/09/2014 02:20 AM, Li Zhong wrote: Or do you mean we don't need to expose any information related to SECTION to userspace? Right, we don't need to expose sections themselves to userspace. Do we? -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC PATCH] memory driver: make phys_index/end_phys_index reflect the start/end section number
On 04/09/2014 10:49 AM, Dave Hansen wrote: On 04/09/2014 02:20 AM, Li Zhong wrote: Or do you mean we don't need to expose any information related to SECTION to userspace? Right, we don't need to expose sections themselves to userspace. Do we? No. the layout in sysfs is based in block_size_bytes so I do not see any need to expose sections to userspace. -Nathan -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC PATCH] memory driver: make phys_index/end_phys_index reflect the start/end section number
On 04/08/2014 02:47 PM, Dave Hansen wrote: On 04/08/2014 11:23 AM, Nathan Fontenot wrote: On 04/08/2014 11:13 AM, Dave Hansen wrote: On 04/08/2014 01:27 AM, Li Zhong wrote: If Dave and others don't have further objections, it seems this small userspace incompatibility could be accepted by most of us, and I don't need to make a version 2. Let me ask another question then. What are the units of phys_index/end_phys_index? How do we expose those units to userspace? The documentation for these files just states that the files contain the first and last section id of memory in the memory block for phys_index and end_phys_index respectively. I'm not sure the values have ever been units of anything, at least not that I remember. sigh There are two units. SECTION_SIZE, which is completely internal to the kernel, and block_size_bytes which used to be the same as SECTION_SIZE, but is not now. Which one of those two is phys_index/end_phys_index in, and if it is in terms of SECTION_SIZE like this patch proposes, how do we tell userspace how large SECTION_SIZE is? block_size_bytes is supposed to tell you how large the sections are. In the case where we lumped a bunch of sections together, we also bumped up block_size_bytes. That's why we currently divide the *ACTUAL* section number in phys_index/end_phys_index by block_size_bytes. That document really needs to be updated to stop referring to sections (at least in the descriptions of the user interface). We can not change the units of phys_index/end_phys_index without also changing block_size_bytes. Re-reading the documentation. You're correct, it needs help. -Nathan -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC PATCH] memory driver: make phys_index/end_phys_index reflect the start/end section number
On 04/08/2014 02:47 PM, Dave Hansen wrote: That document really needs to be updated to stop referring to sections (at least in the descriptions of the user interface). We can not change the units of phys_index/end_phys_index without also changing block_size_bytes. Here is a first pass at updating the documentation. I have tried to update the documentation to refer to memory blocks instead of memory sections where appropriate and added a paragraph to explain that memory blocks are mode of memory sections. Thoughts? -Nathan --- Documentation/memory-hotplug.txt | 113 --- 1 file changed, 59 insertions(+), 54 deletions(-) Index: linux/Documentation/memory-hotplug.txt === --- linux.orig/Documentation/memory-hotplug.txt +++ linux/Documentation/memory-hotplug.txt @@ -88,16 +88,21 @@ phase by hand. 1.3. Unit of Memory online/offline operation -Memory hotplug uses SPARSEMEM memory model. SPARSEMEM divides the whole memory -into chunks of the same size. The chunk is called a section. The size of -a section is architecture dependent. For example, power uses 16MiB, ia64 uses -1GiB. The unit of online/offline operation is one section. (see Section 3.) +Memory hotplug uses SPARSEMEM memory model which allows memory to be divided +into chunks of the same size. These chunks are called sections. The size of +a memory section is architecture dependent. For example, power uses 16MiB, ia64 +uses 1GiB. + +Memory sections are combined into chunks referred to as memory blocks. The +size of a memory block is architecture dependent and represents the logical +unit upon which memory online/offline operations are to be performed. The +default size of a memory block is the same as memory section size unless an +architecture specifies otherwise. (see Section 3.) -To determine the size of sections, please read this file: +To determine the size (in bytes) of a memory block please read this file: /sys/devices/system/memory/block_size_bytes -This file shows the size of sections in byte. --- 2. Kernel Configuration @@ -123,14 +128,15 @@ config options. (CONFIG_ACPI_CONTAINER). This option can be kernel module too. + -4 sysfs files for memory hotplug +3 sysfs files for memory hotplug -All sections have their device information in sysfs. Each section is part of -a memory block under /sys/devices/system/memory as +All memory blocks have their device information in sysfs. Each memory block +is described under /sys/devices/system/memory as /sys/devices/system/memory/memoryXXX -(XXX is the section id.) +(XXX is the memory block id.) Now, XXX is defined as (start_address_of_section / section_size) of the first section contained in the memory block. The files 'phys_index' and @@ -141,13 +147,13 @@ range. Currently there is no way to dete the existence of one should not affect the hotplug capabilities of the memory block. -For example, assume 1GiB section size. A device for a memory starting at +For example, assume 1GiB memory block size. A device for a memory starting at 0x1 is /sys/device/system/memory/memory4 (0x1 / 1Gib = 4) This device covers address range [0x1 ... 0x14000) -Under each section, you can see 4 or 5 files, the end_phys_index file being -a recent addition and not present on older kernels. +Under each memory block, you can see 4 or 5 files, the end_phys_index file +being a recent addition and not present on older kernels. /sys/devices/system/memory/memoryXXX/start_phys_index /sys/devices/system/memory/memoryXXX/end_phys_index @@ -185,6 +191,7 @@ For example: A backlink will also be created: /sys/devices/system/memory/memory9/node0 - ../../node/node0 + 4. Physical memory hot-add phase @@ -227,11 +234,10 @@ You can tell the physical address of new % echo start_address_of_new_memory /sys/devices/system/memory/probe -Then, [start_address_of_new_memory, start_address_of_new_memory + section_size) -memory range is hot-added. In this case, hotplug script is not called (in -current implementation). You'll have to online memory by yourself. -Please see How to online memory in this text. - +Then, [start_address_of_new_memory, start_address_of_new_memory + +memory_block_size] memory range is hot-added. In this case, hotplug script is +not called (in current implementation). You'll have to online memory by +yourself. Please see How to online memory in this text. -- @@ -240,36 +246,36 @@ Please see How to online memory in thi 5.1. State of memory -To see (online/offline) state of memory section, read 'state' file. +To see (online/offline) state of a memory block, read 'state' file. % cat
Re: [RFC PATCH] memory driver: make phys_index/end_phys_index reflect the start/end section number
On 04/10/2014 01:39 AM, Nathan Fontenot wrote: On 04/08/2014 02:47 PM, Dave Hansen wrote: That document really needs to be updated to stop referring to sections (at least in the descriptions of the user interface). We can not change the units of phys_index/end_phys_index without also changing block_size_bytes. Here is a first pass at updating the documentation. I have tried to update the documentation to refer to memory blocks instead of memory sections where appropriate and added a paragraph to explain that memory blocks are mode of memory sections. Thoughts? I think the change is basically ok. So Reviewed-by: Zhang Yanfei zhangyan...@cn.fujitsu.com Only one nitpick below. -Nathan --- Documentation/memory-hotplug.txt | 113 --- 1 file changed, 59 insertions(+), 54 deletions(-) Index: linux/Documentation/memory-hotplug.txt === --- linux.orig/Documentation/memory-hotplug.txt +++ linux/Documentation/memory-hotplug.txt @@ -88,16 +88,21 @@ phase by hand. 1.3. Unit of Memory online/offline operation -Memory hotplug uses SPARSEMEM memory model. SPARSEMEM divides the whole memory -into chunks of the same size. The chunk is called a section. The size of -a section is architecture dependent. For example, power uses 16MiB, ia64 uses -1GiB. The unit of online/offline operation is one section. (see Section 3.) +Memory hotplug uses SPARSEMEM memory model which allows memory to be divided +into chunks of the same size. These chunks are called sections. The size of +a memory section is architecture dependent. For example, power uses 16MiB, ia64 +uses 1GiB. + +Memory sections are combined into chunks referred to as memory blocks. The +size of a memory block is architecture dependent and represents the logical +unit upon which memory online/offline operations are to be performed. The +default size of a memory block is the same as memory section size unless an +architecture specifies otherwise. (see Section 3.) -To determine the size of sections, please read this file: +To determine the size (in bytes) of a memory block please read this file: /sys/devices/system/memory/block_size_bytes -This file shows the size of sections in byte. --- 2. Kernel Configuration @@ -123,14 +128,15 @@ config options. (CONFIG_ACPI_CONTAINER). This option can be kernel module too. + -4 sysfs files for memory hotplug +3 sysfs files for memory hotplug -All sections have their device information in sysfs. Each section is part of -a memory block under /sys/devices/system/memory as +All memory blocks have their device information in sysfs. Each memory block +is described under /sys/devices/system/memory as /sys/devices/system/memory/memoryXXX -(XXX is the section id.) +(XXX is the memory block id.) Now, XXX is defined as (start_address_of_section / section_size) of the first section contained in the memory block. The files 'phys_index' and @@ -141,13 +147,13 @@ range. Currently there is no way to dete the existence of one should not affect the hotplug capabilities of the memory block. -For example, assume 1GiB section size. A device for a memory starting at +For example, assume 1GiB memory block size. A device for a memory starting at 0x1 is /sys/device/system/memory/memory4 (0x1 / 1Gib = 4) This device covers address range [0x1 ... 0x14000) -Under each section, you can see 4 or 5 files, the end_phys_index file being -a recent addition and not present on older kernels. +Under each memory block, you can see 4 or 5 files, the end_phys_index file +being a recent addition and not present on older kernels. /sys/devices/system/memory/memoryXXX/start_phys_index /sys/devices/system/memory/memoryXXX/end_phys_index @@ -185,6 +191,7 @@ For example: A backlink will also be created: /sys/devices/system/memory/memory9/node0 - ../../node/node0 + 4. Physical memory hot-add phase @@ -227,11 +234,10 @@ You can tell the physical address of new % echo start_address_of_new_memory /sys/devices/system/memory/probe -Then, [start_address_of_new_memory, start_address_of_new_memory + section_size) -memory range is hot-added. In this case, hotplug script is not called (in -current implementation). You'll have to online memory by yourself. -Please see How to online memory in this text. - +Then, [start_address_of_new_memory, start_address_of_new_memory + +memory_block_size] memory range is hot-added. In this case, hotplug script is +not called (in current implementation). You'll have to online memory by +yourself. Please see How to online memory in this text. --
Re: [RFC PATCH] memory driver: make phys_index/end_phys_index reflect the start/end section number
On Wed, 2014-04-09 at 08:49 -0700, Dave Hansen wrote: On 04/09/2014 02:20 AM, Li Zhong wrote: Or do you mean we don't need to expose any information related to SECTION to userspace? Right, we don't need to expose sections themselves to userspace. Do we? OK, I agree with that. Yanfei, I recall you once expressed your preference for section numbers? Thanks, Zhong -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC PATCH] memory driver: make phys_index/end_phys_index reflect the start/end section number
On 04/10/2014 11:14 AM, Li Zhong wrote: On Wed, 2014-04-09 at 08:49 -0700, Dave Hansen wrote: On 04/09/2014 02:20 AM, Li Zhong wrote: Or do you mean we don't need to expose any information related to SECTION to userspace? Right, we don't need to expose sections themselves to userspace. Do we? OK, I agree with that. Yanfei, I recall you once expressed your preference for section numbers? Hmmm Looking at the git log: commit d33601644cd3b09afb2edd9474517edc441c8fad Author: Nathan Fontenot nf...@austin.ibm.com Date: Thu Jan 20 10:44:29 2011 -0600 memory hotplug: Update phys_index to [start|end]_section_nr Update the 'phys_index' property of a the memory_block struct to be called start_section_nr, and add a end_section_nr property. The data tracked here is the same but the updated naming is more in line with what is stored here, namely the first and last section number that the memory block spans. The names presented to userspace remain the same, phys_index for start_section_nr and end_phys_index for end_section_nr, to avoid breaking anything in userspace. This also updates the node sysfs code to be aware of the new capability for a memory block to contain multiple memory sections and be aware of the memory block structure name changes (start_section_nr). This requires an additional parameter to unregister_mem_sect_under_nodes so that we know which memory section of the memory block to unregister. Signed-off-by: Nathan Fontenot nf...@austin.ibm.com Reviewed-by: Robin Holt h...@sgi.com Reviewed-by: KAMEZAWA Hiroyuki kamezawa.hir...@jp.fujitsu.com Signed-off-by: Greg Kroah-Hartman gre...@suse.de So obviously, Nathan added the end_phys_index sysfile to present the last section number of a memory block (for end_section_nr), but what he did in the patch seems not matching the log. So what is the motivation of adding this 'end_phys_index' file here? Confused. -- Thanks. Zhang Yanfei -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC PATCH] memory driver: make phys_index/end_phys_index reflect the start/end section number
On Wed, 2014-04-09 at 12:39 -0500, Nathan Fontenot wrote: On 04/08/2014 02:47 PM, Dave Hansen wrote: That document really needs to be updated to stop referring to sections (at least in the descriptions of the user interface). We can not change the units of phys_index/end_phys_index without also changing block_size_bytes. Here is a first pass at updating the documentation. I have tried to update the documentation to refer to memory blocks instead of memory sections where appropriate and added a paragraph to explain that memory blocks are mode of memory sections. Thoughts? If we all agree to hide the information about sections, then I think we also need to update the section id's used for phys_index/end_phys_index, something like following on top of yours? -- diff --git a/Documentation/memory-hotplug.txt b/Documentation/memory-hotplug.txt index 92d15e2..9fbb025 100644 --- a/Documentation/memory-hotplug.txt +++ b/Documentation/memory-hotplug.txt @@ -138,10 +138,7 @@ is described under /sys/devices/system/memory as /sys/devices/system/memory/memoryXXX (XXX is the memory block id.) -Now, XXX is defined as (start_address_of_section / section_size) of the first -section contained in the memory block. The files 'phys_index' and -'end_phys_index' under each directory report the beginning and end section id's -for the memory block covered by the sysfs directory. It is expected that all +For the memory block covered by the sysfs directory. It is expected that all memory sections in this range are present and no memory holes exist in the range. Currently there is no way to determine if there is a memory hole, but the existence of one should not affect the hotplug capabilities of the memory @@ -155,16 +152,14 @@ This device covers address range [0x1 ... 0x14000) Under each memory block, you can see 4 or 5 files, the end_phys_index file being a recent addition and not present on older kernels. -/sys/devices/system/memory/memoryXXX/start_phys_index +/sys/devices/system/memory/memoryXXX/phys_index /sys/devices/system/memory/memoryXXX/end_phys_index /sys/devices/system/memory/memoryXXX/phys_device /sys/devices/system/memory/memoryXXX/state /sys/devices/system/memory/memoryXXX/removable -'phys_index' : read-only and contains section id of the first section - in the memory block, same as XXX. -'end_phys_index' : read-only and contains section id of the last section - in the memory block. +'phys_index' : read-only and contains memory block id, same as XXX. +'end_phys_index' : read-only and contains memory block id, same as XXX. 'state' : read-write at read: contains online/offline state of memory. at write: user can specify online_kernel, -- Not sure whether it is proper to remove end_phys_index, too? Thanks, Zhong -Nathan --- Documentation/memory-hotplug.txt | 113 --- 1 file changed, 59 insertions(+), 54 deletions(-) Index: linux/Documentation/memory-hotplug.txt === --- linux.orig/Documentation/memory-hotplug.txt +++ linux/Documentation/memory-hotplug.txt @@ -88,16 +88,21 @@ phase by hand. 1.3. Unit of Memory online/offline operation -Memory hotplug uses SPARSEMEM memory model. SPARSEMEM divides the whole memory -into chunks of the same size. The chunk is called a section. The size of -a section is architecture dependent. For example, power uses 16MiB, ia64 uses -1GiB. The unit of online/offline operation is one section. (see Section 3.) +Memory hotplug uses SPARSEMEM memory model which allows memory to be divided +into chunks of the same size. These chunks are called sections. The size of +a memory section is architecture dependent. For example, power uses 16MiB, ia64 +uses 1GiB. + +Memory sections are combined into chunks referred to as memory blocks. The +size of a memory block is architecture dependent and represents the logical +unit upon which memory online/offline operations are to be performed. The +default size of a memory block is the same as memory section size unless an +architecture specifies otherwise. (see Section 3.) -To determine the size of sections, please read this file: +To determine the size (in bytes) of a memory block please read this file: /sys/devices/system/memory/block_size_bytes -This file shows the size of sections in byte. --- 2. Kernel Configuration @@ -123,14 +128,15 @@ config options. (CONFIG_ACPI_CONTAINER). This option can be kernel module too. + -4 sysfs files for memory hotplug +3 sysfs files for memory hotplug -All sections have their device information in sysfs. Each section is part of -a memory block under /sys/devices/system/memory as
Re: [RFC PATCH] memory driver: make phys_index/end_phys_index reflect the start/end section number
On 04/08/2014 11:23 AM, Nathan Fontenot wrote: > On 04/08/2014 11:13 AM, Dave Hansen wrote: >> On 04/08/2014 01:27 AM, Li Zhong wrote: >>> If Dave and others don't have further objections, it seems this small >>> userspace incompatibility could be accepted by most of us, and I don't >>> need to make a version 2. >> >> Let me ask another question then. What are the units of >> phys_index/end_phys_index? How do we expose those units to userspace? >> > > The documentation for these files just states that the files contain > the first and last section id of memory in the memory block for > phys_index and end_phys_index respectively. > > I'm not sure the values have ever been units of anything, at least not > that I remember. There are two units. SECTION_SIZE, which is completely internal to the kernel, and block_size_bytes which used to be the same as SECTION_SIZE, but is not now. Which one of those two is phys_index/end_phys_index in, and if it is in terms of SECTION_SIZE like this patch proposes, how do we tell userspace how large SECTION_SIZE is? block_size_bytes is supposed to tell you how large the sections are. In the case where we lumped a bunch of sections together, we also bumped up block_size_bytes. That's why we currently divide the *ACTUAL* section number in phys_index/end_phys_index by block_size_bytes. That document really needs to be updated to stop referring to sections (at least in the descriptions of the user interface). We can not change the units of phys_index/end_phys_index without also changing block_size_bytes. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC PATCH] memory driver: make phys_index/end_phys_index reflect the start/end section number
On 04/08/2014 11:13 AM, Dave Hansen wrote: > On 04/08/2014 01:27 AM, Li Zhong wrote: >> If Dave and others don't have further objections, it seems this small >> userspace incompatibility could be accepted by most of us, and I don't >> need to make a version 2. > > Let me ask another question then. What are the units of > phys_index/end_phys_index? How do we expose those units to userspace? > The documentation for these files just states that the files contain the first and last section id of memory in the memory block for phys_index and end_phys_index respectively. I'm not sure the values have ever been units of anything, at least not that I remember. -Nathan -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC PATCH] memory driver: make phys_index/end_phys_index reflect the start/end section number
On 04/08/2014 01:27 AM, Li Zhong wrote: > If Dave and others don't have further objections, it seems this small > userspace incompatibility could be accepted by most of us, and I don't > need to make a version 2. Let me ask another question then. What are the units of phys_index/end_phys_index? How do we expose those units to userspace? -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC PATCH] memory driver: make phys_index/end_phys_index reflect the start/end section number
On Fri, 2014-04-04 at 10:29 +0900, Yasuaki Ishimatsu wrote: > (2014/04/02 17:56), Li Zhong wrote: > > I noticed the phys_index and end_phys_index under > > /sys/devices/system/memory/memoryXXX/ have the same value, e.g. > > (for the test machine, one memory block has 8 sections, that is > > sections_per_block equals 8) > > > > # cd /sys/devices/system/memory/memory100/ > > # cat phys_index end_phys_index > > 0064 > > 0064 > > > > Seems they should reflect the start/end section number respectively, which > > also matches what is said in Documentation/memory-hotplug.txt > > > > This patch tries to modify that so the two files could show the start/end > > section number of the memory block. > > > > After this change, output of the above example looks like: > > > > # cat phys_index end_phys_index > > 0320 > > 0327 > > > > Signed-off-by: Li Zhong > > --- > > Good catch! This is a bug. So I think the bug should be fixed. > > Reviewed-by: Yasuaki Ishimatsu Thank you all for the review. If Dave and others don't have further objections, it seems this small userspace incompatibility could be accepted by most of us, and I don't need to make a version 2. Thanks, Zhong > Thanks, > Yasuaki Ishimatsu > > > drivers/base/memory.c | 4 ++-- > > 1 file changed, 2 insertions(+), 2 deletions(-) > > > > diff --git a/drivers/base/memory.c b/drivers/base/memory.c > > index bece691..b10f2fa 100644 > > --- a/drivers/base/memory.c > > +++ b/drivers/base/memory.c > > @@ -114,7 +114,7 @@ static ssize_t show_mem_start_phys_index(struct device > > *dev, > > struct memory_block *mem = to_memory_block(dev); > > unsigned long phys_index; > > > > - phys_index = mem->start_section_nr / sections_per_block; > > + phys_index = mem->start_section_nr; > > return sprintf(buf, "%08lx\n", phys_index); > > } > > > > @@ -124,7 +124,7 @@ static ssize_t show_mem_end_phys_index(struct device > > *dev, > > struct memory_block *mem = to_memory_block(dev); > > unsigned long phys_index; > > > > - phys_index = mem->end_section_nr / sections_per_block; > > + phys_index = mem->end_section_nr; > > return sprintf(buf, "%08lx\n", phys_index); > > } > > > > > > > > -- > > To unsubscribe from this list: send the line "unsubscribe linux-kernel" in > > the body of a message to majord...@vger.kernel.org > > More majordomo info at http://vger.kernel.org/majordomo-info.html > > Please read the FAQ at http://www.tux.org/lkml/ > > > > > -- > To unsubscribe from this list: send the line "unsubscribe linux-kernel" in > the body of a message to majord...@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > Please read the FAQ at http://www.tux.org/lkml/ > -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC PATCH] memory driver: make phys_index/end_phys_index reflect the start/end section number
On Fri, 2014-04-04 at 10:29 +0900, Yasuaki Ishimatsu wrote: (2014/04/02 17:56), Li Zhong wrote: I noticed the phys_index and end_phys_index under /sys/devices/system/memory/memoryXXX/ have the same value, e.g. (for the test machine, one memory block has 8 sections, that is sections_per_block equals 8) # cd /sys/devices/system/memory/memory100/ # cat phys_index end_phys_index 0064 0064 Seems they should reflect the start/end section number respectively, which also matches what is said in Documentation/memory-hotplug.txt This patch tries to modify that so the two files could show the start/end section number of the memory block. After this change, output of the above example looks like: # cat phys_index end_phys_index 0320 0327 Signed-off-by: Li Zhong zh...@linux.vnet.ibm.com --- Good catch! This is a bug. So I think the bug should be fixed. Reviewed-by: Yasuaki Ishimatsu isimatu.yasu...@jp.fujitsu.com Thank you all for the review. If Dave and others don't have further objections, it seems this small userspace incompatibility could be accepted by most of us, and I don't need to make a version 2. Thanks, Zhong Thanks, Yasuaki Ishimatsu drivers/base/memory.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/base/memory.c b/drivers/base/memory.c index bece691..b10f2fa 100644 --- a/drivers/base/memory.c +++ b/drivers/base/memory.c @@ -114,7 +114,7 @@ static ssize_t show_mem_start_phys_index(struct device *dev, struct memory_block *mem = to_memory_block(dev); unsigned long phys_index; - phys_index = mem-start_section_nr / sections_per_block; + phys_index = mem-start_section_nr; return sprintf(buf, %08lx\n, phys_index); } @@ -124,7 +124,7 @@ static ssize_t show_mem_end_phys_index(struct device *dev, struct memory_block *mem = to_memory_block(dev); unsigned long phys_index; - phys_index = mem-end_section_nr / sections_per_block; + phys_index = mem-end_section_nr; return sprintf(buf, %08lx\n, phys_index); } -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/ -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/ -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC PATCH] memory driver: make phys_index/end_phys_index reflect the start/end section number
On 04/08/2014 01:27 AM, Li Zhong wrote: If Dave and others don't have further objections, it seems this small userspace incompatibility could be accepted by most of us, and I don't need to make a version 2. Let me ask another question then. What are the units of phys_index/end_phys_index? How do we expose those units to userspace? -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC PATCH] memory driver: make phys_index/end_phys_index reflect the start/end section number
On 04/08/2014 11:13 AM, Dave Hansen wrote: On 04/08/2014 01:27 AM, Li Zhong wrote: If Dave and others don't have further objections, it seems this small userspace incompatibility could be accepted by most of us, and I don't need to make a version 2. Let me ask another question then. What are the units of phys_index/end_phys_index? How do we expose those units to userspace? The documentation for these files just states that the files contain the first and last section id of memory in the memory block for phys_index and end_phys_index respectively. I'm not sure the values have ever been units of anything, at least not that I remember. -Nathan -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC PATCH] memory driver: make phys_index/end_phys_index reflect the start/end section number
On 04/08/2014 11:23 AM, Nathan Fontenot wrote: On 04/08/2014 11:13 AM, Dave Hansen wrote: On 04/08/2014 01:27 AM, Li Zhong wrote: If Dave and others don't have further objections, it seems this small userspace incompatibility could be accepted by most of us, and I don't need to make a version 2. Let me ask another question then. What are the units of phys_index/end_phys_index? How do we expose those units to userspace? The documentation for these files just states that the files contain the first and last section id of memory in the memory block for phys_index and end_phys_index respectively. I'm not sure the values have ever been units of anything, at least not that I remember. sigh There are two units. SECTION_SIZE, which is completely internal to the kernel, and block_size_bytes which used to be the same as SECTION_SIZE, but is not now. Which one of those two is phys_index/end_phys_index in, and if it is in terms of SECTION_SIZE like this patch proposes, how do we tell userspace how large SECTION_SIZE is? block_size_bytes is supposed to tell you how large the sections are. In the case where we lumped a bunch of sections together, we also bumped up block_size_bytes. That's why we currently divide the *ACTUAL* section number in phys_index/end_phys_index by block_size_bytes. That document really needs to be updated to stop referring to sections (at least in the descriptions of the user interface). We can not change the units of phys_index/end_phys_index without also changing block_size_bytes. -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC PATCH] memory driver: make phys_index/end_phys_index reflect the start/end section number
(2014/04/02 17:56), Li Zhong wrote: I noticed the phys_index and end_phys_index under /sys/devices/system/memory/memoryXXX/ have the same value, e.g. (for the test machine, one memory block has 8 sections, that is sections_per_block equals 8) # cd /sys/devices/system/memory/memory100/ # cat phys_index end_phys_index 0064 0064 Seems they should reflect the start/end section number respectively, which also matches what is said in Documentation/memory-hotplug.txt This patch tries to modify that so the two files could show the start/end section number of the memory block. After this change, output of the above example looks like: # cat phys_index end_phys_index 0320 0327 Signed-off-by: Li Zhong --- Good catch! This is a bug. So I think the bug should be fixed. Reviewed-by: Yasuaki Ishimatsu Thanks, Yasuaki Ishimatsu drivers/base/memory.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/base/memory.c b/drivers/base/memory.c index bece691..b10f2fa 100644 --- a/drivers/base/memory.c +++ b/drivers/base/memory.c @@ -114,7 +114,7 @@ static ssize_t show_mem_start_phys_index(struct device *dev, struct memory_block *mem = to_memory_block(dev); unsigned long phys_index; - phys_index = mem->start_section_nr / sections_per_block; + phys_index = mem->start_section_nr; return sprintf(buf, "%08lx\n", phys_index); } @@ -124,7 +124,7 @@ static ssize_t show_mem_end_phys_index(struct device *dev, struct memory_block *mem = to_memory_block(dev); unsigned long phys_index; - phys_index = mem->end_section_nr / sections_per_block; + phys_index = mem->end_section_nr; return sprintf(buf, "%08lx\n", phys_index); } -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/ -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC PATCH] memory driver: make phys_index/end_phys_index reflect the start/end section number
On Wed, Apr 2, 2014 at 12:09 PM, Dave Hansen wrote: > On 04/02/2014 01:56 AM, Li Zhong wrote: >> I noticed the phys_index and end_phys_index under >> /sys/devices/system/memory/memoryXXX/ have the same value, e.g. >> (for the test machine, one memory block has 8 sections, that is >> sections_per_block equals 8) >> >> # cd /sys/devices/system/memory/memory100/ >> # cat phys_index end_phys_index >> 0064 >> 0064 >> >> Seems they should reflect the start/end section number respectively, which >> also matches what is said in Documentation/memory-hotplug.txt > > This changes a user-visible interface. Won't this break userspace? But who uses this? This is totally broken and I think nobody can use meaningfully. I bet we can fix this right now. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC PATCH] memory driver: make phys_index/end_phys_index reflect the start/end section number
On Thu, 2014-04-03 at 11:06 +0800, Zhang Yanfei wrote:
> On 04/03/2014 10:37 AM, Li Zhong wrote:
> > On Thu, 2014-04-03 at 09:37 +0800, Zhang Yanfei wrote:
> >> Add ccing
> >>
> >> On 04/02/2014 04:56 PM, Li Zhong wrote:
> >>> I noticed the phys_index and end_phys_index under
> >>> /sys/devices/system/memory/memoryXXX/ have the same value, e.g.
> >>> (for the test machine, one memory block has 8 sections, that is
> >>> sections_per_block equals 8)
> >>>
> >>> # cd /sys/devices/system/memory/memory100/
> >>> # cat phys_index end_phys_index
> >>> 0064
> >>> 0064
> >>>
> >>> Seems they should reflect the start/end section number respectively,
> >>> which
> >>> also matches what is said in Documentation/memory-hotplug.txt
> >>
> > Hi Yanfei,
> >
> > Thanks for the review.
> >
> >> Indeed. I've noticed this before. The value in 'end_phys_index' doesn't
> >> match what it really means. But, the name itself is vague, it looks like
> >> it is the index of some page frame. (we keep this name for compatibility?)
> >
> > I guess so, Dave just reminded me that the RFC would also break
> > userspace..
> >
> > And now my plan is:
> > leave the code unchanged
> > update the document, state the end_phys_index/phys_index are the same,
> > and means the memory block index
>
> Ah. I doubt whether there is userspace tool which is using the two sysfiles?
> for example, the memory100 directory itself can tell us which block it is.
> So why there is the two files under it give the same meaning.
>
> If there is userspace tool using the two files, does it use 'end_phys_index'
> in the correct way? That said, if a userspace tool knows what the
> 'end_phys_index'
> really mean, does it still need it since we have 'phys_index' with the same
> value?
For the end_phys_index, I totally agree with you. If somebody tries to
use it, say as the end section number, he should finally be able to find
that the value is not what he expects. But who knows ...
For phys_index, I guess it is also reasonable for some userspace tool to
just loop for each memoryXXX directory under /sys/devices/system/memory,
and use the phys_index as the memory block index for the directory,
instead of extracting the XXX from the directory name memoryXXX
Don't know whether there are some generic rules for handling such kind
of compatibility issues...
> > [optional] create two new files start_sec_nr, end_sec_nr if needed
>
> These two are the really meaningful sysfiles for userspace, IMO.
OK, I see.
>
> >
> > Do you have any other suggestions?
>
> No. I think we should wait for other guys to comment more.
OK, let's wait.
Thanks, Zhong
>
> Thanks.
>
> >
> > Thanks, Zhong
> >
> >>
> >> The corresponding member in struct memory_block is:
> >>
> >> struct memory_block {
> >> unsigned long start_section_nr;
> >> unsigned long end_section_nr;
> >> ...
> >>
> >> The two members seem to have the right name, and have the right value in
> >> kernel.
> >>
> >>
> >>>
> >>> This patch tries to modify that so the two files could show the start/end
> >>> section number of the memory block.
> >>>
> >>> After this change, output of the above example looks like:
> >>>
> >>> # cat phys_index end_phys_index
> >>> 0320
> >>> 0327
> >>>
> >>> Signed-off-by: Li Zhong
> >>> ---
> >>> drivers/base/memory.c | 4 ++--
> >>> 1 file changed, 2 insertions(+), 2 deletions(-)
> >>>
> >>> diff --git a/drivers/base/memory.c b/drivers/base/memory.c
> >>> index bece691..b10f2fa 100644
> >>> --- a/drivers/base/memory.c
> >>> +++ b/drivers/base/memory.c
> >>> @@ -114,7 +114,7 @@ static ssize_t show_mem_start_phys_index(struct
> >>> device *dev,
> >>> struct memory_block *mem = to_memory_block(dev);
> >>> unsigned long phys_index;
> >>>
> >>> - phys_index = mem->start_section_nr / sections_per_block;
> >>> + phys_index = mem->start_section_nr;
> >>> return sprintf(buf, "%08lx\n", phys_index);
> >>> }
> >>>
> >>> @@ -124,7 +124,7 @@ static ssize_t show_mem_end_phys_index(struct device
> >>> *dev,
> >>> struct memory_block *mem = to_memory_block(dev);
> >>> unsigned long phys_index;
> >>>
> >>> - phys_index = mem->end_section_nr / sections_per_block;
> >>> + phys_index = mem->end_section_nr;
> >>> return sprintf(buf, "%08lx\n", phys_index);
> >>> }
> >>>
> >>>
> >>>
> >>> --
> >>> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> >>> the body of a message to majord...@vger.kernel.org
> >>> More majordomo info at http://vger.kernel.org/majordomo-info.html
> >>> Please read the FAQ at http://www.tux.org/lkml/
> >>>
> >>
> >>
> >
> >
> > .
> >
>
>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC PATCH] memory driver: make phys_index/end_phys_index reflect the start/end section number
On Wed, Apr 2, 2014 at 12:09 PM, Dave Hansen dave.han...@intel.com wrote: On 04/02/2014 01:56 AM, Li Zhong wrote: I noticed the phys_index and end_phys_index under /sys/devices/system/memory/memoryXXX/ have the same value, e.g. (for the test machine, one memory block has 8 sections, that is sections_per_block equals 8) # cd /sys/devices/system/memory/memory100/ # cat phys_index end_phys_index 0064 0064 Seems they should reflect the start/end section number respectively, which also matches what is said in Documentation/memory-hotplug.txt This changes a user-visible interface. Won't this break userspace? But who uses this? This is totally broken and I think nobody can use meaningfully. I bet we can fix this right now. -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC PATCH] memory driver: make phys_index/end_phys_index reflect the start/end section number
(2014/04/02 17:56), Li Zhong wrote: I noticed the phys_index and end_phys_index under /sys/devices/system/memory/memoryXXX/ have the same value, e.g. (for the test machine, one memory block has 8 sections, that is sections_per_block equals 8) # cd /sys/devices/system/memory/memory100/ # cat phys_index end_phys_index 0064 0064 Seems they should reflect the start/end section number respectively, which also matches what is said in Documentation/memory-hotplug.txt This patch tries to modify that so the two files could show the start/end section number of the memory block. After this change, output of the above example looks like: # cat phys_index end_phys_index 0320 0327 Signed-off-by: Li Zhong zh...@linux.vnet.ibm.com --- Good catch! This is a bug. So I think the bug should be fixed. Reviewed-by: Yasuaki Ishimatsu isimatu.yasu...@jp.fujitsu.com Thanks, Yasuaki Ishimatsu drivers/base/memory.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/base/memory.c b/drivers/base/memory.c index bece691..b10f2fa 100644 --- a/drivers/base/memory.c +++ b/drivers/base/memory.c @@ -114,7 +114,7 @@ static ssize_t show_mem_start_phys_index(struct device *dev, struct memory_block *mem = to_memory_block(dev); unsigned long phys_index; - phys_index = mem-start_section_nr / sections_per_block; + phys_index = mem-start_section_nr; return sprintf(buf, %08lx\n, phys_index); } @@ -124,7 +124,7 @@ static ssize_t show_mem_end_phys_index(struct device *dev, struct memory_block *mem = to_memory_block(dev); unsigned long phys_index; - phys_index = mem-end_section_nr / sections_per_block; + phys_index = mem-end_section_nr; return sprintf(buf, %08lx\n, phys_index); } -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/ -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC PATCH] memory driver: make phys_index/end_phys_index reflect the start/end section number
On Thu, 2014-04-03 at 11:06 +0800, Zhang Yanfei wrote:
On 04/03/2014 10:37 AM, Li Zhong wrote:
On Thu, 2014-04-03 at 09:37 +0800, Zhang Yanfei wrote:
Add ccing
On 04/02/2014 04:56 PM, Li Zhong wrote:
I noticed the phys_index and end_phys_index under
/sys/devices/system/memory/memoryXXX/ have the same value, e.g.
(for the test machine, one memory block has 8 sections, that is
sections_per_block equals 8)
# cd /sys/devices/system/memory/memory100/
# cat phys_index end_phys_index
0064
0064
Seems they should reflect the start/end section number respectively,
which
also matches what is said in Documentation/memory-hotplug.txt
Hi Yanfei,
Thanks for the review.
Indeed. I've noticed this before. The value in 'end_phys_index' doesn't
match what it really means. But, the name itself is vague, it looks like
it is the index of some page frame. (we keep this name for compatibility?)
I guess so, Dave just reminded me that the RFC would also break
userspace..
And now my plan is:
leave the code unchanged
update the document, state the end_phys_index/phys_index are the same,
and means the memory block index
Ah. I doubt whether there is userspace tool which is using the two sysfiles?
for example, the memory100 directory itself can tell us which block it is.
So why there is the two files under it give the same meaning.
If there is userspace tool using the two files, does it use 'end_phys_index'
in the correct way? That said, if a userspace tool knows what the
'end_phys_index'
really mean, does it still need it since we have 'phys_index' with the same
value?
For the end_phys_index, I totally agree with you. If somebody tries to
use it, say as the end section number, he should finally be able to find
that the value is not what he expects. But who knows ...
For phys_index, I guess it is also reasonable for some userspace tool to
just loop for each memoryXXX directory under /sys/devices/system/memory,
and use the phys_index as the memory block index for the directory,
instead of extracting the XXX from the directory name memoryXXX
Don't know whether there are some generic rules for handling such kind
of compatibility issues...
[optional] create two new files start_sec_nr, end_sec_nr if needed
These two are the really meaningful sysfiles for userspace, IMO.
OK, I see.
Do you have any other suggestions?
No. I think we should wait for other guys to comment more.
OK, let's wait.
Thanks, Zhong
Thanks.
Thanks, Zhong
The corresponding member in struct memory_block is:
struct memory_block {
unsigned long start_section_nr;
unsigned long end_section_nr;
...
The two members seem to have the right name, and have the right value in
kernel.
This patch tries to modify that so the two files could show the start/end
section number of the memory block.
After this change, output of the above example looks like:
# cat phys_index end_phys_index
0320
0327
Signed-off-by: Li Zhong zh...@linux.vnet.ibm.com
---
drivers/base/memory.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/base/memory.c b/drivers/base/memory.c
index bece691..b10f2fa 100644
--- a/drivers/base/memory.c
+++ b/drivers/base/memory.c
@@ -114,7 +114,7 @@ static ssize_t show_mem_start_phys_index(struct
device *dev,
struct memory_block *mem = to_memory_block(dev);
unsigned long phys_index;
- phys_index = mem-start_section_nr / sections_per_block;
+ phys_index = mem-start_section_nr;
return sprintf(buf, %08lx\n, phys_index);
}
@@ -124,7 +124,7 @@ static ssize_t show_mem_end_phys_index(struct device
*dev,
struct memory_block *mem = to_memory_block(dev);
unsigned long phys_index;
- phys_index = mem-end_section_nr / sections_per_block;
+ phys_index = mem-end_section_nr;
return sprintf(buf, %08lx\n, phys_index);
}
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
.
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC PATCH] memory driver: make phys_index/end_phys_index reflect the start/end section number
On 04/03/2014 10:37 AM, Li Zhong wrote:
> On Thu, 2014-04-03 at 09:37 +0800, Zhang Yanfei wrote:
>> Add ccing
>>
>> On 04/02/2014 04:56 PM, Li Zhong wrote:
>>> I noticed the phys_index and end_phys_index under
>>> /sys/devices/system/memory/memoryXXX/ have the same value, e.g.
>>> (for the test machine, one memory block has 8 sections, that is
>>> sections_per_block equals 8)
>>>
>>> # cd /sys/devices/system/memory/memory100/
>>> # cat phys_index end_phys_index
>>> 0064
>>> 0064
>>>
>>> Seems they should reflect the start/end section number respectively, which
>>> also matches what is said in Documentation/memory-hotplug.txt
>>
> Hi Yanfei,
>
> Thanks for the review.
>
>> Indeed. I've noticed this before. The value in 'end_phys_index' doesn't
>> match what it really means. But, the name itself is vague, it looks like
>> it is the index of some page frame. (we keep this name for compatibility?)
>
> I guess so, Dave just reminded me that the RFC would also break
> userspace..
>
> And now my plan is:
> leave the code unchanged
> update the document, state the end_phys_index/phys_index are the same,
> and means the memory block index
Ah. I doubt whether there is userspace tool which is using the two sysfiles?
for example, the memory100 directory itself can tell us which block it is.
So why there is the two files under it give the same meaning.
If there is userspace tool using the two files, does it use 'end_phys_index'
in the correct way? That said, if a userspace tool knows what the
'end_phys_index'
really mean, does it still need it since we have 'phys_index' with the same
value?
> [optional] create two new files start_sec_nr, end_sec_nr if needed
These two are the really meaningful sysfiles for userspace, IMO.
>
> Do you have any other suggestions?
No. I think we should wait for other guys to comment more.
Thanks.
>
> Thanks, Zhong
>
>>
>> The corresponding member in struct memory_block is:
>>
>> struct memory_block {
>> unsigned long start_section_nr;
>> unsigned long end_section_nr;
>> ...
>>
>> The two members seem to have the right name, and have the right value in
>> kernel.
>>
>>
>>>
>>> This patch tries to modify that so the two files could show the start/end
>>> section number of the memory block.
>>>
>>> After this change, output of the above example looks like:
>>>
>>> # cat phys_index end_phys_index
>>> 0320
>>> 0327
>>>
>>> Signed-off-by: Li Zhong
>>> ---
>>> drivers/base/memory.c | 4 ++--
>>> 1 file changed, 2 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/drivers/base/memory.c b/drivers/base/memory.c
>>> index bece691..b10f2fa 100644
>>> --- a/drivers/base/memory.c
>>> +++ b/drivers/base/memory.c
>>> @@ -114,7 +114,7 @@ static ssize_t show_mem_start_phys_index(struct device
>>> *dev,
>>> struct memory_block *mem = to_memory_block(dev);
>>> unsigned long phys_index;
>>>
>>> - phys_index = mem->start_section_nr / sections_per_block;
>>> + phys_index = mem->start_section_nr;
>>> return sprintf(buf, "%08lx\n", phys_index);
>>> }
>>>
>>> @@ -124,7 +124,7 @@ static ssize_t show_mem_end_phys_index(struct device
>>> *dev,
>>> struct memory_block *mem = to_memory_block(dev);
>>> unsigned long phys_index;
>>>
>>> - phys_index = mem->end_section_nr / sections_per_block;
>>> + phys_index = mem->end_section_nr;
>>> return sprintf(buf, "%08lx\n", phys_index);
>>> }
>>>
>>>
>>>
>>> --
>>> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
>>> the body of a message to majord...@vger.kernel.org
>>> More majordomo info at http://vger.kernel.org/majordomo-info.html
>>> Please read the FAQ at http://www.tux.org/lkml/
>>>
>>
>>
>
>
> .
>
--
Thanks.
Zhang Yanfei
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC PATCH] memory driver: make phys_index/end_phys_index reflect the start/end section number
On Thu, 2014-04-03 at 09:37 +0800, Zhang Yanfei wrote:
> Add ccing
>
> On 04/02/2014 04:56 PM, Li Zhong wrote:
> > I noticed the phys_index and end_phys_index under
> > /sys/devices/system/memory/memoryXXX/ have the same value, e.g.
> > (for the test machine, one memory block has 8 sections, that is
> > sections_per_block equals 8)
> >
> > # cd /sys/devices/system/memory/memory100/
> > # cat phys_index end_phys_index
> > 0064
> > 0064
> >
> > Seems they should reflect the start/end section number respectively, which
> > also matches what is said in Documentation/memory-hotplug.txt
>
Hi Yanfei,
Thanks for the review.
> Indeed. I've noticed this before. The value in 'end_phys_index' doesn't
> match what it really means. But, the name itself is vague, it looks like
> it is the index of some page frame. (we keep this name for compatibility?)
I guess so, Dave just reminded me that the RFC would also break
userspace..
And now my plan is:
leave the code unchanged
update the document, state the end_phys_index/phys_index are the same,
and means the memory block index
[optional] create two new files start_sec_nr, end_sec_nr if needed
Do you have any other suggestions?
Thanks, Zhong
>
> The corresponding member in struct memory_block is:
>
> struct memory_block {
> unsigned long start_section_nr;
> unsigned long end_section_nr;
> ...
>
> The two members seem to have the right name, and have the right value in
> kernel.
>
>
> >
> > This patch tries to modify that so the two files could show the start/end
> > section number of the memory block.
> >
> > After this change, output of the above example looks like:
> >
> > # cat phys_index end_phys_index
> > 0320
> > 0327
> >
> > Signed-off-by: Li Zhong
> > ---
> > drivers/base/memory.c | 4 ++--
> > 1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/base/memory.c b/drivers/base/memory.c
> > index bece691..b10f2fa 100644
> > --- a/drivers/base/memory.c
> > +++ b/drivers/base/memory.c
> > @@ -114,7 +114,7 @@ static ssize_t show_mem_start_phys_index(struct device
> > *dev,
> > struct memory_block *mem = to_memory_block(dev);
> > unsigned long phys_index;
> >
> > - phys_index = mem->start_section_nr / sections_per_block;
> > + phys_index = mem->start_section_nr;
> > return sprintf(buf, "%08lx\n", phys_index);
> > }
> >
> > @@ -124,7 +124,7 @@ static ssize_t show_mem_end_phys_index(struct device
> > *dev,
> > struct memory_block *mem = to_memory_block(dev);
> > unsigned long phys_index;
> >
> > - phys_index = mem->end_section_nr / sections_per_block;
> > + phys_index = mem->end_section_nr;
> > return sprintf(buf, "%08lx\n", phys_index);
> > }
> >
> >
> >
> > --
> > To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> > the body of a message to majord...@vger.kernel.org
> > More majordomo info at http://vger.kernel.org/majordomo-info.html
> > Please read the FAQ at http://www.tux.org/lkml/
> >
>
>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC PATCH] memory driver: make phys_index/end_phys_index reflect the start/end section number
On Wed, 2014-04-02 at 09:09 -0700, Dave Hansen wrote: > On 04/02/2014 01:56 AM, Li Zhong wrote: > > I noticed the phys_index and end_phys_index under > > /sys/devices/system/memory/memoryXXX/ have the same value, e.g. > > (for the test machine, one memory block has 8 sections, that is > > sections_per_block equals 8) > > > > # cd /sys/devices/system/memory/memory100/ > > # cat phys_index end_phys_index > > 0064 > > 0064 > > > > Seems they should reflect the start/end section number respectively, which > > also matches what is said in Documentation/memory-hotplug.txt > > This changes a user-visible interface. Won't this break userspace? Hi Dave, Hmm, I think so... Thank you for the reminder Do you have some better ideas to fix this? Maybe we should leave the code unchanged, just change the corresponding document (just it seems that the end_phys_index will always be the same as phys_index)? And if the section numbers are really needed, then we could create two new files later, e.g. start_section_nr, end_section_nr? Thanks, Zhong > -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC PATCH] memory driver: make phys_index/end_phys_index reflect the start/end section number
Add ccing
On 04/02/2014 04:56 PM, Li Zhong wrote:
> I noticed the phys_index and end_phys_index under
> /sys/devices/system/memory/memoryXXX/ have the same value, e.g.
> (for the test machine, one memory block has 8 sections, that is
> sections_per_block equals 8)
>
> # cd /sys/devices/system/memory/memory100/
> # cat phys_index end_phys_index
> 0064
> 0064
>
> Seems they should reflect the start/end section number respectively, which
> also matches what is said in Documentation/memory-hotplug.txt
Indeed. I've noticed this before. The value in 'end_phys_index' doesn't
match what it really means. But, the name itself is vague, it looks like
it is the index of some page frame. (we keep this name for compatibility?)
The corresponding member in struct memory_block is:
struct memory_block {
unsigned long start_section_nr;
unsigned long end_section_nr;
...
The two members seem to have the right name, and have the right value in kernel.
>
> This patch tries to modify that so the two files could show the start/end
> section number of the memory block.
>
> After this change, output of the above example looks like:
>
> # cat phys_index end_phys_index
> 0320
> 0327
>
> Signed-off-by: Li Zhong
> ---
> drivers/base/memory.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/base/memory.c b/drivers/base/memory.c
> index bece691..b10f2fa 100644
> --- a/drivers/base/memory.c
> +++ b/drivers/base/memory.c
> @@ -114,7 +114,7 @@ static ssize_t show_mem_start_phys_index(struct device
> *dev,
> struct memory_block *mem = to_memory_block(dev);
> unsigned long phys_index;
>
> - phys_index = mem->start_section_nr / sections_per_block;
> + phys_index = mem->start_section_nr;
> return sprintf(buf, "%08lx\n", phys_index);
> }
>
> @@ -124,7 +124,7 @@ static ssize_t show_mem_end_phys_index(struct device *dev,
> struct memory_block *mem = to_memory_block(dev);
> unsigned long phys_index;
>
> - phys_index = mem->end_section_nr / sections_per_block;
> + phys_index = mem->end_section_nr;
> return sprintf(buf, "%08lx\n", phys_index);
> }
>
>
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/
>
--
Thanks.
Zhang Yanfei
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC PATCH] memory driver: make phys_index/end_phys_index reflect the start/end section number
On 04/02/2014 01:56 AM, Li Zhong wrote: > I noticed the phys_index and end_phys_index under > /sys/devices/system/memory/memoryXXX/ have the same value, e.g. > (for the test machine, one memory block has 8 sections, that is > sections_per_block equals 8) > > # cd /sys/devices/system/memory/memory100/ > # cat phys_index end_phys_index > 0064 > 0064 > > Seems they should reflect the start/end section number respectively, which > also matches what is said in Documentation/memory-hotplug.txt This changes a user-visible interface. Won't this break userspace? -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[RFC PATCH] memory driver: make phys_index/end_phys_index reflect the start/end section number
I noticed the phys_index and end_phys_index under /sys/devices/system/memory/memoryXXX/ have the same value, e.g. (for the test machine, one memory block has 8 sections, that is sections_per_block equals 8) # cd /sys/devices/system/memory/memory100/ # cat phys_index end_phys_index 0064 0064 Seems they should reflect the start/end section number respectively, which also matches what is said in Documentation/memory-hotplug.txt This patch tries to modify that so the two files could show the start/end section number of the memory block. After this change, output of the above example looks like: # cat phys_index end_phys_index 0320 0327 Signed-off-by: Li Zhong --- drivers/base/memory.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/base/memory.c b/drivers/base/memory.c index bece691..b10f2fa 100644 --- a/drivers/base/memory.c +++ b/drivers/base/memory.c @@ -114,7 +114,7 @@ static ssize_t show_mem_start_phys_index(struct device *dev, struct memory_block *mem = to_memory_block(dev); unsigned long phys_index; - phys_index = mem->start_section_nr / sections_per_block; + phys_index = mem->start_section_nr; return sprintf(buf, "%08lx\n", phys_index); } @@ -124,7 +124,7 @@ static ssize_t show_mem_end_phys_index(struct device *dev, struct memory_block *mem = to_memory_block(dev); unsigned long phys_index; - phys_index = mem->end_section_nr / sections_per_block; + phys_index = mem->end_section_nr; return sprintf(buf, "%08lx\n", phys_index); } -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[RFC PATCH] memory driver: make phys_index/end_phys_index reflect the start/end section number
I noticed the phys_index and end_phys_index under /sys/devices/system/memory/memoryXXX/ have the same value, e.g. (for the test machine, one memory block has 8 sections, that is sections_per_block equals 8) # cd /sys/devices/system/memory/memory100/ # cat phys_index end_phys_index 0064 0064 Seems they should reflect the start/end section number respectively, which also matches what is said in Documentation/memory-hotplug.txt This patch tries to modify that so the two files could show the start/end section number of the memory block. After this change, output of the above example looks like: # cat phys_index end_phys_index 0320 0327 Signed-off-by: Li Zhong zh...@linux.vnet.ibm.com --- drivers/base/memory.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/base/memory.c b/drivers/base/memory.c index bece691..b10f2fa 100644 --- a/drivers/base/memory.c +++ b/drivers/base/memory.c @@ -114,7 +114,7 @@ static ssize_t show_mem_start_phys_index(struct device *dev, struct memory_block *mem = to_memory_block(dev); unsigned long phys_index; - phys_index = mem-start_section_nr / sections_per_block; + phys_index = mem-start_section_nr; return sprintf(buf, %08lx\n, phys_index); } @@ -124,7 +124,7 @@ static ssize_t show_mem_end_phys_index(struct device *dev, struct memory_block *mem = to_memory_block(dev); unsigned long phys_index; - phys_index = mem-end_section_nr / sections_per_block; + phys_index = mem-end_section_nr; return sprintf(buf, %08lx\n, phys_index); } -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC PATCH] memory driver: make phys_index/end_phys_index reflect the start/end section number
On 04/02/2014 01:56 AM, Li Zhong wrote: I noticed the phys_index and end_phys_index under /sys/devices/system/memory/memoryXXX/ have the same value, e.g. (for the test machine, one memory block has 8 sections, that is sections_per_block equals 8) # cd /sys/devices/system/memory/memory100/ # cat phys_index end_phys_index 0064 0064 Seems they should reflect the start/end section number respectively, which also matches what is said in Documentation/memory-hotplug.txt This changes a user-visible interface. Won't this break userspace? -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC PATCH] memory driver: make phys_index/end_phys_index reflect the start/end section number
Add ccing
On 04/02/2014 04:56 PM, Li Zhong wrote:
I noticed the phys_index and end_phys_index under
/sys/devices/system/memory/memoryXXX/ have the same value, e.g.
(for the test machine, one memory block has 8 sections, that is
sections_per_block equals 8)
# cd /sys/devices/system/memory/memory100/
# cat phys_index end_phys_index
0064
0064
Seems they should reflect the start/end section number respectively, which
also matches what is said in Documentation/memory-hotplug.txt
Indeed. I've noticed this before. The value in 'end_phys_index' doesn't
match what it really means. But, the name itself is vague, it looks like
it is the index of some page frame. (we keep this name for compatibility?)
The corresponding member in struct memory_block is:
struct memory_block {
unsigned long start_section_nr;
unsigned long end_section_nr;
...
The two members seem to have the right name, and have the right value in kernel.
This patch tries to modify that so the two files could show the start/end
section number of the memory block.
After this change, output of the above example looks like:
# cat phys_index end_phys_index
0320
0327
Signed-off-by: Li Zhong zh...@linux.vnet.ibm.com
---
drivers/base/memory.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/base/memory.c b/drivers/base/memory.c
index bece691..b10f2fa 100644
--- a/drivers/base/memory.c
+++ b/drivers/base/memory.c
@@ -114,7 +114,7 @@ static ssize_t show_mem_start_phys_index(struct device
*dev,
struct memory_block *mem = to_memory_block(dev);
unsigned long phys_index;
- phys_index = mem-start_section_nr / sections_per_block;
+ phys_index = mem-start_section_nr;
return sprintf(buf, %08lx\n, phys_index);
}
@@ -124,7 +124,7 @@ static ssize_t show_mem_end_phys_index(struct device *dev,
struct memory_block *mem = to_memory_block(dev);
unsigned long phys_index;
- phys_index = mem-end_section_nr / sections_per_block;
+ phys_index = mem-end_section_nr;
return sprintf(buf, %08lx\n, phys_index);
}
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
--
Thanks.
Zhang Yanfei
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC PATCH] memory driver: make phys_index/end_phys_index reflect the start/end section number
On Wed, 2014-04-02 at 09:09 -0700, Dave Hansen wrote: On 04/02/2014 01:56 AM, Li Zhong wrote: I noticed the phys_index and end_phys_index under /sys/devices/system/memory/memoryXXX/ have the same value, e.g. (for the test machine, one memory block has 8 sections, that is sections_per_block equals 8) # cd /sys/devices/system/memory/memory100/ # cat phys_index end_phys_index 0064 0064 Seems they should reflect the start/end section number respectively, which also matches what is said in Documentation/memory-hotplug.txt This changes a user-visible interface. Won't this break userspace? Hi Dave, Hmm, I think so... Thank you for the reminder Do you have some better ideas to fix this? Maybe we should leave the code unchanged, just change the corresponding document (just it seems that the end_phys_index will always be the same as phys_index)? And if the section numbers are really needed, then we could create two new files later, e.g. start_section_nr, end_section_nr? Thanks, Zhong -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC PATCH] memory driver: make phys_index/end_phys_index reflect the start/end section number
On Thu, 2014-04-03 at 09:37 +0800, Zhang Yanfei wrote:
Add ccing
On 04/02/2014 04:56 PM, Li Zhong wrote:
I noticed the phys_index and end_phys_index under
/sys/devices/system/memory/memoryXXX/ have the same value, e.g.
(for the test machine, one memory block has 8 sections, that is
sections_per_block equals 8)
# cd /sys/devices/system/memory/memory100/
# cat phys_index end_phys_index
0064
0064
Seems they should reflect the start/end section number respectively, which
also matches what is said in Documentation/memory-hotplug.txt
Hi Yanfei,
Thanks for the review.
Indeed. I've noticed this before. The value in 'end_phys_index' doesn't
match what it really means. But, the name itself is vague, it looks like
it is the index of some page frame. (we keep this name for compatibility?)
I guess so, Dave just reminded me that the RFC would also break
userspace..
And now my plan is:
leave the code unchanged
update the document, state the end_phys_index/phys_index are the same,
and means the memory block index
[optional] create two new files start_sec_nr, end_sec_nr if needed
Do you have any other suggestions?
Thanks, Zhong
The corresponding member in struct memory_block is:
struct memory_block {
unsigned long start_section_nr;
unsigned long end_section_nr;
...
The two members seem to have the right name, and have the right value in
kernel.
This patch tries to modify that so the two files could show the start/end
section number of the memory block.
After this change, output of the above example looks like:
# cat phys_index end_phys_index
0320
0327
Signed-off-by: Li Zhong zh...@linux.vnet.ibm.com
---
drivers/base/memory.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/base/memory.c b/drivers/base/memory.c
index bece691..b10f2fa 100644
--- a/drivers/base/memory.c
+++ b/drivers/base/memory.c
@@ -114,7 +114,7 @@ static ssize_t show_mem_start_phys_index(struct device
*dev,
struct memory_block *mem = to_memory_block(dev);
unsigned long phys_index;
- phys_index = mem-start_section_nr / sections_per_block;
+ phys_index = mem-start_section_nr;
return sprintf(buf, %08lx\n, phys_index);
}
@@ -124,7 +124,7 @@ static ssize_t show_mem_end_phys_index(struct device
*dev,
struct memory_block *mem = to_memory_block(dev);
unsigned long phys_index;
- phys_index = mem-end_section_nr / sections_per_block;
+ phys_index = mem-end_section_nr;
return sprintf(buf, %08lx\n, phys_index);
}
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC PATCH] memory driver: make phys_index/end_phys_index reflect the start/end section number
On 04/03/2014 10:37 AM, Li Zhong wrote:
On Thu, 2014-04-03 at 09:37 +0800, Zhang Yanfei wrote:
Add ccing
On 04/02/2014 04:56 PM, Li Zhong wrote:
I noticed the phys_index and end_phys_index under
/sys/devices/system/memory/memoryXXX/ have the same value, e.g.
(for the test machine, one memory block has 8 sections, that is
sections_per_block equals 8)
# cd /sys/devices/system/memory/memory100/
# cat phys_index end_phys_index
0064
0064
Seems they should reflect the start/end section number respectively, which
also matches what is said in Documentation/memory-hotplug.txt
Hi Yanfei,
Thanks for the review.
Indeed. I've noticed this before. The value in 'end_phys_index' doesn't
match what it really means. But, the name itself is vague, it looks like
it is the index of some page frame. (we keep this name for compatibility?)
I guess so, Dave just reminded me that the RFC would also break
userspace..
And now my plan is:
leave the code unchanged
update the document, state the end_phys_index/phys_index are the same,
and means the memory block index
Ah. I doubt whether there is userspace tool which is using the two sysfiles?
for example, the memory100 directory itself can tell us which block it is.
So why there is the two files under it give the same meaning.
If there is userspace tool using the two files, does it use 'end_phys_index'
in the correct way? That said, if a userspace tool knows what the
'end_phys_index'
really mean, does it still need it since we have 'phys_index' with the same
value?
[optional] create two new files start_sec_nr, end_sec_nr if needed
These two are the really meaningful sysfiles for userspace, IMO.
Do you have any other suggestions?
No. I think we should wait for other guys to comment more.
Thanks.
Thanks, Zhong
The corresponding member in struct memory_block is:
struct memory_block {
unsigned long start_section_nr;
unsigned long end_section_nr;
...
The two members seem to have the right name, and have the right value in
kernel.
This patch tries to modify that so the two files could show the start/end
section number of the memory block.
After this change, output of the above example looks like:
# cat phys_index end_phys_index
0320
0327
Signed-off-by: Li Zhong zh...@linux.vnet.ibm.com
---
drivers/base/memory.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/base/memory.c b/drivers/base/memory.c
index bece691..b10f2fa 100644
--- a/drivers/base/memory.c
+++ b/drivers/base/memory.c
@@ -114,7 +114,7 @@ static ssize_t show_mem_start_phys_index(struct device
*dev,
struct memory_block *mem = to_memory_block(dev);
unsigned long phys_index;
- phys_index = mem-start_section_nr / sections_per_block;
+ phys_index = mem-start_section_nr;
return sprintf(buf, %08lx\n, phys_index);
}
@@ -124,7 +124,7 @@ static ssize_t show_mem_end_phys_index(struct device
*dev,
struct memory_block *mem = to_memory_block(dev);
unsigned long phys_index;
- phys_index = mem-end_section_nr / sections_per_block;
+ phys_index = mem-end_section_nr;
return sprintf(buf, %08lx\n, phys_index);
}
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
.
--
Thanks.
Zhang Yanfei
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
