Re: [PATCH 1/3] powerpc/8xx: Enhance readability of trap types

2021-04-19 Thread Xiongwei Song
On Mon, Apr 19, 2021 at 11:48 PM Christophe Leroy
 wrote:
>
> This patch makes use of trap types in head_8xx.S
>
> Signed-off-by: Christophe Leroy 
> ---
>  arch/powerpc/include/asm/interrupt.h | 29 
>  arch/powerpc/kernel/head_8xx.S   | 49 ++--
>  2 files changed, 47 insertions(+), 31 deletions(-)
>
> diff --git a/arch/powerpc/include/asm/interrupt.h 
> b/arch/powerpc/include/asm/interrupt.h
> index ed2c4042c3d1..cf2c5c3ae716 100644
> --- a/arch/powerpc/include/asm/interrupt.h
> +++ b/arch/powerpc/include/asm/interrupt.h
> @@ -2,13 +2,6 @@
>  #ifndef _ASM_POWERPC_INTERRUPT_H
>  #define _ASM_POWERPC_INTERRUPT_H
>
> -#include 
> -#include 
> -#include 
> -#include 
> -#include 
> -#include 
> -
>  /* BookE/4xx */
>  #define INTERRUPT_CRITICAL_INPUT  0x100
>
> @@ -39,9 +32,11 @@
>  /* BookE/BookS/4xx/8xx */
>  #define INTERRUPT_DATA_STORAGE0x300
>  #define INTERRUPT_INST_STORAGE0x400
> +#define INTERRUPT_EXTERNAL 0x500
>  #define INTERRUPT_ALIGNMENT   0x600
>  #define INTERRUPT_PROGRAM 0x700
>  #define INTERRUPT_SYSCALL 0xc00
> +#define INTERRUPT_TRACE0xd00

The INTERRUPT_TRACE macro is defined in BookS section.
In BookE, 0xd00 stands for debug interrupt, so I defined it as
INTERRUPT_DEBUG.  I understand they are similar things,
but the terminologies are different in reference manuals.

Regards,
Xiongwei


Re: linux-next: build failure after merge of the powerpc tree

2021-04-19 Thread Xiongwei Song
Thank you so much Stephen. Sorry for my negligence.

Should I fix this myself on powerpc tree?

Regards,
Xiongwei

On Mon, Apr 19, 2021 at 5:14 PM Stephen Rothwell  wrote:
>
> Hi all,
>
> After merging the powerpc tree, today's linux-next build (powerpc
> allyesconfig) failed like this:
>
> arch/powerpc/kernel/fadump.c: In function 'crash_fadump':
> arch/powerpc/kernel/fadump.c:731:28: error: 'INTERRUPT_SYSTEM_RESET' 
> undeclared (first use in this function)
>   731 |  if (TRAP(&(fdh->regs)) == INTERRUPT_SYSTEM_RESET) {
>   |^~
> arch/powerpc/kernel/fadump.c:731:28: note: each undeclared identifier is 
> reported only once for each function it appears in
>
> Caused by commit
>
>   7153d4bf0b37 ("powerpc/traps: Enhance readability for trap types")
>
> I have applied the following patch for today.
>
> From: Stephen Rothwell 
> Date: Mon, 19 Apr 2021 19:05:05 +1000
> Subject: [PATCH] fix up for "powerpc/traps: Enhance readability for trap 
> types"
>
> Signed-off-by: Stephen Rothwell 
> ---
>  arch/powerpc/kernel/fadump.c | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/arch/powerpc/kernel/fadump.c b/arch/powerpc/kernel/fadump.c
> index b55b4c23f3b6..000e3b7f3fca 100644
> --- a/arch/powerpc/kernel/fadump.c
> +++ b/arch/powerpc/kernel/fadump.c
> @@ -31,6 +31,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>
>  /*
>   * The CPU who acquired the lock to trigger the fadump crash should
> --
> 2.30.2
>
> --
> Cheers,
> Stephen Rothwell


[PATCH v5] powerpc/traps: Enhance readability for trap types

2021-04-14 Thread Xiongwei Song
From: Xiongwei Song 

Define macros to list ppc interrupt types in interttupt.h, replace the
reference of the trap hex values with these macros.

Referred the hex numbers in arch/powerpc/kernel/exceptions-64e.S,
arch/powerpc/kernel/exceptions-64s.S, arch/powerpc/kernel/head_*.S,
arch/powerpc/kernel/head_booke.h and arch/powerpc/include/asm/kvm_asm.h.

v4-v5:
* Delete unnecessary #ifdef.
* Move INTERRUPT_* macros to interttupt.h, classify for different cpu
  types. Drop traps.h.
* Directly define INTERRUPT_MACHINE_CHECK with 0x200.

v3-v4:
Fix compile issue:
arch/powerpc/kernel/process.c:1473:14: error: 'INTERRUPT_MACHINE_CHECK' 
undeclared (first use in this function); did you mean 'TAINT_MACHINE_CHECK'?
Reported-by: kernel test robot 

v2-v3:
Correct the prefix of trap macros with INTERRUPT_, the previous prefix
is TRAP_, which is not precise. This is suggested by Segher Boessenkool
and Nicholas Piggin.

v1-v2:
Define more trap macros to replace more trap hexs in code, not just for
the __show_regs function. This is suggested by Christophe Leroy.

Signed-off-by: Xiongwei Song 
---
 arch/powerpc/include/asm/interrupt.h  | 48 +--
 arch/powerpc/kernel/fadump.c  |  2 +-
 arch/powerpc/kernel/interrupt.c   |  2 +-
 arch/powerpc/kernel/process.c |  4 ++-
 arch/powerpc/kernel/traps.c   |  6 ++--
 arch/powerpc/kexec/crash.c|  3 +-
 arch/powerpc/mm/book3s64/hash_utils.c |  4 +--
 arch/powerpc/mm/fault.c   | 16 -
 arch/powerpc/perf/core-book3s.c   |  5 +--
 arch/powerpc/xmon/xmon.c  | 20 +++
 10 files changed, 81 insertions(+), 29 deletions(-)

diff --git a/arch/powerpc/include/asm/interrupt.h 
b/arch/powerpc/include/asm/interrupt.h
index 05e7fc4ffb50..86daf1242088 100644
--- a/arch/powerpc/include/asm/interrupt.h
+++ b/arch/powerpc/include/asm/interrupt.h
@@ -9,6 +9,46 @@
 #include 
 #include 
 
+/* BookE/4xx */
+#define INTERRUPT_CRITICAL_INPUT  0x100
+
+/* BookE */
+#define INTERRUPT_DEBUG   0xd00
+#ifdef CONFIG_BOOKE
+#define INTERRUPT_PERFMON 0x260
+#define INTERRUPT_DOORBELL0x280
+#endif
+
+/* BookS/4xx/8xx */
+#define INTERRUPT_MACHINE_CHECK   0x200
+
+/* BookS/8xx */
+#define INTERRUPT_SYSTEM_RESET0x100
+
+/* BookS */
+#define INTERRUPT_DATA_SEGMENT0x380
+#define INTERRUPT_INST_SEGMENT0x480
+#define INTERRUPT_TRACE   0xd00
+#define INTERRUPT_H_DATA_STORAGE  0xe00
+#define INTERRUPT_H_FAC_UNAVAIL   0xf80
+#ifdef CONFIG_PPC_BOOK3S
+#define INTERRUPT_DOORBELL0xa00
+#define INTERRUPT_PERFMON 0xf00
+#endif
+
+/* BookE/BookS/4xx/8xx */
+#define INTERRUPT_DATA_STORAGE0x300
+#define INTERRUPT_INST_STORAGE0x400
+#define INTERRUPT_ALIGNMENT   0x600
+#define INTERRUPT_PROGRAM 0x700
+#define INTERRUPT_SYSCALL 0xc00
+
+/* BookE/BookS/44x */
+#define INTERRUPT_FP_UNAVAIL  0x800
+
+/* BookE/BookS/44x/8xx */
+#define INTERRUPT_DECREMENTER 0x900
+
 static inline void nap_adjust_return(struct pt_regs *regs)
 {
 #ifdef CONFIG_PPC_970_NAP
@@ -70,7 +110,7 @@ static inline void interrupt_enter_prepare(struct pt_regs 
*regs, struct interrup
 * CT_WARN_ON comes here via program_check_exception,
 * so avoid recursion.
 */
-   if (TRAP(regs) != 0x700)
+   if (TRAP(regs) != INTERRUPT_PROGRAM)
CT_WARN_ON(ct_state() != CONTEXT_KERNEL);
}
 #endif
@@ -175,7 +215,8 @@ static inline void interrupt_nmi_enter_prepare(struct 
pt_regs *regs, struct inte
/* Don't do any per-CPU operations until interrupt state is fixed */
 #endif
/* Allow DEC and PMI to be traced when they are soft-NMI */
-   if (TRAP(regs) != 0x900 && TRAP(regs) != 0xf00 && TRAP(regs) != 0x260) {
+   if (TRAP(regs) != INTERRUPT_DECREMENTER &&
+   TRAP(regs) != INTERRUPT_PERFMON) {
state->ftrace_enabled = this_cpu_get_ftrace_enabled();
this_cpu_set_ftrace_enabled(0);
}
@@ -204,7 +245,8 @@ static inline void interrupt_nmi_exit_prepare(struct 
pt_regs *regs, struct inter
 */
 
 #ifdef CONFIG_PPC64
-   if (TRAP(regs) != 0x900 && TRAP(regs) != 0xf00 && TRAP(regs) != 0x260)
+   if (TRAP(regs) != INTERRUPT_DECREMENTER &&
+   TRAP(regs) != INTERRUPT_PERFMON)
this_cpu_set_ftrace_enabled(state->ftrace_enabled);
 
 #ifdef CONFIG_PPC_BOOK3S_64
diff --git a/arch/powerpc/kernel/fadump.c b/arch/powerpc/kernel/fadump.c
index eddf362caedc..b55b4c23f3b6 100644
--- a/arch/powerpc/kernel/fadump.c
+++ b/arch/powerpc/kernel/fadump.c
@@ -728,7 +728,7 @@ void crash_fadump(struct pt_regs *regs, const char *str)
 * If we came in via system reset, wait a while for the secondary
 * CPUs to enter.
 */
-   if (TRAP(&(fdh->regs)) == 0x100) {
+   if (TRAP(&(fdh->reg

Re: [PATCH v4] powerpc/traps: Enhance readability for trap types

2021-04-10 Thread Xiongwei Song


> On Apr 10, 2021, at 8:35 AM, Nicholas Piggin  wrote:
> 
> Thanks for working on this, I think it's a nice cleanup and helps
> non-powerpc people understand the code a bit better.
> 

My pleasure.

> Excerpts from Xiongwei Song's message of April 10, 2021 12:28 am:
>> From: Xiongwei Song 
>> 
>> Create a new header named traps.h, define macros to list ppc interrupt
>> types in traps.h, replace the references of the trap hex values with these
>> macros.
>> 
>> Referred the hex numbers in arch/powerpc/kernel/exceptions-64e.S,
>> arch/powerpc/kernel/exceptions-64s.S and
>> arch/powerpc/include/asm/kvm_asm.h.
>> 
>> Reported-by: kernel test robot 
> 
> It now looks like lkp asked for this whole cleanup patch. I would
> put [kernel test robot ] in your v3->4 changelog
> item.
> 

Agree. I just forgot to delete this line in the patch.

>> Signed-off-by: Xiongwei Song 
>> ---
>> 
>> v3-v4:
>> Fix compile issue:
>> arch/powerpc/kernel/process.c:1473:14: error: 'INTERRUPT_MACHINE_CHECK' 
>> undeclared (first use in this function); did you mean 'TAINT_MACHINE_CHECK'?
>> I didn't add "Reported-by: kernel test robot " here,
>> because it's improper for this patch.
> 
> [...]
> 
>> diff --git a/arch/powerpc/include/asm/traps.h 
>> b/arch/powerpc/include/asm/traps.h
>> new file mode 100644
>> index ..2e64e10afcef
>> --- /dev/null
>> +++ b/arch/powerpc/include/asm/traps.h
>> @@ -0,0 +1,32 @@
>> +/* SPDX-License-Identifier: GPL-2.0 */
>> +#ifndef _ASM_PPC_TRAPS_H
>> +#define _ASM_PPC_TRAPS_H
> 
> These could go in interrupt.h.
> 
>> +#if defined(CONFIG_BOOKE) || defined(CONFIG_4xx)
>> +#define INTERRUPT_MACHINE_CHECK   0x000
>> +#define INTERRUPT_CRITICAL_INPUT  0x100
>> +#define INTERRUPT_ALTIVEC_UNAVAIL 0x200
>> +#define INTERRUPT_PERFMON 0x260
>> +#define INTERRUPT_DOORBELL0x280
>> +#define INTERRUPT_DEBUG   0xd00
>> +#else
>> +#define INTERRUPT_SYSTEM_RESET0x100
>> +#define INTERRUPT_MACHINE_CHECK   0x200
> 
> [...]
> 
>> @@ -1469,7 +1470,9 @@ static void __show_regs(struct pt_regs *regs)
>>  trap = TRAP(regs);
>>  if (!trap_is_syscall(regs) && cpu_has_feature(CPU_FTR_CFAR))
>>  pr_cont("CFAR: "REG" ", regs->orig_gpr3);
>> -if (trap == 0x200 || trap == 0x300 || trap == 0x600) {
>> +if (trap == INTERRUPT_MACHINE_CHECK ||
>> +trap == INTERRUPT_DATA_STORAGE ||
>> +trap == INTERRUPT_ALIGNMENT) {
>>  if (IS_ENABLED(CONFIG_4xx) || IS_ENABLED(CONFIG_BOOKE))
>>  pr_cont("DEAR: "REG" ESR: "REG" ", regs->dar, 
>> regs->dsisr);
>>  else
> 
> This is now a change in behaviour because previously BOOKE/4xx tested
> 0x200, but now it tests 0.

Yes. Previously BOOKE/4xx tested 0x200, but checked this line history, please 
see
the commit below:

commit c54006491dde7d1b8050c5542716b751be92ed80
Author: Anton Blanchard 
Date:   Fri Nov 15 15:41:19 2013 +1100

powerpc: Print DAR and DSISR on machine check oopses

Machine check exceptions set DAR and DSISR, so print them in our
oops output.

Signed-off-by: Anton Blanchard 
Signed-off-by: Benjamin Herrenschmidt 

diff --git a/arch/powerpc/kernel/process.c b/arch/powerpc/kernel/process.c
index 75c2d1009985..37c4103a8cff 100644
--- a/arch/powerpc/kernel/process.c
+++ b/arch/powerpc/kernel/process.c
@@ -864,7 +864,7 @@ void show_regs(struct pt_regs * regs)
trap = TRAP(regs);
if ((regs->trap != 0xc00) && cpu_has_feature(CPU_FTR_CFAR))
printk("CFAR: "REG"\n", regs->orig_gpr3);
-   if (trap == 0x300 || trap == 0x600)
+   if (trap == 0x200 || trap == 0x300 || trap == 0x600)
 #if defined(CONFIG_4xx) || defined(CONFIG_BOOKE)
printk("DEAR: "REG", ESR: "REG"\n", regs->dar, regs->dsisr);
 #else

0x200 aims to test Machine check, but for 64e, the regs->trap should be 0x000 
here,
under the commit comments, I changed the code behavior. Sorry I didn’t add 
comments. 

> 
> That looks wrong for 4xx. 64e does put 0x000 there but I wonder if it 
> should use 0x200 instead.

Ok. Thanks for pointing this out, let me learn about 4xx. 

> Bit difficult to test this stuff, I do have
> some MCE injection patches for QEMU for 64s, might be able to look at
> porting them to 64e although I have no idea about booke machine checks.
> 

Yes, would appreciate your sharing.

> Anyway I don't think this patch should change generated code at all.
> Either change the code first with smaller patches, or make sure you
> keep the tests the same.

Agree.

Regards,
Xiongwei

> 
> Thanks,
> Nick



Re: [PATCH v3] powerpc/traps: Enhance readability for trap types

2021-04-10 Thread Xiongwei Song


> On Apr 10, 2021, at 8:04 AM, Michael Ellerman  wrote:
> 
> Christophe Leroy  writes:
>> Le 08/04/2021 à 16:07, Xiongwei Song a écrit :
>>> From: Xiongwei Song 
>>> 
>>> Create a new header named traps.h, define macros to list ppc interrupt
>>> types in traps.h, replace the reference of the trap hex values with these
>>> macros.
> ...
>>> diff --git a/arch/powerpc/include/asm/interrupt.h 
>>> b/arch/powerpc/include/asm/interrupt.h
>>> index 7c633896d758..5ce9898bc9a6 100644
>>> --- a/arch/powerpc/include/asm/interrupt.h
>>> +++ b/arch/powerpc/include/asm/interrupt.h
>>> @@ -8,6 +8,7 @@
>>>  #include 
>>>  #include 
>>>  #include 
>>> +#include 
>>> 
>>>  struct interrupt_state {
>>>  #ifdef CONFIG_PPC_BOOK3E_64
>>> @@ -59,7 +60,7 @@ static inline void interrupt_enter_prepare(struct pt_regs 
>>> *regs, struct interrup
>>>  * CT_WARN_ON comes here via program_check_exception,
>>>  * so avoid recursion.
>>>  */
>>> -   if (TRAP(regs) != 0x700)
>>> +   if (TRAP(regs) != INTERRUPT_PROGRAM)
>>> CT_WARN_ON(ct_state() != CONTEXT_KERNEL);
>>> }
>>>  #endif
>>> @@ -156,7 +157,8 @@ static inline void interrupt_nmi_enter_prepare(struct 
>>> pt_regs *regs, struct inte
>>> /* Don't do any per-CPU operations until interrupt state is fixed */
>>>  #endif
>>> /* Allow DEC and PMI to be traced when they are soft-NMI */
>>> -   if (TRAP(regs) != 0x900 && TRAP(regs) != 0xf00 && TRAP(regs) != 0x260) {
>>> +   if (TRAP(regs) != INTERRUPT_DECREMENTER &&
>>> +   TRAP(regs) != INTERRUPT_PERFMON) {
>> 
>> I think too long names hinder readability, see later for suggestions.
> 
> I asked for the longer names :)
> 
> I think they make it easier for people who are less familiar with the
> architecture than us to make sense of the names.
> 
> And there's only a couple of cases where it requires splitting a line,
> and they could be converted to use switch if we think it's a problem.
> 
>>> diff --git a/arch/powerpc/mm/fault.c b/arch/powerpc/mm/fault.c
>>> index 0c0b1c2cfb49..641b3feef7ee 100644
>>> --- a/arch/powerpc/mm/fault.c
>>> +++ b/arch/powerpc/mm/fault.c
>>> @@ -588,20 +589,24 @@ void __bad_page_fault(struct pt_regs *regs, int sig)
>>> /* kernel has accessed a bad area */
>>> 
>>> switch (TRAP(regs)) {
>>> -   case 0x300:
>>> -   case 0x380:
>>> -   case 0xe00:
>>> +   case INTERRUPT_DATA_STORAGE:
>>> +#ifdef CONFIG_PPC_BOOK3S
>>> +   case INTERRUPT_DATA_SEGMENT:
>>> +   case INTERRUPT_H_DATA_STORAGE:
>>> +#endif
>> 
>> It would be better to avoid #ifdefs when none where necessary before.
> 
> Yes I agree.
> 
> I think these can all be avoided by defining most of the values
> regardless of what platform we're building for. Only the values that
> overlap need to be kept behind an ifdef.

Ok. 

Regards,
Xiongwei

> 
> cheers



Re: [PATCH v3] powerpc/traps: Enhance readability for trap types

2021-04-10 Thread Xiongwei Song


> On Apr 10, 2021, at 12:14 AM, Christophe Leroy  
> wrote:
> 
> 
> 
> Le 08/04/2021 à 16:07, Xiongwei Song a écrit :
>> From: Xiongwei Song 
>> Create a new header named traps.h, define macros to list ppc interrupt
>> types in traps.h, replace the reference of the trap hex values with these
>> macros.
>> Referred the hex number in arch/powerpc/kernel/exceptions-64e.S,
>> arch/powerpc/kernel/exceptions-64s.S and
>> arch/powerpc/include/asm/kvm_asm.h.
>> v2-v3:
>> Correct the prefix of trap macros with INTERRUPT_, the previous prefix
>> is TRAP_, which is not precise. This is suggested by Segher Boessenkool
>> and Nicholas Piggin.
>> v1-v2:
>> Define more trap macros to replace more trap hexs in code, not just for
>> the __show_regs function. This is suggested by Christophe Leroy.
>> Signed-off-by: Xiongwei Song 
>> ---
>>  arch/powerpc/include/asm/interrupt.h  |  9 +---
>>  arch/powerpc/include/asm/ptrace.h |  3 ++-
>>  arch/powerpc/include/asm/traps.h  | 32 +++
>>  arch/powerpc/kernel/interrupt.c   |  3 ++-
>>  arch/powerpc/kernel/process.c |  5 -
>>  arch/powerpc/mm/book3s64/hash_utils.c |  5 +++--
>>  arch/powerpc/mm/fault.c   | 21 +++---
>>  arch/powerpc/perf/core-book3s.c   |  5 +++--
>>  arch/powerpc/xmon/xmon.c  | 16 +++---
>>  9 files changed, 78 insertions(+), 21 deletions(-)
>>  create mode 100644 arch/powerpc/include/asm/traps.h
>> diff --git a/arch/powerpc/include/asm/interrupt.h 
>> b/arch/powerpc/include/asm/interrupt.h
>> index 7c633896d758..5ce9898bc9a6 100644
>> --- a/arch/powerpc/include/asm/interrupt.h
>> +++ b/arch/powerpc/include/asm/interrupt.h
>> @@ -8,6 +8,7 @@
>>  #include 
>>  #include 
>>  #include 
>> +#include 
>>struct interrupt_state {
>>  #ifdef CONFIG_PPC_BOOK3E_64
>> @@ -59,7 +60,7 @@ static inline void interrupt_enter_prepare(struct pt_regs 
>> *regs, struct interrup
>>   * CT_WARN_ON comes here via program_check_exception,
>>   * so avoid recursion.
>>   */
>> -if (TRAP(regs) != 0x700)
>> +if (TRAP(regs) != INTERRUPT_PROGRAM)
>>  CT_WARN_ON(ct_state() != CONTEXT_KERNEL);
>>  }
>>  #endif
>> @@ -156,7 +157,8 @@ static inline void interrupt_nmi_enter_prepare(struct 
>> pt_regs *regs, struct inte
>>  /* Don't do any per-CPU operations until interrupt state is fixed */
>>  #endif
>>  /* Allow DEC and PMI to be traced when they are soft-NMI */
>> -if (TRAP(regs) != 0x900 && TRAP(regs) != 0xf00 && TRAP(regs) != 0x260) {
>> +if (TRAP(regs) != INTERRUPT_DECREMENTER &&
>> +TRAP(regs) != INTERRUPT_PERFMON) {
> 
> I think too long names hinder readability, see later for suggestions.
> 
>>  state->ftrace_enabled = this_cpu_get_ftrace_enabled();
>>  this_cpu_set_ftrace_enabled(0);
>>  }
>> @@ -180,7 +182,8 @@ static inline void interrupt_nmi_exit_prepare(struct 
>> pt_regs *regs, struct inter
>>  nmi_exit();
>>#ifdef CONFIG_PPC64
>> -if (TRAP(regs) != 0x900 && TRAP(regs) != 0xf00 && TRAP(regs) != 0x260)
>> +if (TRAP(regs) != INTERRUPT_DECREMENTER &&
>> +TRAP(regs) != INTERRUPT_PERFMON)
>>  this_cpu_set_ftrace_enabled(state->ftrace_enabled);
>>#ifdef CONFIG_PPC_BOOK3S_64
>> diff --git a/arch/powerpc/include/asm/ptrace.h 
>> b/arch/powerpc/include/asm/ptrace.h
>> index f10498e1b3f6..7a17e0365d43 100644
>> --- a/arch/powerpc/include/asm/ptrace.h
>> +++ b/arch/powerpc/include/asm/ptrace.h
>> @@ -21,6 +21,7 @@
>>#include 
>>  #include 
>> +#include 
>>#ifndef __ASSEMBLY__
>>  struct pt_regs
>> @@ -237,7 +238,7 @@ static inline bool trap_is_unsupported_scv(struct 
>> pt_regs *regs)
>>static inline bool trap_is_syscall(struct pt_regs *regs)
>>  {
>> -return (trap_is_scv(regs) || TRAP(regs) == 0xc00);
>> +return (trap_is_scv(regs) || TRAP(regs) == INTERRUPT_SYSCALL);
>>  }
>>static inline bool trap_norestart(struct pt_regs *regs)
>> diff --git a/arch/powerpc/include/asm/traps.h 
>> b/arch/powerpc/include/asm/traps.h
>> new file mode 100644
>> index ..cb416a17097c
>> --- /dev/null
>> +++ b/arch/powerpc/include/asm/traps.h
>> @@ -0,0 +1,32 @@
>> +/* SPDX-License-Identifier: GPL-2.0 */
>> +#ifndef _ASM_PPC

[PATCH v4] powerpc/traps: Enhance readability for trap types

2021-04-09 Thread Xiongwei Song
From: Xiongwei Song 

Create a new header named traps.h, define macros to list ppc interrupt
types in traps.h, replace the references of the trap hex values with these
macros.

Referred the hex numbers in arch/powerpc/kernel/exceptions-64e.S,
arch/powerpc/kernel/exceptions-64s.S and
arch/powerpc/include/asm/kvm_asm.h.

Reported-by: kernel test robot 
Signed-off-by: Xiongwei Song 
---

v3-v4:
Fix compile issue:
arch/powerpc/kernel/process.c:1473:14: error: 'INTERRUPT_MACHINE_CHECK' 
undeclared (first use in this function); did you mean 'TAINT_MACHINE_CHECK'?
I didn't add "Reported-by: kernel test robot " here,
because it's improper for this patch.

v2-v3:
Correct the prefix of trap macros with INTERRUPT_, the previous prefix
is TRAP_, which is not precise. This is suggested by Segher Boessenkool
and Nicholas Piggin.

v1-v2:
Define more trap macros to replace more trap hexs in code, not just for
the __show_regs function. This is suggested by Christophe Leroy.

---
 arch/powerpc/include/asm/interrupt.h  |  9 +---
 arch/powerpc/include/asm/ptrace.h |  3 ++-
 arch/powerpc/include/asm/traps.h  | 32 +++
 arch/powerpc/kernel/interrupt.c   |  3 ++-
 arch/powerpc/kernel/process.c |  5 -
 arch/powerpc/mm/book3s64/hash_utils.c |  5 +++--
 arch/powerpc/mm/fault.c   | 21 +++---
 arch/powerpc/perf/core-book3s.c   |  5 +++--
 arch/powerpc/xmon/xmon.c  | 16 +++---
 9 files changed, 78 insertions(+), 21 deletions(-)
 create mode 100644 arch/powerpc/include/asm/traps.h

diff --git a/arch/powerpc/include/asm/interrupt.h 
b/arch/powerpc/include/asm/interrupt.h
index 05e7fc4ffb50..4fd904fb5d59 100644
--- a/arch/powerpc/include/asm/interrupt.h
+++ b/arch/powerpc/include/asm/interrupt.h
@@ -8,6 +8,7 @@
 #include 
 #include 
 #include 
+#include 
 
 static inline void nap_adjust_return(struct pt_regs *regs)
 {
@@ -70,7 +71,7 @@ static inline void interrupt_enter_prepare(struct pt_regs 
*regs, struct interrup
 * CT_WARN_ON comes here via program_check_exception,
 * so avoid recursion.
 */
-   if (TRAP(regs) != 0x700)
+   if (TRAP(regs) != INTERRUPT_PROGRAM)
CT_WARN_ON(ct_state() != CONTEXT_KERNEL);
}
 #endif
@@ -175,7 +176,8 @@ static inline void interrupt_nmi_enter_prepare(struct 
pt_regs *regs, struct inte
/* Don't do any per-CPU operations until interrupt state is fixed */
 #endif
/* Allow DEC and PMI to be traced when they are soft-NMI */
-   if (TRAP(regs) != 0x900 && TRAP(regs) != 0xf00 && TRAP(regs) != 0x260) {
+   if (TRAP(regs) != INTERRUPT_DECREMENTER &&
+   TRAP(regs) != INTERRUPT_PERFMON) {
state->ftrace_enabled = this_cpu_get_ftrace_enabled();
this_cpu_set_ftrace_enabled(0);
}
@@ -204,7 +206,8 @@ static inline void interrupt_nmi_exit_prepare(struct 
pt_regs *regs, struct inter
 */
 
 #ifdef CONFIG_PPC64
-   if (TRAP(regs) != 0x900 && TRAP(regs) != 0xf00 && TRAP(regs) != 0x260)
+   if (TRAP(regs) != INTERRUPT_DECREMENTER &&
+   TRAP(regs) != INTERRUPT_PERFMON)
this_cpu_set_ftrace_enabled(state->ftrace_enabled);
 
 #ifdef CONFIG_PPC_BOOK3S_64
diff --git a/arch/powerpc/include/asm/ptrace.h 
b/arch/powerpc/include/asm/ptrace.h
index 95600f3a6523..07ff8629e776 100644
--- a/arch/powerpc/include/asm/ptrace.h
+++ b/arch/powerpc/include/asm/ptrace.h
@@ -21,6 +21,7 @@
 
 #include 
 #include 
+#include 
 
 #ifndef __ASSEMBLY__
 struct pt_regs
@@ -237,7 +238,7 @@ static inline bool trap_is_unsupported_scv(struct pt_regs 
*regs)
 
 static inline bool trap_is_syscall(struct pt_regs *regs)
 {
-   return (trap_is_scv(regs) || TRAP(regs) == 0xc00);
+   return (trap_is_scv(regs) || TRAP(regs) == INTERRUPT_SYSCALL);
 }
 
 static inline bool trap_norestart(struct pt_regs *regs)
diff --git a/arch/powerpc/include/asm/traps.h b/arch/powerpc/include/asm/traps.h
new file mode 100644
index ..2e64e10afcef
--- /dev/null
+++ b/arch/powerpc/include/asm/traps.h
@@ -0,0 +1,32 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef _ASM_PPC_TRAPS_H
+#define _ASM_PPC_TRAPS_H
+
+#if defined(CONFIG_BOOKE) || defined(CONFIG_4xx)
+#define INTERRUPT_MACHINE_CHECK   0x000
+#define INTERRUPT_CRITICAL_INPUT  0x100
+#define INTERRUPT_ALTIVEC_UNAVAIL 0x200
+#define INTERRUPT_PERFMON 0x260
+#define INTERRUPT_DOORBELL0x280
+#define INTERRUPT_DEBUG   0xd00
+#else
+#define INTERRUPT_SYSTEM_RESET0x100
+#define INTERRUPT_MACHINE_CHECK   0x200
+#define INTERRUPT_DATA_SEGMENT0x380
+#define INTERRUPT_INST_SEGMENT0x480
+#define INTERRUPT_DOORBELL0xa00
+#define INTERRUPT_TRACE   0xd00
+#define INTERRUPT_H_DATA_STORAGE  0xe00
+#define INTERRUPT_PERFMON 0xf00
+#define INTERRUPT_H_FAC_UNAVAIL 

[PATCH v3] powerpc/traps: Enhance readability for trap types

2021-04-08 Thread Xiongwei Song
From: Xiongwei Song 

Create a new header named traps.h, define macros to list ppc interrupt
types in traps.h, replace the reference of the trap hex values with these
macros.

Referred the hex number in arch/powerpc/kernel/exceptions-64e.S,
arch/powerpc/kernel/exceptions-64s.S and
arch/powerpc/include/asm/kvm_asm.h.

v2-v3:
Correct the prefix of trap macros with INTERRUPT_, the previous prefix
is TRAP_, which is not precise. This is suggested by Segher Boessenkool
and Nicholas Piggin.

v1-v2:
Define more trap macros to replace more trap hexs in code, not just for
the __show_regs function. This is suggested by Christophe Leroy.

Signed-off-by: Xiongwei Song 
---
 arch/powerpc/include/asm/interrupt.h  |  9 +---
 arch/powerpc/include/asm/ptrace.h |  3 ++-
 arch/powerpc/include/asm/traps.h  | 32 +++
 arch/powerpc/kernel/interrupt.c   |  3 ++-
 arch/powerpc/kernel/process.c |  5 -
 arch/powerpc/mm/book3s64/hash_utils.c |  5 +++--
 arch/powerpc/mm/fault.c   | 21 +++---
 arch/powerpc/perf/core-book3s.c   |  5 +++--
 arch/powerpc/xmon/xmon.c  | 16 +++---
 9 files changed, 78 insertions(+), 21 deletions(-)
 create mode 100644 arch/powerpc/include/asm/traps.h

diff --git a/arch/powerpc/include/asm/interrupt.h 
b/arch/powerpc/include/asm/interrupt.h
index 7c633896d758..5ce9898bc9a6 100644
--- a/arch/powerpc/include/asm/interrupt.h
+++ b/arch/powerpc/include/asm/interrupt.h
@@ -8,6 +8,7 @@
 #include 
 #include 
 #include 
+#include 
 
 struct interrupt_state {
 #ifdef CONFIG_PPC_BOOK3E_64
@@ -59,7 +60,7 @@ static inline void interrupt_enter_prepare(struct pt_regs 
*regs, struct interrup
 * CT_WARN_ON comes here via program_check_exception,
 * so avoid recursion.
 */
-   if (TRAP(regs) != 0x700)
+   if (TRAP(regs) != INTERRUPT_PROGRAM)
CT_WARN_ON(ct_state() != CONTEXT_KERNEL);
}
 #endif
@@ -156,7 +157,8 @@ static inline void interrupt_nmi_enter_prepare(struct 
pt_regs *regs, struct inte
/* Don't do any per-CPU operations until interrupt state is fixed */
 #endif
/* Allow DEC and PMI to be traced when they are soft-NMI */
-   if (TRAP(regs) != 0x900 && TRAP(regs) != 0xf00 && TRAP(regs) != 0x260) {
+   if (TRAP(regs) != INTERRUPT_DECREMENTER &&
+   TRAP(regs) != INTERRUPT_PERFMON) {
state->ftrace_enabled = this_cpu_get_ftrace_enabled();
this_cpu_set_ftrace_enabled(0);
}
@@ -180,7 +182,8 @@ static inline void interrupt_nmi_exit_prepare(struct 
pt_regs *regs, struct inter
nmi_exit();
 
 #ifdef CONFIG_PPC64
-   if (TRAP(regs) != 0x900 && TRAP(regs) != 0xf00 && TRAP(regs) != 0x260)
+   if (TRAP(regs) != INTERRUPT_DECREMENTER &&
+   TRAP(regs) != INTERRUPT_PERFMON)
this_cpu_set_ftrace_enabled(state->ftrace_enabled);
 
 #ifdef CONFIG_PPC_BOOK3S_64
diff --git a/arch/powerpc/include/asm/ptrace.h 
b/arch/powerpc/include/asm/ptrace.h
index f10498e1b3f6..7a17e0365d43 100644
--- a/arch/powerpc/include/asm/ptrace.h
+++ b/arch/powerpc/include/asm/ptrace.h
@@ -21,6 +21,7 @@
 
 #include 
 #include 
+#include 
 
 #ifndef __ASSEMBLY__
 struct pt_regs
@@ -237,7 +238,7 @@ static inline bool trap_is_unsupported_scv(struct pt_regs 
*regs)
 
 static inline bool trap_is_syscall(struct pt_regs *regs)
 {
-   return (trap_is_scv(regs) || TRAP(regs) == 0xc00);
+   return (trap_is_scv(regs) || TRAP(regs) == INTERRUPT_SYSCALL);
 }
 
 static inline bool trap_norestart(struct pt_regs *regs)
diff --git a/arch/powerpc/include/asm/traps.h b/arch/powerpc/include/asm/traps.h
new file mode 100644
index ..cb416a17097c
--- /dev/null
+++ b/arch/powerpc/include/asm/traps.h
@@ -0,0 +1,32 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef _ASM_PPC_TRAPS_H
+#define _ASM_PPC_TRAPS_H
+
+#if defined(CONFIG_BOOKE) || defined(CONFIG_4xx)
+#define INTERRUPT_MACHINE_CHECK   0x000
+#define INTERRUPT_CRITICAL_INPUT  0x100
+#define INTERRUPT_ALTIVEC_UNAVAIL 0x200
+#define INTERRUPT_PERFMON 0x260
+#define INTERRUPT_DOORBELL0x280
+#define INTERRUPT_DEBUG   0xd00
+#elif defined(CONFIG_PPC_BOOK3S)
+#define INTERRUPT_SYSTEM_RESET0x100
+#define INTERRUPT_MACHINE_CHECK   0x200
+#define INTERRUPT_DATA_SEGMENT0x380
+#define INTERRUPT_INST_SEGMENT0x480
+#define INTERRUPT_DOORBELL0xa00
+#define INTERRUPT_TRACE   0xd00
+#define INTERRUPT_H_DATA_STORAGE  0xe00
+#define INTERRUPT_PERFMON 0xf00
+#define INTERRUPT_H_FAC_UNAVAIL   0xf80
+#endif
+
+#define INTERRUPT_DATA_STORAGE0x300
+#define INTERRUPT_INST_STORAGE0x400
+#define INTERRUPT_ALIGNMENT   0x600
+#define INTERRUPT_PROGRAM 0x700
+#define INTERRUPT_FP_UNAVAIL  0x800
+#define INTERRUPT_DECREMENTER 0x900
+#define INTERRUPT_SYSCALL 0xc00
+
+#en

Re: [PATCH v2] powerpc/traps: Enhance readability for trap types

2021-04-05 Thread Xiongwei Song


Regards,
Xiongwei




> On Apr 2, 2021, at 8:36 AM, Nicholas Piggin  wrote:
> 
> Excerpts from Segher Boessenkool's message of April 2, 2021 2:11 am:
>> On Thu, Apr 01, 2021 at 10:55:58AM +0800, Xiongwei Song wrote:
>>> Segher Boessenkool  于2021年4月1日周四 上午6:15写道:
>>> 
>>>> On Wed, Mar 31, 2021 at 08:58:17PM +1100, Michael Ellerman wrote:
>>>>> So perhaps:
>>>>> 
>>>>>  EXC_SYSTEM_RESET
>>>>>  EXC_MACHINE_CHECK
>>>>>  EXC_DATA_STORAGE
>>>>>  EXC_DATA_SEGMENT
>>>>>  EXC_INST_STORAGE
>>>>>  EXC_INST_SEGMENT
>>>>>  EXC_EXTERNAL_INTERRUPT
>>>>>  EXC_ALIGNMENT
>>>>>  EXC_PROGRAM_CHECK
>>>>>  EXC_FP_UNAVAILABLE
>>>>>  EXC_DECREMENTER
>>>>>  EXC_HV_DECREMENTER
>>>>>  EXC_SYSTEM_CALL
>>>>>  EXC_HV_DATA_STORAGE
>>>>>  EXC_PERF_MONITOR
>>>> 
>>>> These are interrupt (vectors), not exceptions.  It doesn't matter all
>>>> that much, but confusing things more isn't useful either!  There can be
>>>> multiple exceptions that all can trigger the same interrupt.
>>>> 
>>>> When looking at the reference manual of e500 and e600 from NXP
>>> official, they call them as interrupts.While looking at the "The
>>> Programming Environments"
>>> that is also from NXP, they call them exceptions. Looks like there is
>>> no explicit distinction between interrupts and exceptions.
>> 
>> The architecture documents have always called it interrupts.  The PEM
>> says it calls them exceptions instead, but they are called interrupts in
>> the architecture (and the PEM says that, too).
>> 
>>> Here is the "The Programming Environments" link:
>>> https://www.nxp.com.cn/docs/en/user-guide/MPCFPE_AD_R1.pdf
>> 
>> That document is 24 years old.  The architecture is still published,
>> new versions regularly.
>> 
>>> As far as I know, the values of interrupts or exceptions above are defined
>>> explicitly in reference manual or the programming environments.
>> 
>> They are defined in the architecture.
>> 
>>> Could
>>> you please provide more details about multiple exceptions with the same
>>> interrupts?
>> 
>> The simplest example is 700, program interrupt.  There are many causes
>> for it, including all the exceptions in FPSCR: VX, ZX, OX, UX, XX, and
>> VX is actually divided into nine separate cases itself.  There also are
>> the various causes of privileged instruction type program interrupts,
>> and  the trap type program interrupt, but the FEX ones are most obvious
>> here.
> 
> Also:
> 
> * Some interrupts have no corresponding exception (system call and 
> system call vectored). This is not just semantics or a bug in the ISA
> because it is different from other synchronous interrupts: instructions 
> which cause exceptions (e.g., a page fault) do not complete before 
> taking the interrupt whereas sc does.
> 
> * It's quite usual for an exception to not cause an interrupt 
> immediately (MSR[EE]=0, HMEER) or never cause one and be cleared by 
> other means (msgclr, mtDEC, mtHMER, etc).
> 
> * It's possible for an exception to cause different interrupts!
> A decrementer exception usually causes a decrementer interrupt, but it
> can cause a system reset interrupt if the processor was in a power
> saving mode. A data storage exception can cause a DSI or HDSI interrupt
> depending on LPCR settings, and many other examples.
> 
> So I agree with Segher on this. We should use interrupt for interrupts, 
> reduce exception except where we really mean it, and move away from vec 
> and trap (I've got this wrong in the past too I admit). We don't have to 
> do it all immediately, but new code should go in this direction.

Appreciate these details about exceptions and interrupts. Looks like interrupt
is the correct term here. I’m glad to create the v3 patch with INTERRUPT_
prefix.  

Regards,
Xiongwei

> 
> Thanks,
> Nick



Re: [PATCH v2] powerpc/traps: Enhance readability for trap types

2021-04-05 Thread Xiongwei Song


Regards,
Xiongwei




> On Apr 1, 2021, at 4:01 PM, Nicholas Piggin  wrote:
> 
> Excerpts from Michael Ellerman's message of April 1, 2021 12:39 pm:
>> Segher Boessenkool  writes:
>>> On Wed, Mar 31, 2021 at 08:58:17PM +1100, Michael Ellerman wrote:
 So perhaps:
 
  EXC_SYSTEM_RESET
  EXC_MACHINE_CHECK
  EXC_DATA_STORAGE
  EXC_DATA_SEGMENT
  EXC_INST_STORAGE
  EXC_INST_SEGMENT
  EXC_EXTERNAL_INTERRUPT
  EXC_ALIGNMENT
  EXC_PROGRAM_CHECK
  EXC_FP_UNAVAILABLE
  EXC_DECREMENTER
  EXC_HV_DECREMENTER
  EXC_SYSTEM_CALL
  EXC_HV_DATA_STORAGE
  EXC_PERF_MONITOR
>>> 
>>> These are interrupt (vectors), not exceptions.  It doesn't matter all
>>> that much, but confusing things more isn't useful either!  There can be
>>> multiple exceptions that all can trigger the same interrupt.
>> 
>> Yeah I know, but I think that ship has already sailed as far as the
>> naming we have in the kernel.
> 
> It has, but there are also several other ships also sailing in different 
> directions. It could be worse though, at least they are not sideways in 
> the Suez.
> 
>> We have over 250 uses of "exc", and several files called "exception"
>> something.
>> 
>> Using "interrupt" can also be confusing because Linux uses that to mean
>> "external interrupt".
>> 
>> But I dunno, maybe INT or VEC is clearer? .. or TRAP :)
> 
> We actually already have defines that follow Segher's suggestion, it's 
> just that they're hidden away in a KVM header.
> 
> #define BOOK3S_INTERRUPT_SYSTEM_RESET   0x100
> #define BOOK3S_INTERRUPT_MACHINE_CHECK  0x200
> #define BOOK3S_INTERRUPT_DATA_STORAGE   0x300
> #define BOOK3S_INTERRUPT_DATA_SEGMENT   0x380
> #define BOOK3S_INTERRUPT_INST_STORAGE   0x400
> #define BOOK3S_INTERRUPT_INST_SEGMENT   0x480
> #define BOOK3S_INTERRUPT_EXTERNAL   0x500
> #define BOOK3S_INTERRUPT_EXTERNAL_HV0x502
> #define BOOK3S_INTERRUPT_ALIGNMENT  0x600
> 
> It would take just a small amount of work to move these to general 
> powerpc header, add #ifdefs for Book E/S where the numbers differ,
> and remove the BOOK3S_ prefix.
> 
> I don't mind INTERRUPT_ but INT_ would be okay too. VEC_ actually
> doesn't match what Book E does (which is some weirdness to map some
> of them to match Book S but not all, arguably we should clean that
> up too and just use vector numbers consistently, but the INTERRUPT_
> prefix would still be valid if we did that).
> 
> BookE KVM entry will still continue to use a different convention
> there so I would leave all those KVM defines in place for now, we
> might do another pass on them later.

Thanks for the comments. 

> 
> Thanks,
> Nick



Re: [PATCH v2] powerpc/traps: Enhance readability for trap types

2021-04-05 Thread Xiongwei Song


> On Apr 2, 2021, at 12:11 AM, Segher Boessenkool  
> wrote:
> 
> On Thu, Apr 01, 2021 at 10:55:58AM +0800, Xiongwei Song wrote:
>> Segher Boessenkool  于2021年4月1日周四 上午6:15写道:
>> 
>>> On Wed, Mar 31, 2021 at 08:58:17PM +1100, Michael Ellerman wrote:
>>>> So perhaps:
>>>> 
>>>>  EXC_SYSTEM_RESET
>>>>  EXC_MACHINE_CHECK
>>>>  EXC_DATA_STORAGE
>>>>  EXC_DATA_SEGMENT
>>>>  EXC_INST_STORAGE
>>>>  EXC_INST_SEGMENT
>>>>  EXC_EXTERNAL_INTERRUPT
>>>>  EXC_ALIGNMENT
>>>>  EXC_PROGRAM_CHECK
>>>>  EXC_FP_UNAVAILABLE
>>>>  EXC_DECREMENTER
>>>>  EXC_HV_DECREMENTER
>>>>  EXC_SYSTEM_CALL
>>>>  EXC_HV_DATA_STORAGE
>>>>  EXC_PERF_MONITOR
>>> 
>>> These are interrupt (vectors), not exceptions.  It doesn't matter all
>>> that much, but confusing things more isn't useful either!  There can be
>>> multiple exceptions that all can trigger the same interrupt.
>>> 
>>> When looking at the reference manual of e500 and e600 from NXP
>> official, they call them as interrupts.While looking at the "The
>> Programming Environments"
>> that is also from NXP, they call them exceptions. Looks like there is
>> no explicit distinction between interrupts and exceptions.
> 
> The architecture documents have always called it interrupts.  The PEM
> says it calls them exceptions instead, but they are called interrupts in
> the architecture (and the PEM says that, too).
> 
>> Here is the "The Programming Environments" link:
>> https://www.nxp.com.cn/docs/en/user-guide/MPCFPE_AD_R1.pdf
> 
> That document is 24 years old.  The architecture is still published,
> new versions regularly.
> 
>> As far as I know, the values of interrupts or exceptions above are defined
>> explicitly in reference manual or the programming environments.
> 
> They are defined in the architecture.
> 
>> Could
>> you please provide more details about multiple exceptions with the same
>> interrupts?
> 
> The simplest example is 700, program interrupt.  There are many causes
> for it, including all the exceptions in FPSCR: VX, ZX, OX, UX, XX, and
> VX is actually divided into nine separate cases itself.  There also are
> the various causes of privileged instruction type program interrupts,
> and  the trap type program interrupt, but the FEX ones are most obvious
> here.

Thanks for the explanation.

Regards,
Xiongwei

> 
> 
> Segher



[PATCH v2] powerpc/traps: Enhance readability for trap types

2021-03-30 Thread Xiongwei Song
From: Xiongwei Song 

Create a new header named traps.h, define macros to list ppc exception
types in traps.h, replace the reference of the real trap values with
these macros.

Signed-off-by: Xiongwei Song 
---
 arch/powerpc/include/asm/interrupt.h  |  7 ---
 arch/powerpc/include/asm/ptrace.h |  3 ++-
 arch/powerpc/include/asm/traps.h  | 19 +++
 arch/powerpc/kernel/interrupt.c   |  2 +-
 arch/powerpc/kernel/process.c |  3 ++-
 arch/powerpc/kernel/traps.c   |  5 +++--
 arch/powerpc/kexec/crash.c|  3 ++-
 arch/powerpc/kvm/book3s_hv_builtin.c  |  5 +++--
 arch/powerpc/mm/book3s64/hash_utils.c |  5 +++--
 arch/powerpc/mm/fault.c   | 17 +
 arch/powerpc/perf/core-book3s.c   |  5 +++--
 arch/powerpc/xmon/xmon.c  | 21 +++--
 12 files changed, 62 insertions(+), 33 deletions(-)
 create mode 100644 arch/powerpc/include/asm/traps.h

diff --git a/arch/powerpc/include/asm/interrupt.h 
b/arch/powerpc/include/asm/interrupt.h
index 7c633896d758..d4c935ba8e16 100644
--- a/arch/powerpc/include/asm/interrupt.h
+++ b/arch/powerpc/include/asm/interrupt.h
@@ -8,6 +8,7 @@
 #include 
 #include 
 #include 
+#include 
 
 struct interrupt_state {
 #ifdef CONFIG_PPC_BOOK3E_64
@@ -59,7 +60,7 @@ static inline void interrupt_enter_prepare(struct pt_regs 
*regs, struct interrup
 * CT_WARN_ON comes here via program_check_exception,
 * so avoid recursion.
 */
-   if (TRAP(regs) != 0x700)
+   if (TRAP(regs) != TRAP_PROG)
CT_WARN_ON(ct_state() != CONTEXT_KERNEL);
}
 #endif
@@ -156,7 +157,7 @@ static inline void interrupt_nmi_enter_prepare(struct 
pt_regs *regs, struct inte
/* Don't do any per-CPU operations until interrupt state is fixed */
 #endif
/* Allow DEC and PMI to be traced when they are soft-NMI */
-   if (TRAP(regs) != 0x900 && TRAP(regs) != 0xf00 && TRAP(regs) != 0x260) {
+   if (TRAP(regs) != TRAP_DEC && TRAP(regs) != TRAP_PMI && TRAP(regs) != 
0x260) {
state->ftrace_enabled = this_cpu_get_ftrace_enabled();
this_cpu_set_ftrace_enabled(0);
}
@@ -180,7 +181,7 @@ static inline void interrupt_nmi_exit_prepare(struct 
pt_regs *regs, struct inter
nmi_exit();
 
 #ifdef CONFIG_PPC64
-   if (TRAP(regs) != 0x900 && TRAP(regs) != 0xf00 && TRAP(regs) != 0x260)
+   if (TRAP(regs) != TRAP_DEC && TRAP(regs) != TRAP_PMI && TRAP(regs) != 
0x260)
this_cpu_set_ftrace_enabled(state->ftrace_enabled);
 
 #ifdef CONFIG_PPC_BOOK3S_64
diff --git a/arch/powerpc/include/asm/ptrace.h 
b/arch/powerpc/include/asm/ptrace.h
index f10498e1b3f6..04726fb43a9d 100644
--- a/arch/powerpc/include/asm/ptrace.h
+++ b/arch/powerpc/include/asm/ptrace.h
@@ -21,6 +21,7 @@
 
 #include 
 #include 
+#include 
 
 #ifndef __ASSEMBLY__
 struct pt_regs
@@ -237,7 +238,7 @@ static inline bool trap_is_unsupported_scv(struct pt_regs 
*regs)
 
 static inline bool trap_is_syscall(struct pt_regs *regs)
 {
-   return (trap_is_scv(regs) || TRAP(regs) == 0xc00);
+   return (trap_is_scv(regs) || TRAP(regs) == TRAP_SYSCALL);
 }
 
 static inline bool trap_norestart(struct pt_regs *regs)
diff --git a/arch/powerpc/include/asm/traps.h b/arch/powerpc/include/asm/traps.h
new file mode 100644
index ..a31b6122de23
--- /dev/null
+++ b/arch/powerpc/include/asm/traps.h
@@ -0,0 +1,19 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef _ASM_PPC_TRAPS_H
+#define _ASM_PPC_TRAPS_H
+
+#define TRAP_RESET   0x100 /* System reset */
+#define TRAP_MCE 0x200 /* Machine check */
+#define TRAP_DSI 0x300 /* Data storage */
+#define TRAP_DSEGI   0x380 /* Data segment */
+#define TRAP_ISI 0x400 /* Instruction storage */
+#define TRAP_ISEGI   0x480 /* Instruction segment */
+#define TRAP_ALIGN   0x600 /* Alignment */
+#define TRAP_PROG0x700 /* Program */
+#define TRAP_DEC 0x900 /* Decrementer */
+#define TRAP_SYSCALL 0xc00 /* System call */
+#define TRAP_TRACEI  0xd00 /* Trace */
+#define TRAP_FPA 0xe00 /* Floating-point Assist */
+#define TRAP_PMI 0xf00 /* Performance monitor */
+
+#endif /* _ASM_PPC_TRAPS_H */
diff --git a/arch/powerpc/kernel/interrupt.c b/arch/powerpc/kernel/interrupt.c
index c4dd4b8f9cfa..fc9a40cbbcae 100644
--- a/arch/powerpc/kernel/interrupt.c
+++ b/arch/powerpc/kernel/interrupt.c
@@ -456,7 +456,7 @@ notrace unsigned long interrupt_exit_kernel_prepare(struct 
pt_regs *regs, unsign
 * CT_WARN_ON comes here via program_check_exception,
 * so avoid recursion.
 */
-   if (TRAP(regs) != 0x700)
+   if (TRAP(regs) != TRAP_PROG)
CT_WARN_ON(ct_state() == CONTEXT_USER);
 
kuap = kuap_get_and_assert_locked();
diff --git a/arch/powerpc/kernel/process.c b/arch/powerpc/kernel/process.c
index b966c8e0cead..0d4167

Re: [PATCH] powerpc/process: Enhance readability for trap types.

2021-03-28 Thread Xiongwei Song


> On Mar 28, 2021, at 11:21 PM, Christophe Leroy  
> wrote:
> 
> 
> 
> Le 28/03/2021 à 16:35, Xiongwei Song a écrit :
>> From: Xiongwei Song 
>> Define macros to enhance the code readability on ppc trap types.
> 
> Good idea
> 
>> Signed-off-by: Xiongwei Song 
>> ---
>>  arch/powerpc/kernel/process.c | 6 +-
>>  1 file changed, 5 insertions(+), 1 deletion(-)
>> diff --git a/arch/powerpc/kernel/process.c b/arch/powerpc/kernel/process.c
>> index 3231c2df9e26..3bbd3cf353a7 100644
>> --- a/arch/powerpc/kernel/process.c
>> +++ b/arch/powerpc/kernel/process.c
>> @@ -1451,6 +1451,10 @@ static void print_msr_bits(unsigned long val)
>>  #define LAST_VOLATILE   12
>>  #endif
>>  +#define TRAP_MC  0x200 /* Machine Check */
> 
> I think usually we use MCE, so TRAP_MCE would be better

Ok.
> 
>> +#define TRAP_DSI 0x300 /* DSI exception */
>> +#define TRAP_AM  0x600 /* Alignment exception */
> 
> Don't know what AM means. TRAP_ALIGN would be more explicit.

No Problem.
> 
>> +
> 
> The defines should go in a header file, for instance asm/ptrace.h in order to 
> be re-used in other files.

Agree.
> 
> You should do more. You can find other places to improve with:
> 
> git grep "trap ==" arch/powerpc/
> git grep "TRAP(regs) ==" arch/powerpc/

Just ran “git grep”, looks like the work is much bigger than what I imagined.
> 
>>  static void __show_regs(struct pt_regs *regs)
>>  {
>>  int i, trap;
>> @@ -1465,7 +1469,7 @@ static void __show_regs(struct pt_regs *regs)
>>  trap = TRAP(regs);
>>  if (!trap_is_syscall(regs) && cpu_has_feature(CPU_FTR_CFAR))
>>  pr_cont("CFAR: "REG" ", regs->orig_gpr3);
>> -if (trap == 0x200 || trap == 0x300 || trap == 0x600) {
>> +if (trap == TRAP_MC || trap == TRAP_DSI || trap == TRAP_AM) {
>>  if (IS_ENABLED(CONFIG_4xx) || IS_ENABLED(CONFIG_BOOKE))
>>  pr_cont("DEAR: "REG" ESR: "REG" ", regs->dar, 
>> regs->dsisr);
>>  else

Thanks for the response. I will send v2.

 Regards,
Xiongwei

[PATCH] powerpc/process: Enhance readability for trap types.

2021-03-28 Thread Xiongwei Song
From: Xiongwei Song 

Define macros to enhance the code readability on ppc trap types.

Signed-off-by: Xiongwei Song 
---
 arch/powerpc/kernel/process.c | 6 +-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/arch/powerpc/kernel/process.c b/arch/powerpc/kernel/process.c
index 3231c2df9e26..3bbd3cf353a7 100644
--- a/arch/powerpc/kernel/process.c
+++ b/arch/powerpc/kernel/process.c
@@ -1451,6 +1451,10 @@ static void print_msr_bits(unsigned long val)
 #define LAST_VOLATILE  12
 #endif
 
+#define TRAP_MC  0x200 /* Machine Check */
+#define TRAP_DSI 0x300 /* DSI exception */
+#define TRAP_AM  0x600 /* Alignment exception */
+
 static void __show_regs(struct pt_regs *regs)
 {
int i, trap;
@@ -1465,7 +1469,7 @@ static void __show_regs(struct pt_regs *regs)
trap = TRAP(regs);
if (!trap_is_syscall(regs) && cpu_has_feature(CPU_FTR_CFAR))
pr_cont("CFAR: "REG" ", regs->orig_gpr3);
-   if (trap == 0x200 || trap == 0x300 || trap == 0x600) {
+   if (trap == TRAP_MC || trap == TRAP_DSI || trap == TRAP_AM) {
if (IS_ENABLED(CONFIG_4xx) || IS_ENABLED(CONFIG_BOOKE))
pr_cont("DEAR: "REG" ESR: "REG" ", regs->dar, 
regs->dsisr);
else
-- 
2.17.1



[PATCH] tracing: Remove redundant subject

2018-02-21 Thread Xiongwei Song
There are two consecutive 'we' to represent subject, remove one of the two.

Signed-off-by: Xiongwei Song 
---
 Documentation/trace/events.txt | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/Documentation/trace/events.txt b/Documentation/trace/events.txt
index 1d486660b40f..813b140cfe2c 100644
--- a/Documentation/trace/events.txt
+++ b/Documentation/trace/events.txt
@@ -878,10 +878,10 @@ The following commands are supported:
   Because the default sort key above is 'hitcount', the above shows a
   the list of call_sites by increasing hitcount, so that at the bottom
   we see the functions that made the most kmalloc calls during the
-  run.  If instead we we wanted to see the top kmalloc callers in
-  terms of the number of bytes requested rather than the number of
-  calls, and we wanted the top caller to appear at the top, we can use
-  the 'sort' parameter, along with the 'descending' modifier:
+  run.  If instead we wanted to see the top kmalloc callers in terms
+  of the number of bytes requested rather than the number of calls,
+  and we wanted the top caller to appear at the top, we can use the
+  'sort' parameter, along with the 'descending' modifier:
 
 # echo 'hist:key=call_site.sym:val=bytes_req:sort=bytes_req.descending' > \
/sys/kernel/debug/tracing/events/kmem/kmalloc/trigger
-- 
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[PATCH] tracing: Fix incorrect file name

2018-02-04 Thread Xiongwei Song
There is no file named 'enabled' in the directory tracing/events. It should
be the file 'enable'.

Signed-off-by: Xiongwei Song 
---
 Documentation/trace/events.txt | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/Documentation/trace/events.txt b/Documentation/trace/events.txt
index 2cc08d4a326e..1d486660b40f 100644
--- a/Documentation/trace/events.txt
+++ b/Documentation/trace/events.txt
@@ -294,7 +294,7 @@ PID listed in the set_event_pid file.
 
 # cd /sys/kernel/debug/tracing
 # echo $$ > set_event_pid
-# echo 1 > events/enabled
+# echo 1 > events/enable
 
 Will only trace events for the current task.
 
-- 
2.15.1



Re: [PATCH] cgroup: remove incorrect check on the return value of css_alloc

2018-01-22 Thread Xiongwei Song

> On 22 Jan 2018, at 11:55 PM, Tejun Heo  wrote:
> 
> Hello,
> 
> On Mon, Jan 22, 2018 at 09:38:51PM +0800, Xiongwei Song wrote:
>> The function css_alloc never return NULL, it may return normal pointer or
> 
> It's a calling a controller implemented method.  I'd much rather keep
> the extra protection.
> 
>> error codes that made by ERR_PTR, so !css is always false, we need use
>> IS_ERR to check it, and if this is true, we should use ERR_CAST handle it.
> 
> It's of the same type.  Why would it need casting?

Okay, Thanks for your comment.

Apologies for the noise.

Thanks
Xiongwei
> 
> Thanks.
> 
> -- 
> tejun


[PATCH] cgroup: remove incorrect check on the return value of css_alloc

2018-01-22 Thread Xiongwei Song
The function css_alloc never return NULL, it may return normal pointer or
error codes that made by ERR_PTR, so !css is always false, we need use
IS_ERR to check it, and if this is true, we should use ERR_CAST handle it.

Signed-off-by: Xiongwei Song 
---
 kernel/cgroup/cgroup.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/kernel/cgroup/cgroup.c b/kernel/cgroup/cgroup.c
index adac5b91d2b8..a442fd9ad744 100644
--- a/kernel/cgroup/cgroup.c
+++ b/kernel/cgroup/cgroup.c
@@ -4728,10 +4728,8 @@ static struct cgroup_subsys_state *css_create(struct 
cgroup *cgrp,
lockdep_assert_held(&cgroup_mutex);
 
css = ss->css_alloc(parent_css);
-   if (!css)
-   css = ERR_PTR(-ENOMEM);
if (IS_ERR(css))
-   return css;
+   return ERR_CAST(css);
 
init_and_link_css(css, ss, cgrp);
 
-- 
2.15.1



[PATCH] xfs: destroy mutex pag_ici_reclaim_lock before free xfs_perag_t structure

2018-01-11 Thread Xiongwei Song
The mutex pag_ici_reclaim_lock of xfs_perag_t structure is initialized in
xfs_initialize_perag. If happen errors in xfs_initialize_perag, or free
resources in xfs_free_perag, wo need to destroy the mutex before free
perag.

Signed-off-by: Xiongwei Song 
---
 fs/xfs/xfs_mount.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/fs/xfs/xfs_mount.c b/fs/xfs/xfs_mount.c
index c879b517cc94..98fd41cbb9e1 100644
--- a/fs/xfs/xfs_mount.c
+++ b/fs/xfs/xfs_mount.c
@@ -162,6 +162,7 @@ xfs_free_perag(
ASSERT(pag);
ASSERT(atomic_read(&pag->pag_ref) == 0);
xfs_buf_hash_destroy(pag);
+   mutex_destroy(&pag->pag_ici_reclaim_lock);
call_rcu(&pag->rcu_head, __xfs_free_perag);
}
 }
@@ -248,6 +249,7 @@ xfs_initialize_perag(
 out_hash_destroy:
xfs_buf_hash_destroy(pag);
 out_free_pag:
+   mutex_destroy(&pag->pag_ici_reclaim_lock);
kmem_free(pag);
 out_unwind_new_pags:
/* unwind any prior newly initialized pags */
@@ -256,6 +258,7 @@ xfs_initialize_perag(
if (!pag)
break;
xfs_buf_hash_destroy(pag);
+   mutex_destroy(&pag->pag_ici_reclaim_lock);
kmem_free(pag);
}
return error;
-- 
2.15.1



[PATCH] ARM: locomo: fix free dev incorrectly

2018-01-09 Thread Xiongwei Song
In function locomo_init_one_child, If kzalloc call is failed for dev we
would goto out label, then call kfree for dev, however, dev is NULL, we
shouldn't do this.

Signed-off-by: Xiongwei Song 
---
 arch/arm/common/locomo.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/arch/arm/common/locomo.c b/arch/arm/common/locomo.c
index 51936bde1eb2..fb21b5ade391 100644
--- a/arch/arm/common/locomo.c
+++ b/arch/arm/common/locomo.c
@@ -256,10 +256,9 @@ locomo_init_one_child(struct locomo *lchip, struct 
locomo_dev_info *info)
NO_IRQ : lchip->irq_base + info->irq[0];
 
ret = device_register(&dev->dev);
-   if (ret) {
- out:
+   if (ret)
kfree(dev);
-   }
+ out:
return ret;
 }
 
-- 
2.15.1



[PATCH v2] staging: android: check the return value of register_shrinker

2018-01-04 Thread Xiongwei Song
register_shrinker call is made in ashmem_init, it may return error code,
so we need to check it.

Signed-off-by: Xiongwei Song 
---
v1->v2: remove unlikely()
---
 drivers/staging/android/ashmem.c | 8 +++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/drivers/staging/android/ashmem.c b/drivers/staging/android/ashmem.c
index 4e8947923904..5d7aa55efb22 100644
--- a/drivers/staging/android/ashmem.c
+++ b/drivers/staging/android/ashmem.c
@@ -854,12 +854,18 @@ static int __init ashmem_init(void)
goto out_free2;
}
 
-   register_shrinker(&ashmem_shrinker);
+   ret = register_shrinker(&ashmem_shrinker);
+   if (ret) {
+   pr_err("failed to register shrinker!\n");
+   goto out_demisc;
+   }
 
pr_info("initialized\n");
 
return 0;
 
+out_demisc:
+   misc_deregister(&ashmem_misc);
 out_free2:
kmem_cache_destroy(ashmem_range_cachep);
 out_free1:
-- 
2.15.1



Re: [PATCH] staging: android: check the return value of register_shrinker

2018-01-04 Thread Xiongwei Song
2018-01-04 22:55 GMT+08:00 Dan Carpenter :
> On Thu, Jan 04, 2018 at 08:08:53AM +0800, Xiongwei Song wrote:
>> register_shrinker call is made in ashmem_init, it may return error code,
>> so we need to check it.
>>
>> Signed-off-by: Xiongwei Song 
>> ---
>>  drivers/staging/android/ashmem.c | 8 +++-
>>  1 file changed, 7 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/staging/android/ashmem.c 
>> b/drivers/staging/android/ashmem.c
>> index 4e8947923904..0b23c3e25cd4 100644
>> --- a/drivers/staging/android/ashmem.c
>> +++ b/drivers/staging/android/ashmem.c
>> @@ -854,12 +854,18 @@ static int __init ashmem_init(void)
>>   goto out_free2;
>>   }
>>
>> - register_shrinker(&ashmem_shrinker);
>> + ret = register_shrinker(&ashmem_shrinker);
>> + if (unlikely(ret)) {
>
> Don't add the unlikely() here.  It hurts readability and this is not a
> fast path.  I know the other callers all have it...  :/

OK. Thanks for your suggestion. I'll send v2 out.



Thanks,
Xiongwei

>
> regards,
> dan carpenter
>
>
>
>
>
>
>
>
>
>
>
>
>
>
>
>
>
>
>
>
>
>
>


[PATCH] dm bufio: fix missed destroy of mutex c->lock

2018-01-04 Thread Xiongwei Song
The mutex c->lock is initialized in dm_bufio_client_create, however,
it is not destroyed before free the structure of dm_bufio_client in
dm_bufio_client_destroy.

Signed-off-by: Xiongwei Song 
---
 drivers/md/dm-bufio.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/md/dm-bufio.c b/drivers/md/dm-bufio.c
index c546b567f3b5..53c0d5d2039a 100644
--- a/drivers/md/dm-bufio.c
+++ b/drivers/md/dm-bufio.c
@@ -1811,6 +1811,7 @@ void dm_bufio_client_destroy(struct dm_bufio_client *c)
BUG_ON(c->n_buffers[i]);
 
dm_io_client_destroy(c->dm_io);
+   mutex_destroy(&c->lock);
kfree(c);
 }
 EXPORT_SYMBOL_GPL(dm_bufio_client_destroy);
-- 
2.15.1



[PATCH] staging: android: check the return value of register_shrinker

2018-01-03 Thread Xiongwei Song
register_shrinker call is made in ashmem_init, it may return error code,
so we need to check it.

Signed-off-by: Xiongwei Song 
---
 drivers/staging/android/ashmem.c | 8 +++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/drivers/staging/android/ashmem.c b/drivers/staging/android/ashmem.c
index 4e8947923904..0b23c3e25cd4 100644
--- a/drivers/staging/android/ashmem.c
+++ b/drivers/staging/android/ashmem.c
@@ -854,12 +854,18 @@ static int __init ashmem_init(void)
goto out_free2;
}
 
-   register_shrinker(&ashmem_shrinker);
+   ret = register_shrinker(&ashmem_shrinker);
+   if (unlikely(ret)) {
+   pr_err("failed to register shrinker!\n");
+   goto out_demisc;
+   }
 
pr_info("initialized\n");
 
return 0;
 
+out_demisc:
+   misc_deregister(&ashmem_misc);
 out_free2:
kmem_cache_destroy(ashmem_range_cachep);
 out_free1:
-- 
2.15.1



Re: [PATCH] xfs: destroy mutex qi_tree_lock before free xfs_quotainfo_t object

2018-01-02 Thread Xiongwei Song
2018-01-03 4:38 GMT+08:00 Darrick J. Wong :
> On Sun, Dec 24, 2017 at 08:34:47PM +0800, Xiongwei Song wrote:
>> The mutex qi_tree_lock of xfs_quotainfo_t object was initialized when
>> calling xfs_qm_init_quotainfo, but it was not destroyed before free
>> xfs_quotainfo_t object when calling xfs_qm_destroy_quotainfo, this was
>> incorrect, so destroy it in function xfs_qm_destroy_quotainfo.
>
> Already in for-next, but thank you for catching this.
>
> https://git.kernel.org/pub/scm/fs/xfs/xfs-linux.git/commit/?h=for-next&id=2196881566225f3c3428d1a5f847a992944daa5b


Apologies for the noise.


Yours sincerely,
Xiongwei


> --D
>
>> Signed-off-by: Xiongwei Song 
>> ---
>>  fs/xfs/xfs_qm.c | 1 +
>>  1 file changed, 1 insertion(+)
>>
>> diff --git a/fs/xfs/xfs_qm.c b/fs/xfs/xfs_qm.c
>> index ec952dfad359..deceef5cbbf3 100644
>> --- a/fs/xfs/xfs_qm.c
>> +++ b/fs/xfs/xfs_qm.c
>> @@ -737,6 +737,7 @@ xfs_qm_destroy_quotainfo(
>>   qi->qi_pquotaip = NULL;
>>   }
>>   mutex_destroy(&qi->qi_quotaofflock);
>> + mutex_destroy(&qi->qi_tree_lock);
>>   kmem_free(qi);
>>   mp->m_quotainfo = NULL;
>>  }
>> --
>> 2.15.1
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
>> the body of a message to majord...@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH v3] drm/ttm: check the return value of kzalloc

2018-01-02 Thread Xiongwei Song
In the function ttm_page_alloc_init, kzalloc call is made for variable
_manager, we need to check its return value, it may return NULL.

Signed-off-by: Xiongwei Song 
---
v2->v3: delete goto expression
v1->v2: delete kfree _manager
---
 drivers/gpu/drm/ttm/ttm_page_alloc.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/gpu/drm/ttm/ttm_page_alloc.c 
b/drivers/gpu/drm/ttm/ttm_page_alloc.c
index b5ba6441489f..5d252fb27a82 100644
--- a/drivers/gpu/drm/ttm/ttm_page_alloc.c
+++ b/drivers/gpu/drm/ttm/ttm_page_alloc.c
@@ -1007,6 +1007,8 @@ int ttm_page_alloc_init(struct ttm_mem_global *glob, 
unsigned max_pages)
pr_info("Initializing pool allocator\n");
 
_manager = kzalloc(sizeof(*_manager), GFP_KERNEL);
+   if (!_manager)
+   return -ENOMEM;
 
ttm_page_pool_init_locked(&_manager->wc_pool, GFP_HIGHUSER, "wc", 0);
 
-- 
2.15.1



[PATCH v2] drm/ttm: check the return value of kzalloc

2018-01-02 Thread Xiongwei Song
In the function ttm_page_alloc_init, kzalloc call is made for variable
_manager, we need to check its return value, it may return NULL.

Signed-off-by: Xiongwei Song 
---
v1->v2: delete kfree _manager
---
 drivers/gpu/drm/ttm/ttm_page_alloc.c | 6 ++
 1 file changed, 6 insertions(+)

diff --git a/drivers/gpu/drm/ttm/ttm_page_alloc.c 
b/drivers/gpu/drm/ttm/ttm_page_alloc.c
index b5ba6441489f..583d73edb7df 100644
--- a/drivers/gpu/drm/ttm/ttm_page_alloc.c
+++ b/drivers/gpu/drm/ttm/ttm_page_alloc.c
@@ -1007,6 +1007,10 @@ int ttm_page_alloc_init(struct ttm_mem_global *glob, 
unsigned max_pages)
pr_info("Initializing pool allocator\n");
 
_manager = kzalloc(sizeof(*_manager), GFP_KERNEL);
+   if (!_manager) {
+   ret = -ENOMEM;
+   goto out;
+   }
 
ttm_page_pool_init_locked(&_manager->wc_pool, GFP_HIGHUSER, "wc", 0);
 
@@ -1041,6 +1045,8 @@ int ttm_page_alloc_init(struct ttm_mem_global *glob, 
unsigned max_pages)
ttm_pool_mm_shrink_init(_manager);
 
return 0;
+out:
+   return ret;
 }
 
 void ttm_page_alloc_fini(void)
-- 
2.15.1



[PATCH] drm/ttm: optimize errors checking and free _manager when finishing

2017-12-31 Thread Xiongwei Song
In the function ttm_page_alloc_init, kzalloc call is made for variable
_manager, we need to check its return value, it may return NULL.

In the function ttm_page_alloc_fini, we need to call kfree for variable
_manager, instead of make _manager NULL directly.

Signed-off-by: Xiongwei Song 
---
 drivers/gpu/drm/ttm/ttm_page_alloc.c | 13 +++--
 1 file changed, 11 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/ttm/ttm_page_alloc.c 
b/drivers/gpu/drm/ttm/ttm_page_alloc.c
index b5ba6441489f..e20a0b8e352b 100644
--- a/drivers/gpu/drm/ttm/ttm_page_alloc.c
+++ b/drivers/gpu/drm/ttm/ttm_page_alloc.c
@@ -1007,6 +1007,10 @@ int ttm_page_alloc_init(struct ttm_mem_global *glob, 
unsigned max_pages)
pr_info("Initializing pool allocator\n");
 
_manager = kzalloc(sizeof(*_manager), GFP_KERNEL);
+   if (!_manager) {
+   ret = -ENOMEM;
+   goto out;
+   }
 
ttm_page_pool_init_locked(&_manager->wc_pool, GFP_HIGHUSER, "wc", 0);
 
@@ -1034,13 +1038,17 @@ int ttm_page_alloc_init(struct ttm_mem_global *glob, 
unsigned max_pages)
   &glob->kobj, "pool");
if (unlikely(ret != 0)) {
kobject_put(&_manager->kobj);
-   _manager = NULL;
-   return ret;
+   goto out_free_mgr;
}
 
ttm_pool_mm_shrink_init(_manager);
 
return 0;
+out_free_mgr:
+   kfree(_manager);
+   _manager = NULL;
+out:
+   return ret;
 }
 
 void ttm_page_alloc_fini(void)
@@ -1055,6 +1063,7 @@ void ttm_page_alloc_fini(void)
ttm_page_pool_free(&_manager->pools[i], FREE_ALL_PAGES, true);
 
kobject_put(&_manager->kobj);
+   kfree(_manager);
_manager = NULL;
 }
 
-- 
2.15.1



[PATCH] staging: android: check for error from register_shrinker in ion_heap_init_shrinker

2017-12-29 Thread Xiongwei Song
The function register_shrinker in ion_heap_init_shrinker may return an
error, check it out. Meanwhile, ion_heap_init_shrinker has to a return
value.

Signed-off-by: Xiongwei Song 
---
 drivers/staging/android/ion/ion.c  | 8 ++--
 drivers/staging/android/ion/ion.h  | 2 +-
 drivers/staging/android/ion/ion_heap.c | 5 +++--
 3 files changed, 10 insertions(+), 5 deletions(-)

diff --git a/drivers/staging/android/ion/ion.c 
b/drivers/staging/android/ion/ion.c
index bb2c4b07badd..57e0d8035b2e 100644
--- a/drivers/staging/android/ion/ion.c
+++ b/drivers/staging/android/ion/ion.c
@@ -529,6 +529,7 @@ void ion_device_add_heap(struct ion_heap *heap)
 {
struct dentry *debug_file;
struct ion_device *dev = internal_dev;
+   int ret;
 
if (!heap->ops->allocate || !heap->ops->free)
pr_err("%s: can not add heap with invalid ops struct.\n",
@@ -540,8 +541,11 @@ void ion_device_add_heap(struct ion_heap *heap)
if (heap->flags & ION_HEAP_FLAG_DEFER_FREE)
ion_heap_init_deferred_free(heap);
 
-   if ((heap->flags & ION_HEAP_FLAG_DEFER_FREE) || heap->ops->shrink)
-   ion_heap_init_shrinker(heap);
+   if ((heap->flags & ION_HEAP_FLAG_DEFER_FREE) || heap->ops->shrink) {
+   ret = ion_heap_init_shrinker(heap);
+   if (ret)
+   pr_err("%s: Failed to register shrinker\n", __func__);
+   }
 
heap->dev = dev;
down_write(&dev->lock);
diff --git a/drivers/staging/android/ion/ion.h 
b/drivers/staging/android/ion/ion.h
index c24c6e230654..a238f23c9116 100644
--- a/drivers/staging/android/ion/ion.h
+++ b/drivers/staging/android/ion/ion.h
@@ -230,7 +230,7 @@ int ion_alloc(size_t len,
  * this function will be called to setup a shrinker to shrink the freelists
  * and call the heap's shrink op.
  */
-void ion_heap_init_shrinker(struct ion_heap *heap);
+int ion_heap_init_shrinker(struct ion_heap *heap);
 
 /**
  * ion_heap_init_deferred_free -- initialize deferred free functionality
diff --git a/drivers/staging/android/ion/ion_heap.c 
b/drivers/staging/android/ion/ion_heap.c
index 2af86a2d94fb..772dad65396e 100644
--- a/drivers/staging/android/ion/ion_heap.c
+++ b/drivers/staging/android/ion/ion_heap.c
@@ -297,11 +297,12 @@ static unsigned long ion_heap_shrink_scan(struct shrinker 
*shrinker,
return freed;
 }
 
-void ion_heap_init_shrinker(struct ion_heap *heap)
+int ion_heap_init_shrinker(struct ion_heap *heap)
 {
heap->shrinker.count_objects = ion_heap_shrink_count;
heap->shrinker.scan_objects = ion_heap_shrink_scan;
heap->shrinker.seeks = DEFAULT_SEEKS;
heap->shrinker.batch = 0;
-   register_shrinker(&heap->shrinker);
+
+   return register_shrinker(&heap->shrinker);
 }
-- 
2.15.1



[PATCH] xfs: destroy mutex qi_tree_lock before free xfs_quotainfo_t object

2017-12-24 Thread Xiongwei Song
The mutex qi_tree_lock of xfs_quotainfo_t object was initialized when
calling xfs_qm_init_quotainfo, but it was not destroyed before free
xfs_quotainfo_t object when calling xfs_qm_destroy_quotainfo, this was
incorrect, so destroy it in function xfs_qm_destroy_quotainfo.

Signed-off-by: Xiongwei Song 
---
 fs/xfs/xfs_qm.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/fs/xfs/xfs_qm.c b/fs/xfs/xfs_qm.c
index ec952dfad359..deceef5cbbf3 100644
--- a/fs/xfs/xfs_qm.c
+++ b/fs/xfs/xfs_qm.c
@@ -737,6 +737,7 @@ xfs_qm_destroy_quotainfo(
qi->qi_pquotaip = NULL;
}
mutex_destroy(&qi->qi_quotaofflock);
+   mutex_destroy(&qi->qi_tree_lock);
kmem_free(qi);
mp->m_quotainfo = NULL;
 }
-- 
2.15.1



[PATCH] vfio: mdev: make a couple of functions and structure vfio_mdev_driver static

2017-12-21 Thread Xiongwei Song
The functions vfio_mdev_probe, vfio_mdev_remove and the structure
vfio_mdev_driver are only used in this file, so make them static.

Clean up sparse warnings:
drivers/vfio/mdev/vfio_mdev.c:114:5: warning: no previous prototype
for 'vfio_mdev_probe' [-Wmissing-prototypes]
drivers/vfio/mdev/vfio_mdev.c:121:6: warning: no previous prototype
for 'vfio_mdev_remove' [-Wmissing-prototypes]

Signed-off-by: Xiongwei Song 
---
 drivers/vfio/mdev/vfio_mdev.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/vfio/mdev/vfio_mdev.c b/drivers/vfio/mdev/vfio_mdev.c
index fa848a701b8b..d230620fe02d 100644
--- a/drivers/vfio/mdev/vfio_mdev.c
+++ b/drivers/vfio/mdev/vfio_mdev.c
@@ -111,19 +111,19 @@ static const struct vfio_device_ops vfio_mdev_dev_ops = {
.mmap   = vfio_mdev_mmap,
 };
 
-int vfio_mdev_probe(struct device *dev)
+static int vfio_mdev_probe(struct device *dev)
 {
struct mdev_device *mdev = to_mdev_device(dev);
 
return vfio_add_group_dev(dev, &vfio_mdev_dev_ops, mdev);
 }
 
-void vfio_mdev_remove(struct device *dev)
+static void vfio_mdev_remove(struct device *dev)
 {
vfio_del_group_dev(dev);
 }
 
-struct mdev_driver vfio_mdev_driver = {
+static struct mdev_driver vfio_mdev_driver = {
.name   = "vfio_mdev",
.probe  = vfio_mdev_probe,
.remove = vfio_mdev_remove,
-- 
2.15.1



[PATCH] bpf: make function xdp_do_generic_redirect_map() static

2017-12-18 Thread Xiongwei Song
The function xdp_do_generic_redirect_map() is only used in this file, so
make it static.

Clean up sparse warning:
net/core/filter.c:2687:5: warning: no previous prototype
for 'xdp_do_generic_redirect_map' [-Wmissing-prototypes]

Signed-off-by: Xiongwei Song 
---
 net/core/filter.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/net/core/filter.c b/net/core/filter.c
index 754abe1041b7..130b842c3a15 100644
--- a/net/core/filter.c
+++ b/net/core/filter.c
@@ -2684,8 +2684,9 @@ static int __xdp_generic_ok_fwd_dev(struct sk_buff *skb, 
struct net_device *fwd)
return 0;
 }
 
-int xdp_do_generic_redirect_map(struct net_device *dev, struct sk_buff *skb,
-   struct bpf_prog *xdp_prog)
+static int xdp_do_generic_redirect_map(struct net_device *dev,
+  struct sk_buff *skb,
+  struct bpf_prog *xdp_prog)
 {
struct redirect_info *ri = this_cpu_ptr(&redirect_info);
unsigned long map_owner = ri->map_owner;
-- 
2.15.1



[PATCH] drm/tilcdc: make tilcdc_mode_hvtotal() static

2017-12-02 Thread Xiongwei Song
The function tilcdc_mode_hvtotal is local to the source and does not need
to be in global scope, so make it static.

drivers/gpu/drm/tilcdc/tilcdc_crtc.c:297:6: warning: no previous prototype for 
'tilcdc_mode_hvtotal' [-Wmissing-prototypes]
 uint tilcdc_mode_hvtotal(const struct drm_display_mode *mode)

Signed-off-by: Xiongwei Song 
---
 drivers/gpu/drm/tilcdc/tilcdc_crtc.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/tilcdc/tilcdc_crtc.c 
b/drivers/gpu/drm/tilcdc/tilcdc_crtc.c
index 6ef4d1a..c4804d9 100644
--- a/drivers/gpu/drm/tilcdc/tilcdc_crtc.c
+++ b/drivers/gpu/drm/tilcdc/tilcdc_crtc.c
@@ -294,7 +294,7 @@ static void tilcdc_crtc_set_clk(struct drm_crtc *crtc)
LCDC_V2_CORE_CLK_EN);
 }
 
-uint tilcdc_mode_hvtotal(const struct drm_display_mode *mode)
+static uint tilcdc_mode_hvtotal(const struct drm_display_mode *mode)
 {
return (uint) div_u64(1000llu * mode->htotal * mode->vtotal,
  mode->clock);
-- 
2.9.5