lib/ref_tracker: improve printing stats

In case the library is tracking busy subsystem, simply
printing stack for every active reference will spam log
with long, hard to read, redundant stack traces. To improve
readabilty following changes have been made:
- reports are printed per stack_handle - log is more compact,
- added display name for ref_tracker_dir - it will differentiate
  multiple subsystems,
- stack trace is printed indented, in the same printk call,
- info about dropped references is printed as well.

Signed-off-by: Andrzej Hajda <andrzej.hajda@intel.com>
Reviewed-by: Andi Shyti <andi.shyti@linux.intel.com>
Reviewed-by: Eric Dumazet <edumazet@google.com>
Signed-off-by: Jakub Kicinski <kuba@kernel.org>

diff --git a/include/linux/ref_tracker.h b/include/linux/ref_tracker.h
index 87a92f2bec1b88beefc902545ee68dcfa12eea5f..19a69e7809d6c16736d95c760ef01086bef9b9e5 100644 (file)
--- a/include/linux/ref_tracker.h
+++ b/include/linux/ref_tracker.h
@@ -17,12 +17,15 @@ struct ref_tracker_dir {
        bool                    dead;
        struct list_head        list; /* List of active trackers */
        struct list_head        quarantine; /* List of dead trackers */
+       char                    name[32];
 #endif
 };
 
 #ifdef CONFIG_REF_TRACKER
+
 static inline void ref_tracker_dir_init(struct ref_tracker_dir *dir,
-                                       unsigned int quarantine_count)
+                                       unsigned int quarantine_count,
+                                       const char *name)
 {
        INIT_LIST_HEAD(&dir->list);
        INIT_LIST_HEAD(&dir->quarantine);
@@ -31,6 +34,7 @@ static inline void ref_tracker_dir_init(struct ref_tracker_dir *dir,
        dir->dead = false;
        refcount_set(&dir->untracked, 1);
        refcount_set(&dir->no_tracker, 1);
+       strscpy(dir->name, name, sizeof(dir->name));
        stack_depot_init();
 }
 
@@ -51,7 +55,8 @@ int ref_tracker_free(struct ref_tracker_dir *dir,
 #else /* CONFIG_REF_TRACKER */
 
 static inline void ref_tracker_dir_init(struct ref_tracker_dir *dir,
-                                       unsigned int quarantine_count)
+                                       unsigned int quarantine_count,
+                                       const char *name)
 {
 }
 

diff --git a/lib/ref_tracker.c b/lib/ref_tracker.c
index d4eb0929af8f96cfd32cb77e1ee3856fb869f891..2ffe79c90c1771652b92e00be2a93eaf76b71486 100644 (file)
--- a/lib/ref_tracker.c
+++ b/lib/ref_tracker.c
@@ -1,11 +1,16 @@
 // SPDX-License-Identifier: GPL-2.0-or-later
+
+#define pr_fmt(fmt) "ref_tracker: " fmt
+
 #include <linux/export.h>
+#include <linux/list_sort.h>
 #include <linux/ref_tracker.h>
 #include <linux/slab.h>
 #include <linux/stacktrace.h>
 #include <linux/stackdepot.h>
 
 #define REF_TRACKER_STACK_ENTRIES 16
+#define STACK_BUF_SIZE 1024
 
 struct ref_tracker {
        struct list_head        head;   /* anchor into dir->list or dir->quarantine */
@@ -14,24 +19,87 @@ struct ref_tracker {
        depot_stack_handle_t    free_stack_handle;
 };
 
-void ref_tracker_dir_print_locked(struct ref_tracker_dir *dir,
-                                 unsigned int display_limit)
+struct ref_tracker_dir_stats {
+       int total;
+       int count;
+       struct {
+               depot_stack_handle_t stack_handle;
+               unsigned int count;
+       } stacks[];
+};
+
+static struct ref_tracker_dir_stats *
+ref_tracker_get_stats(struct ref_tracker_dir *dir, unsigned int limit)
 {
+       struct ref_tracker_dir_stats *stats;
        struct ref_tracker *tracker;
-       unsigned int i = 0;
 
-       lockdep_assert_held(&dir->lock);
+       stats = kmalloc(struct_size(stats, stacks, limit),
+                       GFP_NOWAIT | __GFP_NOWARN);
+       if (!stats)
+               return ERR_PTR(-ENOMEM);
+       stats->total = 0;
+       stats->count = 0;
 
        list_for_each_entry(tracker, &dir->list, head) {
-               if (i < display_limit) {
-                       pr_err("leaked reference.\n");
-                       if (tracker->alloc_stack_handle)
-                               stack_depot_print(tracker->alloc_stack_handle);
-                       i++;
-               } else {
-                       break;
+               depot_stack_handle_t stack = tracker->alloc_stack_handle;
+               int i;
+
+               ++stats->total;
+               for (i = 0; i < stats->count; ++i)
+                       if (stats->stacks[i].stack_handle == stack)
+                               break;
+               if (i >= limit)
+                       continue;
+               if (i >= stats->count) {
+                       stats->stacks[i].stack_handle = stack;
+                       stats->stacks[i].count = 0;
+                       ++stats->count;
                }
+               ++stats->stacks[i].count;
+       }
+
+       return stats;
+}
+
+void ref_tracker_dir_print_locked(struct ref_tracker_dir *dir,
+                                 unsigned int display_limit)
+{
+       struct ref_tracker_dir_stats *stats;
+       unsigned int i = 0, skipped;
+       depot_stack_handle_t stack;
+       char *sbuf;
+
+       lockdep_assert_held(&dir->lock);
+
+       if (list_empty(&dir->list))
+               return;
+
+       stats = ref_tracker_get_stats(dir, display_limit);
+       if (IS_ERR(stats)) {
+               pr_err("%s@%pK: couldn't get stats, error %pe\n",
+                      dir->name, dir, stats);
+               return;
        }
+
+       sbuf = kmalloc(STACK_BUF_SIZE, GFP_NOWAIT | __GFP_NOWARN);
+
+       for (i = 0, skipped = stats->total; i < stats->count; ++i) {
+               stack = stats->stacks[i].stack_handle;
+               if (sbuf && !stack_depot_snprint(stack, sbuf, STACK_BUF_SIZE, 4))
+                       sbuf[0] = 0;
+               pr_err("%s@%pK has %d/%d users at\n%s\n", dir->name, dir,
+                      stats->stacks[i].count, stats->total, sbuf);
+               skipped -= stats->stacks[i].count;
+       }
+
+       if (skipped)
+               pr_err("%s@%pK skipped reports about %d/%d users.\n",
+                      dir->name, dir, skipped, stats->total);
+
+       kfree(sbuf);
+
+       kfree(stats);
 }
 EXPORT_SYMBOL(ref_tracker_dir_print_locked);
 

diff --git a/lib/test_ref_tracker.c b/lib/test_ref_tracker.c
index 19d7dec70cc62f0a7b274cd5ec8bf8cec62b0af3..49970a7c96f3f46196508d1d967315ae1da52210 100644 (file)
--- a/lib/test_ref_tracker.c
+++ b/lib/test_ref_tracker.c
@@ -64,7 +64,7 @@ static int __init test_ref_tracker_init(void)
 {
        int i;
 
-       ref_tracker_dir_init(&ref_dir, 100);
+       ref_tracker_dir_init(&ref_dir, 100, "selftest");
 
        timer_setup(&test_ref_tracker_timer, test_ref_tracker_timer_func, 0);
        mod_timer(&test_ref_tracker_timer, jiffies + 1);

diff --git a/net/core/dev.c b/net/core/dev.c
index b3c13e0419356b943e90b1f46dd7e035c6ec1a9c..5ca7a133d3be5b1ddf010ee68d95a156c2c6ba6c 100644 (file)
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -10630,7 +10630,7 @@ struct net_device *alloc_netdev_mqs(int sizeof_priv, const char *name,
        dev = PTR_ALIGN(p, NETDEV_ALIGN);
        dev->padded = (char *)dev - (char *)p;
 
-       ref_tracker_dir_init(&dev->refcnt_tracker, 128);
+       ref_tracker_dir_init(&dev->refcnt_tracker, 128, name);
 #ifdef CONFIG_PCPU_DEV_REFCNT
        dev->pcpu_refcnt = alloc_percpu(int);
        if (!dev->pcpu_refcnt)

diff --git a/net/core/net_namespace.c b/net/core/net_namespace.c
index 3e3598cd49f23e8202c4dd9b8c04edb77d296af7..f4183c4c1ec82fa0c067ac76a56e06c24527639e 100644 (file)
--- a/net/core/net_namespace.c
+++ b/net/core/net_namespace.c
@@ -308,7 +308,7 @@ EXPORT_SYMBOL_GPL(get_net_ns_by_id);
 /* init code that must occur even if setup_net() is not called. */
 static __net_init void preinit_net(struct net *net)
 {
-       ref_tracker_dir_init(&net->notrefcnt_tracker, 128);
+       ref_tracker_dir_init(&net->notrefcnt_tracker, 128, "net notrefcnt");
 }
 
 /*
@@ -322,7 +322,7 @@ static __net_init int setup_net(struct net *net, struct user_namespace *user_ns)
        LIST_HEAD(net_exit_list);
 
        refcount_set(&net->ns.count, 1);
-       ref_tracker_dir_init(&net->refcnt_tracker, 128);
+       ref_tracker_dir_init(&net->refcnt_tracker, 128, "net refcnt");
 
        refcount_set(&net->passive, 1);
        get_random_bytes(&net->hash_mix, sizeof(u32));