[PATCH draft] mm: use pagefault_disable() to check for disabled pagefaults in the handler
zip --- This will replacement patch nr 6 if we have an agreement :) So this is my suggestion to fix the irq context problem. We should not modify pagefault_disabled() to check for in_atomic(), otherwise we can't reuse it for might_fault()/might_sleep() checks. This also allows to clean it up later (if ever :) ). Thanks! zip --- Introduce faulthandler_disabled() and use it to check for irq context and disabled pagefaults (via pagefault_disable()) in the pagefault handlers. Please note that we keep the in_atomic() checks in place. This is needed to detect irq context (in which case preemption is always properly disabled). In contrast, preempt_disable() should never be used to disable pagefaults. With !CONFIG_PREEMPT_COUNT, preempt_disable() doesn't modify the preempt counter, and therefore the result of in_atomic() differs. We validate that condition by using might_fault() checks when calling might_sleep(). Therefore, we add an comment to faulthandler_disabled(), describing why this is needed. faulthandler_disabled() and pagefault_disable() are defined in linux/uaccess.h, so let's properly add that include to all relevant files. This patch is based on a patch from Thomas Gleixner. Signed-off-by: David Hildenbrand --- arch/alpha/mm/fault.c | 5 ++--- arch/arc/mm/fault.c| 2 +- arch/arm/mm/fault.c| 2 +- arch/arm64/mm/fault.c | 2 +- arch/avr32/mm/fault.c | 4 ++-- arch/cris/mm/fault.c | 6 +++--- arch/frv/mm/fault.c| 4 ++-- arch/ia64/mm/fault.c | 4 ++-- arch/m32r/mm/fault.c | 8 arch/m68k/mm/fault.c | 4 ++-- arch/metag/mm/fault.c | 2 +- arch/microblaze/mm/fault.c | 8 arch/mips/mm/fault.c | 4 ++-- arch/mn10300/mm/fault.c| 4 ++-- arch/nios2/mm/fault.c | 2 +- arch/parisc/kernel/traps.c | 4 ++-- arch/parisc/mm/fault.c | 4 ++-- arch/powerpc/mm/fault.c| 9 + arch/s390/mm/fault.c | 2 +- arch/score/mm/fault.c | 3 ++- arch/sh/mm/fault.c | 5 +++-- arch/sparc/mm/fault_32.c | 4 ++-- arch/sparc/mm/fault_64.c | 4 ++-- arch/sparc/mm/init_64.c| 2 +- arch/tile/mm/fault.c | 4 ++-- arch/um/kernel/trap.c | 4 ++-- arch/unicore32/mm/fault.c | 2 +- arch/x86/mm/fault.c| 5 +++-- arch/xtensa/mm/fault.c | 4 ++-- include/linux/uaccess.h| 12 30 files changed, 72 insertions(+), 57 deletions(-) diff --git a/arch/alpha/mm/fault.c b/arch/alpha/mm/fault.c index 9d0ac09..4a905bd 100644 --- a/arch/alpha/mm/fault.c +++ b/arch/alpha/mm/fault.c @@ -23,8 +23,7 @@ #include #include #include - -#include +#include extern void die_if_kernel(char *,struct pt_regs *,long, unsigned long *); @@ -107,7 +106,7 @@ do_page_fault(unsigned long address, unsigned long mmcsr, /* If we're in an interrupt context, or have no user context, we must not take the fault. */ - if (!mm || in_atomic()) + if (!mm || faulthandler_disabled()) goto no_context; #ifdef CONFIG_ALPHA_LARGE_VMALLOC diff --git a/arch/arc/mm/fault.c b/arch/arc/mm/fault.c index 6a2e006..d948e4e 100644 --- a/arch/arc/mm/fault.c +++ b/arch/arc/mm/fault.c @@ -86,7 +86,7 @@ void do_page_fault(unsigned long address, struct pt_regs *regs) * If we're in an interrupt or have no user * context, we must not take the fault.. */ - if (in_atomic() || !mm) + if (faulthandler_disabled() || !mm) goto no_context; if (user_mode(regs)) diff --git a/arch/arm/mm/fault.c b/arch/arm/mm/fault.c index 6333d9c..0d629b8 100644 --- a/arch/arm/mm/fault.c +++ b/arch/arm/mm/fault.c @@ -276,7 +276,7 @@ do_page_fault(unsigned long addr, unsigned int fsr, struct pt_regs *regs) * If we're in an interrupt or have no user * context, we must not take the fault.. */ - if (in_atomic() || !mm) + if (faulthandler_disabled() || !mm) goto no_context; if (user_mode(regs)) diff --git a/arch/arm64/mm/fault.c b/arch/arm64/mm/fault.c index 96da131..0948d32 100644 --- a/arch/arm64/mm/fault.c +++ b/arch/arm64/mm/fault.c @@ -211,7 +211,7 @@ static int __kprobes do_page_fault(unsigned long addr, unsigned int esr, * If we're in an interrupt or have no user context, we must not take * the fault. */ - if (in_atomic() || !mm) + if (faulthandler_disabled() || !mm) goto no_context; if (user_mode(regs)) diff --git a/arch/avr32/mm/fault.c b/arch/avr32/mm/fault.c index d223a8b..c035339 100644 --- a/arch/avr32/mm/fault.c +++ b/arch/avr32/mm/fault.c @@ -14,11 +14,11 @@ #include #include #include +#include #include #include #include -#include #ifdef CONFIG_KPROBES static inline int notify_page_fault(struct pt_regs *regs, int trap) @@ -81,7 +81,7 @@ asmlinkage void do_page_fault(unsigned long ecr, struct pt_regs *regs) *
[PATCH draft] mm: use pagefault_disable() to check for disabled pagefaults in the handler
zip --- This will replacement patch nr 6 if we have an agreement :) So this is my suggestion to fix the irq context problem. We should not modify pagefault_disabled() to check for in_atomic(), otherwise we can't reuse it for might_fault()/might_sleep() checks. This also allows to clean it up later (if ever :) ). Thanks! zip --- Introduce faulthandler_disabled() and use it to check for irq context and disabled pagefaults (via pagefault_disable()) in the pagefault handlers. Please note that we keep the in_atomic() checks in place. This is needed to detect irq context (in which case preemption is always properly disabled). In contrast, preempt_disable() should never be used to disable pagefaults. With !CONFIG_PREEMPT_COUNT, preempt_disable() doesn't modify the preempt counter, and therefore the result of in_atomic() differs. We validate that condition by using might_fault() checks when calling might_sleep(). Therefore, we add an comment to faulthandler_disabled(), describing why this is needed. faulthandler_disabled() and pagefault_disable() are defined in linux/uaccess.h, so let's properly add that include to all relevant files. This patch is based on a patch from Thomas Gleixner. Signed-off-by: David Hildenbrand d...@linux.vnet.ibm.com --- arch/alpha/mm/fault.c | 5 ++--- arch/arc/mm/fault.c| 2 +- arch/arm/mm/fault.c| 2 +- arch/arm64/mm/fault.c | 2 +- arch/avr32/mm/fault.c | 4 ++-- arch/cris/mm/fault.c | 6 +++--- arch/frv/mm/fault.c| 4 ++-- arch/ia64/mm/fault.c | 4 ++-- arch/m32r/mm/fault.c | 8 arch/m68k/mm/fault.c | 4 ++-- arch/metag/mm/fault.c | 2 +- arch/microblaze/mm/fault.c | 8 arch/mips/mm/fault.c | 4 ++-- arch/mn10300/mm/fault.c| 4 ++-- arch/nios2/mm/fault.c | 2 +- arch/parisc/kernel/traps.c | 4 ++-- arch/parisc/mm/fault.c | 4 ++-- arch/powerpc/mm/fault.c| 9 + arch/s390/mm/fault.c | 2 +- arch/score/mm/fault.c | 3 ++- arch/sh/mm/fault.c | 5 +++-- arch/sparc/mm/fault_32.c | 4 ++-- arch/sparc/mm/fault_64.c | 4 ++-- arch/sparc/mm/init_64.c| 2 +- arch/tile/mm/fault.c | 4 ++-- arch/um/kernel/trap.c | 4 ++-- arch/unicore32/mm/fault.c | 2 +- arch/x86/mm/fault.c| 5 +++-- arch/xtensa/mm/fault.c | 4 ++-- include/linux/uaccess.h| 12 30 files changed, 72 insertions(+), 57 deletions(-) diff --git a/arch/alpha/mm/fault.c b/arch/alpha/mm/fault.c index 9d0ac09..4a905bd 100644 --- a/arch/alpha/mm/fault.c +++ b/arch/alpha/mm/fault.c @@ -23,8 +23,7 @@ #include linux/smp.h #include linux/interrupt.h #include linux/module.h - -#include asm/uaccess.h +#include linux/uaccess.h extern void die_if_kernel(char *,struct pt_regs *,long, unsigned long *); @@ -107,7 +106,7 @@ do_page_fault(unsigned long address, unsigned long mmcsr, /* If we're in an interrupt context, or have no user context, we must not take the fault. */ - if (!mm || in_atomic()) + if (!mm || faulthandler_disabled()) goto no_context; #ifdef CONFIG_ALPHA_LARGE_VMALLOC diff --git a/arch/arc/mm/fault.c b/arch/arc/mm/fault.c index 6a2e006..d948e4e 100644 --- a/arch/arc/mm/fault.c +++ b/arch/arc/mm/fault.c @@ -86,7 +86,7 @@ void do_page_fault(unsigned long address, struct pt_regs *regs) * If we're in an interrupt or have no user * context, we must not take the fault.. */ - if (in_atomic() || !mm) + if (faulthandler_disabled() || !mm) goto no_context; if (user_mode(regs)) diff --git a/arch/arm/mm/fault.c b/arch/arm/mm/fault.c index 6333d9c..0d629b8 100644 --- a/arch/arm/mm/fault.c +++ b/arch/arm/mm/fault.c @@ -276,7 +276,7 @@ do_page_fault(unsigned long addr, unsigned int fsr, struct pt_regs *regs) * If we're in an interrupt or have no user * context, we must not take the fault.. */ - if (in_atomic() || !mm) + if (faulthandler_disabled() || !mm) goto no_context; if (user_mode(regs)) diff --git a/arch/arm64/mm/fault.c b/arch/arm64/mm/fault.c index 96da131..0948d32 100644 --- a/arch/arm64/mm/fault.c +++ b/arch/arm64/mm/fault.c @@ -211,7 +211,7 @@ static int __kprobes do_page_fault(unsigned long addr, unsigned int esr, * If we're in an interrupt or have no user context, we must not take * the fault. */ - if (in_atomic() || !mm) + if (faulthandler_disabled() || !mm) goto no_context; if (user_mode(regs)) diff --git a/arch/avr32/mm/fault.c b/arch/avr32/mm/fault.c index d223a8b..c035339 100644 --- a/arch/avr32/mm/fault.c +++ b/arch/avr32/mm/fault.c @@ -14,11 +14,11 @@ #include linux/pagemap.h #include linux/kdebug.h #include linux/kprobes.h +#include linux/uaccess.h #include asm/mmu_context.h #include asm/sysreg.h #include asm/tlb.h -#include
