All of lore.kernel.org
 help / color / mirror / Atom feed
From: David Ahern <dsa@cumulusnetworks.com>
To: Edward Cree <ec429@cantab.net>,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: linux-kernel@vger.kernel.org
Subject: Re: [RFC] sysfs: defer instantiation of default attrs
Date: Sat, 20 Feb 2016 09:35:25 -0700	[thread overview]
Message-ID: <56C895CD.9070702@cumulusnetworks.com> (raw)
In-Reply-To: <alpine.DEB.2.20.1602201041100.23349@dev-full.penguins>

Hi Ed:

Thank for taking the to look into this.

On 2/20/16 3:42 AM, Edward Cree wrote:
> Testing this in a VM, and with udevd disabled (being too lazy to do it
>   properly), created 1024 bridges.  On ls'ing
>   /sys/class/net/bridge*/queues/*/, saw free memory drop by 64kB,
>   suggesting that much had been saved by deferral.  It's not very much,
>   hopefully deferring attribute groups will do better.

That sounds consistent with what I found by cutting out attributes in 
netdev_register_kobject. e.g., from my patch:

diff --git a/net/core/net-sysfs.c b/net/core/net-sysfs.c
index b6c8a6629b39..f0df828a2f20 100644
--- a/net/core/net-sysfs.c
+++ b/net/core/net-sysfs.c
@@ -1524,7 +1524,8 @@ int netdev_register_kobject(struct net_device *ndev)
         int error = 0;

         device_initialize(dev);
-       dev->class = &net_class;
+       if (!netif_is_lwt(ndev))
+               dev->class = &net_class;
         dev->platform_data = ndev;
         dev->groups = groups;

@@ -1535,7 +1536,8 @@ int netdev_register_kobject(struct net_device *ndev)
         if (*groups)
                 groups++;

-       *groups++ = &netstat_group;
+       if (!netif_is_lwt(ndev)) {
+               *groups++ = &netstat_group;

  #if IS_ENABLED(CONFIG_WIRELESS_EXT) || IS_ENABLED(CONFIG_CFG80211)
         if (ndev->ieee80211_ptr)


The big memory savings came in when I bypassed register_queue_kobjects:

         error = device_add(dev);
         if (error)
                 return error;

-       error = register_queue_kobjects(ndev);
-       if (error) {
-               device_del(dev);
-               return error;
+       if (!netif_is_lwt(ndev)) {
+               error = register_queue_kobjects(ndev);
+               if (error) {
+                       device_del(dev);
+                       return error;
+               }
         }

         pm_runtime_set_memalloc_noio(dev, true);


Yes, this is a bit Draconian and does have impacts to tools looking for 
/sys files but the return is huge when you look at 1000's of netdevices 
(ports, vlans, bridges, bonds).

      reply	other threads:[~2016-02-20 16:35 UTC|newest]

Thread overview: 2+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-02-20 10:42 [RFC] sysfs: defer instantiation of default attrs Edward Cree
2016-02-20 16:35 ` David Ahern [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=56C895CD.9070702@cumulusnetworks.com \
    --to=dsa@cumulusnetworks.com \
    --cc=ec429@cantab.net \
    --cc=gregkh@linuxfoundation.org \
    --cc=linux-kernel@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.