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

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

[Ingo Molnar]

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

Re: [patch] Real-Time Preemption, -RT-2.6.13-rc4-V0.7.53-01, High Resolution Timers & RCU-tasklist features

[Lee Revell]

On Thu, 2005-08-11 at 13:00 +0200, Ingo Molnar wrote:
> 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.
> 

George,

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

Can we get a real help text here:

Clock & Timer Selection
> 1. Legacy Timer Support (LEGACY_TIMER) (NEW)
  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

Re: [patch] Real-Time Preemption, -RT-2.6.13-rc4-V0.7.53-01, High Resolution Timers & RCU-tasklist features

[Lee Revell]

On Thu, 2005-08-11 at 23:07 -0400, Lee Revell wrote:
> Very nice to see this going in (via) the RT patch.
> 

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

Re: [patch] Real-Time Preemption, -RT-2.6.13-rc4-V0.7.53-01, High Resolution Timers & RCU-tasklist features

[Ingo Molnar]

* Lee Revell <rlrevell@joe-job.com> wrote:

> On Thu, 2005-08-11 at 23:07 -0400, Lee Revell wrote:
> > Very nice to see this going in (via) the RT patch.
> > 
> 
> Also, does not compile for me with ACPI PM timer selected:

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

Re: [patch] Real-Time Preemption, -RT-2.6.13-rc4-V0.7.53-01, High Resolution Timers & RCU-tasklist features

[Thomas Gleixner]

On Thu, 2005-08-11 at 23:19 -0400, Lee Revell wrote:
> On Thu, 2005-08-11 at 23:07 -0400, Lee Revell wrote:
> > Very nice to see this going in (via) the RT patch.
> > 
> 
> Also, does not compile for me with ACPI PM timer selected:

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

tglx

Re: [patch] Real-Time Preemption, -RT-2.6.13-rc4-V0.7.53-01, High Resolution Timers & RCU-tasklist features

[Ingo Molnar]

* Lee Revell <rlrevell@joe-job.com> wrote:

> On Thu, 2005-08-11 at 13:00 +0200, Ingo Molnar wrote:
> > 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.
> > 
> 
> George,
> 
> Very nice to see this going in (via) the RT patch.
> 
> Can we get a real help text here:
> 
> Clock & Timer Selection
> > 1. Legacy Timer Support (LEGACY_TIMER) (NEW)
>   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.

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

Re: [patch] Real-Time Preemption, -RT-2.6.13-rc4-V0.7.53-01, High Resolution Timers & RCU-tasklist features

[Ryan Brown]

is there a patch available for -rc6?

On 8/11/05, Ingo Molnar <mingo@elte.hu> wrote:
> 
> 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
> -
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/
>

Re: [patch] Real-Time Preemption, -RT-2.6.13-rc4-V0.7.53-01, High Resolution Timers & RCU-tasklist features

[Lee Revell]

On Sat, 2005-08-13 at 12:28 +1200, Ryan Brown wrote:
> 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

Re: [patch] Real-Time Preemption, -RT-2.6.13-rc4-V0.7.53-01, High Resolution Timers & RCU-tasklist features

[George Anzinger]

[-- Attachment #1: Type: text/plain, Size: 538 bytes --]

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   george@mvista.com
HRT (High-res-timers):  http://sourceforge.net/projects/high-res-timers/

[-- Attachment #2: fix-nmi-enter.patch --]
[-- Type: text/plain, Size: 1296 bytes --]

Source: MontaVista Software, Inc. George Anzinger <george@mvista.com>
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.

Signed-off-by: George Anzinger<george@mvista.com>

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

Index: linux-2.6.13-rc/include/linux/hardirq.h
===================================================================
--- linux-2.6.13-rc.orig/include/linux/hardirq.h
+++ 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)

Re: [patch] Real-Time Preemption, -RT-2.6.13-rc4-V0.7.53-01, High Resolution Timers & RCU-tasklist features

[Ingo Molnar]

* George Anzinger <george@mvista.com> wrote:

> 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.

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

	Ingo

Re: [patch] Real-Time Preemption, -RT-2.6.13-rc4-V0.7.53-01, High Resolution Timers & RCU-tasklist features

[Ingo Molnar]

* Ingo Molnar <mingo@elte.hu> wrote:

> * George Anzinger <george@mvista.com> wrote:
> 
> > 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.
> 
> ah - thanks, applied. Might explain some of the recent SMP weirdnesses 
> i'm seeing. Attributed them to the HRT patch ;-)

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

Re: [patch] Real-Time Preemption, -RT-2.6.13-rc4-V0.7.53-01, High Resolution Timers & RCU-tasklist features

[George Anzinger]

Ingo Molnar wrote:
> * Ingo Molnar <mingo@elte.hu> wrote:
> 
> 
>>* George Anzinger <george@mvista.com> wrote:
>>
>>
>>>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.
>>
>>ah - thanks, applied. Might explain some of the recent SMP weirdnesses 
>>i'm seeing. Attributed them to the HRT patch ;-)
> 
> 
> 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.
> 
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   george@mvista.com
HRT (High-res-timers):  http://sourceforge.net/projects/high-res-timers/

Re: [patch] Real-Time Preemption, -RT-2.6.13-rc4-V0.7.53-01, High Resolution Timers & RCU-tasklist features

[Thomas Gleixner]

On Mon, 2005-08-15 at 16:39 -0700, George Anzinger wrote:
> 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.

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

[patch] Real-Time Preemption, -RT-2.6.13-rc6-V0.7.53-11

[Ingo Molnar]

* Ryan Brown <some.nzguy@gmail.com> wrote:

> is there a patch available for -rc6?

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

Re: [patch] Real-Time Preemption, -RT-2.6.13-rc6-V0.7.53-11

[Peter Zijlstra]

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

--- linux-2.6.13-rc6-git7-RT-V0.7.53-11/drivers/usb/core/hcd.c~ 2005-08-15 21:23:45.000000000 +0200
+++ linux-2.6.13-rc6-git7-RT-V0.7.53-11/drivers/usb/core/hcd.c  2005-08-15 22:03:33.000000000 +0200
@@ -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?

--- linux-2.6.13-rc6-git7-RT-V0.7.53-11/drivers/usb/core/hcd.c~ 2005-08-15 22:03:33.000000000 +0200
+++ linux-2.6.13-rc6-git7-RT-V0.7.53-11/drivers/usb/core/hcd.c  2005-08-15 22:32:54.000000000 +0200
@@ -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 <a.p.zijlstra@chello.nl>

Re: [patch] Real-Time Preemption, -RT-2.6.13-rc6-V0.7.53-11

[Ingo Molnar]

* Peter Zijlstra <a.p.zijlstra@chello.nl> wrote:

> --- linux-2.6.13-rc6-git7-RT-V0.7.53-11/drivers/usb/core/hcd.c~ 2005-08-15 21:23:45.000000000 +0200
> +++ linux-2.6.13-rc6-git7-RT-V0.7.53-11/drivers/usb/core/hcd.c  2005-08-15 22:03:33.000000000 +0200
> @@ -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;
>  }

i'm wondering whether we could/should also fix this upstream - and 
whether this [making the IRQ flags disabling a NOP on -RT] is the right 
fix. Why does the USB hcd.c code do this in the first place? Disabling 
interrupts during usb_hcd_giveback_urb() [but not holding the urb->lock] 
might serialize on UP, but it has no serialization effect on SMP and is 
hence potentially buggy. Is there something i'm missing about this code?

the normal way of using urb->lock would be spin_lock_irqsave() and 
spin_lock_irqrestore(), not the 'detached' method seen above.

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

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.

> --- linux-2.6.13-rc6-git7-RT-V0.7.53-11/drivers/usb/core/hcd.c~ 2005-08-15 22:03:33.000000000 +0200
> +++ linux-2.6.13-rc6-git7-RT-V0.7.53-11/drivers/usb/core/hcd.c  2005-08-15 22:32:54.000000000 +0200
> @@ -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);

same question: why are interrupts being kept disabled longer, and why is 
usb_hcd_giveback_urb() called with interrupts disabled? (I tried to use 
spin_lock_irqsave/irqrestore() in earlier -RT versions, but people 
reported hangs and USB misbehavior, which might be related. I'm worried 
that your _nort patch could cause similar misbehavior.)

how about (naively) extending the urb->lock to cover 
usb_hcd_giveback_urb() calls too - does that cause a deadlock or is it 
unsafe in some other way?

	Ingo

Re: [patch] Real-Time Preemption, -RT-2.6.13-rc6-V0.7.53-11

[Alan Stern]

On Tue, 16 Aug 2005, Ingo Molnar wrote:

> * Peter Zijlstra <a.p.zijlstra@chello.nl> wrote:
> 
> > --- linux-2.6.13-rc6-git7-RT-V0.7.53-11/drivers/usb/core/hcd.c~ 2005-08-15 21:23:45.000000000 +0200
> > +++ linux-2.6.13-rc6-git7-RT-V0.7.53-11/drivers/usb/core/hcd.c  2005-08-15 22:03:33.000000000 +0200
> > @@ -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;
> >  }
> 
> i'm wondering whether we could/should also fix this upstream - and 
> whether this [making the IRQ flags disabling a NOP on -RT] is the right 
> fix. Why does the USB hcd.c code do this in the first place? Disabling 
> interrupts during usb_hcd_giveback_urb() [but not holding the urb->lock] 
> might serialize on UP, but it has no serialization effect on SMP and is 
> hence potentially buggy. Is there something i'm missing about this code?
> 
> the normal way of using urb->lock would be spin_lock_irqsave() and 
> spin_lock_irqrestore(), not the 'detached' method seen above.

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.

> > similar fix, completions need not have irqs disabled on PREEMPT_RT 
> > right?
> 
> 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.
> 
> > --- linux-2.6.13-rc6-git7-RT-V0.7.53-11/drivers/usb/core/hcd.c~ 2005-08-15 22:03:33.000000000 +0200
> > +++ linux-2.6.13-rc6-git7-RT-V0.7.53-11/drivers/usb/core/hcd.c  2005-08-15 22:32:54.000000000 +0200
> > @@ -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);
> 
> same question: why are interrupts being kept disabled longer, and why is 
> usb_hcd_giveback_urb() called with interrupts disabled? (I tried to use 
> spin_lock_irqsave/irqrestore() in earlier -RT versions, but people 
> reported hangs and USB misbehavior, which might be related. I'm worried 
> that your _nort patch could cause similar misbehavior.)

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

> how about (naively) extending the urb->lock to cover 
> usb_hcd_giveback_urb() calls too - does that cause a deadlock or is it 
> unsafe in some other 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

Re: [patch] Real-Time Preemption, -RT-2.6.13-rc6-V0.7.53-11

[Ingo Molnar]

* Alan Stern <stern@rowland.harvard.edu> wrote:

> 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.

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.

Signed-off-by: Ingo Molnar <mingo@elte.hu>

Index: linux/drivers/usb/core/hcd.c
===================================================================
--- linux.orig/drivers/usb/core/hcd.c
+++ 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).

Re: [patch] Real-Time Preemption, -RT-2.6.13-rc6-V0.7.53-11

[Alan Stern]

On Tue, 16 Aug 2005, Ingo Molnar wrote:

> * Alan Stern <stern@rowland.harvard.edu> wrote:
> 
> > 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.
> 
> 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.

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
===================================================================
--- usb-2.6.orig/drivers/usb/core/message.c
+++ 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);
 }
 
 

Re: [patch] Real-Time Preemption, -RT-2.6.13-rc6-V0.7.53-11

[Ingo Molnar]

* Alan Stern <stern@rowland.harvard.edu> wrote:

> 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.

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

	Ingo

Re: [patch] Real-Time Preemption, -RT-2.6.13-rc6-V0.7.53-11

[David Brownell]

> > > 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.

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.


> > > 			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.
> > 
> > so we can remove it altogether, via the patch below?

Sounds dangerous to me.


> > 					(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.
>
> 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.

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

Re: [patch] Real-Time Preemption, -RT-2.6.13-rc6-V0.7.53-11

[Alan Stern]

On Tue, 16 Aug 2005, David Brownell wrote:

> > > > 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.
> 
> 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.

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.


>   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.

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

Re: [patch] Real-Time Preemption, -RT-2.6.13-rc6-V0.7.53-11

[David Brownell]

> > > > > 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.
> > 
> > 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.
>
> Maybe Ingo's priorities are sufficiently different that he thinks the
> return _will_ be worthwhile.

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.  :)


>		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.

Yes, both HCDs and usbcore.


> >   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 ...
>
> I'm in favor of #2 on general principle.

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.


>		The issue you mention,
> maintaining bandwidth reservations (and hardware data structures) for
> periodic endpoint queues, is not all that difficult to handle.

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

Re: [patch] Real-Time Preemption, -RT-2.6.13-rc6-V0.7.53-11

[Ingo Molnar]

* David Brownell <david-b@pacbell.net> wrote:

> > >   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 ...
> >
> > I'm in favor of #2 on general principle.
> 
> Which principle would that be, though?  :)

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

Re: [patch] Real-Time Preemption, -RT-2.6.13-rc6-V0.7.53-11

[David Brownell]

> > > > We couldn't switch to #2 with patches that simple.  They'd in fact
> > > > be rather involved ...
> > >
> > > I'm in favor of #2 on general principle.
> > 
> > Which principle would that be, though?  :)
>
> it's the basic Linux kernel principle of never disabling interrupts, 
> unless really, really necessary.

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.


> 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.

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.


> 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.)

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".


> 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)

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


> 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.

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

Re: [patch] Real-Time Preemption, -RT-2.6.13-rc6-V0.7.53-11

[Alan Stern]

On Thu, 18 Aug 2005, Ingo Molnar wrote:

> but fortunately this is a relatively straightforward process, and it
> seems Alan has identified most of the affected functions. 

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.


On Wed, 17 Aug 2005, David Brownell wrote:

> 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.

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

> 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.

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.)

> 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".

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

Re: [patch] Real-Time Preemption, -RT-2.6.13-rc6-V0.7.53-11

[Ingo Molnar]

* David Brownell <david-b@pacbell.net> wrote:

> > > > > We couldn't switch to #2 with patches that simple.  They'd in fact
> > > > > be rather involved ...
> > > >
> > > > I'm in favor of #2 on general principle.
> > > 
> > > Which principle would that be, though?  :)
> >
> > it's the basic Linux kernel principle of never disabling interrupts, 
> > unless really, really necessary.
> 
> 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.

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.

> > 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.
> 
> 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.

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

> > 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.)
> 
> 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".

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.

> > 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)
> 
> I think all the locks inside usbcore and the HCDs will end up being 
> "irqsafe"...

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

Re: [patch] Real-Time Preemption, -RT-2.6.13-rc6-V0.7.53-11

[Ingo Molnar]

* Alan Stern <stern@rowland.harvard.edu> wrote:

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

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

	Ingo

Index: linux/drivers/usb/core/devio.c
===================================================================
--- linux.orig/drivers/usb/core/devio.c
+++ 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;

2.6.13-rc6-rt1

[Ingo Molnar]

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

Re: 2.6.13-rc6-rt1

[Michal Schmidt]

[-- Attachment #1: Type: text/plain, Size: 725 bytes --]

Ingo Molnar wrote:
> 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.

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

[-- Attachment #2: ketchup-rt.diff --]
[-- Type: text/plain, Size: 648 bytes --]

--- ketchup-0.9	2005-08-16 14:06:20.000000000 +0200
+++ ketchup-0.9+rt	2005-08-16 14:24:05.000000000 +0200
@@ -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):

Re: 2.6.13-rc6-rt1

[Matt Mackall]

On Tue, Aug 16, 2005 at 02:32:01PM +0200, Michal Schmidt wrote:
> Ingo Molnar wrote:
> >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.
> 
> 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?

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.

Re: 2.6.13-rc6-rt1

[Esben Nielsen]

On Fri, 26 Aug 2005, Matt Mackall wrote:

> On Tue, Aug 16, 2005 at 02:32:01PM +0200, Michal Schmidt wrote:
> > Ingo Molnar wrote:
> > >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.
> > 
> > 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?
> 
> 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.
> 
I added the line in the patch below to be able to get Ingo's older
patches.

Esben

diff -r 1342be306020 ketchup
--- a/ketchup   Sat Aug 27 01:12:42 2005
+++ b/ketchup   Tue Aug 30 00:30:23 2005
@@ -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

[patch] KGDB for Real-Time Preemption systems

[George Anzinger]

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   george@mvista.com
HRT (High-res-timers):  http://sourceforge.net/projects/high-res-timers/

Re: [patch] KGDB for Real-Time Preemption systems

[Ingo Molnar]

* George Anzinger <george@mvista.com> wrote:

> 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.

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

Re: [patch] KGDB for Real-Time Preemption systems

[George Anzinger]

Ingo Molnar wrote:
> * George Anzinger <george@mvista.com> wrote:
> 
> 
>>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.
> 
> 
> 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.

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   george@mvista.com
HRT (High-res-timers):  http://sourceforge.net/projects/high-res-timers/

Re: [patch] KGDB for Real-Time Preemption systems

[Serge Noiraud]

[-- Attachment #1: Type: text/plain, Size: 772 bytes --]

mercredi 17 Août 2005 02:53, George Anzinger wrote/a écrit :
> 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.

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.

[-- Attachment #2: diff.kgdb-ga-rt --]
[-- Type: text/x-diff, Size: 1233 bytes --]

--- kgdb-ga-rt.patch.org	2005-09-05 12:00:17.103019648 +0200
+++ kgdb-ga-rt.patch	2005-09-05 12:08:04.561955088 +0200
@@ -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__ */

Re: [patch] KGDB for Real-Time Preemption systems

[George Anzinger]

Serge Noiraud wrote:
> mercredi 17 Août 2005 02:53, George Anzinger wrote/a écrit :
> 
>>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.
> 
> 
> 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.

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   george@mvista.com
HRT (High-res-timers):  http://sourceforge.net/projects/high-res-timers/

Re: [patch] KGDB for Real-Time Preemption systems

[Serge Noiraud]

mercredi 17 Août 2005 02:53, George Anzinger wrote/a écrit :
> 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.

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

Re: [patch] KGDB for Real-Time Preemption systems

[George Anzinger]

Serge Noiraud wrote:
> mercredi 17 Août 2005 02:53, George Anzinger wrote/a écrit :
> 
>>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.
> 
> 
> I'm trying this kgdb patch with 2.6.13 and I get the following errors.
> Is there something I forgot ?

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
> 
> ...
>   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
> -
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/
> 

-- 
George Anzinger   george@mvista.com
HRT (High-res-timers):  http://sourceforge.net/projects/high-res-timers/

Re: [patch] KGDB for Real-Time Preemption systems

[Serge Noiraud]

mercredi 7 Septembre 2005 23:16, George Anzinger wrote/a écrit :
> Serge Noiraud wrote:
...
> >
> > I'm trying this kgdb patch with 2.6.13 and I get the following errors.
> > Is there something I forgot ?
>
> 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...)
I don't get those errors with CONFIG_KGDB=n
bellow I put the diff between a working . config and a non working .config
>
> George
>
> > ...
> >   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:
...
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.

--- .config.orig    2005-08-01 10:29:22.740759504 +0200
+++ .config_dbg 2005-08-01 10:40:30.534239464 +0200
@@ -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

 #

Re: [patch] KGDB for Real-Time Preemption systems

[George Anzinger]

Serge Noiraud wrote:
> mercredi 7 Septembre 2005 23:16, George Anzinger wrote/a écrit :
> 
>>Serge Noiraud wrote:
> 
> ...
> 
>>>I'm trying this kgdb patch with 2.6.13 and I get the following errors.
>>>Is there something I forgot ?

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/
>>
>>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...)
> 
> I don't get those errors with CONFIG_KGDB=n
> bellow I put the diff between a working . config and a non working .config
> 
>>George
>>
>>
>>>...
>>>  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:
> 
> ...
> 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!

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.

> #
> 
~
> -# 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 <--------------------- This one is doing it............
> +CONFIG_DEBUG_KOBJECT=y
> +CONFIG_DEBUG_HIGHMEM=y
~
> +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

The following are not in the latest kgdb...............
> +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  <--- I recommend against this one, see notes at front of kgdb patch
> +CONFIG_KGDB_CONSOLE=y            <--- Likewise use this only if you have only one serial port and no VGA
> +CONFIG_KGDB_SYSRQ=y
> 
>  #
> 
> -
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/
> 

-- 
George Anzinger   george@mvista.com
HRT (High-res-timers):  http://sourceforge.net/projects/high-res-timers/

Re: [patch] Real-Time Preemption, -RT-2.6.13-rc4-V0.7.53-01, High Resolution Timers & RCU-tasklist features

[Guillaume Foliard]

[-- Attachment #1: Type: text/plain, Size: 677 bytes --]

Hi,

Here is the compilation error I had with 0.7.53-02 :


kernel/rcupdate.c: In function 'rcu_init':
kernel/rcupdate.c:818: error: invalid storage class for function 'rcu_torture_init'
kernel/rcupdate.c:820: warning: implicit declaration of function 'rcu_torture_init'
kernel/rcupdate.c: At top level:
kernel/rcupdate.c:1187: warning: conflicting types for 'rcu_torture_init'
kernel/rcupdate.c:1187: error: static declaration of 'rcu_torture_init' follows non-static declaration
kernel/rcupdate.c:820: error: previous implicit declaration of 'rcu_torture_init' was here

The .config is attached.
"gcc --version" gives : gcc (GCC) 4.0.2 20050806 (prerelease) (Debian 4.0.1-4)

[-- Attachment #2: config.gz --]
[-- Type: application/x-gzip, Size: 9055 bytes --]

Re: [patch] Real-Time Preemption, -RT-2.6.13-rc4-V0.7.53-01, High Resolution Timers & RCU-tasklist features

[Ingo Molnar]

On Thu, 11 Aug 2005, Guillaume Foliard wrote:

> Hi,
> 
> Here is the compilation error I had with 0.7.53-02 :

thanks - i've uploaded the -53-05 patch which should fix this - does it
build/work for you now?

	Ingo

Re: [patch] Real-Time Preemption, -RT-2.6.13-rc4-V0.7.53-01, High Resolution Timers & RCU-tasklist features

[Guillaume Foliard]

On Friday 12 August 2005 14:53, Ingo Molnar wrote:
> On Thu, 11 Aug 2005, Guillaume Foliard wrote:
> > Hi,
> >
> > Here is the compilation error I had with 0.7.53-02 :
>
> thanks - i've uploaded the -53-05 patch which should fix this - does it
> build/work for you now?

I've tried -53-07. Build is ok. Kernel has booted and is running.

Thank you.

Guillaume

Re: [patch] Real-Time Preemption, -RT-2.6.13-rc4-V0.7.53-01, High Resolution Timers & RCU-tasklist features

[Guillaume Foliard]

On Friday 12 August 2005 21:27, Guillaume Foliard wrote:
> On Friday 12 August 2005 14:53, Ingo Molnar wrote:
> > On Thu, 11 Aug 2005, Guillaume Foliard wrote:
> > > Hi,
> > >
> > > Here is the compilation error I had with 0.7.53-02 :
> >
> > thanks - i've uploaded the -53-05 patch which should fix this - does it
> > build/work for you now?
>
> I've tried -53-07. Build is ok. Kernel has booted and is running.

I'm running dmesg from times to times and just catched this one :

BUG: kblockd/0/84: leaked RT prio 98 (110)?
 [<c0139e02>] up_mutex+0xd1/0x11e (8)
 [<c014b0a1>] kmem_cache_alloc+0x5d/0xe1 (32)
 [<c02efd97>] __scsi_get_command+0x23/0x66 (32)
 [<c02efe0f>] scsi_get_command+0x35/0x9e (28)
 [<c02f5a97>] scsi_prep_fn+0x120/0x220 (28)
 [<c02a35f8>] elv_next_request+0x5c/0x1bd (36)
 [<c029e7ff>] get_device+0x1b/0x24 (12)
 [<c02f5c8b>] scsi_request_fn+0x4b/0x3a0 (16)
 [<c02ac92f>] as_work_handler+0x2b/0x3d (40)
 [<c013093d>] worker_thread+0x193/0x259 (16)
 [<c02ac904>] as_work_handler+0x0/0x3d (32)
 [<c011b8e2>] default_wake_function+0x0/0x22 (32)
 [<c01307aa>] worker_thread+0x0/0x259 (32)
 [<c0135b73>] kthread+0x98/0x9c (4)
 [<c0135adb>] kthread+0x0/0x9c (24)
 [<c0101011>] kernel_thread_helper+0x5/0xb (16)
---------------------------
| preempt count: 00000001 ]
| 1-level deep critical section nesting:
----------------------------------------
.. [<c0139d4e>] .... up_mutex+0x1d/0x11e
.....[<00000000>] ..   ( <= _stext+0x3feffde0/0x47)


Also, during boot this one occured :

WARNING: swapper/1 changed soft IRQ-flags.
 [<c032ce28>] rh_call_control+0xd9/0x3f2 (8)
 [<c013a82b>] sub_preempt_count+0x18/0x1c (20)
 [<c0139da4>] up_mutex+0x73/0x11e (28)
 [<c032df0b>] hcd_submit_urb+0x1e1/0x22e (32)
 [<c032ee5a>] usb_start_wait_urb+0x69/0x10c (44)
 [<c013a82b>] sub_preempt_count+0x18/0x1c (64)
 [<c013934d>] __init_rt_mutex+0x37/0x56 (12)
 [<c013a440>] _spin_lock_init+0x30/0x3c (16)
 [<c032ef8c>] usb_internal_control_msg+0x8f/0x9b (28)
 [<c032f02c>] usb_control_msg+0x94/0xa8 (28)
 [<c032f8ed>] usb_get_descriptor+0x94/0xc8 (56)
 [<c014b001>] cache_flusharray+0x6a/0xad (40)
 [<c032fd2a>] usb_get_device_descriptor+0x62/0x8e (32)
 [<c032abda>] usb_set_device_state+0x18/0x54 (24)
 [<c032d860>] register_root_hub+0x6e/0x148 (20)
 [<c032e69c>] usb_add_hcd+0x1ef/0x3aa (40)
 [<c0332c9d>] usb_hcd_pci_probe+0x1cd/0x2f9 (56)
 [<c02445de>] pci_match_device+0x1e/0xc8 (20)
 [<c02446cc>] __pci_device_probe+0x44/0x4f (28)
 [<c0244700>] pci_device_probe+0x29/0x3f (20)
 [<c029fa56>] driver_probe_device+0x3d/0xa7 (20)
 [<c048c86f>] klist_dec_and_del+0x1a/0x1e (12)
 [<c029fb34>] __driver_attach+0x0/0x4e (16)
 [<c029fb73>] __driver_attach+0x3f/0x4e (4)
 [<c029f17c>] bus_for_each_dev+0x5b/0x7b (20)
 [<c029fba8>] driver_attach+0x26/0x2a (44)
 [<c029fb34>] __driver_attach+0x0/0x4e (16)
 [<c029f61a>] bus_add_driver+0x6e/0xce (4)
 [<c0244932>] pci_register_driver+0x89/0xab (32)
 [<c0617b70>] init+0x1d/0x25 (24)
 [<c05fe83d>] do_initcalls+0x54/0xce (8)
 [<c01002a2>] init+0x0/0x1ae (16)
 [<c01002d5>] init+0x33/0x1ae (16)
 [<c010100c>] kernel_thread_helper+0x0/0xb (12)
 [<c0101011>] kernel_thread_helper+0x5/0xb (4)
---------------------------
| preempt count: 00000000 ]
| 0-level deep critical section nesting:
----------------------------------------



Guillaume

Re: [patch] Real-Time Preemption, -RT-2.6.13-rc4-V0.7.53-01, High Resolution Timers & RCU-tasklist features

[Chuck Harding]

On Fri, 12 Aug 2005, Guillaume Foliard wrote:

> On Friday 12 August 2005 14:53, Ingo Molnar wrote:
>> On Thu, 11 Aug 2005, Guillaume Foliard wrote:
>>> Hi,
>>>
>>> Here is the compilation error I had with 0.7.53-02 :
>>
>> thanks - i've uploaded the -53-05 patch which should fix this - does it
>> build/work for you now?
>
> I've tried -53-07. Build is ok. Kernel has booted and is running.
>
> Thank you.
>
> Guillaume
>

I still get a compile error in drivers/ide/ide-taskfile.c because the
conditionals around references to flag are not consistant. The patch
I sent for -53-06 should still work to correct this.

-- 
Charles D. (Chuck) Harding <charding@llnl.gov>  Voice: 925-423-8879
Senior Computer Associate         ICCD            Fax: 925-423-6961
Lawrence Livermore National Laboratory      Computation Directorate
Livermore, CA USA  http://www.llnl.gov  GPG Public Key ID: B9EB6601
------------------ http://tinyurl.com/5w5ey -----------------------
-- Fad: In one era and out the other. --