Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1557585 > unrolled thread

Re: [PATCH v4 06/15] lockdep: Make save_trace can skip stack tracing of the current

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  Re: [PATCH v4 06/15] lockdep: Make save_trace can skip stack tracing  of the current Peter Zijlstra <peterz@infradead.org> - 2017-01-12 17:40 +0100
    Re: [PATCH v4 06/15] lockdep: Make save_trace can skip stack tracing  of the current Byungchul Park <byungchul.park@lge.com> - 2017-01-13 01:20 +0100

#1557585 — Re: [PATCH v4 06/15] lockdep: Make save_trace can skip stack tracing of the current

On Fri, Dec 09, 2016 at 02:12:02PM +0900, Byungchul Park wrote:
> Currently, save_trace() always performs save_stack_trace() for the
> current. However, crossrelease needs to use stack trace data of another
> context instead of the current. So add a parameter for skipping stack
> tracing of the current and make it use trace data, which is already
> saved by crossrelease framework.
> 
> Signed-off-by: Byungchul Park <byungchul.park@lge.com>
> ---
>  kernel/locking/lockdep.c | 33 ++++++++++++++++++++-------------
>  1 file changed, 20 insertions(+), 13 deletions(-)
> 
> diff --git a/kernel/locking/lockdep.c b/kernel/locking/lockdep.c
> index 3eaa11c..11580ec 100644
> --- a/kernel/locking/lockdep.c
> +++ b/kernel/locking/lockdep.c
> @@ -387,15 +387,22 @@ static void print_lockdep_off(const char *bug_msg)
>  #endif
>  }
>  
> -static int save_trace(struct stack_trace *trace)
> +static int save_trace(struct stack_trace *trace, int skip_tracing)
>  {
> -	trace->nr_entries = 0;
> -	trace->max_entries = MAX_STACK_TRACE_ENTRIES - nr_stack_trace_entries;
> -	trace->entries = stack_trace + nr_stack_trace_entries;
> +	unsigned int nr_avail = MAX_STACK_TRACE_ENTRIES - nr_stack_trace_entries;
>  
> -	trace->skip = 3;
> -
> -	save_stack_trace(trace);
> +	if (skip_tracing) {
> +		trace->nr_entries = min(trace->nr_entries, nr_avail);
> +		memcpy(stack_trace + nr_stack_trace_entries, trace->entries,
> +				trace->nr_entries * sizeof(trace->entries[0]));
> +		trace->entries = stack_trace + nr_stack_trace_entries;
> +	} else {
> +		trace->nr_entries = 0;
> +		trace->max_entries = nr_avail;
> +		trace->entries = stack_trace + nr_stack_trace_entries;
> +		trace->skip = 3;
> +		save_stack_trace(trace);
> +	}
>  
>  	/*
>  	 * Some daft arches put -1 at the end to indicate its a full trace.

That's pretty nasty semantics.. so when skip_tracing it modifies trace
in-place.

[toc] | [next] | [standalone]

#1557927

On Thu, Jan 12, 2017 at 05:37:57PM +0100, Peter Zijlstra wrote:
> On Fri, Dec 09, 2016 at 02:12:02PM +0900, Byungchul Park wrote:
> > Currently, save_trace() always performs save_stack_trace() for the
> > current. However, crossrelease needs to use stack trace data of another
> > context instead of the current. So add a parameter for skipping stack
> > tracing of the current and make it use trace data, which is already
> > saved by crossrelease framework.
> > 
> > Signed-off-by: Byungchul Park <byungchul.park@lge.com>
> > ---
> >  kernel/locking/lockdep.c | 33 ++++++++++++++++++++-------------
> >  1 file changed, 20 insertions(+), 13 deletions(-)
> > 
> > diff --git a/kernel/locking/lockdep.c b/kernel/locking/lockdep.c
> > index 3eaa11c..11580ec 100644
> > --- a/kernel/locking/lockdep.c
> > +++ b/kernel/locking/lockdep.c
> > @@ -387,15 +387,22 @@ static void print_lockdep_off(const char *bug_msg)
> >  #endif
> >  }
> >  
> > -static int save_trace(struct stack_trace *trace)
> > +static int save_trace(struct stack_trace *trace, int skip_tracing)
> >  {
> > -	trace->nr_entries = 0;
> > -	trace->max_entries = MAX_STACK_TRACE_ENTRIES - nr_stack_trace_entries;
> > -	trace->entries = stack_trace + nr_stack_trace_entries;
> > +	unsigned int nr_avail = MAX_STACK_TRACE_ENTRIES - nr_stack_trace_entries;
> >  
> > -	trace->skip = 3;
> > -
> > -	save_stack_trace(trace);
> > +	if (skip_tracing) {
> > +		trace->nr_entries = min(trace->nr_entries, nr_avail);
> > +		memcpy(stack_trace + nr_stack_trace_entries, trace->entries,
> > +				trace->nr_entries * sizeof(trace->entries[0]));
> > +		trace->entries = stack_trace + nr_stack_trace_entries;
> > +	} else {
> > +		trace->nr_entries = 0;
> > +		trace->max_entries = nr_avail;
> > +		trace->entries = stack_trace + nr_stack_trace_entries;
> > +		trace->skip = 3;
> > +		save_stack_trace(trace);
> > +	}
> >  
> >  	/*
> >  	 * Some daft arches put -1 at the end to indicate its a full trace.
> 
> That's pretty nasty semantics.. so when skip_tracing it modifies trace
> in-place.

I agree. Let me think more and enhance it.

Thank you,
Byungchul

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web