[PATCH -rt] update kmap_atomic on !HIGHMEM

[PATCH -rt] update kmap_atomic on !HIGHMEM

[Daniel Walker]

I got some scheduling while atomic on x86-64 , and since x86-64 doesn't seem
to have HIGHMEM there's no workaround for kmap_atomic() .

This patch adds the same as i386 HIGHMEM for !HIGHMEM.

Signed-Off-By: Daniel Walker <dwalker@mvista.com>

---
 include/linux/highmem.h |   14 +++++++++++++-
 1 files changed, 13 insertions(+), 1 deletion(-)

Index: linux-2.6.19/include/linux/highmem.h
===================================================================
--- linux-2.6.19.orig/include/linux/highmem.h
+++ linux-2.6.19/include/linux/highmem.h
@@ -42,13 +42,25 @@ static inline void *kmap(struct page *pa
 
 #define kunmap(page) do { (void) (page); } while (0)
 
+#ifdef CONFIG_PREEMPT_RT
+/*
+ * kmap and kmunmap are above, and they don't really do anything
+ * interesting.
+ */
+# define kmap_atomic(page, idx)		kmap(page)
+# define kmap_atomic_pfn(pfn, idx)	kmap(pfn_to_page(pfn))
+# define kunmap_atomic(kvaddr, idx)	kunmap(kvaddr)
+#else
 #define kmap_atomic(page, idx) \
 	({ pagefault_disable(); page_address(page); })
 #define kunmap_atomic(addr, idx)	do { pagefault_enable(); } while (0)
 #define kmap_atomic_pfn(pfn, idx)	kmap_atomic(pfn_to_page(pfn), (idx))
-#define kmap_atomic_to_page(ptr)	virt_to_page(ptr)
 #endif
 
+#define kmap_atomic_to_page(ptr)	virt_to_page(ptr)
+
+#endif /* ARCH_HAS_KMAP */
+
 #endif /* CONFIG_HIGHMEM */
 
 /* when CONFIG_HIGHMEM is not set these will be plain clear/copy_page */
--

Re: [PATCH -rt] update kmap_atomic on !HIGHMEM

[Ingo Molnar]

* Daniel Walker <dwalker@mvista.com> wrote:

> I got some scheduling while atomic on x86-64 , [...]

please post those messages too.

	Ingo

Re: [PATCH -rt] update kmap_atomic on !HIGHMEM

[Daniel Walker]

On Wed, 2006-12-27 at 21:54 +0100, Ingo Molnar wrote:
> * Daniel Walker <dwalker@mvista.com> wrote:
> 
> > I got some scheduling while atomic on x86-64 , [...]
> 
> please post those messages too.
> 
> 	Ingo

Ok .. This is from 2.6.20-rc2-rt0 tho .

BUG: scheduling while atomic: kswapd0/0x00000001/244, CPU#1

Call Trace:
[<ffffffff80268f80>] __sched_text_start+0xb0/0xb9f
[<ffffffff8028fa8c>] task_rq_lock+0x4c/0x90
[<ffffffff8028fdd3>] rt_mutex_setprio+0xb3/0xd0
[<ffffffff802bb2c4>] add_preempt_count+0x14/0xf0
[<ffffffff802b1353>] task_blocks_on_rt_mutex+0x163/0x1d0
[<ffffffff80269da5>] schedule+0xe5/0x110
[<ffffffff8026afa2>] rt_spin_lock_slowlock+0x102/0x1b0
[<ffffffff802c9b71>] swap_duplicate+0x51/0x100
[<ffffffff802c94b2>] move_to_swap_cache+0x32/0x70
[<ffffffff802cf4ba>] shmem_writepage+0xaa/0x1a0
[<ffffffff802c35e0>] shrink_inactive_list+0x430/0x950
[<ffffffff802134f4>] shrink_zone+0xe4/0x110
[<ffffffff8025e629>] kswapd+0x379/0x510
[<ffffffff802a8270>] autoremove_wake_function+0x0/0x30
[<ffffffff8025e2b0>] kswapd+0x0/0x510
[<ffffffff80234c89>] kthread+0xd9/0x120
[<ffffffff8022981c>] schedule_tail+0xdc/0x130
[<ffffffff802a80a0>] keventd_create_kthread+0x0/0x90
[<ffffffff80265ef8>] child_rip+0xa/0x12
[<ffffffff802a80a0>] keventd_create_kthread+0x0/0x90
[<ffffffff80234bb0>] kthread+0x0/0x120
[<ffffffff80265eee>] child_rip+0x0/0x12

---------------------------
| preempt count: 00000001 ]
| 1-level deep critical section nesting:
----------------------------------------
 .. [<ffffffff802cc111>] .... shmem_swp_entry+0x31/0x1a0
 .....[<00000000>] ..   ( <= 0x0)

Re: [PATCH -rt] update kmap_atomic on !HIGHMEM

[Ingo Molnar]

* Daniel Walker <dwalker@mvista.com> wrote:

> I got some scheduling while atomic on x86-64 , and since x86-64 
> doesn't seem to have HIGHMEM there's no workaround for kmap_atomic() .
> 
> This patch adds the same as i386 HIGHMEM for !HIGHMEM.

the problem is that this does not disable pagefaulting while 
kmap-atomic. Could you try the patch below, does it solve the assert?

	Ingo

------------------------->
Subject: [patch] clean up the page fault disabling logic
From: Ingo Molnar <mingo@elte.hu>

decouple the pagefault-disabled logic from the preempt count.

Signed-off-by: Ingo Molnar <mingo@elte.hu>
---
 arch/arm/mm/fault.c     |    2 +-
 arch/i386/mm/fault.c    |    2 +-
 arch/mips/mm/fault.c    |    2 +-
 arch/powerpc/mm/fault.c |    2 +-
 arch/x86_64/mm/fault.c  |    2 +-
 include/linux/sched.h   |    1 +
 include/linux/uaccess.h |   33 +++------------------------------
 kernel/fork.c           |    1 +
 mm/memory.c             |   20 ++++++++++++++++++++
 9 files changed, 30 insertions(+), 35 deletions(-)

Index: linux/arch/arm/mm/fault.c
===================================================================
--- linux.orig/arch/arm/mm/fault.c
+++ linux/arch/arm/mm/fault.c
@@ -230,7 +230,7 @@ do_page_fault(unsigned long addr, unsign
 	 * If we're in an interrupt or have no user
 	 * context, we must not take the fault..
 	 */
-	if (in_atomic() || !mm)
+	if (in_atomic() || !mm || current->pagefault_disabled)
 		goto no_context;
 
 	/*
Index: linux/arch/i386/mm/fault.c
===================================================================
--- linux.orig/arch/i386/mm/fault.c
+++ linux/arch/i386/mm/fault.c
@@ -382,7 +382,7 @@ fastcall notrace void __kprobes do_page_
 	 * If we're in an interrupt, have no user context or are running in an
 	 * atomic region then we must not take the fault..
 	 */
-	if (in_atomic() || !mm)
+	if (in_atomic() || !mm || current->pagefault_disabled)
 		goto bad_area_nosemaphore;
 
 	/* When running in the kernel we expect faults to occur only to
Index: linux/arch/mips/mm/fault.c
===================================================================
--- linux.orig/arch/mips/mm/fault.c
+++ linux/arch/mips/mm/fault.c
@@ -69,7 +69,7 @@ asmlinkage void do_page_fault(struct pt_
 	 * If we're in an interrupt or have no user
 	 * context, we must not take the fault..
 	 */
-	if (in_atomic() || !mm)
+	if (in_atomic() || !mm || current->pagefault_disabled)
 		goto bad_area_nosemaphore;
 
 	down_read(&mm->mmap_sem);
Index: linux/arch/powerpc/mm/fault.c
===================================================================
--- linux.orig/arch/powerpc/mm/fault.c
+++ linux/arch/powerpc/mm/fault.c
@@ -196,7 +196,7 @@ int __kprobes notrace do_page_fault(stru
 	}
 #endif /* !(CONFIG_4xx || CONFIG_BOOKE)*/
 
-	if (in_atomic() || mm == NULL) {
+	if (in_atomic() || mm == NULL || current->pagefault_disabled) {
 		if (!user_mode(regs))
 			return SIGSEGV;
 		/* in_atomic() in user mode is really bad,
Index: linux/arch/x86_64/mm/fault.c
===================================================================
--- linux.orig/arch/x86_64/mm/fault.c
+++ linux/arch/x86_64/mm/fault.c
@@ -405,7 +405,7 @@ asmlinkage void __kprobes do_page_fault(
 	 * If we're in an interrupt or have no user
 	 * context, we must not take the fault..
 	 */
-	if (unlikely(in_atomic() || !mm))
+	if (unlikely(in_atomic() || !mm || current->pagefault_disabled))
 		goto bad_area_nosemaphore;
 
  again:
Index: linux/include/linux/sched.h
===================================================================
--- linux.orig/include/linux/sched.h
+++ linux/include/linux/sched.h
@@ -1147,6 +1147,7 @@ struct task_struct {
 	/* mutex deadlock detection */
 	struct mutex_waiter *blocked_on;
 #endif
+	int pagefault_disabled;
 #ifdef CONFIG_TRACE_IRQFLAGS
 	unsigned int irq_events;
 	int hardirqs_enabled;
Index: linux/include/linux/uaccess.h
===================================================================
--- linux.orig/include/linux/uaccess.h
+++ linux/include/linux/uaccess.h
@@ -6,37 +6,10 @@
 
 /*
  * These routines enable/disable the pagefault handler in that
- * it will not take any locks and go straight to the fixup table.
- *
- * They have great resemblance to the preempt_disable/enable calls
- * and in fact they are identical; this is because currently there is
- * no other way to make the pagefault handlers do this. So we do
- * disable preemption but we don't necessarily care about that.
+ * it will not take any MM locks and go straight to the fixup table.
  */
-static inline void pagefault_disable(void)
-{
-	inc_preempt_count();
-	/*
-	 * make sure to have issued the store before a pagefault
-	 * can hit.
-	 */
-	barrier();
-}
-
-static inline void pagefault_enable(void)
-{
-	/*
-	 * make sure to issue those last loads/stores before enabling
-	 * the pagefault handler again.
-	 */
-	barrier();
-	dec_preempt_count();
-	/*
-	 * make sure we do..
-	 */
-	barrier();
-	preempt_check_resched();
-}
+extern void pagefault_disable(void);
+extern void pagefault_enable(void);
 
 #ifndef ARCH_HAS_NOCACHE_UACCESS
 
Index: linux/kernel/fork.c
===================================================================
--- linux.orig/kernel/fork.c
+++ linux/kernel/fork.c
@@ -1128,6 +1128,7 @@ static struct task_struct *copy_process(
 	p->hardirq_context = 0;
 	p->softirq_context = 0;
 #endif
+	p->pagefault_disabled = 0;
 #ifdef CONFIG_LOCKDEP
 	p->lockdep_depth = 0; /* no locks held yet */
 	p->curr_chain_key = 0;
Index: linux/mm/memory.c
===================================================================
--- linux.orig/mm/memory.c
+++ linux/mm/memory.c
@@ -2481,6 +2481,26 @@ unlock:
 	return VM_FAULT_MINOR;
 }
 
+void pagefault_disable(void)
+{
+	current->pagefault_disabled++;
+	/*
+	 * make sure to have issued the store before a pagefault
+	 * can hit.
+	 */
+	barrier();
+}
+
+void pagefault_enable(void)
+{
+	/*
+	 * make sure to issue those last loads/stores before enabling
+	 * the pagefault handler again.
+	 */
+	barrier();
+	current->pagefault_disabled--;
+}
+
 /*
  * By the time we get here, we already hold the mm semaphore
  */

Re: [PATCH -rt] update kmap_atomic on !HIGHMEM

[Ingo Molnar]

plus on i386 the fix below is needed as well.

	Ingo

---
 include/asm-i386/highmem.h |    6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

Index: linux/include/asm-i386/highmem.h
===================================================================
--- linux.orig/include/asm-i386/highmem.h
+++ linux/include/asm-i386/highmem.h
@@ -81,9 +81,9 @@ struct page *__kmap_atomic_to_page(void 
  * on PREEMPT_RT kmap_atomic() is a wrapper that uses kmap():
  */
 #ifdef CONFIG_PREEMPT_RT
-#  define kmap_atomic(page, type)	kmap(page)
-#  define kmap_atomic_pfn(pfn, type)	kmap(pfn_to_page(pfn))
-#  define kunmap_atomic(kvaddr, type)	kunmap_virt(kvaddr)
+#  define kmap_atomic(page, type)	({ pagefault_disable(); kmap(page); })
+#  define kmap_atomic_pfn(pfn, type)	kmap_atomic(pfn_to_page(pfn), type)
+#  define kunmap_atomic(kvaddr, type)	do { pagefault_enable(); kunmap_virt(kvaddr); } while (0)
 #  define kmap_atomic_to_page(kvaddr)	kmap_to_page(kvaddr)
 #else
 # define kmap_atomic(page, type)	__kmap_atomic(page, type)

Re: [PATCH -rt] update kmap_atomic on !HIGHMEM

[Daniel Walker]

On Wed, 2006-12-27 at 22:25 +0100, Ingo Molnar wrote:
> * Daniel Walker <dwalker@mvista.com> wrote:
> 
> > I got some scheduling while atomic on x86-64 , and since x86-64 
> > doesn't seem to have HIGHMEM there's no workaround for kmap_atomic() .
> > 
> > This patch adds the same as i386 HIGHMEM for !HIGHMEM.
> 
> the problem is that this does not disable pagefaulting while 
> kmap-atomic. Could you try the patch below, does it solve the assert?
> 

That goes for the other CONFIG_HIGHMEM changes also? I mean if we can't
do it here we shouldn't do it else where..

(note, I just used testscripts/ltpstress.sh from LTP to reproduce this.)

Daniel 

Re: [PATCH -rt] update kmap_atomic on !HIGHMEM

[Daniel Walker]

On Wed, 2006-12-27 at 22:31 +0100, Ingo Molnar wrote:
> plus on i386 the fix below is needed as well.
> 

We do it on most other arches .. PowerPC , and I think ARM too.

Daniel

Re: [PATCH -rt] update kmap_atomic on !HIGHMEM

[Ingo Molnar]

* Daniel Walker <dwalker@mvista.com> wrote:

> On Wed, 2006-12-27 at 22:31 +0100, Ingo Molnar wrote:
> > plus on i386 the fix below is needed as well.
> > 
> 
> We do it on most other arches .. PowerPC , and I think ARM too.

I dont think they currently work with PREEMPT_RT and HIGHMEM enabled, 
but if/once they do they'll need a similar fix.

	Ingo

Re: [PATCH -rt] update kmap_atomic on !HIGHMEM

[Daniel Walker]

On Wed, 2006-12-27 at 22:25 +0100, Ingo Molnar wrote:
> 
> +void pagefault_disable(void)
> +{
> +       current->pagefault_disabled++;
> +       /*
> +        * make sure to have issued the store before a pagefault
> +        * can hit.
> +        */
> +       barrier();
> +}
> +
> +void pagefault_enable(void)
> +{
> +       /*
> +        * make sure to issue those last loads/stores before enabling
> +        * the pagefault handler again.
> +        */
> +       barrier();
> +       current->pagefault_disabled--;
> +}
> +

I'll test it, but clearly I'm not going to get the same scheduling while
atomic using this change.. So may as well just put it in -rt and see if
something crashes ..

Daniel

Re: [PATCH -rt] update kmap_atomic on !HIGHMEM

[Daniel Walker]

On Wed, 2006-12-27 at 22:25 +0100, Ingo Molnar wrote:
> * Daniel Walker <dwalker@mvista.com> wrote:
> 
> > I got some scheduling while atomic on x86-64 , and since x86-64 
> > doesn't seem to have HIGHMEM there's no workaround for kmap_atomic() .
> > 
> > This patch adds the same as i386 HIGHMEM for !HIGHMEM.
> 
> the problem is that this does not disable pagefaulting while 
> kmap-atomic. Could you try the patch below, does it solve the assert?
> 


It hasn't crashed after about an hour of stress.. One comment on the
patch tho , the pagefault_enable/pagefault_disable need module exports,
I saw the build complaining about them.

Daniel