From: Eric Dumazet <edumazet@google.com>
To: Andrzej Hajda <andrzej.hajda@intel.com>
Cc: LKML <linux-kernel@vger.kernel.org>,
intel-gfx@lists.freedesktop.org, dri-devel@lists.freedesktop.org,
netdev <netdev@vger.kernel.org>,
Jani Nikula <jani.nikula@linux.intel.com>,
Daniel Vetter <daniel@ffwll.ch>,
Lucas De Marchi <lucas.demarchi@intel.com>,
Chris Wilson <chris.p.wilson@intel.com>,
Dmitry Vyukov <dvyukov@google.com>,
Jakub Kicinski <kuba@kernel.org>
Subject: Re: [PATCH 2/9] lib/ref_tracker: compact stacktraces before printing
Date: Thu, 17 Feb 2022 07:23:31 -0800 [thread overview]
Message-ID: <CANn89iKgzztLA3y6V+vw3RiyoScC3K=1Z1_gajj8H56wGWDw6A@mail.gmail.com> (raw)
In-Reply-To: <20220217140441.1218045-3-andrzej.hajda@intel.com>
On Thu, Feb 17, 2022 at 6:05 AM Andrzej Hajda <andrzej.hajda@intel.com> wrote:
>
> In cases references are taken alternately on multiple exec paths leak
> report can grow substantially, sorting and grouping leaks by stack_handle
> allows to compact it.
>
> Signed-off-by: Andrzej Hajda <andrzej.hajda@intel.com>
> Reviewed-by: Chris Wilson <chris.p.wilson@intel.com>
> ---
> lib/ref_tracker.c | 35 +++++++++++++++++++++++++++--------
> 1 file changed, 27 insertions(+), 8 deletions(-)
>
> diff --git a/lib/ref_tracker.c b/lib/ref_tracker.c
> index 1b0c6d645d64a..0e9c7d2828ccb 100644
> --- a/lib/ref_tracker.c
> +++ b/lib/ref_tracker.c
> @@ -1,5 +1,6 @@
> // SPDX-License-Identifier: GPL-2.0-or-later
> #include <linux/export.h>
> +#include <linux/list_sort.h>
> #include <linux/ref_tracker.h>
> #include <linux/slab.h>
> #include <linux/stacktrace.h>
> @@ -14,23 +15,41 @@ struct ref_tracker {
> depot_stack_handle_t free_stack_handle;
> };
>
> +static int ref_tracker_cmp(void *priv, const struct list_head *a, const struct list_head *b)
> +{
> + const struct ref_tracker *ta = list_entry(a, const struct ref_tracker, head);
> + const struct ref_tracker *tb = list_entry(b, const struct ref_tracker, head);
> +
> + return ta->alloc_stack_handle - tb->alloc_stack_handle;
> +}
> +
> void __ref_tracker_dir_print(struct ref_tracker_dir *dir,
> unsigned int display_limit)
> {
> + unsigned int i = 0, count = 0;
> struct ref_tracker *tracker;
> - unsigned int i = 0;
> + depot_stack_handle_t stack;
>
> lockdep_assert_held(&dir->lock);
>
> + if (list_empty(&dir->list))
> + return;
> +
> + list_sort(NULL, &dir->list, ref_tracker_cmp);
What is going to be the cost of sorting a list with 1,000,000 items in it ?
I just want to make sure we do not trade printing at most ~10 references
(from netdev_wait_allrefs()) to a soft lockup :/ with no useful info
if something went terribly wrong.
I suggest that you do not sort a potential big list, and instead
attempt to allocate an array of @display_limits 'struct stack_counts'
I suspect @display_limits will always be kept to a reasonable value
(less than 100 ?)
struct stack_counts {
depot_stack_handle_t stack_handle;
unsigned int count;
}
Then, iterating the list and update the array (that you can keep
sorted by ->stack_handle)
Then after iterating, print the (at_most) @display_limits handles
found in the temp array.
> +
> 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 {
> + if (i++ >= display_limit)
> break;
> - }
> + if (!count++)
> + stack = tracker->alloc_stack_handle;
> + if (stack == tracker->alloc_stack_handle &&
> + !list_is_last(&tracker->head, &dir->list))
> + continue;
> +
> + pr_err("leaked %d references.\n", count);
> + if (stack)
> + stack_depot_print(stack);
> + count = 0;
> }
> }
> EXPORT_SYMBOL(__ref_tracker_dir_print);
> --
> 2.25.1
>
next prev parent reply other threads:[~2022-02-17 15:23 UTC|newest]
Thread overview: 21+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-02-17 14:04 [PATCH 0/9] drm/i915: use ref_tracker library for tracking wakerefs Andrzej Hajda
2022-02-17 14:04 ` [PATCH 1/9] lib/ref_tracker: add unlocked leak print helper Andrzej Hajda
2022-02-17 14:04 ` [PATCH 2/9] lib/ref_tracker: compact stacktraces before printing Andrzej Hajda
2022-02-17 15:23 ` Eric Dumazet [this message]
2022-02-17 15:25 ` Eric Dumazet
2022-02-18 10:54 ` Andrzej Hajda
2022-02-18 13:01 ` Eric Dumazet
2022-02-17 14:04 ` [PATCH 3/9] lib/ref_tracker: __ref_tracker_dir_print improve printing Andrzej Hajda
2022-02-17 15:38 ` Eric Dumazet
2022-02-18 10:11 ` Andrzej Hajda
2022-02-17 14:04 ` [PATCH 4/9] lib/ref_tracker: add printing to memory buffer Andrzej Hajda
2022-02-17 14:04 ` [PATCH 5/9] lib/ref_tracker: improve allocation flags Andrzej Hajda
2022-02-17 15:13 ` Eric Dumazet
2022-02-18 10:28 ` Andrzej Hajda
2022-02-18 13:05 ` Eric Dumazet
2022-02-17 14:04 ` [PATCH 6/9] drm/i915: Separate wakeref tracking Andrzej Hajda
2022-02-17 14:48 ` [Intel-gfx] " Ville Syrjälä
2022-02-18 10:32 ` Andrzej Hajda
2022-02-17 14:04 ` [PATCH 7/9] drm/i915: Track leaked gt->wakerefs Andrzej Hajda
2022-02-17 14:04 ` [PATCH 8/9] drm/i915: Correct type of wakeref variable Andrzej Hajda
2022-02-17 14:04 ` [PATCH 9/9] drm/i915: replace Intel internal tracker with kernel core ref_tracker Andrzej Hajda
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='CANn89iKgzztLA3y6V+vw3RiyoScC3K=1Z1_gajj8H56wGWDw6A@mail.gmail.com' \
--to=edumazet@google.com \
--cc=andrzej.hajda@intel.com \
--cc=chris.p.wilson@intel.com \
--cc=daniel@ffwll.ch \
--cc=dri-devel@lists.freedesktop.org \
--cc=dvyukov@google.com \
--cc=intel-gfx@lists.freedesktop.org \
--cc=jani.nikula@linux.intel.com \
--cc=kuba@kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=lucas.demarchi@intel.com \
--cc=netdev@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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).