diff mbox series

[13/15] stackdepot: add backwards links to hash table buckets

Message ID e9ed24afd386d12e01c1169c17531f9ce54c0044.1693328501.git.andreyknvl@google.com (mailing list archive)
State New
Headers show
Series stackdepot: allow evicting stack traces | expand

Commit Message

andrey.konovalov@linux.dev Aug. 29, 2023, 5:11 p.m. UTC
From: Andrey Konovalov <andreyknvl@google.com>

Maintain links in the stack records to previous entries within the
hash table buckets.

This is preparatory patch for implementing the eviction of stack records
from the stack depot.

Signed-off-by: Andrey Konovalov <andreyknvl@google.com>
---
 lib/stackdepot.c | 4 ++++
 1 file changed, 4 insertions(+)

Comments

Marco Elver Aug. 30, 2023, 9:24 a.m. UTC | #1
On Tue, Aug 29, 2023 at 07:11PM +0200, andrey.konovalov@linux.dev wrote:
> From: Andrey Konovalov <andreyknvl@google.com>
> 
> Maintain links in the stack records to previous entries within the
> hash table buckets.
> 
> This is preparatory patch for implementing the eviction of stack records
> from the stack depot.
> 
> Signed-off-by: Andrey Konovalov <andreyknvl@google.com>
> ---
>  lib/stackdepot.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/lib/stackdepot.c b/lib/stackdepot.c
> index a84c0debbb9e..641db97d8c7c 100644
> --- a/lib/stackdepot.c
> +++ b/lib/stackdepot.c
> @@ -58,6 +58,7 @@ union handle_parts {
>  
>  struct stack_record {
>  	struct stack_record *next;	/* Link in hash table or freelist */
> +	struct stack_record *prev;	/* Link in hash table */

At this point this could be a normal list_head? Then you don't have to
roll your own doubly-linked list manipulation (and benefit from things
like CONFIG_LIST_DEBUG).

>  	u32 hash;			/* Hash in hash table */
>  	u32 size;			/* Number of stored frames */
>  	union handle_parts handle;
> @@ -493,6 +494,9 @@ depot_stack_handle_t __stack_depot_save(unsigned long *entries,
>  
>  		if (new) {
>  			new->next = *bucket;
> +			new->prev = NULL;
> +			if (*bucket)
> +				(*bucket)->prev = new;
>  			*bucket = new;
>  			found = new;
>  		}
> -- 
> 2.25.1
>
Andrey Konovalov Sept. 13, 2023, 5:07 p.m. UTC | #2
On Wed, Aug 30, 2023 at 11:24 AM Marco Elver <elver@google.com> wrote:
>
> > diff --git a/lib/stackdepot.c b/lib/stackdepot.c
> > index a84c0debbb9e..641db97d8c7c 100644
> > --- a/lib/stackdepot.c
> > +++ b/lib/stackdepot.c
> > @@ -58,6 +58,7 @@ union handle_parts {
> >
> >  struct stack_record {
> >       struct stack_record *next;      /* Link in hash table or freelist */
> > +     struct stack_record *prev;      /* Link in hash table */
>
> At this point this could be a normal list_head? Then you don't have to
> roll your own doubly-linked list manipulation (and benefit from things
> like CONFIG_LIST_DEBUG).

Yeah, I think this makes sense. Will do in v2. Thanks!
diff mbox series

Patch

diff --git a/lib/stackdepot.c b/lib/stackdepot.c
index a84c0debbb9e..641db97d8c7c 100644
--- a/lib/stackdepot.c
+++ b/lib/stackdepot.c
@@ -58,6 +58,7 @@  union handle_parts {
 
 struct stack_record {
 	struct stack_record *next;	/* Link in hash table or freelist */
+	struct stack_record *prev;	/* Link in hash table */
 	u32 hash;			/* Hash in hash table */
 	u32 size;			/* Number of stored frames */
 	union handle_parts handle;
@@ -493,6 +494,9 @@  depot_stack_handle_t __stack_depot_save(unsigned long *entries,
 
 		if (new) {
 			new->next = *bucket;
+			new->prev = NULL;
+			if (*bucket)
+				(*bucket)->prev = new;
 			*bucket = new;
 			found = new;
 		}