| Message ID | 20230528113545.650533-1-linmiaohe@huawei.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | x86/mce: remove unused mce_vaddr | expand |
On Sun, May 28, 2023 at 07:35:45PM +0800, Miaohe Lin wrote: > Since commit a6e3cf70b772 ("x86/mce: Change to not send SIGBUS error during > copy from user"), mce_vaddr is not used anymore. Remove it and clean up the > relevant code. Hi Miaohe, Not so sure that the 'clean up' part is useful. See below. > > Signed-off-by: Miaohe Lin <linmiaohe@huawei.com> > --- > arch/x86/kernel/cpu/mce/severity.c | 7 +------ > include/linux/sched.h | 1 - > 2 files changed, 1 insertion(+), 7 deletions(-) > > diff --git a/arch/x86/kernel/cpu/mce/severity.c b/arch/x86/kernel/cpu/mce/severity.c > index c4477162c07d..0acc0039de81 100644 > --- a/arch/x86/kernel/cpu/mce/severity.c > +++ b/arch/x86/kernel/cpu/mce/severity.c > @@ -252,12 +252,7 @@ static bool is_copy_from_user(struct pt_regs *regs) > return false; > } > > - if (fault_in_kernel_space(addr)) > - return false; > - > - current->mce_vaddr = (void __user *)addr; > - > - return true; > + return !fault_in_kernel_space(addr); > } Refactoring the return is unnecessary and seems less readable. How about removing the assignment, and leaving the rest, as is: diff --git a/arch/x86/kernel/cpu/mce/severity.c b/arch/x86/kernel/cpu/mce/severity.c index c4477162c07d..1c03221ddcb1 100644 --- a/arch/x86/kernel/cpu/mce/severity.c +++ b/arch/x86/kernel/cpu/mce/severity.c @@ -255,8 +255,6 @@ static bool is_copy_from_user(struct pt_regs *regs) if (fault_in_kernel_space(addr)) return false; - current->mce_vaddr = (void __user *)addr; - return true; } Alison > > /* > diff --git a/include/linux/sched.h b/include/linux/sched.h > index eed5d65b8d1f..3054a7087230 100644 > --- a/include/linux/sched.h > +++ b/include/linux/sched.h > @@ -1493,7 +1493,6 @@ struct task_struct { > #endif > > #ifdef CONFIG_X86_MCE > - void __user *mce_vaddr; > __u64 mce_kflags; > u64 mce_addr; > __u64 mce_ripv : 1, > -- > 2.27.0 >
On 2023/5/31 3:16, Alison Schofield wrote: > On Sun, May 28, 2023 at 07:35:45PM +0800, Miaohe Lin wrote: >> Since commit a6e3cf70b772 ("x86/mce: Change to not send SIGBUS error during >> copy from user"), mce_vaddr is not used anymore. Remove it and clean up the >> relevant code. > > Hi Miaohe, > > Not so sure that the 'clean up' part is useful. See below. > > >> >> Signed-off-by: Miaohe Lin <linmiaohe@huawei.com> >> --- >> arch/x86/kernel/cpu/mce/severity.c | 7 +------ >> include/linux/sched.h | 1 - >> 2 files changed, 1 insertion(+), 7 deletions(-) >> >> diff --git a/arch/x86/kernel/cpu/mce/severity.c b/arch/x86/kernel/cpu/mce/severity.c >> index c4477162c07d..0acc0039de81 100644 >> --- a/arch/x86/kernel/cpu/mce/severity.c >> +++ b/arch/x86/kernel/cpu/mce/severity.c >> @@ -252,12 +252,7 @@ static bool is_copy_from_user(struct pt_regs *regs) >> return false; >> } >> >> - if (fault_in_kernel_space(addr)) >> - return false; >> - >> - current->mce_vaddr = (void __user *)addr; >> - >> - return true; >> + return !fault_in_kernel_space(addr); >> } > > > Refactoring the return is unnecessary and seems less readable. > How about removing the assignment, and leaving the rest, as is: > > diff --git a/arch/x86/kernel/cpu/mce/severity.c b/arch/x86/kernel/cpu/mce/severity.c > index c4477162c07d..1c03221ddcb1 100644 > --- a/arch/x86/kernel/cpu/mce/severity.c > +++ b/arch/x86/kernel/cpu/mce/severity.c > @@ -255,8 +255,6 @@ static bool is_copy_from_user(struct pt_regs *regs) > if (fault_in_kernel_space(addr)) > return false; > > - current->mce_vaddr = (void __user *)addr; > - > return true; > } > Sounds good to me. Will do it in v2. Thanks for your advice.
diff --git a/arch/x86/kernel/cpu/mce/severity.c b/arch/x86/kernel/cpu/mce/severity.c index c4477162c07d..0acc0039de81 100644 --- a/arch/x86/kernel/cpu/mce/severity.c +++ b/arch/x86/kernel/cpu/mce/severity.c @@ -252,12 +252,7 @@ static bool is_copy_from_user(struct pt_regs *regs) return false; } - if (fault_in_kernel_space(addr)) - return false; - - current->mce_vaddr = (void __user *)addr; - - return true; + return !fault_in_kernel_space(addr); } /* diff --git a/include/linux/sched.h b/include/linux/sched.h index eed5d65b8d1f..3054a7087230 100644 --- a/include/linux/sched.h +++ b/include/linux/sched.h @@ -1493,7 +1493,6 @@ struct task_struct { #endif #ifdef CONFIG_X86_MCE - void __user *mce_vaddr; __u64 mce_kflags; u64 mce_addr; __u64 mce_ripv : 1,
Since commit a6e3cf70b772 ("x86/mce: Change to not send SIGBUS error during copy from user"), mce_vaddr is not used anymore. Remove it and clean up the relevant code. Signed-off-by: Miaohe Lin <linmiaohe@huawei.com> --- arch/x86/kernel/cpu/mce/severity.c | 7 +------ include/linux/sched.h | 1 - 2 files changed, 1 insertion(+), 7 deletions(-)