| Message ID | 20170719044946.22030-7-benh@kernel.crashing.org (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | 41b464e5e5675476a5fd923aea8d5a87d2493d93 |
| Headers | show |
Benjamin Herrenschmidt <benh@kernel.crashing.org> a écrit : > Define a common page_fault_is_write() helper and use it > > Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org> > --- > arch/powerpc/mm/fault.c | 18 +++++++++++------- > 1 file changed, 11 insertions(+), 7 deletions(-) > > diff --git a/arch/powerpc/mm/fault.c b/arch/powerpc/mm/fault.c > index f257965b54b5..26ec0dd4f419 100644 > --- a/arch/powerpc/mm/fault.c > +++ b/arch/powerpc/mm/fault.c > @@ -183,6 +183,16 @@ static int mm_fault_error(struct pt_regs *regs, > unsigned long addr, int fault) > } > > /* > + * Define the correct "is_write" bit in error_code based > + * on the processor family > + */ > +#if (defined(CONFIG_4xx) || defined(CONFIG_BOOKE)) > +#define page_fault_is_write(__err) ((__err) & ESR_DST) > +#else > +#define page_fault_is_write(__err) ((__err) & DSISR_ISSTORE) > +#endif Doesn't linux kernel codying style make preference to static inline functions instead of macros ? > + > +/* > * For 600- and 800-family processors, the error_code parameter is DSISR > * for a data fault, SRR1 for an instruction fault. For 400-family > processors > * the error_code parameter is ESR for a data fault, 0 for an instruction > @@ -202,18 +212,12 @@ static int __do_page_fault(struct pt_regs > *regs, unsigned long address, > struct mm_struct *mm = current->mm; > unsigned int flags = FAULT_FLAG_ALLOW_RETRY | FAULT_FLAG_KILLABLE; > int code = SEGV_MAPERR; > - int is_write = 0; > int is_exec = TRAP(regs) == 0x400; > int is_user = user_mode(regs); > + int is_write = page_fault_is_write(error_code); > int fault; > int rc = 0, store_update_sp = 0; > > -#if !(defined(CONFIG_4xx) || defined(CONFIG_BOOKE)) > - is_write = error_code & DSISR_ISSTORE; > -#else > - is_write = error_code & ESR_DST; > -#endif /* CONFIG_4xx || CONFIG_BOOKE */ > - > #ifdef CONFIG_PPC_ICSWX > /* > * we need to do this early because this "data storage > -- > 2.13.3
On Sat, 2017-07-22 at 18:40 +0200, LEROY Christophe wrote: > Benjamin Herrenschmidt <benh@kernel.crashing.org> a écrit : > > > Define a common page_fault_is_write() helper and use it > > > > Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org> > > --- > > arch/powerpc/mm/fault.c | 18 +++++++++++------- > > 1 file changed, 11 insertions(+), 7 deletions(-) > > > > diff --git a/arch/powerpc/mm/fault.c b/arch/powerpc/mm/fault.c > > index f257965b54b5..26ec0dd4f419 100644 > > --- a/arch/powerpc/mm/fault.c > > +++ b/arch/powerpc/mm/fault.c > > @@ -183,6 +183,16 @@ static int mm_fault_error(struct pt_regs *regs, > > unsigned long addr, int fault) > > } > > > > /* > > + * Define the correct "is_write" bit in error_code based > > + * on the processor family > > + */ > > +#if (defined(CONFIG_4xx) || defined(CONFIG_BOOKE)) > > +#define page_fault_is_write(__err) ((__err) & ESR_DST) > > +#else > > +#define page_fault_is_write(__err) ((__err) & DSISR_ISSTORE) > > +#endif > > Doesn't linux kernel codying style make preference to static inline > functions instead of macros ? Doesn't really matter, yes that could be, but then I would have to add a !! to make them proper bools I think unless I keep things as int, etc... not sure how well gcc will do with it. > > > + > > +/* > > * For 600- and 800-family processors, the error_code parameter is DSISR > > * for a data fault, SRR1 for an instruction fault. For 400-family > > processors > > * the error_code parameter is ESR for a data fault, 0 for an instruction > > @@ -202,18 +212,12 @@ static int __do_page_fault(struct pt_regs > > *regs, unsigned long address, > > struct mm_struct *mm = current->mm; > > unsigned int flags = FAULT_FLAG_ALLOW_RETRY | FAULT_FLAG_KILLABLE; > > int code = SEGV_MAPERR; > > - int is_write = 0; > > int is_exec = TRAP(regs) == 0x400; > > int is_user = user_mode(regs); > > + int is_write = page_fault_is_write(error_code); > > int fault; > > int rc = 0, store_update_sp = 0; > > > > -#if !(defined(CONFIG_4xx) || defined(CONFIG_BOOKE)) > > - is_write = error_code & DSISR_ISSTORE; > > -#else > > - is_write = error_code & ESR_DST; > > -#endif /* CONFIG_4xx || CONFIG_BOOKE */ > > - > > #ifdef CONFIG_PPC_ICSWX > > /* > > * we need to do this early because this "data storage > > -- > > 2.13.3 > >
LEROY Christophe <christophe.leroy@c-s.fr> writes: > Benjamin Herrenschmidt <benh@kernel.crashing.org> a écrit : > >> Define a common page_fault_is_write() helper and use it >> >> Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org> >> --- >> arch/powerpc/mm/fault.c | 18 +++++++++++------- >> 1 file changed, 11 insertions(+), 7 deletions(-) >> >> diff --git a/arch/powerpc/mm/fault.c b/arch/powerpc/mm/fault.c >> index f257965b54b5..26ec0dd4f419 100644 >> --- a/arch/powerpc/mm/fault.c >> +++ b/arch/powerpc/mm/fault.c >> @@ -183,6 +183,16 @@ static int mm_fault_error(struct pt_regs *regs, >> unsigned long addr, int fault) >> } >> >> /* >> + * Define the correct "is_write" bit in error_code based >> + * on the processor family >> + */ >> +#if (defined(CONFIG_4xx) || defined(CONFIG_BOOKE)) >> +#define page_fault_is_write(__err) ((__err) & ESR_DST) >> +#else >> +#define page_fault_is_write(__err) ((__err) & DSISR_ISSTORE) >> +#endif > > Doesn't linux kernel codying style make preference to static inline > functions instead of macros ? In general yes. Especially for things that look like functions. Ben was worried the code gen would be worse if it was a static inline returning bool, I'll apply it and have a look. cheers
diff --git a/arch/powerpc/mm/fault.c b/arch/powerpc/mm/fault.c index f257965b54b5..26ec0dd4f419 100644 --- a/arch/powerpc/mm/fault.c +++ b/arch/powerpc/mm/fault.c @@ -183,6 +183,16 @@ static int mm_fault_error(struct pt_regs *regs, unsigned long addr, int fault) } /* + * Define the correct "is_write" bit in error_code based + * on the processor family + */ +#if (defined(CONFIG_4xx) || defined(CONFIG_BOOKE)) +#define page_fault_is_write(__err) ((__err) & ESR_DST) +#else +#define page_fault_is_write(__err) ((__err) & DSISR_ISSTORE) +#endif + +/* * For 600- and 800-family processors, the error_code parameter is DSISR * for a data fault, SRR1 for an instruction fault. For 400-family processors * the error_code parameter is ESR for a data fault, 0 for an instruction @@ -202,18 +212,12 @@ static int __do_page_fault(struct pt_regs *regs, unsigned long address, struct mm_struct *mm = current->mm; unsigned int flags = FAULT_FLAG_ALLOW_RETRY | FAULT_FLAG_KILLABLE; int code = SEGV_MAPERR; - int is_write = 0; int is_exec = TRAP(regs) == 0x400; int is_user = user_mode(regs); + int is_write = page_fault_is_write(error_code); int fault; int rc = 0, store_update_sp = 0; -#if !(defined(CONFIG_4xx) || defined(CONFIG_BOOKE)) - is_write = error_code & DSISR_ISSTORE; -#else - is_write = error_code & ESR_DST; -#endif /* CONFIG_4xx || CONFIG_BOOKE */ - #ifdef CONFIG_PPC_ICSWX /* * we need to do this early because this "data storage
Define a common page_fault_is_write() helper and use it Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org> --- arch/powerpc/mm/fault.c | 18 +++++++++++------- 1 file changed, 11 insertions(+), 7 deletions(-)