Re: [PATCH] drivers: base: new memory config sysfs driver for large memory systems

Re: [PATCH] drivers: base: new memory config sysfs driver for large memory systems

[Greg Kroah-Hartman]

On Thu, Jul 25, 2013 at 04:11:20PM -0500, Seth Jennings wrote:
> From: Nathan Fontenot <nfont@linux.vnet.ibm.com>
> 
> Large memory systems (1TB or more) experience boot delays on the order
> of minutes due to the initializing the memory configuration part of
> sysfs at /sys/devices/system/memory/.
> 
> ppc64 has a memory block size of 256M and (I think) x86 is 128M.  With 1TB
> of RAM and a 256M block size, that's 4k memory blocks with 20 sysfs
> entries per block that's around 80k items that need be created at boot
> time in sysfs.  Some systems go up to 16TB where the issue is
> even more severe.
> 
> This patch is a prototype for a new sysfs memory layout where the
> entries are created on demand by writing memory block numbers into a
> "show" and "hide" files to create and destroy the memory block
> configuration attributes in sysfs.  This would decouple the number of
> sysfs entries created at boot time from the memory size, resulting in a
> sysfs initialization time that doesn't increase and memory size
> increase.
> 
> Signed-off-by: Seth Jennings <sjenning@linux.vnet.ibm.com>
> Signed-off-by: Nathan Fontenot <nfont@linux.vnet.ibm.com>

How does this tie into the patches Nathan sent yesterday for memory
hotplug stuff that I thought modified the same part of the kernel?

thanks,

greg k-h

Re: [PATCH] drivers: base: new memory config sysfs driver for large memory systems

[Seth Jennings]

On Thu, Jul 25, 2013 at 04:38:00PM -0700, Greg Kroah-Hartman wrote:
> On Thu, Jul 25, 2013 at 04:11:20PM -0500, Seth Jennings wrote:
> > From: Nathan Fontenot <nfont@linux.vnet.ibm.com>
> > 
> > Large memory systems (1TB or more) experience boot delays on the order
> > of minutes due to the initializing the memory configuration part of
> > sysfs at /sys/devices/system/memory/.
> > 
> > ppc64 has a memory block size of 256M and (I think) x86 is 128M.  With 1TB
> > of RAM and a 256M block size, that's 4k memory blocks with 20 sysfs
> > entries per block that's around 80k items that need be created at boot
> > time in sysfs.  Some systems go up to 16TB where the issue is
> > even more severe.
> > 
> > This patch is a prototype for a new sysfs memory layout where the
> > entries are created on demand by writing memory block numbers into a
> > "show" and "hide" files to create and destroy the memory block
> > configuration attributes in sysfs.  This would decouple the number of
> > sysfs entries created at boot time from the memory size, resulting in a
> > sysfs initialization time that doesn't increase and memory size
> > increase.
> > 
> > Signed-off-by: Seth Jennings <sjenning@linux.vnet.ibm.com>
> > Signed-off-by: Nathan Fontenot <nfont@linux.vnet.ibm.com>
> 
> How does this tie into the patches Nathan sent yesterday for memory
> hotplug stuff that I thought modified the same part of the kernel?

So this patch introduces the new layout in a new file
drives/base/memfs.c (which, in light of your last comment should
probably be something more like largememory.c).

It doesn't clash with Nathan's, but it doesn't contain the new
is_memblock_[removable|offline] functions or the new "release"
attribute.  But that can be added easily.

Seth

Re: [PATCH] drivers: base: new memory config sysfs driver for large memory systems

[Greg Kroah-Hartman]

On Fri, Jul 26, 2013 at 09:33:44AM -0500, Seth Jennings wrote:
> On Thu, Jul 25, 2013 at 04:38:00PM -0700, Greg Kroah-Hartman wrote:
> > On Thu, Jul 25, 2013 at 04:11:20PM -0500, Seth Jennings wrote:
> > > From: Nathan Fontenot <nfont@linux.vnet.ibm.com>
> > > 
> > > Large memory systems (1TB or more) experience boot delays on the order
> > > of minutes due to the initializing the memory configuration part of
> > > sysfs at /sys/devices/system/memory/.
> > > 
> > > ppc64 has a memory block size of 256M and (I think) x86 is 128M.  With 1TB
> > > of RAM and a 256M block size, that's 4k memory blocks with 20 sysfs
> > > entries per block that's around 80k items that need be created at boot
> > > time in sysfs.  Some systems go up to 16TB where the issue is
> > > even more severe.
> > > 
> > > This patch is a prototype for a new sysfs memory layout where the
> > > entries are created on demand by writing memory block numbers into a
> > > "show" and "hide" files to create and destroy the memory block
> > > configuration attributes in sysfs.  This would decouple the number of
> > > sysfs entries created at boot time from the memory size, resulting in a
> > > sysfs initialization time that doesn't increase and memory size
> > > increase.
> > > 
> > > Signed-off-by: Seth Jennings <sjenning@linux.vnet.ibm.com>
> > > Signed-off-by: Nathan Fontenot <nfont@linux.vnet.ibm.com>
> > 
> > How does this tie into the patches Nathan sent yesterday for memory
> > hotplug stuff that I thought modified the same part of the kernel?
> 
> So this patch introduces the new layout in a new file
> drives/base/memfs.c (which, in light of your last comment should
> probably be something more like largememory.c).
> 
> It doesn't clash with Nathan's, but it doesn't contain the new
> is_memblock_[removable|offline] functions or the new "release"
> attribute.  But that can be added easily.

Can you two please work together on this so I we don't have to deal with
competing patch sets and get confused as to what is really going on
here?

thanks,

greg k-h

Re: [PATCH] drivers: base: new memory config sysfs driver for large memory systems

[Greg Kroah-Hartman]

On Thu, Jul 25, 2013 at 04:11:20PM -0500, Seth Jennings wrote:
> +#define MEMFS_CLASS_NAME	"memoryfs"

One question, a "*fs" name in the kernel usually implies it is a
separate filesystem, which this isn't at all, it's just a "normal"
class/subsystem in the kernel.  So how about "memory" instead?

thanks,

greg k-h

Re: [PATCH] drivers: base: new memory config sysfs driver for large memory systems

[Seth Jennings]

On Thu, Jul 25, 2013 at 04:40:07PM -0700, Greg Kroah-Hartman wrote:
> On Thu, Jul 25, 2013 at 04:11:20PM -0500, Seth Jennings wrote:
> > +#define MEMFS_CLASS_NAME	"memoryfs"
> 
> One question, a "*fs" name in the kernel usually implies it is a
> separate filesystem, which this isn't at all, it's just a "normal"
> class/subsystem in the kernel.  So how about "memory" instead?

"memory" is the name used by the current sysfs memory layout code in
drivers/base/memory.c. So it can't be the same unless we are going to
create a toggle a boot time to select between the models, which is
something I am looking to add if this code/design is acceptable to
people.

The design is that people with large memory systems would pass a boot
parameter that selects this alternate layout, so that the majority
of non-large-memory users and any userspace programs that depend on the
old layout would be unaffected.

In the meantime, the name "memfs" was chosen for the RFC so that people
could compile and run the new model concurrently with the current model.

Seth

Re: [PATCH] drivers: base: new memory config sysfs driver for large memory systems

[Greg Kroah-Hartman]

On Fri, Jul 26, 2013 at 09:42:51AM -0500, Seth Jennings wrote:

Sorry for the delay, google decided to mark your responses as "spam" :(

> On Thu, Jul 25, 2013 at 04:40:07PM -0700, Greg Kroah-Hartman wrote:
> > On Thu, Jul 25, 2013 at 04:11:20PM -0500, Seth Jennings wrote:
> > > +#define MEMFS_CLASS_NAME	"memoryfs"
> > 
> > One question, a "*fs" name in the kernel usually implies it is a
> > separate filesystem, which this isn't at all, it's just a "normal"
> > class/subsystem in the kernel.  So how about "memory" instead?
> 
> "memory" is the name used by the current sysfs memory layout code in
> drivers/base/memory.c. So it can't be the same unless we are going to
> create a toggle a boot time to select between the models, which is
> something I am looking to add if this code/design is acceptable to
> people.

I know it can't be the same, but this is like "memory_v2" or something,
right?  I suggest you make it an either/or option, given that you feel
the existing layout just will not work properly for you.

> The design is that people with large memory systems would pass a boot
> parameter that selects this alternate layout, so that the majority
> of non-large-memory users and any userspace programs that depend on the
> old layout would be unaffected.
> 
> In the meantime, the name "memfs" was chosen for the RFC so that people
> could compile and run the new model concurrently with the current model.

It's a really bad name for a driver subsystem, please don't use it.

thanks,

greg k-h

Re: [PATCH] drivers: base: new memory config sysfs driver for large memory systems

[Dave Hansen]

On 08/01/2013 01:57 PM, Greg Kroah-Hartman wrote:
>> > "memory" is the name used by the current sysfs memory layout code in
>> > drivers/base/memory.c. So it can't be the same unless we are going to
>> > create a toggle a boot time to select between the models, which is
>> > something I am looking to add if this code/design is acceptable to
>> > people.
> I know it can't be the same, but this is like "memory_v2" or something,
> right?  I suggest you make it an either/or option, given that you feel
> the existing layout just will not work properly for you.

If there are existing tools or applications that look for memory hotplug
events, how does this interact with those?  I know you guys have control
over the ppc software that actually performs the probe/online
operations, but what about other apps?

I also don't seem to see the original post to LKML.  Did you send
privately to Greg, then he cc'd LKML on his reply?

Re: [PATCH] drivers: base: new memory config sysfs driver for large memory systems

[Seth Jennings]

On Thu, Aug 01, 2013 at 03:13:03PM -0700, Dave Hansen wrote:
> On 08/01/2013 01:57 PM, Greg Kroah-Hartman wrote:
> >> > "memory" is the name used by the current sysfs memory layout code in
> >> > drivers/base/memory.c. So it can't be the same unless we are going to
> >> > create a toggle a boot time to select between the models, which is
> >> > something I am looking to add if this code/design is acceptable to
> >> > people.
> > I know it can't be the same, but this is like "memory_v2" or something,
> > right?  I suggest you make it an either/or option, given that you feel
> > the existing layout just will not work properly for you.
> 
> If there are existing tools or applications that look for memory hotplug
> events, how does this interact with those?  I know you guys have control
> over the ppc software that actually performs the probe/online
> operations, but what about other apps?

After taking a closer look, I've decided to rework this to preserve more
of the existing layout.  Should be posting it next Monday.

> 
> I also don't seem to see the original post to LKML.  Did you send
> privately to Greg, then he cc'd LKML on his reply?

Yeah :-/  My mail relay settings were messed up and my system tried to
deliver the mail directly to recipients; some of which worked and some
failed (spam/firewall filters, etc).  Sigh...

Seth