Real-Time Preemption, -RT-2.6.13-rc4-V0.7.53-01, High Resolution Timers & RCU-tasklist features

i have released the -53-01 Real-Time Preemption patch, which can be downloaded from:

http://redhat.com/~mingo/realtime-preempt/

there are two new features in this release, which justified the jump from .52 to .53:

- the inclusion of the High Resolution Timers patch, written by George Anzinger, and ported/improved/cleaned-up by Thomas Gleixner.

- the inclusion of the RCU tasklist_lock patch from Paul McKenney.

the HRT patch from George Anzinger is a crutial piece of real-time infrastructure. The version included in the -RT tree supports both PIT and local APIC timer driven variable-rate timer interrupts.

Thomas Glexiner, besides porting it to PREEMPT_RT, cleaning it up, adding the local APIC timer support has also added the CONFIG_HIGH_RES_TIMERS_DYN_PRIO feature, which pushes priority inheritance into the high-res timer space. Furthermore, Thomas has extended nanosleep to use HR timers, if the task is RT. This makes it easier to test HRT functionality.

NOTE: there's a new softirq, softirq-hrtimer (PID 8 on UP x86), which should be chrt-ed to higher than SCHED_FIFO-50 if HR timer interrupts are the most important latencies in the system. E.g.:

chrt -f 90 -p 8

the RCU tasklist-lock patch is a small but important feature from Paul McKenney which enables good HRT latencies: the HRT patch, when using POSIX timers, would use a signal-sending codepath that depends on the tasklist_lock, and thus the (quite high) tasklist_lock latencies controlled the latency of HR timers - defeating much of the benefits of HR timers. With the RCU tasklist-lock the signal sending path is now RCU-read locked, with no locking dependency, and thus excellent worst-case latencies.

given these changes, some (mostly build-related) regressions are to be expected. Only the x86 architecture is expected to work for now.

to build a -V0.7.53-01 tree, the following patches should to be applied:

http://kernel.org/pub/linux/kernel/v2.6/linux-2.6.12.tar.bz2
http://kernel.org/pub/linux/kernel/v2.6/testing/patch-2.6.13-rc4.bz2
http://redhat.com/~mingo/realtime-preempt/realtime-preempt-2.6.13-rc4-RT-V0.7.53-01

patches, bugreports and any other feedback welcome,

Ingo

George,

Very nice to see this going in (via) the RT patch.

Can we get a real help text here:

Clock & Timer Selection

2. HPET Timer Support (HPET_TIMER)
3. High Resolution Timer Support (HIGH_RES_TIMERS) (NEW)
choice[1-3?]: ?

You may have either HPET, High Resolution, or Legacy timer support.

This would be a great place to put some of your extensive docs about the various timer sources and issues on x86. I have always thought the HRT docs were the best source on the net for this info, and I refer people to it whenever someone has a question about timers on the linux audio lists.

The docs for "High Resolution Timer clock source" are great.

Can we get more docs here:

HRT Softirg dynamic priority adjustment (HIGH_RES_TIMERS_DYN_PRIO)

        ^^^ typo

[N/y/?] (NEW) ?

This option enables the dynamic priority adjustment of the high resolution timer soft interrupt

No point documenting this one if it's expected to go away though.

Lee

Also, does not compile for me with ACPI PM timer selected:

CC arch/i386/kernel/timers/hrtimer_pm.o
In file included from include/asm/hrtime.h:220,

                 from include/linux/hrtime.h:58,
                 from arch/i386/kernel/timers/hrtimer_pm.c:11:

include/asm/hrtime-Macpi.h: In function 'stake_cpuctr':
include/asm/hrtime-Macpi.h:110: warning: no return statement in function returning non-void
arch/i386/kernel/timers/hrtimer_pm.c: In function 'high_res_init_pm':
arch/i386/kernel/timers/hrtimer_pm.c:141: warning: format '%lu' expects type 'long unsigned int', but argument 2 has type 'unsigned int' arch/i386/kernel/timers/hrtimer_pm.c:141: warning: format '%03lu' expects type 'long unsigned int', but argument 3 has type 'unsigned int' arch/i386/kernel/timers/hrtimer_pm.c:182: error: invalid lvalue in assignment
make[2]: *** [arch/i386/kernel/timers/hrtimer_pm.o] Error 1 make[1]: *** [arch/i386/kernel/timers] Error 2
make: *** [arch/i386/kernel] Error 2

Lee

i fixed the build errors, but HIGH_RES_TIMER_ACPI_PM does not boot - so i've disabled it in the -53-03 patch for the time being.

Ingo

I did not come around yet to adapt the PM timer to the overall changes I made.

tglx

this feature is from Thomas - and most of the PREEMPT_RT integration work too (and hence all the bugs are purely his fault too! ;-).

In the -53-03 patch i've made HIGH_RES_TIMERS_DYN_PRIO always-on for PREEMPT_RT, but for other preemption models it's still selectable individually, so some more docs would indeed be useful.

Ingo

is there a patch available for -rc6?

Not yet. Whatever you see here:

http://people.redhat.com/mingo/realtime-preempt/

is the latest version.

Lee

Ingo, all

I, silly person that I am, configured an RT, SMP, PREEMPT_DEBUG system.
Someone put code in the NMI path to modify the preempt count which,
often as not will generate a PREEMPT_DEBUG message as there is no tell what state the preempt count is in on an NMI interrupt. I have sent the attached patch to Andrew on this, but meanwhile, if you want RT, SMP, PREEMPT_DEBUG you will be much better off with this.

George Anzinger   [email protected]
HRT (High-res-timers):  http://sourceforge.net/projects/high-res-timers/

Source: MontaVista Software, Inc. George Anzinger <[email protected]>
Type: Defect Fix

Description:

Modifying a word from NMI code runs the very real risk of loosing either then new or the old bits. Remember, we can not prevent an NMI interrupt from ANYWHERE, inparticular between the read and the write of a read modify write sequence.

This patch removes the update of the preempt count from the NMI path.

 hardirq.h |    9 ++++++---
 1 files changed, 6 insertions(+), 3 deletions(-)

Index: linux-2.6.13-rc/include/linux/hardirq.h
===================================================================
@@ -98,9 +98,12 @@ extern void synchronize_irq(unsigned int
 #else
 # define synchronize_irq(irq)	barrier()
 #endif
-
-#define nmi_enter()		irq_enter()
-#define nmi_exit()		sub_preempt_count(HARDIRQ_OFFSET)
+/*
+ * Re think these.  NMI _must_not_ share data words with non-nmi code
+ * Meanwhile, just do a no-op.
+ */
+#define nmi_enter()	/*	irq_enter()  */
+#define nmi_exit()	/*	sub_preempt_count(HARDIRQ_OFFSET) */
 
 #ifndef CONFIG_VIRT_CPU_ACCOUNTING
 static inline void account_user_vtime(struct task_struct *tsk)

ah - thanks, applied. Might explain some of the recent SMP weirdnesses i'm seeing. Attributed them to the HRT patch ;-)

Ingo

i'm still seeing weird crashes under SMP, which go away if i disable CONFIG_HIGH_RES_TIMERS. (this after i fixed a couple of other SMP bugs in the HRT code) It happens sometime during the bootup, after enabling the network but before users can log in. There's no good debug info, just a hang that comes from all CPUs trying to get some debug info out but crashing deeply.

Ingo

I haven't looked at this new code all that closely as yet. One thing I did notice is that there is an assumption that the "timer being delivered flag" can be shared between LR timers and HR timers. I suspect this is wrong as the delivery code is in seperate threads (I assume). This could lead to del_timer_async missing a timer.

In the prior patch we just ignored the del_timer_async issue for HR timers (code I plan to do soon). This WAS taken care of in earlier kernels by a reuse of one of the list link fields, but Andrew convince me that this was _not_ good.

So, my guess, a nanosleep for an RT task (I think you said these are promoted to HR) is completing and over writing the deliver in progress flag for a LR timer which just happens to have a del_timer_sync going on at the same time.

George Anzinger   [email protected]
HRT (High-res-timers):  http://sourceforge.net/projects/high-res-timers/

You're right, I found this yesterday night. Silly me.

This happened when I moved the hr timers into the tvec_t_base_s structure due to Olegs changes to the timer_list and the deletion code.

tglx

yes, i've just uploaded the -53-11 release, which is against 2.6.13-rc6.

the -53-11 release includes a number of fixes, in particular a fix for hard lockups that occur mostly on SMP systems, but also might occur on UP systems.

to build a -53-11 tree, the following patches should to be applied:

http://kernel.org/pub/linux/kernel/v2.6/linux-2.6.12.tar.bz2
http://kernel.org/pub/linux/kernel/v2.6/testing/patch-2.6.13-rc6.bz2
http://redhat.com/~mingo/realtime-preempt/realtime-preempt-2.6.13-rc6-RT-V0.7.53-11

Ingo

Hi Ingo,

two small buglets for my config. This is the first .53 kernel that worked for me (admittedly -07 was the last I tried). It feels a bit odd but that might be the lack of sleep.

two traces, two patches.

Peter Zijlstra

kernel: WARNING: swapper/1 changed soft IRQ-flags.
kernel:  [<c01046b3>] dump_stack+0x23/0x30 (20)
kernel:  [<c0147e2f>] illegal_API_call+0x4f/0x60 (20)
kernel:  [<c0147f31>] __local_irq_save+0x31/0x40 (8)
kernel:  [<c02d4a2e>] rh_call_control+0x16e/0x3e0 (88)
kernel:  [<c02d4f53>] rh_urb_enqueue+0x33/0x60 (20)
kernel:  [<c02d5c04>] hcd_submit_urb+0x194/0x1c0 (44)
kernel:  [<c02d6a01>] usb_submit_urb+0x1e1/0x2c0 (44)
kernel:  [<c02d6d1e>] usb_start_wait_urb+0x6e/0x120 (164)
kernel:  [<c02d6e69>] usb_internal_control_msg+0x99/0xb0 (32)
kernel:  [<c02d6f12>] usb_control_msg+0x92/0xb0 (56)
kernel:  [<c02d781a>] usb_get_descriptor+0x9a/0xe0 (76)
kernel:  [<c02d7cb5>] usb_get_device_descriptor+0x65/0xa0 (44)
kernel:  [<c02d5507>] register_root_hub+0x77/0x160 (44)
kernel:  [<c02d63c2>] usb_add_hcd+0x172/0x3b0 (56)
kernel:  [<c02dafec>] usb_hcd_pci_probe+0x27c/0x3b0 (56)
kernel:  [<c022ea26>] __pci_device_probe+0x56/0x70 (28)
kernel:  [<c022ea74>] pci_device_probe+0x34/0x60 (24)
kernel:  [<c028156e>] driver_probe_device+0x3e/0xc0 (36)
kernel:  [<c02816ef>] __driver_attach+0x4f/0x60 (24)
kernel:  [<c0280b12>] bus_for_each_dev+0x62/0x90 (48)
kernel:  [<c028172d>] driver_attach+0x2d/0x30 (24)
kernel:  [<c0281068>] bus_add_driver+0x88/0xf0 (36)
kernel:  [<c0281b4d>] driver_register+0x5d/0x70 (32)
kernel:  [<c022ed8a>] pci_register_driver+0x9a/0xc0 (28)
kernel:  [<c048c7a9>] ohci_hcd_pci_init+0x39/0x40 (16)
kernel:  [<c0471b02>] do_initcalls+0x32/0xf0 (36)
kernel:  [<c0471bea>] do_basic_setup+0x2a/0x30 (8)
kernel:  [<c01003fa>] init+0x8a/0x290 (24)
kernel:  [<c01015f9>] kernel_thread_helper+0x5/0xc (1049518108)
kernel: ---------------------------
kernel: | preempt count: 00000000 ]
kernel: | 0-level deep critical section nesting:
kernel: ----------------------------------------

kernel:
kernel: ------------------------------
kernel: | showing all locks held by: | (swapper/1 [c171b4f0, 116]):
kernel: ------------------------------
kernel:
kernel: #001: [c07f6a74] {(struct semaphore *)(&hwif->gendev_rel_sem)}
kernel: ... acquired at: init_hwif_data+0x94/0x190
kernel:
kernel: #002: [c07f75f4] {(struct semaphore *)(&hwif->gendev_rel_sem)}
kernel: ... acquired at: init_hwif_data+0x94/0x190
kernel:
kernel: #003: [c07f8174] {(struct semaphore *)(&hwif->gendev_rel_sem)}
kernel: ... acquired at: init_hwif_data+0x94/0x190
kernel:
kernel: #004: [c07f8cf4] {(struct semaphore *)(&hwif->gendev_rel_sem)}
kernel: ... acquired at: init_hwif_data+0x94/0x190
kernel:
kernel: #005: [c07f9874] {(struct semaphore *)(&hwif->gendev_rel_sem)}
kernel: ... acquired at: init_hwif_data+0x94/0x190
kernel:
kernel: #006: [c07fa3f4] {(struct semaphore *)(&hwif->gendev_rel_sem)}
kernel: ... acquired at: init_hwif_data+0x94/0x190
kernel:
kernel: #007: [c07faf74] {(struct semaphore *)(&hwif->gendev_rel_sem)}
kernel: ... acquired at: init_hwif_data+0x94/0x190
kernel:
kernel: #008: [c07fbaf4] {(struct semaphore *)(&hwif->gendev_rel_sem)}
kernel: ... acquired at: init_hwif_data+0x94/0x190
kernel:
kernel: #009: [c07fc674] {(struct semaphore *)(&hwif->gendev_rel_sem)}
kernel: ... acquired at: init_hwif_data+0x94/0x190
kernel:
kernel: #010: [c07fd1f4] {(struct semaphore *)(&hwif->gendev_rel_sem)}
kernel: ... acquired at: init_hwif_data+0x94/0x190
kernel:
kernel: #011: [efe6fa00] {(struct semaphore *)(&dev->sem)}
kernel: ... acquired at: __driver_attach+0x21/0x60
kernel:
kernel: #012: [c03dcfe4] {kernel_sem.lock}
kernel: ... acquired at: __reacquire_kernel_lock+0x38/0x80
kernel:
kernel: #013: [c0429144] {usb_bus_list_lock.lock}
kernel: ... acquired at: register_root_hub+0x5b/0x160

@@ -506,13 +506,11 @@ error:
        }
 
        /* any errors get returned through the urb completion */
-       local_irq_save (flags);
+       local_irq_save_nort (flags);
        spin_lock (&urb->lock);
        if (urb->status == -EINPROGRESS)
                urb->status = status;
        spin_unlock (&urb->lock);
        usb_hcd_giveback_urb (hcd, urb, NULL);
-       local_irq_restore (flags);
+       local_irq_restore_nort (flags);
        return 0;

}

kernel: WARNING: softirq-timer/0/4 changed soft IRQ-flags.
kernel:  [<c01046b3>] dump_stack+0x23/0x30 (20)
kernel:  [<c0147e2f>] illegal_API_call+0x4f/0x60 (20)
kernel:  [<c0147f31>] __local_irq_save+0x31/0x40 (8)
kernel:  [<c02d4cda>] usb_hcd_poll_rh_status+0x5a/0x180 (48)
kernel:  [<c02d4e16>] rh_timer_func+0x16/0x20 (12)
kernel:  [<c0131b30>] run_timer_softirq+0x210/0x430 (64)
kernel:  [<c012d5af>] ksoftirqd+0xff/0x1f0 (48)
kernel:  [<c013f816>] kthread+0xb6/0xf0 (48)
kernel:  [<c01015f9>] kernel_thread_helper+0x5/0xc (268668956)
kernel: ---------------------------
kernel: | preempt count: 00000000 ]
kernel: | 0-level deep critical section nesting:
kernel: ----------------------------------------

kernel:
kernel: ------------------------------
kernel: | showing all locks held by: | (softirq-timer/0/4 [effc5510, 98]):
kernel: ------------------------------
kernel:

similar fix, completions need not have irqs disabled on PREEMPT_RT right?

@@ -538,7 +538,7 @@ void usb_hcd_poll_rh_status(struct usb_h
        if (length > 0) {
 
                /* try to complete the status urb */
-               local_irq_save (flags);
+               local_irq_save_nort (flags);
                spin_lock(&hcd_root_hub_lock);
                urb = hcd->status_urb;
                if (urb) {

@@ -562,7 +562,7 @@ void usb_hcd_poll_rh_status(struct usb_h
                        usb_hcd_giveback_urb (hcd, urb, NULL);
                else
                        hcd->poll_pending = 1;
-               local_irq_restore (flags);
+               local_irq_restore_nort (flags);
        }
 
        /* The USB 2.0 spec says 256 ms.  This is close enough and won't

Peter Zijlstra <[email protected]>

correct, PREEMPT_RT is very strict about the use of the interrupt flags. A fair portion of the now-illegal API uses are also SMP bugs on upstream, so these details are worth pursuing.

I don't know much about the real-time preemption work, but I can explain what the code was supposed to be doing.

Interrupts are disabled during usb_hcd_giveback_urb because that's how it was done originally and nobody has made an effort to remove this assumption from the USB device drivers. There's no real reason for it other than historical inertia. It's not done for serialization -- there's no need for serialization since an URB can't be resubmitted before the previous callback occurs (unless a driver is badly broken). The "detached" method is used simply to avoid an extra pair of enable/disable instructions.

Personally I think it would be an improvement if we changed things to allow callbacks with interrupts enabled. This would require a lot of auditing of USB drivers, but in the end it should prove worthwhile.

Same answer as above: The call is done with interrupts disabled because it was always done that way.

It would cause a deadlock. Not to mention that this is not what urb->lock is intended for (protection of urb->status).

Alan Stern

so we can remove it altogether, via the patch below? (If there's any unsafe driver, it should already be unsafe on SMP, and with the proliferation of HT and dual-core CPUs, SMP will be the norm within a year or so - so the sooner we trigger any breakages, the better i guess.)

i'll give it a whirl in the -RT tree.

Ingo

----

remove unnecessarily irqs-off sections from hcd.c.

Index: linux/drivers/usb/core/hcd.c
===================================================================
@@ -506,13 +506,11 @@ error:
 	}
 
 	/* any errors get returned through the urb completion */
-	local_irq_save (flags);
-	spin_lock (&urb->lock);
+	spin_lock_irqsave(&urb->lock, flags);
 	if (urb->status == -EINPROGRESS)
 		urb->status = status;
-	spin_unlock (&urb->lock);
+	spin_unlock_irqrestore(&urb->lock, flags);
 	usb_hcd_giveback_urb (hcd, urb, NULL);
-	local_irq_restore (flags);
 	return 0;
 }
 

@@ -540,8 +538,7 @@ void usb_hcd_poll_rh_status(struct usb_h
 	if (length > 0) {
 
 		/* try to complete the status urb */
-		local_irq_save (flags);
-		spin_lock(&hcd_root_hub_lock);
+		spin_lock_irqsave(&hcd_root_hub_lock, flags);
 		urb = hcd->status_urb;
 		if (urb) {
 			spin_lock(&urb->lock);

@@ -557,14 +554,13 @@ void usb_hcd_poll_rh_status(struct usb_h
 			spin_unlock(&urb->lock);
 		} else
 			length = 0;
-		spin_unlock(&hcd_root_hub_lock);
+		spin_unlock_irqrestore(&hcd_root_hub_lock, flags);
 
 		/* local irqs are always blocked in completions */
 		if (length > 0)
 			usb_hcd_giveback_urb (hcd, urb, NULL);
 		else
 			hcd->poll_pending = 1;
-		local_irq_restore (flags);
 	}
 
 	/* The USB 2.0 spec says 256 ms.  This is close enough and won't

@@ -647,17 +643,15 @@ static int usb_rh_urb_dequeue (struct us
 	} else {				/* Status URB */
 		if (!hcd->uses_new_polling)
 			del_timer_sync (&hcd->rh_timer);
-		local_irq_disable ();
-		spin_lock (&hcd_root_hub_lock);
+		spin_lock_irq(&hcd_root_hub_lock);
 		if (urb == hcd->status_urb) {
 			hcd->status_urb = NULL;
 			urb->hcpriv = NULL;
 		} else
 			urb = NULL;		/* wasn't fully queued */
-		spin_unlock (&hcd_root_hub_lock);
+		spin_unlock_irq(&hcd_root_hub_lock);
 		if (urb)
 			usb_hcd_giveback_urb (hcd, urb, NULL);
-		local_irq_enable ();
 	}
 
 	return 0;

@@ -1367,15 +1361,13 @@ hcd_endpoint_disable (struct usb_device 
 	WARN_ON (!HC_IS_RUNNING (hcd->state) && hcd->state != HC_STATE_HALT &&
 			udev->state != USB_STATE_NOTATTACHED);
 
-	local_irq_disable ();
-
 	/* FIXME move most of this into message.c as part of its
 	 * endpoint disable logic
 	 */
 
 	/* ep is already gone from udev->ep_{in,out}[]; no more submits */
 rescan:
-	spin_lock (&hcd_data_lock);
+	spin_lock_irq(&hcd_data_lock);
 	list_for_each_entry (urb, &ep->urb_list, urb_list) {
 		int	tmp;
 

@@ -1388,13 +1380,13 @@ rescan:
 		if (urb->status != -EINPROGRESS)
 			continue;
 		usb_get_urb (urb);
-		spin_unlock (&hcd_data_lock);
+		spin_unlock_irq(&hcd_data_lock);
 
-		spin_lock (&urb->lock);
+		spin_lock_irq(&urb->lock);
 		tmp = urb->status;
 		if (tmp == -EINPROGRESS)
 			urb->status = -ESHUTDOWN;
-		spin_unlock (&urb->lock);
+		spin_unlock_irq(&urb->lock);
 
 		/* kick hcd unless it's already returning this */
 		if (tmp == -EINPROGRESS) {

@@ -1417,8 +1409,7 @@ rescan:
 		/* list contents may have changed */
 		goto rescan;
 	}
-	spin_unlock (&hcd_data_lock);
-	local_irq_enable ();
+	spin_unlock_irq(&hcd_data_lock);
 
 	/* synchronize with the hardware, so old configuration state
 	 * clears out immediately (and will be freed).

In general yes, the patch should be okay. But there are a few things to check for. Perhaps most of the USB drivers don't care whether interrupts are enabled or not in their completion routines. However I know of at least one where it does matter. The current code _is_ SMP-safe, but it won't remain UP-safe unless you add this patch in addition to your own.

Alan Stern

P.S.: Another routine worth examining is async_completed() in drivers/usb/core/devio.c.

Index: usb-2.6/drivers/usb/core/message.c
===================================================================
@@ -224,8 +224,9 @@ static void sg_clean (struct usb_sg_requ
 static void sg_complete (struct urb *urb, struct pt_regs *regs)
 {
 	struct usb_sg_request	*io = (struct usb_sg_request *) urb->context;
+	unsigned long flags;
 
-	spin_lock (&io->lock);
+	spin_lock_irqsave (&io->lock, flags);
 
 	/* In 2.5 we require hcds' endpoint queues not to progress after fault
 	 * reports, until the completion callback (this!) returns.  That lets

@@ -259,7 +260,7 @@ static void sg_complete (struct urb *urb
 		 * unlink pending urbs so they won't rx/tx bad data.
 		 * careful: unlink can sometimes be synchronous...
 		 */
-		spin_unlock (&io->lock);
+		spin_unlock_irqrestore (&io->lock, flags);
 		for (i = 0, found = 0; i < io->entries; i++) {
 			if (!io->urbs [i] || !io->urbs [i]->dev)
 				continue;

@@ -272,7 +273,7 @@ static void sg_complete (struct urb *urb
 			} else if (urb == io->urbs [i])
 				found = 1;
 		}
-		spin_lock (&io->lock);
+		spin_lock_irqsave (&io->lock, flags);
 	}
 	urb->dev = NULL;
 

@@ -282,7 +283,7 @@ static void sg_complete (struct urb *urb
 	if (!io->count)
 		complete (&io->complete);
 
-	spin_unlock (&io->lock);
+	spin_unlock_irqrestore (&io->lock, flags);
 }
 
 

ah, indeed. I've added this to the -RT tree too.

Ingo

Also Host Controller Drivers (HCDs). You do sort of have to remember who's calling this routine. It's normally an HCD in the middle of its IRQ processing, tending hardware.

I'd actually say the reason that has IRQs disabled is because of the amount of work that would have been involved in changing that assumption ... I actually did look at what it'd take to let IRQs be enabled during USB completion callbacks a while back, and concluded it'd be a lot of work for hardly any return.

Sounds dangerous to me.

And in general, all drivers have been _allowed_ to know that IRQs are disabled in completion handlers ever since the earliest 2.2 kernels that included USB support.

Changes like that -- interactions between at least three obvious subsystems (usbcore, usb drivers, hcds) and lots of unobvious ones (whoever the drivers talk to) -- sound rather error prone to me. Worth taking very cautiously, with lots of regression testing of stress loads on some of the funkier hardware.

Not that I'm deeply opposed to such changes, you understand, but there seem to be three choices here:

1 ALWAYS complete() with IRQs disabled

2 NEVER complete() with them disabled

3 SOMETIMEs complete() with them disabled.

Right now we're with #1 which is simple, consistent and guaranteed.

We couldn't switch to #2 with patches that simple. They'd in fact be rather involved, because there is logic like "If the endpoint's queue is empty when the completion handler returns, then deschedule that queueue" inside IRQ handlers. Basic things, like correctness, for periodic scheduling depend on such logic.

And I don't see much point to #3. It's got all the DIS-advantages of #1 and #2 and the advantages of neither ...

- Dave

Maybe Ingo's priorities are sufficiently different that he thinks the return _will_ be worthwhile. We've already mentioned the work involved in auditing all the USB drivers. You bring up the point that the HCDs may need adjustments also.

I'm in favor of #2 on general principle. The issue you mention, maintaining bandwidth reservations (and hardware data structures) for periodic endpoint queues, is not all that difficult to handle. Are there any other parts of the HCDs you can think of that would be affected by such a change? (I can't think of any for uhci-hcd, although I haven't looked through the code to make sure.)

Alan Stern

Maybe. I think I didn't catch all the discussion either; what I saw was questioning the USB-specific changes related to the interrupt thread work. That touches on a lot of other things too, more than a few of which raise interesting issues. :)

Yes, both HCDs and usbcore.

Which principle would that be, though? :)

I think "Keep It Simple, Stupid" argues strongly against #3, and also -- at least for now -- against #2 as well.

There were other places I remember logic that relied on such API guarantees, but I couldn't list them all now.

In any case, I'm not going to invest time to rewrite any HCDs to help make that happen. I'm sure there are other people competent to do that though, and some of them would surely bring some new insights into how the USB stack can be improved.

In fact I think this sort of thing might usefully be looked at along with other performance related issues. That whole stack seems functional enough now that it might well be time to look at thing that slow it down, or which it slows down.

- Dave

it's the basic Linux kernel principle of never disabling interrupts, unless really, really necessary.

but the main issue isnt with disabling interrupts in general, the issue is with "naked" (i.e. lock-less) disabling of interrupts. Let me try to explain. Stuff like:

	spin_lock_irq(&lock);
	stuff1();
	spin_unlock(&lock);
	stuff2();
	local_irq_enable();

is outright dangerous, because it could hide SMP bugs that do not trigger on UP. SMP systems are becoming the norm these days, so we cannot hide behind "most people use UP" arguments anymore. IRQ flags management needs to be tightened up. (The good news is that it can be done gradually and i'm doing it, so it's no extra work for you.)

so in the process of identifying naked IRQ-flags use i asked why the USB code was doing it, and i'm happy that the answer is "no good reason, mostly historic". (naked IRQ flags use also happens to be a problem for PREEMPT_RT, where i also have a debug warning about such IRQ flags assymetries, but you need not worry about that one.)

the only logistical problem with "unifying" the IRQ flags operations with their respective spin_lock functions is their transitive nature, e.g. in the above example, if stuff2() does:

	stuff2()
	{
		spin_lock(&lock2);
		stuff3();
		spin_unlock(&lock2);
	}

then the irqs-off condition needs to be propagated into the function:

	stuff2()
	{
		spin_lock_irq(&lock2);
		stuff3();
		spin_unlock_irq(&lock2);
	}

IFF 'lock2' can be used from an interrupt or softirq context. Obviously the latter code form is much more preferred, because it self-documents that lock2 is irq-safe. Nested, implicit irqs-off assumptions are another common source of locking bugs.

but fortunately this is a relatively straightforward process, and it seems Alan has identified most of the affected functions. I'd be happy if you could point out more candidates. In the worst-case, if any function is missed by accident then it's easy to debug it. I'm now testing the patches posted in this thread in the -RT tree, they are looking good so far.

to make such cleanups of irq-flags use easier i'm also thinking about automating the process of checking for the irq-safety of spinlocks, by adding a new spinlock type via:

	DEFINE_SPINLOCK_IRQSAFE(lock2);

(and a spin_lock_init_irqsafe() function too)

this will be useful for the whole kernel, not properly managing the irq flags is a common locking mistake. With this new debug feature enabled, the kernel will warn if an irq-safe lock is taken with interrupts still enabled.

furthermore, the debug feature will also warn if a spinlock _not_ marked irqsafe is used from an interrupt context. This is another common type of locking mistake.

the spinlock-consolidation patch currently cooking in -mm (to be merged to 2.6.14) makes it easy to add such new spinlock debugging features without having to change assembly code in 22 architectures.

[ we could even take this one step further and completely automate
irq-safe locks - i.e. not manage irq-flags at all and only have spin_lock() and spin_unlock(). That would cause some minimal runtime overhead for irqsafe locks though. But i digress. ]

Ingo

And "really, really" depends on context. I know that you're working in some contexts where "really, really" means "virtually never", but that's not the only context folk work with.

Of course, "never ... unless really really necessary" can fight against the principle of "as simple as practical". Tradeoffs somehow never seem far away in engineering, somehow.

Sure, but NONE of the code in question started out that way. And last time I audited USB locking code, there was none like that. So I don't know where that pseudocode came from; if any code is like that today, it could be hiding non-SMP bugs too. The only cases where "local_irq_enable" is used, it's paired with "local_irq_disable".

And the only places either is used relate to some tricky lock hierarchy games that need to be played when canceling URBs ... where usbcore has to account for the fact that the URB being canceled may _at that instant_ be in the middle of being given back to the device driver (from the HCD, via usbcore) by an IRQ handler on another CPU. There's no "naked disabling" at all; it's used exclusively when two different irq-safe locks need to be coordinated.

But as I pointed out, that answer was incomplete. Or maybe it only addressed some of the code paths, not all of them. Not all the issues are "mostly historic".

I think all the locks inside usbcore and the HCDs will end up being "irqsafe"...

That seems like a good idea. New Linux developers have often gotten confused about the rules associated with a given spinlock; and even when something starts out with good comments, it likely doesn't stay that way. Automatic warning about simple mistakes will be a big win.

- Dave

Ooh... Don't assume that! I only identified the two that occurred to me at the moment. There could be plenty of others -- I haven't looked.

I don't think making this change will involve any loss of simplicity. See below.

Are you two talking about the same thing? I gather that by "naked disabling" Ingo means something like (see hcd_endpoint_disable):

	local_irq_disable ();

whereas Dave appears to be talking about nested spinlocks (see hcd_unlink_urb):

	spin_lock_irqsave (&urb->lock, flags);
	spin_lock (&hcd_data_lock);

There's no question that nested locking is important and necessary. The calls to local_irq_disable are more dubious. AFAICT each place they occur in hcd.c, it is solely to fulfill the guarantee that usb_hcd_giveback_urb calls the completion handler with local interrupts disabled. It has no bearing at all on matters of concurrency, such as cancelling an URB that is at that instant being given back -- the spinlocks take care of that. (Or more accurately, they would if the local_irq_{dis,en}enable calls were removed and the corresponding outermost calls to spin_lock were replaced by spin_lock_irq.)

It's true that I haven't examined all the code paths. Leaving aside the completion handlers themselves, however, I don't think there is anything in usbcore or the HCDs that would really be affected by enabling local interrupts during the callbacks. If there is, it's almost certainly a bug.

Take for example the issue you mentioned before, about descheduling empty endpoint queues when the completion handler returns. By enabling local interrupts, we would allow the possibility that an unrelated IRQ routine might enqueue or dequeue an entry while the handler was running. But that possibility has always existed, since the unrelated IRQ routine could in any case be running on a different CPU.

Alan Stern

do you imply that in the USB case the code and the locking somehow gets simpler? It doesnt. Explicit management of the IRQ flags when combined with spinlocks is _never_ good, for multiple reasons. We do it only very rarely in the core kernel code, and even then we comment it thickly. E.g. we used to have a single case of such code in do_exit() and we got rid of it.

the simple solution is to always save/restore irq flags when taking the irq-safe lock.

so could you give me some other than "mostly historic" explanation? It seems there's some spin_lock() use within the USB code - if most of the USB locks are irq-safe, then all those spin_lock()s should be converted to use spin_lock_irqsave/restore. Depending on an external local_irq_disable() to have interrupts disabled is extremely fragile. It might easily be code that works fine right now, but it's an extremely unrobust concept.

great - then the solution is to use irq save/restore for all of them, and dont manage irq flags explicitly. I'll cook up a patch.

Ingo

i've added the patch below to be on the safe side.

Ingo

Index: linux/drivers/usb/core/devio.c
===================================================================
@@ -274,10 +274,11 @@ static void async_completed(struct urb *
         struct async *as = (struct async *)urb->context;
         struct dev_state *ps = as->ps;
 	struct siginfo sinfo;
+	unsigned long flags;
 
-        spin_lock(&ps->lock);
-        list_move_tail(&as->asynclist, &ps->async_completed);
-        spin_unlock(&ps->lock);
+	spin_lock_irqsave(&ps->lock, flags);
+	list_move_tail(&as->asynclist, &ps->async_completed);
+	spin_unlock_irqrestore(&ps->lock, flags);
 	if (as->signr) {
 		sinfo.si_signo = as->signr;
 		sinfo.si_errno = as->urb->status;

i've released the 2.6.13-rc6-rt1 tree, which can be downloaded from the usual place:

http://redhat.com/~mingo/realtime-preempt/

as the name already suggests, i've switched to a new, simplified naming scheme, which follows the usual naming convention of trees tracking the mainline kernel. The numbering will be restarted for every new upstream kernel the -RT tree is merged to.

the 2.6.13-rc6-rt1 release includes a number of fixes. Changes since -53-11:

- more HRT fixes (Thomas Gleixner)

- more RCU-tasklist-lock fixes (Paul E. McKenney)

- IPC message-queue and IPC messages wakeup fixes (Daniel Walker)

- VIA VT8237 southbridge quirks to fix IOAPIC issues (Karsten Wiese)

- NMI preemption-count fix (George Anzinger)

- various latency tracer fixes: lost trace entries, SMP weirdnesses (me)

to build a 2.6.13-rc6-rt1 tree, the following patches should to be applied:

http://kernel.org/pub/linux/kernel/v2.6/linux-2.6.12.tar.bz2
http://kernel.org/pub/linux/kernel/v2.6/testing/patch-2.6.13-rc6.bz2
http://redhat.com/~mingo/realtime-preempt/patch-2.6.13-rc6-rt1

Ingo

Great! With this naming scheme it is easy to teach Matt Mackall's ketchup script about the -RT tree. The modified ketchup script can be downloaded from:
http://www.uamt.feec.vutbr.cz/rizeni/pom/ketchup-0.9+rt

Matt, would you release a new ketchup version with this support for Ingo's tree?

Michal

@@ -307,7 +307,11 @@ version_info = {
     '2.6-mjb': (latest_mjb,
                  kernel_url + "/people/mbligh/%(prebase)s/patch-%(full)s.bz2",
                  r'patch-(2.6.*?).bz2',
-                 1, "Martin Bligh's random collection 'o crap")
+                 1, "Martin Bligh's random collection 'o crap"),
+    '2.6-rt': (latest_dir,
+                "http://people.redhat.com/mingo/realtime-preempt/patch-%(full)s",
+		r'patch-(2.6.*?)',
+		0, "Ingo Molnar's realtime-preempt kernel")
     }
 
 def version_url(ver, sign = 0):

Thanks. I've put this in my version, which is now exported as a Mercurial repo at:

http://selenic.com/repo/ketchup

This version also has -git support, finally.

Mathematics is the supreme nostalgia of our time.

I added the line in the patch below to be able to get Ingo's older patches.

Esben

diff -r 1342be306020 ketchup
@@ -367,6 +367,7 @@
 
     # the jgarzik memorial hack
     url2 = re.sub("/snapshots/", "/snapshots/old/", url)
+    url2 = re.sub("/realtime-preempt/", "/realtime-preempt/older/", url2)    
     if url2 != url:
         if download(url2, file): return file
 

I have put a version of KGDB for x86 RT kernels here:
http://source.mvista.com/~ganzinger/

The common_kgdb_cfi_.... stuff creates debug records for entry.S and friends so that you can "bt" through them. Apply in this order:
Ingo's patch
kgdb-ga-rt.patch
common_kgdb_cfi_annotations.patch

This is, more or less, the same kgdb that is in Andrew's mm tree changed to fix the RT issues.

George Anzinger   [email protected]
HRT (High-res-timers):  http://sourceforge.net/projects/high-res-timers/

great. For the time being i wont add it to the -RT tree (because KGDB is not destined for upstream merging it seems), but it sure is a useful development/debugging add-on.

Ingo

I agree on not adding it. Tom Rini is working on a version the Andrew seems inclined to merge. When that happens I will most likely put together enhancements to it to bring it up to what this one does. Meanwhile I am trying to capture some of Tom's changes in this one. Also, it is MUCH easier for me to maintain as a seperate patch.

George Anzinger   [email protected]
HRT (High-res-timers):  http://sourceforge.net/projects/high-res-timers/

mercredi 17 Août 2005 02:53, George Anzinger wrote/a écrit :

Hi, everybody

I found two bugs in kgdb-ga-rt patch.

The first one : if CONFIG_SMP is not set, we have a compile error The second one : if CONFIG_KGDB is not set, we have a link error I send you a diff patch to correct this. I am not sure the last patch is correct, but it works.

@@ -1861,11 +1861,11 @@
 +	gdb_i386errcode = err_code;
 +	kgdb_info.called_from = __builtin_return_address(0);
 +	kgdb_info.why = str;
++#ifdef CONFIG_SMP
 +	kgdb_info.cpus_waiting[trap_cpu].regs = linux_regs;
 +	kgdb_info.cpus_waiting[trap_cpu].task = current;
 +	kgdb_info.cpus_waiting[trap_cpu].pid = 
 +		(current->pid) ? : (PID_MAX + trap_cpu);
-+#ifdef CONFIG_SMP
 +	/*
 +	 * OK, we can now communicate, lets tell gdb about the sync.
 +	 * but only if we had a problem.

@@ -5078,7 +5078,7 @@
 ===================================================================
 --- /dev/null
 +++ linux-2.6.13-rc/include/asm-i386/kgdb_local.h
-@@ -0,0 +1,174 @@
+@@ -0,0 +1,178 @@
 +#ifndef __KGDB_LOCAL
 +#define ___KGDB_LOCAL
 +#include <linux/config.h>

@@ -5248,7 +5248,11 @@
 +#endif
 +#define INIT_KDEBUG putDebugChar("+");
 +extern struct  notifier_block kgdb_notify_struct;
++#ifdef CONFIG_KGDB 
 +#define KGDB_NOTIFY {&kgdb_notify_struct}
++#else				/* CONFIG_KGDB */
++#define KGDB_NOTIFY {NULL}
++#endif				/* CONFIG_KGDB */
 +#else
 +#define KGDB_NOTIFY {NULL}
 +#endif                          /* __ASSEMBLY__ */

The reported bugs are now rolled into the kgdb patch. Also, there is a new README.txt. I also included, in the kgdb patch, an updated gdb macro file (Documentation/i386/kgdb/gdbinit.hw) which has a per_cpu macro to:

given a per_cpu structure name and the cpu number returns the address of that structure, properly typed.

I am also putting my current version of time_stamp_tool. This is the replacement for kgdb_ts() which I have removed from the kgdb patch. Still a little rough but it has promise of being arch independent.

George Anzinger   [email protected]
HRT (High-res-timers):  http://sourceforge.net/projects/high-res-timers/

mercredi 17 Août 2005 02:53, George Anzinger wrote/a écrit :

I'm trying this kgdb patch with 2.6.13 and I get the following errors. Is there something I forgot ?

...
INSTALL sound/usb/snd-usb-audio.ko
INSTALL sound/usb/snd-usb-lib.ko
INSTALL sound/usb/usx2y/snd-usb-usx2y.ko
if [ -r System.map -a -x /sbin/depmod ]; then /sbin/depmod -ae -F System.map
-b /var/tmp/kernel-2.6.13-rt4-root -r 2.6.13-rt4; fi
WARNING: /var/tmp/kernel-2.6.13-rt4-root/lib/modules/2.6.13-rt4/kernel/net/sunrpc/sunrpc.ko needs unknown symbol preempt_locks
WARNING: /var/tmp/kernel-2.6.13-rt4-root/lib/modules/2.6.13-rt4/kernel/net/appletalk/appletalk.ko needs unknown symbol preempt_locks
WARNING: /var/tmp/kernel-2.6.13-rt4-root/lib/modules/2.6.13-rt4/kernel/fs/reiserfs/reiserfs.ko needs unknown symbol preempt_locks
WARNING: /var/tmp/kernel-2.6.13-rt4-root/lib/modules/2.6.13-rt4/kernel/fs/ntfs/ntfs.ko needs unknown symbol preempt_locks
WARNING: /var/tmp/kernel-2.6.13-rt4-root/lib/modules/2.6.13-rt4/kernel/fs/nfs/nfs.ko needs unknown symbol preempt_locks
WARNING: /var/tmp/kernel-2.6.13-rt4-root/lib/modules/2.6.13-rt4/kernel/fs/minix/minix.ko needs unknown symbol preempt_locks
WARNING: /var/tmp/kernel-2.6.13-rt4-root/lib/modules/2.6.13-rt4/kernel/fs/jbd/jbd.ko needs unknown symbol preempt_locks
WARNING: /var/tmp/kernel-2.6.13-rt4-root/lib/modules/2.6.13-rt4/kernel/fs/ext3/ext3.ko needs unknown symbol preempt_locks
WARNING: /var/tmp/kernel-2.6.13-rt4-root/lib/modules/2.6.13-rt4/kernel/fs/cifs/cifs.ko needs unknown symbol preempt_locks
WARNING: /var/tmp/kernel-2.6.13-rt4-root/lib/modules/2.6.13-rt4/kernel/fs/affs/affs.ko needs unknown symbol preempt_locks
WARNING: /var/tmp/kernel-2.6.13-rt4-root/lib/modules/2.6.13-rt4/kernel/drivers/scsi/libata.ko needs unknown symbol preempt_locks
WARNING: /var/tmp/kernel-2.6.13-rt4-root/lib/modules/2.6.13-rt4/kernel/drivers/scsi/ide-scsi.ko needs unknown symbol preempt_locks
WARNING: /var/tmp/kernel-2.6.13-rt4-root/lib/modules/2.6.13-rt4/kernel/drivers/scsi/gdth.ko needs unknown symbol preempt_locks
WARNING: /var/tmp/kernel-2.6.13-rt4-root/lib/modules/2.6.13-rt4/kernel/drivers/md/raid6.ko needs unknown symbol preempt_locks
WARNING: /var/tmp/kernel-2.6.13-rt4-root/lib/modules/2.6.13-rt4/kernel/drivers/md/raid5.ko needs unknown symbol preempt_locks
WARNING: /var/tmp/kernel-2.6.13-rt4-root/lib/modules/2.6.13-rt4/kernel/drivers/ide/ide-floppy.ko needs unknown symbol preempt_locks
WARNING: /var/tmp/kernel-2.6.13-rt4-root/lib/modules/2.6.13-rt4/kernel/drivers/block/pktcdvd.ko needs unknown symbol preempt_locks
WARNING: /var/tmp/kernel-2.6.13-rt4-root/lib/modules/2.6.13-rt4/kernel/drivers/block/loop.ko needs unknown symbol preempt_locks
make[3]: *** [_modinst_post] Erreur 1
erreur: Mauvais status de sortie pour /var/tmp/rpm-tmp.51405 (%install)

Erreur de construction de RPM:
Mauvais status de sortie pour /var/tmp/rpm-tmp.51405 (%install)
make[2]: *** [rpm] Erreur 1
make[1]: *** [rpm] Erreur 2
make: *** [rpm] Erreur 2

This related to kgdb? I.e. does it go away if you either turn off kgdb at configure time or just don't patch with kgdb? (It sure seems unrelated, but...)

George

George Anzinger   [email protected]
HRT (High-res-timers):  http://sourceforge.net/projects/high-res-timers/

mercredi 7 Septembre 2005 23:16, George Anzinger wrote/a écrit :

...

I don't get those errors with CONFIG_KGDB=n
bellow I put the diff between a working . config and a non working .config

...
If I redo the make command only ( not make rpm ) I obtain the following :

# make
  CHK     include/linux/version.h
make[1]: « arch/i386/kernel/asm-offsets.s » est à jour.
  CHK     include/linux/compile.h
  CHK     usr/initramfs_list
Kernel: arch/i386/boot/bzImage is ready  (#1)
  Building modules, stage 2.
  MODPOST
*** Warning: "preempt_locks" [net/sunrpc/sunrpc.ko] undefined!
*** Warning: "preempt_locks" [net/appletalk/appletalk.ko] undefined!
*** Warning: "preempt_locks" [fs/reiserfs/reiserfs.ko] undefined!
*** Warning: "preempt_locks" [fs/ntfs/ntfs.ko] undefined!
*** Warning: "preempt_locks" [fs/nfs/nfs.ko] undefined!
*** Warning: "preempt_locks" [fs/minix/minix.ko] undefined!
*** Warning: "preempt_locks" [fs/jbd/jbd.ko] undefined!
*** Warning: "preempt_locks" [fs/ext3/ext3.ko] undefined!
*** Warning: "preempt_locks" [fs/cifs/cifs.ko] undefined!
*** Warning: "preempt_locks" [fs/affs/affs.ko] undefined!
*** Warning: "preempt_locks" [drivers/scsi/libata.ko] undefined!
*** Warning: "preempt_locks" [drivers/scsi/ide-scsi.ko] undefined!
*** Warning: "preempt_locks" [drivers/scsi/gdth.ko] undefined!
*** Warning: "preempt_locks" [drivers/md/raid6.ko] undefined!
*** Warning: "preempt_locks" [drivers/md/raid5.ko] undefined!
*** Warning: "preempt_locks" [drivers/ide/ide-floppy.ko] undefined!
*** Warning: "preempt_locks" [drivers/block/pktcdvd.ko] undefined!
*** Warning: "preempt_locks" [drivers/block/loop.ko] undefined!
#

If I apply the following patch to my .config, I get the errors :
I tried with and without LTT. I get the same problem.

@@ -35,15 +35,16 @@
 CONFIG_IKCONFIG_PROC=y
 # CONFIG_CPUSETS is not set
 CONFIG_EMBEDDED=y
-# CONFIG_KALLSYMS is not set
-# CONFIG_KALLSYMS_ALL is not set
-# CONFIG_KALLSYMS_EXTRA_PASS is not set
+CONFIG_KALLSYMS=y
+CONFIG_KALLSYMS_ALL=y
+CONFIG_KALLSYMS_EXTRA_PASS=y
 CONFIG_PRINTK=y
 CONFIG_BUG=y
 CONFIG_BASE_FULL=y
 CONFIG_FUTEX=y
 CONFIG_EPOLL=y
 # CONFIG_CC_OPTIMIZE_FOR_SIZE is not set
+CONFIG_LTT=y
 CONFIG_SHMEM=y
 CONFIG_CC_ALIGN_FUNCTIONS=0
 CONFIG_CC_ALIGN_LABELS=0

@@ -283,7 +284,7 @@
 CONFIG_PCI_MSI=y
 CONFIG_PCI_LEGACY_PROC=y
 # CONFIG_PCI_NAMES is not set
-# CONFIG_PCI_DEBUG is not set
+CONFIG_PCI_DEBUG=y
 CONFIG_ISA_DMA_API=y
 CONFIG_ISA=y
 # CONFIG_EISA is not set

@@ -329,7 +330,7 @@
 # CONFIG_STANDALONE is not set
 CONFIG_PREVENT_FIRMWARE_BUILD=y
 CONFIG_FW_LOADER=m
-# CONFIG_DEBUG_DRIVER is not set
+CONFIG_DEBUG_DRIVER=y
 CONFIG_NET=y
 
 #

@@ -2780,7 +2781,10 @@
 # CONFIG_HUGETLBFS is not set
 # CONFIG_HUGETLB_PAGE is not set
 CONFIG_RAMFS=y
-# CONFIG_RELAYFS_FS is not set
+CONFIG_RELAYFS_FS=y
+CONFIG_KLOG_CHANNEL=y
+CONFIG_KLOG_CHANNEL_AUTOENABLE=y
+CONFIG_KLOG_CHANNEL_SHIFT=21
 CONFIG_SUPERMOUNT=m
 # CONFIG_SUPERMOUNT_DEBUG is not set
 

@@ -2930,36 +2931,38 @@
 #
 # Kernel hacking
 #
-# CONFIG_PRINTK_TIME is not set
-# CONFIG_PRINTK_IGNORE_LOGLEVEL is not set 
-# CONFIG_DEBUG_KERNEL is not set
+CONFIG_PRINTK_TIME=y
+CONFIG_PRINTK_IGNORE_LOGLEVEL=y
+CONFIG_DEBUG_KERNEL=y
 CONFIG_MAGIC_SYSRQ=y
 CONFIG_LOG_BUF_SHIFT=17
 CONFIG_DETECT_SOFTLOCKUP=y
-# CONFIG_SCHEDSTATS is not set
-# CONFIG_DEBUG_SLAB is not set
-CONFIG_DEBUG_PREEMPT=n
-CONFIG_DEBUG_IRQ_FLAGS=n
+CONFIG_SCHEDSTATS=y
+CONFIG_DEBUG_SLAB=y
+CONFIG_DEBUG_PREEMPT=y
+CONFIG_DEBUG_IRQ_FLAGS=y
 CONFIG_WAKEUP_TIMING=y
-CONFIG_WAKEUP_LATENCY_HIST=n
+CONFIG_WAKEUP_LATENCY_HIST=y
 CONFIG_PREEMPT_TRACE=y
-CONFIG_CRITICAL_PREEMPT_TIMING=n  
-CONFIG_CRITICAL_IRQSOFF_TIMING=n   
+CONFIG_CRITICAL_PREEMPT_TIMING=y   
+CONFIG_PREEMPT_OFF_HIST=y
+CONFIG_CRITICAL_IRQSOFF_TIMING=y   
+CONFIG_INTERRUPT_OFF_HIST=y
 CONFIG_CRITICAL_TIMING=y
 CONFIG_LATENCY_TIMING=y
-# CONFIG_LATENCY_TRACE is not set
-# CONFIG_RT_DEADLOCK_DETECT is not set
-# CONFIG_DEBUG_RT_LOCKING_MODE is not set
-# CONFIG_DEBUG_KOBJECT is not set
-# CONFIG_DEBUG_HIGHMEM is not set
-# CONFIG_DEBUG_BUGVERBOSE is not set
-# CONFIG_DEBUG_INFO is not set
-# CONFIG_DEBUG_FS is not set
-# CONFIG_USE_FRAME_POINTER is not set
-# CONFIG_EARLY_PRINTK is not set
-# CONFIG_DEBUG_STACKOVERFLOW is not set
+CONFIG_LATENCY_TRACE=y
+CONFIG_RT_DEADLOCK_DETECT=y
+CONFIG_DEBUG_RT_LOCKING_MODE=y
+CONFIG_DEBUG_KOBJECT=y
+CONFIG_DEBUG_HIGHMEM=y
+CONFIG_DEBUG_BUGVERBOSE=y
+CONFIG_DEBUG_INFO=y
+CONFIG_DEBUG_FS=y
+CONFIG_USE_FRAME_POINTER=y
+CONFIG_EARLY_PRINTK=y
+CONFIG_DEBUG_STACKOVERFLOW=y
 # CONFIG_KPROBES is not set
-# CONFIG_DEBUG_STACK_USAGE is not set
+CONFIG_DEBUG_STACK_USAGE=y
 
 #
 # Page alloc debug is incompatible with Software Suspend on i386

@@ -2967,7 +2968,27 @@
 # CONFIG_4KSTACKS is not set
 CONFIG_X86_FIND_SMP_CONFIG=y
 CONFIG_X86_MPPARSE=y
-# CONFIG_KGDB is not set
+CONFIG_KGDB=y
+CONFIG_KGDB_9600BAUD=y
+# CONFIG_KGDB_19200BAUD is not set
+# CONFIG_KGDB_38400BAUD is not set
+# CONFIG_KGDB_57600BAUD is not set
+# CONFIG_KGDB_115200BAUD is not set
+CONFIG_KGDB_PORT=0x3f8
+CONFIG_KGDB_IRQ=4
+CONFIG_KGDB_MORE=y
+CONFIG_KGDB_OPTIONS="-O1"
+CONFIG_NO_KGDB_CPUS=8
+CONFIG_KGDB_TS=y
+# CONFIG_KGDB_TS_64 is not set
+CONFIG_KGDB_TS_128=y
+# CONFIG_KGDB_TS_256 is not set
+# CONFIG_KGDB_TS_512 is not set
+# CONFIG_KGDB_TS_1024 is not set
+CONFIG_STACK_OVERFLOW_TEST=y
+CONFIG_TRAP_BAD_SYSCALL_EXITS=y
+CONFIG_KGDB_CONSOLE=y
+CONFIG_KGDB_SYSRQ=y
 
 #
 

Where did you get the kgdb you are using? It looks like kgdb_ts is in this version, but it it not in the one on my website http://source.mvista.com/~ganzinger/

preempt_locks is being accessed from a module but is not exported. This is turned on with CONFIG_DEBUG_RT_LOCKING_MODE so change that and it should build.

~

.....................................

George Anzinger   [email protected]
HRT (High-res-timers):  http://sourceforge.net/projects/high-res-timers/