> Fengguang, if you're still listening, could you please rerun the tests > on top of ce07a9415f26, with the attached patches also applied? Ping!? it would be very good to get feedback on this asap. > From e7840ad76515f0b5061fcdd098b57b7c01b61482 Mon Sep 17 00:00:00 2001 > Message-Id: > From: Josh Poimboeuf > Date: Thu, 5 Oct 2017 09:43:59 -0500 > Subject: [PATCH 1/2] unwinder fixes > > --- > arch/x86/kernel/unwind_frame.c | 33 ++++++++++++++++++++++++++++++--- > 1 file changed, 30 insertions(+), 3 deletions(-) > > diff --git a/arch/x86/kernel/unwind_frame.c b/arch/x86/kernel/unwind_frame.c > index b9389d72b2f7..0ecc42e34cc4 100644 > --- a/arch/x86/kernel/unwind_frame.c > +++ b/arch/x86/kernel/unwind_frame.c > @@ -33,7 +33,7 @@ static void unwind_dump(struct unwind_state *state) > struct stack_info stack_info = {0}; > unsigned long visit_mask = 0; > > - if (dumped_before) > + if (IS_ENABLED(CONFIG_X86_32) || dumped_before) > return; > > dumped_before = true; > @@ -42,7 +42,8 @@ static void unwind_dump(struct unwind_state *state) > state->stack_info.type, state->stack_info.next_sp, > state->stack_mask, state->graph_idx); > > - for (sp = state->orig_sp; sp; sp = PTR_ALIGN(stack_info.next_sp, sizeof(long))) { > + for (sp = PTR_ALIGN(state->orig_sp, sizeof(long)); sp; > + sp = PTR_ALIGN(stack_info.next_sp, sizeof(long))) { > if (get_stack_info(sp, state->task, &stack_info, &visit_mask)) > break; > > @@ -84,6 +85,12 @@ static size_t regs_size(struct pt_regs *regs) > return sizeof(*regs); > } > > +#ifdef CONFIG_X86_32 > +#define KERNEL_REGS_SIZE (sizeof(struct pt_regs) - 2*sizeof(long)) > +#else > +#define KERNEL_REGS_SIZE (sizeof(struct pt_regs)) > +#endif > + > static bool in_entry_code(unsigned long ip) > { > char *addr = (char *)ip; > @@ -183,6 +190,7 @@ static bool is_last_task_frame(struct unwind_state *state) > * This determines if the frame pointer actually contains an encoded pointer to > * pt_regs on the stack. See ENCODE_FRAME_POINTER. > */ > +#ifdef CONFIG_X86_64 > static struct pt_regs *decode_frame_pointer(unsigned long *bp) > { > unsigned long regs = (unsigned long)bp; > @@ -192,6 +200,17 @@ static struct pt_regs *decode_frame_pointer(unsigned long *bp) > > return (struct pt_regs *)(regs & ~0x1); > } > +#else > +static struct pt_regs *decode_frame_pointer(unsigned long *bp) > +{ > + unsigned long regs = (unsigned long)bp; > + > + if (regs & 0x80000000) > + return NULL; > + > + return (struct pt_regs *)(regs | 0x80000000); > +} > +#endif > > static bool update_stack_state(struct unwind_state *state, > unsigned long *next_bp) > @@ -211,7 +230,7 @@ static bool update_stack_state(struct unwind_state *state, > regs = decode_frame_pointer(next_bp); > if (regs) { > frame = (unsigned long *)regs; > - len = regs_size(regs); > + len = KERNEL_REGS_SIZE; > state->got_irq = true; > } else { > frame = next_bp; > @@ -235,6 +254,14 @@ static bool update_stack_state(struct unwind_state *state, > frame < prev_frame_end) > return false; > > + /* > + * On 32-bit with user mode regs, make sure the last two regs are safe > + * to access: > + */ > + if (IS_ENABLED(CONFIG_X86_32) && regs && user_mode(regs) && > + !on_stack(info, frame, len + 2*sizeof(long))) > + return false; > + > /* Move state to the next frame: */ > if (regs) { > state->regs = regs; > -- > 2.13.6 > > From 62105550632bfbd2e5e2f3768a37958a6872ec1e Mon Sep 17 00:00:00 2001 > Message-Id: <62105550632bfbd2e5e2f3768a37958a6872ec1e.1507215196.git.jpoimboe@redhat.com> > In-Reply-To: > References: > From: Peter Zijlstra > Date: Thu, 5 Oct 2017 09:44:33 -0500 > Subject: [PATCH 2/2] lockdep fixes > > --- > kernel/locking/lockdep.c | 48 ++++++++++++++++++++---------------------------- > 1 file changed, 20 insertions(+), 28 deletions(-) > > diff --git a/kernel/locking/lockdep.c b/kernel/locking/lockdep.c > index 841828ba35b9..6d540bdb24b3 100644 > --- a/kernel/locking/lockdep.c > +++ b/kernel/locking/lockdep.c > @@ -1827,10 +1827,10 @@ check_prev_add(struct task_struct *curr, struct held_lock *prev, > struct held_lock *next, int distance, struct stack_trace *trace, > int (*save)(struct stack_trace *trace)) > { > + struct lock_list *uninitialized_var(target_entry); > struct lock_list *entry; > - int ret; > struct lock_list this; > - struct lock_list *uninitialized_var(target_entry); > + int ret; > > /* > * Prove that the new -> dependency would not > @@ -1844,8 +1844,17 @@ check_prev_add(struct task_struct *curr, struct held_lock *prev, > this.class = hlock_class(next); > this.parent = NULL; > ret = check_noncircular(&this, hlock_class(prev), &target_entry); > - if (unlikely(!ret)) > + if (unlikely(!ret)) { > + if (!trace->entries) { > + /* > + * If @save fails here, the printing might trigger > + * a WARN but because of the !nr_entries it should > + * not do bad things. > + */ > + save(trace); > + } > return print_circular_bug(&this, target_entry, next, prev); > + } > else if (unlikely(ret < 0)) > return print_bfs_bug(ret); > > @@ -1892,7 +1901,7 @@ check_prev_add(struct task_struct *curr, struct held_lock *prev, > return print_bfs_bug(ret); > > > - if (save && !save(trace)) > + if (!trace->entries && !save(trace)) > return 0; > > /* > @@ -1912,20 +1921,6 @@ check_prev_add(struct task_struct *curr, struct held_lock *prev, > if (!ret) > return 0; > > - /* > - * Debugging printouts: > - */ > - if (verbose(hlock_class(prev)) || verbose(hlock_class(next))) { > - graph_unlock(); > - printk("\n new dependency: "); > - print_lock_name(hlock_class(prev)); > - printk(KERN_CONT " => "); > - print_lock_name(hlock_class(next)); > - printk(KERN_CONT "\n"); > - dump_stack(); > - if (!graph_lock()) > - return 0; > - } > return 2; > } > > @@ -1940,8 +1935,12 @@ check_prevs_add(struct task_struct *curr, struct held_lock *next) > { > int depth = curr->lockdep_depth; > struct held_lock *hlock; > - struct stack_trace trace; > - int (*save)(struct stack_trace *trace) = save_trace; > + struct stack_trace trace = { > + .nr_entries = 0, > + .max_entries = 0, > + .entries = NULL, > + .skip = 0, > + }; > > /* > * Debugging checks. > @@ -1967,18 +1966,11 @@ check_prevs_add(struct task_struct *curr, struct held_lock *next) > */ > if (hlock->read != 2 && hlock->check) { > int ret = check_prev_add(curr, hlock, next, > - distance, &trace, save); > + distance, &trace, save_trace); > if (!ret) > return 0; > > /* > - * Stop saving stack_trace if save_trace() was > - * called at least once: > - */ > - if (save && ret == 2) > - save = NULL; > - > - /* > * Stop after the first non-trylock entry, > * as non-trylock entries have added their > * own direct dependencies already, so this > -- > 2.13.6 >