Idea for reducing sysfs memory usage

Idea for reducing sysfs memory usage

[Edward Cree]

Sorry if this has been suggested before, but if so I couldn't find it.
Short version: could a sysfs dir reference a list of default attributes
rather than having to instantiate them all?

Long version:

If I'm understanding correctly, when creating a sysfs dir for a kobject, we
call populate_dir() and create a sysfs file for each of its default_attrs.
This leads to some memory allocations, which apparently can really add up
when, for instance, using a large number of netdevices (mostly software
devices like VLANs and bonds whose sysfs files will probably never actually
be used anyway).  At netdev1.1, a use case was presented for thousands -
perhaps even hundreds of thousands - of such netdevices, at which point the
memory overhead for all those sysfs files becomes quite significant.

Thus I was wondering if it would be feasible to replace the instantiation of
these default attributes with, in principle, a pointer to the default_attr
list, which already contains the methods needed to show or store the
attribute, and need not be duplicated for each sysfs dir referring to it.
In practice it would probably make sense to build a fast-lookup data
structure from the list when the latter is first created, since the list is
(as I understand it) never changed.

Then for instance, a lookup on the directory would search both the default
attrs list and the rbtree of runtime-added nodes.  I don't think it would
ever be necessary for runtime code to _remove_ a default attribute from a
dir, but if it were, then perhaps a negative entry could be added to the
rbtree, and that tree could always be searched first - then, if the negative
entry was found, the search of the default list would be skipped.
Of course, all the other operations on the directory would similarly have to
be modified to take account of there being two places in which an attribute
could be defined, but I believe (perhaps naïvely) that this would not be
excessively painful.

This could potentially save memory whenever large numbers of directories of
the same 'type' are created in sysfs; not just netdevices, but also for
instance PCI devices and their 'power' subdirectories.  While it would
slightly slow down sysfs accesses, I can't quite imagine a situation in
which the speed of sysfs would be critical to system performance - though
perhaps that is just my ignorance.

Is there an obvious reason why I can't do this, or should I dive in and try
to implement it?  (Or should I first just try to estimate how much memory it
would save, to find out whether it would be worth the complexity?)

Not subscribed to list; please Cc on replies.

--
-ed

Re: Idea for reducing sysfs memory usage

[Greg Kroah-Hartman]

On Tue, Feb 16, 2016 at 11:46:49PM +0000, Edward Cree wrote:
> Sorry if this has been suggested before, but if so I couldn't find it.
> Short version: could a sysfs dir reference a list of default attributes
> rather than having to instantiate them all?

Shorter version, why do you think it is?  :)

Have you done some testing of the amount of memory that sysfs entries
consume and found any problems with it?  Remember, a s390 system with
20000 disks works just fine with a very little memory footprint, a lot
of work has gone into making sysfs consume tiny amounts of memory.

thanks,

greg k-h

Re: Idea for reducing sysfs memory usage

[Edward Cree]

On 16/02/16 23:55, Greg Kroah-Hartman wrote:
> On Tue, Feb 16, 2016 at 11:46:49PM +0000, Edward Cree wrote:
>> Sorry if this has been suggested before, but if so I couldn't find it.
>> Short version: could a sysfs dir reference a list of default attributes
>> rather than having to instantiate them all?
> Shorter version, why do you think it is?  :)
>
> Have you done some testing of the amount of memory that sysfs entries
> consume and found any problems with it?
Two reasons:
a) in his netdev1.1 talk "Scaling the Number of Network Interfaces on 
Linux",
    David Ahern claimed a memory overhead of (iirc) about 45kB per 
netdevice,
    of which he attributed (again, iirc) about 20kB to sysfs entries.  
He also
    indicated that this was a problem for his use case.  (My apologies to
    David if I've misrepresented him.  CCed him so he can correct me.)
b) my reading of the code suggested it was allocating stuff for every 
call to
    sysfs_create_file() in the loop in populate_dir().
Having re-read __kernfs_new_node() and struct kernfs_node, I now realise I
misinterpreted them - the name isn't being allocated at all 
(kstrdup_const())
and the struct kernfs_node consists chiefly (if not entirely) of fields
specific to the individual file rather than shareable between multiple
instances.  So there isn't any memory we can save here.

Sorry for the noise.

--
-ed

Re: Idea for reducing sysfs memory usage

[Greg Kroah-Hartman]

On Tue, Feb 16, 2016 at 11:46:49PM +0000, Edward Cree wrote:
> If I'm understanding correctly, when creating a sysfs dir for a kobject, we
> call populate_dir() and create a sysfs file for each of its default_attrs.
> This leads to some memory allocations, which apparently can really add up
> when, for instance, using a large number of netdevices (mostly software
> devices like VLANs and bonds whose sysfs files will probably never actually
> be used anyway).  At netdev1.1, a use case was presented for thousands -
> perhaps even hundreds of thousands - of such netdevices, at which point the
> memory overhead for all those sysfs files becomes quite significant.

Oops, missed this before.  Exactly what is that memory overhead?  Do you
have numbers?

Again, we can easily handle tens of thousands of devices today, hundred
of thousands also probably work fine, I haven't heard of anyone really
needing/wanting them except for odd benchmarks.  If you have a script I
can use to test with, I'd be glad to do so.  I'd also be interested in
the use-case for this type of thing, what is it good for?

But you are going to have to come up with some real numbers to show that
sysfs really is the issue here, and it's not something a bit higher up
the stack.

thanks,

greg k-h

Re: Idea for reducing sysfs memory usage

[David Ahern]

On 2/16/16 5:37 PM, Edward Cree wrote:
> On 16/02/16 23:55, Greg Kroah-Hartman wrote:
>> On Tue, Feb 16, 2016 at 11:46:49PM +0000, Edward Cree wrote:
>>> Sorry if this has been suggested before, but if so I couldn't find it.
>>> Short version: could a sysfs dir reference a list of default attributes
>>> rather than having to instantiate them all?
>> Shorter version, why do you think it is?  :)
>>
>> Have you done some testing of the amount of memory that sysfs entries
>> consume and found any problems with it?
> Two reasons:
> a) in his netdev1.1 talk "Scaling the Number of Network Interfaces on
> Linux",
>     David Ahern claimed a memory overhead of (iirc) about 45kB per
> netdevice,
>     of which he attributed (again, iirc) about 20kB to sysfs entries. He
> also
>     indicated that this was a problem for his use case.  (My apologies to
>     David if I've misrepresented him.  CCed him so he can correct me.)

Close enough. :-)

I analyzed memory allocations for the creation of a dummy netdevice. All 
total 36,450 bytes were requested resulting in 43,944 bytes allocated. 
Of that kobject and sysfs is 14,568. So kobject and sysfs is roughly 1/3 
the memory overhead of a netdevice and is also a significant overhead in 
the time to create the netdevice.

David

Re: Idea for reducing sysfs memory usage

[Greg Kroah-Hartman]

On Wed, Feb 17, 2016 at 12:37:38AM +0000, Edward Cree wrote:
> On 16/02/16 23:55, Greg Kroah-Hartman wrote:
> >On Tue, Feb 16, 2016 at 11:46:49PM +0000, Edward Cree wrote:
> >>Sorry if this has been suggested before, but if so I couldn't find it.
> >>Short version: could a sysfs dir reference a list of default attributes
> >>rather than having to instantiate them all?
> >Shorter version, why do you think it is?  :)
> >
> >Have you done some testing of the amount of memory that sysfs entries
> >consume and found any problems with it?
> Two reasons:
> a) in his netdev1.1 talk "Scaling the Number of Network Interfaces on
> Linux",
>    David Ahern claimed a memory overhead of (iirc) about 45kB per netdevice,
>    of which he attributed (again, iirc) about 20kB to sysfs entries.  He
> also
>    indicated that this was a problem for his use case.  (My apologies to
>    David if I've misrepresented him.  CCed him so he can correct me.)

How many sysfs entries are you creating for that 20kb?  And how did you
measure it?  If you don't access the files, the backing store is not
allocated, saving you a lot of memory.  If you do access it, it will be
freed later on afterward, so it's sometimes really hard to measure this
accurately.

> b) my reading of the code suggested it was allocating stuff for every call
> to
>    sysfs_create_file() in the loop in populate_dir().
> Having re-read __kernfs_new_node() and struct kernfs_node, I now realise I
> misinterpreted them - the name isn't being allocated at all
> (kstrdup_const())
> and the struct kernfs_node consists chiefly (if not entirely) of fields
> specific to the individual file rather than shareable between multiple
> instances.  So there isn't any memory we can save here.

That's good to verify that we have already solved this, thanks :)

> Sorry for the noise.

Not a problem.

greg k-h

Re: Idea for reducing sysfs memory usage

[David Ahern]

On 2/16/16 5:47 PM, Greg Kroah-Hartman wrote:
> How many sysfs entries are you creating for that 20kb?  And how did you
> measure it?  If you don't access the files, the backing store is not
> allocated, saving you a lot of memory.  If you do access it, it will be
> freed later on afterward, so it's sometimes really hard to measure this
> accurately.

Using the kmem tracepoints:

perf record  -g \
-e 
kmem:kmalloc,kmem:kmem_cache_alloc,kmem:mm_page_alloc,kmem:kfree,kmem:kmem_cache_free,kmem:mm_page_free 
        -- ip link add dummy1 type dummy

First go round I painstakenly examined each allocation and location.
After that I modified perf-kmem to do the summary for me for other use 
cases.


I can forward the full notes if you want them. Here's the first few and 
last few:

################
# sysfs -- start

# directory

# device_add -> kobject_add -- register with generic layer

ip  1732 [005] 27128.556548: kmem:kmalloc: ptr=0xffff8800ba9a5d60 
bytes_req=7 bytes_alloc=32 gfp_flags=37748928
	ffffffff8113e5ce __kmalloc_track_caller ([kernel.kallsyms])
	ffffffff81116643 kstrdup ([kernel.kallsyms])
	ffffffff8111667c kstrdup_const ([kernel.kallsyms])
	ffffffff811a6bf5 __kernfs_new_node ([kernel.kallsyms])
	ffffffff811a7397 kernfs_new_node ([kernel.kallsyms])
	ffffffff811a7813 kernfs_create_dir_ns ([kernel.kallsyms])
	ffffffff811a95f0 sysfs_create_dir_ns ([kernel.kallsyms])
	ffffffff8126f76f kobject_add_internal ([kernel.kallsyms])
	ffffffff8126fa62 kobject_add ([kernel.kallsyms])
	ffffffff81319317 device_add ([kernel.kallsyms])
	ffffffff81408528 netdev_register_kobject ([kernel.kallsyms])
	ffffffff813f2cbb register_netdevice ([kernel.kallsyms])
	ffffffff81401314 rtnl_newlink ([kernel.kallsyms])

# directory

ip  1732 [005] 27128.556555: kmem:kmem_cache_alloc: 
ptr=0xffff8801399d6018 bytes_req=168 bytes_alloc=168 gfp_flags=37781696
	ffffffff8113ee09 kmem_cache_alloc ([kernel.kallsyms])
	ffffffff811a6c0e __kernfs_new_node ([kernel.kallsyms])
	ffffffff811a7397 kernfs_new_node ([kernel.kallsyms])
	ffffffff811a7813 kernfs_create_dir_ns ([kernel.kallsyms])
	ffffffff811a95f0 sysfs_create_dir_ns ([kernel.kallsyms])
	ffffffff8126f76f kobject_add_internal ([kernel.kallsyms])
	ffffffff8126fa62 kobject_add ([kernel.kallsyms])
	ffffffff81319317 device_add ([kernel.kallsyms])
	ffffffff81408528 netdev_register_kobject ([kernel.kallsyms])
	ffffffff813f2cbb register_netdevice ([kernel.kallsyms])
	ffffffff81401314 rtnl_newlink ([kernel.kallsyms])

# device_add -> device_create_file(dev, &dev_attr_uevent) --> 
sysfs_create_file(&dev->kobj, &attr->attr)

ip  1732 [005] 27128.556583: kmem:kmem_cache_alloc: 
ptr=0xffff8801399d60c0 bytes_req=168 bytes_alloc=168 gfp_flags=37781696
	ffffffff8113ee09 kmem_cache_alloc ([kernel.kallsyms])
	ffffffff811a6c0e __kernfs_new_node ([kernel.kallsyms])
	ffffffff811a7397 kernfs_new_node ([kernel.kallsyms])
	ffffffff811a8ac7 __kernfs_create_file ([kernel.kallsyms])
	ffffffff811a9310 sysfs_add_file_mode_ns ([kernel.kallsyms])
	ffffffff811a9370 sysfs_create_file_ns ([kernel.kallsyms])
	ffffffff81317f3b device_create_file ([kernel.kallsyms])
	ffffffff81319342 device_add ([kernel.kallsyms])
	ffffffff81408528 netdev_register_kobject ([kernel.kallsyms])
	ffffffff813f2cbb register_netdevice ([kernel.kallsyms])
	ffffffff81401314 rtnl_newlink ([kernel.kallsyms])

...

ip  1732 [005] 27128.560734: kmem:kmem_cache_alloc: 
ptr=0xffff8800b94758a0 bytes_req=168 bytes_alloc=168 gfp_flags=37781696
	ffffffff8113ee09 kmem_cache_alloc ([kernel.kallsyms])
	ffffffff811a6c0e __kernfs_new_node ([kernel.kallsyms])
	ffffffff811a7397 kernfs_new_node ([kernel.kallsyms])
	ffffffff811a8ac7 __kernfs_create_file ([kernel.kallsyms])
	ffffffff811a9310 sysfs_add_file_mode_ns ([kernel.kallsyms])
	ffffffff811a9cd2 internal_create_group ([kernel.kallsyms])
	ffffffff811a9e20 sysfs_create_group ([kernel.kallsyms])
	ffffffff814083ca netdev_queue_update_kobjects ([kernel.kallsyms])
	ffffffff81408583 netdev_register_kobject ([kernel.kallsyms])
	ffffffff813f2cbb register_netdevice ([kernel.kallsyms])
	ffffffff81401314 rtnl_newlink ([kernel.kallsyms])

ip  1732 [005] 27128.560789: kmem:kmem_cache_alloc: 
ptr=0xffff880139cd0400 bytes_req=256 bytes_alloc=256 gfp_flags=37748928
	ffffffff8113ee09 kmem_cache_alloc ([kernel.kallsyms])
	ffffffff813df48a __alloc_skb ([kernel.kallsyms])
	ffffffff81270485 kobject_uevent_env ([kernel.kallsyms])
	ffffffff81270703 kobject_uevent ([kernel.kallsyms])
	ffffffff814083da netdev_queue_update_kobjects ([kernel.kallsyms])
	ffffffff81408583 netdev_register_kobject ([kernel.kallsyms])
	ffffffff813f2cbb register_netdevice ([kernel.kallsyms])
	ffffffff81401314 rtnl_newlink ([kernel.kallsyms])

ip  1732 [005] 27128.560798: kmem:kmalloc: ptr=0xffff8800bab97c00 
bytes_req=512 bytes_alloc=512 gfp_flags=37814976
	ffffffff8113e5ce __kmalloc_track_caller ([kernel.kallsyms])
	ffffffff813def36 __kmalloc_reserve.isra.50 ([kernel.kallsyms])
	ffffffff813df4bc __alloc_skb ([kernel.kallsyms])
	ffffffff81270485 kobject_uevent_env ([kernel.kallsyms])
	ffffffff81270703 kobject_uevent ([kernel.kallsyms])
	ffffffff814083da netdev_queue_update_kobjects ([kernel.kallsyms])
	ffffffff81408583 netdev_register_kobject ([kernel.kallsyms])
	ffffffff813f2cbb register_netdevice ([kernel.kallsyms])
	ffffffff81401314 rtnl_newlink ([kernel.kallsyms])

# end netdev_register_kobject
###############################

Re: Idea for reducing sysfs memory usage

[Edward Cree]

On 17/02/16 00:47, Greg Kroah-Hartman wrote:
> How many sysfs entries are you creating for that 20kb?  And how did you
> measure it?  If you don't access the files, the backing store is not
> allocated, saving you a lot of memory.
Thinking about this some more, could we not do the same thing with the
struct kernfs_nodes, i.e. only allocate them when someone first accesses
the file?  Or simpler, defer allocation of all the files in the dir until
someone first touches the directory?  Of course it would add a little
latency to that first access, but (barring differences in cache warmth) it
would subtract the same amount of time from the initial dir creation, and
in the case where no-one ever looks at the directory, it would save the
memory.
I did find a patch series from 2009 doing something vaguely similar[1], but
a) it looks like it wasn't applied and b) it appears to involve a function
pointer in struct sysfs_elem_dir to say how to populate the directory.
All we need here is to get our kobj (which we have in priv member of struct
kernfs_node) and call populate_dir().  (And remove whatever flag or mark we
used to say "not populated yet".  And some locking will be needed.)

Again, no hard numbers on how much memory this would save, nor evidence
that the "no-one ever touches the dir" case happens in practice...

[1] http://thread.gmane.org/gmane.linux.kernel/904418
--
-ed

Re: Idea for reducing sysfs memory usage

[Greg Kroah-Hartman]

On Wed, Feb 17, 2016 at 01:26:27AM +0000, Edward Cree wrote:
> On 17/02/16 00:47, Greg Kroah-Hartman wrote:
> >How many sysfs entries are you creating for that 20kb?  And how did you
> >measure it?  If you don't access the files, the backing store is not
> >allocated, saving you a lot of memory.
> Thinking about this some more, could we not do the same thing with the
> struct kernfs_nodes, i.e. only allocate them when someone first accesses
> the file?  Or simpler, defer allocation of all the files in the dir until
> someone first touches the directory?  Of course it would add a little
> latency to that first access, but (barring differences in cache warmth) it
> would subtract the same amount of time from the initial dir creation, and
> in the case where no-one ever looks at the directory, it would save the
> memory.
> I did find a patch series from 2009 doing something vaguely similar[1], but
> a) it looks like it wasn't applied and b) it appears to involve a function
> pointer in struct sysfs_elem_dir to say how to populate the directory.
> All we need here is to get our kobj (which we have in priv member of struct
> kernfs_node) and call populate_dir().  (And remove whatever flag or mark we
> used to say "not populated yet".  And some locking will be needed.)
> 
> Again, no hard numbers on how much memory this would save, nor evidence
> that the "no-one ever touches the dir" case happens in practice...

someone always touches the dir to start with, if you are using udev in
your system and you have any rule files for those types of devices.  So
watch out for that.

If you want to try to implement something like this, please attempt it,
I'll always review patches.  But currently, I have no plans on changing
anything on my own, sorry.

thanks,

greg k-h

Re: Idea for reducing sysfs memory usage

[Greg Kroah-Hartman]

On Tue, Feb 16, 2016 at 05:44:54PM -0700, David Ahern wrote:
> On 2/16/16 5:37 PM, Edward Cree wrote:
> >On 16/02/16 23:55, Greg Kroah-Hartman wrote:
> >>On Tue, Feb 16, 2016 at 11:46:49PM +0000, Edward Cree wrote:
> >>>Sorry if this has been suggested before, but if so I couldn't find it.
> >>>Short version: could a sysfs dir reference a list of default attributes
> >>>rather than having to instantiate them all?
> >>Shorter version, why do you think it is?  :)
> >>
> >>Have you done some testing of the amount of memory that sysfs entries
> >>consume and found any problems with it?
> >Two reasons:
> >a) in his netdev1.1 talk "Scaling the Number of Network Interfaces on
> >Linux",
> >    David Ahern claimed a memory overhead of (iirc) about 45kB per
> >netdevice,
> >    of which he attributed (again, iirc) about 20kB to sysfs entries. He
> >also
> >    indicated that this was a problem for his use case.  (My apologies to
> >    David if I've misrepresented him.  CCed him so he can correct me.)
> 
> Close enough. :-)
> 
> I analyzed memory allocations for the creation of a dummy netdevice. All
> total 36,450 bytes were requested resulting in 43,944 bytes allocated. Of
> that kobject and sysfs is 14,568. So kobject and sysfs is roughly 1/3 the
> memory overhead of a netdevice and is also a significant overhead in the
> time to create the netdevice.

Time?  Hm, I thought we reduced the time down by a lot a few years ago,
any pointers to where we are spending too much time?  The PPC people did
some work on our list traversal issues for thousands of devices, perhaps
that doesn't scale to 10s of thousands?

As for 1/3 the memory needed, is that acceptable given all that the
interface and functionality that the code gives you?  14k does seem
pretty large to me, how many attributes/files are assigned to each
device?

thanks,

greg k-h