Skip to contentSkip to conversation

Fix __ipipe_pin_range_globally

PATCH In review Jan Kiszka 2 participants 5 messages Oct 7, 2007

Subsystems (2)
lore
Share & tools
lore

Jan Kiszka

author
#0 lore raw

PATCH 1/1 Fix __ipipe_pin_range_globally

This patch fixes another bug of I-pipe for 2.6.22:

Due to the introduction of a pgd page cache (quicklist) into that kernel, __ipipe_pin_range_globally no longer addressed all spots that need to be updated after vmalloc'ed memory was mapped into the kernel address range. The result was that, after inserting modular Xenomai, new application sometimes received an outdated pgd from the quicklist, and the next timer IRQ triggered a minor fault over xeno_nucleus. As handling faults inside non-root domains with the Linux handler doesn't fly, the box blew up sooner or later.

So I've reworked __ipipe_pin_range_globally, basing it on pgd_list, the list of all pgd pages (in use or cached) in the system, and folding __ipipe_pin_range_mapping into it. That makes __ipipe_pin_range_globally an arch-specific thing from now on.

So far the quicklist is only biting us on i386, but I would suggest to check if/how we can apply this new pattern on other archs as well.

Jan

PS: UP is now stable with latest Xenomai here, but SMP unfortunately still misbehaves (I suspect host timer issues).

---
 arch/i386/mm/fault.c  |   16 +++++++++++-----
 include/linux/ipipe.h |    7 ++-----
 mm/memory.c           |   14 --------------
 3 files changed, 13 insertions(+), 24 deletions(-)
arch/i386/mm/fault.c +11 -5
Index: linux-2.6.22.7/arch/i386/mm/fault.c
===================================================================
@@ -650,16 +650,22 @@ void vmalloc_sync_all(void)
 }
 
 #ifdef CONFIG_IPIPE
-int __ipipe_pin_range_mapping(struct mm_struct *mm,
-			      unsigned long start, unsigned long end)
+void __ipipe_pin_range_globally(unsigned long start, unsigned long end)
 {
 	unsigned long next, addr = start;
+	unsigned long flags;
+	struct page *page;
 
 	do {
 		next = pgd_addr_end(addr, end);
-		vmalloc_sync_one(mm->pgd, addr);
-	} while (addr = next, addr != end);
 
-	return 0;
+		spin_lock_irqsave(&pgd_lock, flags);
+		for (page = pgd_list; page; page = (struct page *)page->index)
+			if (!vmalloc_sync_one(page_address(page), addr)) {
+				BUG_ON(page != pgd_list);
+				break;
+			}
+		spin_unlock_irqrestore(&pgd_lock, flags);
+	} while (addr = next, addr != end);
 }
 #endif /* CONFIG_IPIPE */
include/linux/ipipe.h +2 -5
Index: linux-2.6.22.7/include/linux/ipipe.h
===================================================================
@@ -223,11 +223,6 @@ static inline void ipipe_irq_unlock(unsi
 	__ipipe_unlock_irq(ipipe_current_domain, irq);
 }
 
-struct mm_struct;
-
-int __ipipe_pin_range_mapping(struct mm_struct *mm,
-			      unsigned long start, unsigned long end);
-
 #ifndef __ipipe_sync_pipeline
 #define __ipipe_sync_pipeline(syncmask) __ipipe_sync_stage(syncmask)
 #endif
@@ -298,6 +293,8 @@ static inline void ipipe_init_notify(str
 		__ipipe_dispatch_event(IPIPE_EVENT_INIT,p);
 }
 
+struct mm_struct;
+
 static inline void ipipe_cleanup_notify(struct mm_struct *mm)
 {
 	if (__ipipe_event_monitored_p(IPIPE_EVENT_CLEANUP))
mm/memory.c +0 -14
Index: linux-2.6.22.7/mm/memory.c
===================================================================
@@ -2959,18 +2959,4 @@ int ipipe_disable_ondemand_mappings(stru
 }
 
 EXPORT_SYMBOL(ipipe_disable_ondemand_mappings);
-
-void __ipipe_pin_range_globally(unsigned long start, unsigned long end)
-{
-	struct task_struct *p;
-
-	read_lock(&tasklist_lock);
-
-	for_each_process(p)
-		if (p->mm)
-			__ipipe_pin_range_mapping(p->mm, start, end);
-
-	read_unlock(&tasklist_lock);
-}
-
 #endif
#1 lore raw
Quoted 11 lines · Jan Kiszka

This patch fixes another bug of I-pipe for 2.6.22:

Due to the introduction of a pgd page cache (quicklist) into that kernel, __ipipe_pin_range_globally no longer addressed all spots that need to be updated after vmalloc'ed memory was mapped into the kernel address range. The result was that, after inserting modular Xenomai, new application sometimes received an outdated pgd from the quicklist, and the next timer IRQ triggered a minor fault over xeno_nucleus. As handling faults inside non-root domains with the Linux handler doesn't fly, the box blew up sooner or later.

Good spot. This said, the page cache is fairly old stuff, introduced a long time ago and already present in 2.6.10, so this means that all patches featuring the on-demand mapping disable support do have the same problem.

Quoted 5 lines

So I've reworked __ipipe_pin_range_globally, basing it on pgd_list, the list of all pgd pages (in use or cached) in the system, and folding __ipipe_pin_range_mapping into it. That makes __ipipe_pin_range_globally an arch-specific thing from now on.

Quoted 8 lines

So far the quicklist is only biting us on i386, but I would suggest to check if/how we can apply this new pattern on other archs as well.

Jan

PS: UP is now stable with latest Xenomai here, but SMP unfortunately still misbehaves (I suspect host timer issues).

I still have a problem with UP here, but this one is due to a Xenomai bug -- host timer is no more forwarded when the nucleus timer starts. Does disabling NOHZ & HIRES get things working on your setup?

Signature · 1 line
Philippe.
author
#2 lore raw
Quoted 13 lines · Philippe Gerum
Quoted 11 lines · Jan Kiszka

This patch fixes another bug of I-pipe for 2.6.22:

Due to the introduction of a pgd page cache (quicklist) into that kernel, __ipipe_pin_range_globally no longer addressed all spots that need to be updated after vmalloc'ed memory was mapped into the kernel address range. The result was that, after inserting modular Xenomai, new application sometimes received an outdated pgd from the quicklist, and the next timer IRQ triggered a minor fault over xeno_nucleus. As handling faults inside non-root domains with the Linux handler doesn't fly, the box blew up sooner or later.

Quoted 4 lines

Good spot. This said, the page cache is fairly old stuff, introduced a long time ago and already present in 2.6.10, so this means that all patches featuring the on-demand mapping disable support do have the same problem.

Indeed. But somehow the switch to quicklist or some other pieces of 2.6.22 must have changed the preconditions of this issue. I'm using Xenomai in modular form since ages on my notebook but only got that lockups over 2.6.22. Anyway, so we should back-port my patch and also spread it to the other archs.

Quoted 11 lines
Quoted 11 lines

So I've reworked __ipipe_pin_range_globally, basing it on pgd_list, the list of all pgd pages (in use or cached) in the system, and folding __ipipe_pin_range_mapping into it. That makes __ipipe_pin_range_globally an arch-specific thing from now on.

So far the quicklist is only biting us on i386, but I would suggest to check if/how we can apply this new pattern on other archs as well.

Jan

Quoted 8 lines
Quoted 3 lines

PS: UP is now stable with latest Xenomai here, but SMP unfortunately still misbehaves (I suspect host timer issues).

I still have a problem with UP here, but this one is due to a Xenomai bug -- host timer is no more forwarded when the nucleus timer starts. Does disabling NOHZ & HIRES get things working on your setup?

Yes, I have HIRES on, and I guess that's the point: My current impression is that there are some bits in Xenomai missing to migrate running hires timers from Linux's lapic clockevent device over xntimers. The effect here is that CPU0 continues (probably due to higher timer load) while CPU1 stops scheduling timers:

CPU  SCHEDULED   FIRED       TIMEOUT    INTERVAL   HANDLER      NAME
0    2729        2727        31168      -          NULL         [host-timer/0]
0    11          10          305103844  1000000000  xnpod_watch  [watchdog]
1    11          10          309365472  1000000000  xnpod_watch  [watchdog]

Jan

#3 lore raw
Quoted 19 lines · Jan Kiszka
Quoted 13 lines · Philippe Gerum
Quoted 11 lines · Jan Kiszka

This patch fixes another bug of I-pipe for 2.6.22:

Due to the introduction of a pgd page cache (quicklist) into that kernel, __ipipe_pin_range_globally no longer addressed all spots that need to be updated after vmalloc'ed memory was mapped into the kernel address range. The result was that, after inserting modular Xenomai, new application sometimes received an outdated pgd from the quicklist, and the next timer IRQ triggered a minor fault over xeno_nucleus. As handling faults inside non-root domains with the Linux handler doesn't fly, the box blew up sooner or later.

Quoted 4 lines

Good spot. This said, the page cache is fairly old stuff, introduced a long time ago and already present in 2.6.10, so this means that all patches featuring the on-demand mapping disable support do have the same problem.

Quoted 4 lines

Indeed. But somehow the switch to quicklist or some other pieces of 2.6.22 must have changed the preconditions of this issue. I'm using Xenomai in modular form since ages on my notebook but only got that lockups over 2.6.22.

We've been pretty lucky it seems, or most users end up compiling the support statically.

Quoted 2 lines

Anyway, so we should back-port my patch and also
spread it to the other archs.

When applicable, yes.

Quoted 27 lines
Quoted 12 lines
Quoted 12 lines

So I've reworked __ipipe_pin_range_globally, basing it on pgd_list, the list of all pgd pages (in use or cached) in the system, and folding __ipipe_pin_range_mapping into it. That makes __ipipe_pin_range_globally an arch-specific thing from now on.

So far the quicklist is only biting us on i386, but I would suggest to check if/how we can apply this new pattern on other archs as well.

Jan

Quoted 8 lines
Quoted 3 lines

PS: UP is now stable with latest Xenomai here, but SMP unfortunately still misbehaves (I suspect host timer issues).

I still have a problem with UP here, but this one is due to a Xenomai bug -- host timer is no more forwarded when the nucleus timer starts. Does disabling NOHZ & HIRES get things working on your setup?

Yes, I have HIRES on, and I guess that's the point: My current impression is that there are some bits in Xenomai missing to migrate running hires timers from Linux's lapic clockevent device over xntimers. The effect here is that CPU0 continues (probably due to higher timer load) while CPU1 stops scheduling timers:

Quoted 5 lines
CPU  SCHEDULED   FIRED       TIMEOUT    INTERVAL   HANDLER      NAME
0    2729        2727        31168      -          NULL         [host-timer/0]
0    11          10          305103844  1000000000  xnpod_watch  [watchdog]
1    11          10          309365472  1000000000  xnpod_watch  [watchdog]

The issue I see would be different it seems. I can reproduce the problem in UP + PIT mode, LAPIC off.

Quoted 2 lines

Jan

Signature · 1 line
Philippe.
#4 lore raw
Quoted 20 lines · Philippe Gerum
Quoted 2 lines · Jan Kiszka
Quoted 2 lines

I still have a problem with UP here, but this one is due to a

Xenomai

Quoted 1 line
Quoted 1 line

bug -- host timer is no more forwarded when the nucleus timer

starts.

Quoted 6 lines
Quoted 2 lines

Does disabling NOHZ & HIRES get things working on your setup?

Yes, I have HIRES on, and I guess that's the point: My current impression is that there are some bits in Xenomai missing to migrate running hires timers from Linux's lapic clockevent device over

xntimers.

Quoted 5 lines

The effect here is that CPU0 continues (probably due to higher timer load) while CPU1 stops scheduling timers:

CPU  SCHEDULED   FIRED       TIMEOUT    INTERVAL   HANDLER      NAME
0    2729        2727        31168      -          NULL

[host-timer/0]

Quoted 1 line

0 11 10 305103844 1000000000 xnpod_watch

Quoted 8 lines

[watchdog]

Quoted 1 line

1 11 10 309365472 1000000000 xnpod_watch

[watchdog]

Quoted 1 line

The issue I see would be different it seems. I can reproduce the problem
in UP + PIT mode, LAPIC off.

Ok, there is very likely a basic flaw in the logic for sharing the oneshot mode between the clock event manager and Xenomai, when it comes to relaying the host tick from the latter; something like a silly catch 22 issue. I'll send more on this later, but this probably impacts all setups, LAPIC enabled or not. More on this later, I need to sort out a few things more.

Signature · 1 line
Philippe.