From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S932487AbeDWTLe (ORCPT ); Mon, 23 Apr 2018 15:11:34 -0400 Received: from mail-pf0-f196.google.com ([209.85.192.196]:39046 "EHLO mail-pf0-f196.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S932453AbeDWTLZ (ORCPT ); Mon, 23 Apr 2018 15:11:25 -0400 X-Google-Smtp-Source: AIpwx48h13dwowxV0k0BocaHAMys4UzxUwjXazDOZA3lvv3GZNMq7NV6WJEUG31zeTpq0NGV2zNg3A== Date: Mon, 23 Apr 2018 12:11:23 -0700 (PDT) X-Google-Original-Date: Mon, 23 Apr 2018 11:57:00 PDT (-0700) Subject: Re: [REVIEW][PATCH 14/22] signal/riscv: Use force_sig_fault where appropriate In-Reply-To: <20180420143811.9994-14-ebiederm@xmission.com> CC: linux-arch@vger.kernel.org, linux-kernel@vger.kernel.org, ebiederm@xmission.com, albert@sifive.com, linux-riscv@lists.infradead.org From: Palmer Dabbelt To: ebiederm@xmission.com Message-ID: Mime-Version: 1.0 (MHng) Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 8bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Fri, 20 Apr 2018 07:38:03 PDT (-0700), ebiederm@xmission.com wrote: > Filling in struct siginfo before calling force_sig_info a tedious and > error prone process, where once in a great while the wrong fields > are filled out, and siginfo has been inconsistently cleared. > > Simplify this process by using the helper force_sig_fault. Which > takes as a parameters all of the information it needs, ensures > all of the fiddly bits of filling in struct siginfo are done properly > and then calls force_sig_info. > > In short about a 5 line reduction in code for every time force_sig_info > is called, which makes the calling function clearer. > > Cc: Palmer Dabbelt > Cc: Albert Ou > Cc: linux-riscv@lists.infradead.org > Signed-off-by: "Eric W. Biederman" > --- > arch/riscv/kernel/traps.c | 9 +-------- > 1 file changed, 1 insertion(+), 8 deletions(-) > > diff --git a/arch/riscv/kernel/traps.c b/arch/riscv/kernel/traps.c > index 48aa6471cede..3087940008f4 100644 > --- a/arch/riscv/kernel/traps.c > +++ b/arch/riscv/kernel/traps.c > @@ -66,14 +66,7 @@ void die(struct pt_regs *regs, const char *str) > static inline void do_trap_siginfo(int signo, int code, > unsigned long addr, struct task_struct *tsk) > { > - siginfo_t info; > - > - clear_siginfo(&info); > - info.si_signo = signo; > - info.si_errno = 0; > - info.si_code = code; > - info.si_addr = (void __user *)addr; > - force_sig_info(signo, &info, tsk); > + force_sig_fault(signo, code, (void __user *)addr, tsk); > } > > void do_trap(struct pt_regs *regs, int signo, int code, If I understand this correctly, any change in behavior this causes would have been a bug on our end not filling out siginfo correctly? In that case then feel free to add an Acked-by: Palmer Dabbelt Thanks! From mboxrd@z Thu Jan 1 00:00:00 1970 From: Palmer Dabbelt Subject: Re: [REVIEW][PATCH 14/22] signal/riscv: Use force_sig_fault where appropriate Date: Mon, 23 Apr 2018 12:11:23 -0700 (PDT) Message-ID: References: <20180420143811.9994-14-ebiederm@xmission.com> Mime-Version: 1.0 (MHng) Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 8bit Return-path: In-Reply-To: <20180420143811.9994-14-ebiederm@xmission.com> Sender: linux-kernel-owner@vger.kernel.org Cc: linux-arch@vger.kernel.org, linux-kernel@vger.kernel.org, ebiederm@xmission.com, albert@sifive.com, linux-riscv@lists.infradead.org List-Id: linux-arch.vger.kernel.org On Fri, 20 Apr 2018 07:38:03 PDT (-0700), ebiederm@xmission.com wrote: > Filling in struct siginfo before calling force_sig_info a tedious and > error prone process, where once in a great while the wrong fields > are filled out, and siginfo has been inconsistently cleared. > > Simplify this process by using the helper force_sig_fault. Which > takes as a parameters all of the information it needs, ensures > all of the fiddly bits of filling in struct siginfo are done properly > and then calls force_sig_info. > > In short about a 5 line reduction in code for every time force_sig_info > is called, which makes the calling function clearer. > > Cc: Palmer Dabbelt > Cc: Albert Ou > Cc: linux-riscv@lists.infradead.org > Signed-off-by: "Eric W. Biederman" > --- > arch/riscv/kernel/traps.c | 9 +-------- > 1 file changed, 1 insertion(+), 8 deletions(-) > > diff --git a/arch/riscv/kernel/traps.c b/arch/riscv/kernel/traps.c > index 48aa6471cede..3087940008f4 100644 > --- a/arch/riscv/kernel/traps.c > +++ b/arch/riscv/kernel/traps.c > @@ -66,14 +66,7 @@ void die(struct pt_regs *regs, const char *str) > static inline void do_trap_siginfo(int signo, int code, > unsigned long addr, struct task_struct *tsk) > { > - siginfo_t info; > - > - clear_siginfo(&info); > - info.si_signo = signo; > - info.si_errno = 0; > - info.si_code = code; > - info.si_addr = (void __user *)addr; > - force_sig_info(signo, &info, tsk); > + force_sig_fault(signo, code, (void __user *)addr, tsk); > } > > void do_trap(struct pt_regs *regs, int signo, int code, If I understand this correctly, any change in behavior this causes would have been a bug on our end not filling out siginfo correctly? In that case then feel free to add an Acked-by: Palmer Dabbelt Thanks! From mboxrd@z Thu Jan 1 00:00:00 1970 From: palmer@sifive.com (Palmer Dabbelt) Date: Mon, 23 Apr 2018 12:11:23 -0700 (PDT) Subject: [REVIEW][PATCH 14/22] signal/riscv: Use force_sig_fault where appropriate In-Reply-To: <20180420143811.9994-14-ebiederm@xmission.com> Message-ID: To: linux-riscv@lists.infradead.org List-Id: linux-riscv.lists.infradead.org On Fri, 20 Apr 2018 07:38:03 PDT (-0700), ebiederm at xmission.com wrote: > Filling in struct siginfo before calling force_sig_info a tedious and > error prone process, where once in a great while the wrong fields > are filled out, and siginfo has been inconsistently cleared. > > Simplify this process by using the helper force_sig_fault. Which > takes as a parameters all of the information it needs, ensures > all of the fiddly bits of filling in struct siginfo are done properly > and then calls force_sig_info. > > In short about a 5 line reduction in code for every time force_sig_info > is called, which makes the calling function clearer. > > Cc: Palmer Dabbelt > Cc: Albert Ou > Cc: linux-riscv at lists.infradead.org > Signed-off-by: "Eric W. Biederman" > --- > arch/riscv/kernel/traps.c | 9 +-------- > 1 file changed, 1 insertion(+), 8 deletions(-) > > diff --git a/arch/riscv/kernel/traps.c b/arch/riscv/kernel/traps.c > index 48aa6471cede..3087940008f4 100644 > --- a/arch/riscv/kernel/traps.c > +++ b/arch/riscv/kernel/traps.c > @@ -66,14 +66,7 @@ void die(struct pt_regs *regs, const char *str) > static inline void do_trap_siginfo(int signo, int code, > unsigned long addr, struct task_struct *tsk) > { > - siginfo_t info; > - > - clear_siginfo(&info); > - info.si_signo = signo; > - info.si_errno = 0; > - info.si_code = code; > - info.si_addr = (void __user *)addr; > - force_sig_info(signo, &info, tsk); > + force_sig_fault(signo, code, (void __user *)addr, tsk); > } > > void do_trap(struct pt_regs *regs, int signo, int code, If I understand this correctly, any change in behavior this causes would have been a bug on our end not filling out siginfo correctly? In that case then feel free to add an Acked-by: Palmer Dabbelt Thanks!