Messages in this thread | | | Subject | Re: [PATCH v2 09/16] powerpc/watchpoint: Convert thread_struct->hw_brk to an array | From | Ravi Bangoria <> | Date | Wed, 1 Apr 2020 14:36:21 +0530 |
| |
>> static void set_debug_reg_defaults(struct thread_struct *thread) >> { >> - thread->hw_brk.address = 0; >> - thread->hw_brk.type = 0; >> - thread->hw_brk.len = 0; >> - thread->hw_brk.hw_len = 0; >> - if (ppc_breakpoint_available()) >> - set_breakpoint(&thread->hw_brk); >> + int i; >> + >> + for (i = 0; i < nr_wp_slots(); i++) { > > Maybe you could add the following that you added other places: > > struct arch_hw_breakpoint null_brk = {0}; > > Then do > > thread->hw_brk[i] = null_brk;
Yes that's better.
[...]
>> +static void switch_hw_breakpoint(struct task_struct *new) >> +{ >> + int i; >> + >> + for (i = 0; i < nr_wp_slots(); i++) { >> + if (unlikely(!hw_brk_match(this_cpu_ptr(¤t_brk[i]), >> + &new->thread.hw_brk[i]))) { >> + __set_breakpoint(&new->thread.hw_brk[i], i); >> + } > > Or could be: > > if (likely(hw_brk_match(this_cpu_ptr(¤t_brk[i]), > &new->thread.hw_brk[i]))) > continue; > __set_breakpoint(&new->thread.hw_brk[i], i); >
Sure.
[...]
>> @@ -128,8 +131,10 @@ static void do_signal(struct task_struct *tsk) >> * user space. The DABR will have been cleared if it >> * triggered inside the kernel. >> */ >> - if (tsk->thread.hw_brk.address && tsk->thread.hw_brk.type) >> - __set_breakpoint(&tsk->thread.hw_brk, 0); >> + for (i = 0; i < nr_wp_slots(); i++) { >> + if (tsk->thread.hw_brk[i].address && tsk->thread.hw_brk[i].type) >> + __set_breakpoint(&tsk->thread.hw_brk[i], i); >> + } > > thread.hwbrk also exists when CONFIG_PPC_ADV_DEBUG_REGS is selected. > > You could replace the #ifndef CONFIG_PPC_ADV_DEBUG_REGS by an if (!IS_ENABLED(CONFIG_PPC_ADV_DEBUG_REGS)) and then no need of an ifdef when declaring the int i;
Makes sense. Will change it.
Ravi
| |