Re: [PATCH] sched/clock: prevent tracing recursion in sched_clock_cpu()
On 03/06/2014 07:51 PM, Steven Rostedt wrote:
On Thu, 06 Mar 2014 14:25:28 +0900
Fernando Luis Vázquez Cao wrote:
From: Fernando Luis Vazquez Cao
Prevent tracing of preempt_disable/enable() in sched_clock_cpu().
When CONFIG_DEBUG_PREEMPT is enabled, preempt_disable/enable() are
traced and this causes trace_clock() users (and probably others) to
go into an infinite recursion. Systems with a stable sched_clock()
are not affected.
This problem is similar to that fixed by upstream commit 95ef1e52922
("KVM guest: prevent tracing recursion with kvmclock").
Also similar to: 569d6557ab957.
Acked-by: Steven Rostedt
Thank you four your review, Peter, Steven.
By the way, who is going to pick this patch? Do you want
me to resend with Steven's Acked-by added?
- Fernando
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@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] sched/clock: prevent tracing recursion in sched_clock_cpu()
On 03/06/2014 07:51 PM, Steven Rostedt wrote: On Thu, 06 Mar 2014 14:25:28 +0900 Fernando Luis Vázquez Cao fernando...@lab.ntt.co.jp wrote: From: Fernando Luis Vazquez Cao ferna...@oss.ntt.co.jp Prevent tracing of preempt_disable/enable() in sched_clock_cpu(). When CONFIG_DEBUG_PREEMPT is enabled, preempt_disable/enable() are traced and this causes trace_clock() users (and probably others) to go into an infinite recursion. Systems with a stable sched_clock() are not affected. This problem is similar to that fixed by upstream commit 95ef1e52922 (KVM guest: prevent tracing recursion with kvmclock). Also similar to: 569d6557ab957. Acked-by: Steven Rostedt rost...@goodmis.org Thank you four your review, Peter, Steven. By the way, who is going to pick this patch? Do you want me to resend with Steven's Acked-by added? - Fernando -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH] sched/clock: prevent tracing recursion in sched_clock_cpu()
From: Fernando Luis Vazquez Cao
Prevent tracing of preempt_disable/enable() in sched_clock_cpu().
When CONFIG_DEBUG_PREEMPT is enabled, preempt_disable/enable() are
traced and this causes trace_clock() users (and probably others) to
go into an infinite recursion. Systems with a stable sched_clock()
are not affected.
This problem is similar to that fixed by upstream commit 95ef1e52922
("KVM guest: prevent tracing recursion with kvmclock").
Cc: Linus Torvalds
Cc: Andrew Morton
Cc: Ingo Molnar
Cc: Steven Rostedt
Signed-off-by: Fernando Luis Vazquez Cao
---
diff -urNp linux-3.14-rc5-orig/kernel/sched/clock.c
linux-3.14-rc5/kernel/sched/clock.c
--- linux-3.14-rc5-orig/kernel/sched/clock.c2014-03-06 13:37:43.567720550
+0900
+++ linux-3.14-rc5/kernel/sched/clock.c 2014-03-06 13:41:56.937100949 +0900
@@ -301,14 +301,14 @@ u64 sched_clock_cpu(int cpu)
if (unlikely(!sched_clock_running))
return 0ull;
- preempt_disable();
+ preempt_disable_notrace();
scd = cpu_sdc(cpu);
if (cpu != smp_processor_id())
clock = sched_clock_remote(scd);
else
clock = sched_clock_local(scd);
- preempt_enable();
+ preempt_enable_notrace();
return clock;
}
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH] sched/clock: prevent tracing recursion in sched_clock_cpu()
From: Fernando Luis Vazquez Cao ferna...@oss.ntt.co.jp Prevent tracing of preempt_disable/enable() in sched_clock_cpu(). When CONFIG_DEBUG_PREEMPT is enabled, preempt_disable/enable() are traced and this causes trace_clock() users (and probably others) to go into an infinite recursion. Systems with a stable sched_clock() are not affected. This problem is similar to that fixed by upstream commit 95ef1e52922 (KVM guest: prevent tracing recursion with kvmclock). Cc: Linus Torvalds torva...@linux-foundation.org Cc: Andrew Morton a...@linux-foundation.org Cc: Ingo Molnar mi...@kernel.org Cc: Steven Rostedt rost...@goodmis.org Signed-off-by: Fernando Luis Vazquez Cao ferna...@oss.ntt.co.jp --- diff -urNp linux-3.14-rc5-orig/kernel/sched/clock.c linux-3.14-rc5/kernel/sched/clock.c --- linux-3.14-rc5-orig/kernel/sched/clock.c2014-03-06 13:37:43.567720550 +0900 +++ linux-3.14-rc5/kernel/sched/clock.c 2014-03-06 13:41:56.937100949 +0900 @@ -301,14 +301,14 @@ u64 sched_clock_cpu(int cpu) if (unlikely(!sched_clock_running)) return 0ull; - preempt_disable(); + preempt_disable_notrace(); scd = cpu_sdc(cpu); if (cpu != smp_processor_id()) clock = sched_clock_remote(scd); else clock = sched_clock_local(scd); - preempt_enable(); + preempt_enable_notrace(); return clock; } -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@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 2/4] nohz: Synchronize sleep time stats with seqlock
(2013年08月17日 01:46), Frederic Weisbecker wrote:
On Fri, Aug 16, 2013 at 06:26:54PM +0200, Oleg Nesterov wrote:
On 08/16, Frederic Weisbecker wrote:
On Fri, Aug 16, 2013 at 06:02:01PM +0200, Oleg Nesterov wrote:
+ do {
+ seq = read_seqcount_begin(>sleeptime_seq);
+ if (ts->idle_active && nr_iowait_cpu(cpu) > 0) {
+ ktime_t delta = ktime_sub(now, ts->idle_entrytime);
+ iowait = ktime_add(ts->iowait_sleeptime, delta);
+ } else {
+ iowait = ts->iowait_sleeptime;
+ }
+ } while (read_seqcount_retry(>sleeptime_seq, seq));
Unless I missread this patch, this is still racy a bit.
Suppose it is called on CPU_0 and cpu == 1. Suppose that
ts->idle_active == T and nr_iowait_cpu(cpu) == 1.
So we return iowait_sleeptime + delta.
Suppose that we call get_cpu_iowait_time_us() again. By this time
the task which incremented ->nr_iowait can be woken up on another
CPU, and it can do atomic_dec(rq->nr_iowait). So the next time
we return iowait_sleeptime, and this is not monotonic again.
Hmm, by the time it decrements nr_iowait, it returned from schedule() and
so idle had flushed the pending iowait sleeptime.
Suppose a task does io_schedule() on CPU_0, and increments the counter.
This CPU becomes idle and nr_iowait_cpu(0) == 1.
Then this task is woken up, but try_to_wake_up() selects another CPU != 0.
It returns from schedule() and decrements the same counter, it doesn't
do raw_rq/etc again. nr_iowait_cpu(0) becomes 0.
In fact the task can even migrate to another CPU right after raw_rq().
Ah I see now. So that indeed yet another race.
I am sorry for chiming in late.
That precisely the race I described here:
https://lkml.org/lkml/2013/7/2/3
I should have been more concise in my explanation. I apologize.
Should we flush that iowait to the src CPU? But then it means we must handle
concurrent updates to iowait_sleeptime, idle_sleeptime from the migration
code and from idle enter / exit.
So I fear we need a seqlock.
Or we can live with that and still account the whole idle time slept until
tick_nohz_stop_idle() to iowait if we called tick_nohz_start_idle() with nr_iowait
> 0.
All we need is just a new field in ts-> that records on which state we entered
idle.
Another approach could be to shadow ->iowait_sleeptime as
suggested here: https://lkml.org/lkml/2013/7/2/165
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@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 2/4] nohz: Synchronize sleep time stats with seqlock
(2013年08月19日 20:10), Peter Zijlstra wrote: On Fri, Aug 16, 2013 at 06:46:28PM +0200, Frederic Weisbecker wrote: Option A: Should we flush that iowait to the src CPU? But then it means we must handle concurrent updates to iowait_sleeptime, idle_sleeptime from the migration code and from idle enter / exit. So I fear we need a seqlock. Option B: Or we can live with that and still account the whole idle time slept until tick_nohz_stop_idle() to iowait if we called tick_nohz_start_idle() with nr_iowait > 0. All we need is just a new field in ts-> that records on which state we entered idle. What do you think? I think option B is unworkable. Afaict it could basically caused unlimited iowait time. Suppose we have a load-balancer that tries it bestestest to sort-left (ie. run a task on the lowest 'free' cpu possible) -- the power aware folks are pondering such schemes. Now suppose we have a small burst of activity and the rightmost cpu gets to run something that goes to sleep on iowait. We'd accrue iowait on that cpu until it wakes up, which could be days from now if the load stays low enough, even though the task got to run almost instantly on another cpu. So no, if we need per-cpu iowait time we have to do A. Since we already have atomics in the io_schedule*() paths, please replace those with (seq)locks. Also see if you can place the entire iowait accounting thing in a separate cacheline. I considered option A for a while but, fearing it would be considered overkill, took a different approach: create a shadow copy of ->iowait_sleeptime that is always kept monotonic (artificially in some cases) and use that to compute the values exported through /proc. That said, if deemed acceptable, option A is the one I would choose. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@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 2/4] nohz: Synchronize sleep time stats with seqlock
(2013年08月19日 20:10), Peter Zijlstra wrote: On Fri, Aug 16, 2013 at 06:46:28PM +0200, Frederic Weisbecker wrote: Option A: Should we flush that iowait to the src CPU? But then it means we must handle concurrent updates to iowait_sleeptime, idle_sleeptime from the migration code and from idle enter / exit. So I fear we need a seqlock. Option B: Or we can live with that and still account the whole idle time slept until tick_nohz_stop_idle() to iowait if we called tick_nohz_start_idle() with nr_iowait 0. All we need is just a new field in ts- that records on which state we entered idle. What do you think? I think option B is unworkable. Afaict it could basically caused unlimited iowait time. Suppose we have a load-balancer that tries it bestestest to sort-left (ie. run a task on the lowest 'free' cpu possible) -- the power aware folks are pondering such schemes. Now suppose we have a small burst of activity and the rightmost cpu gets to run something that goes to sleep on iowait. We'd accrue iowait on that cpu until it wakes up, which could be days from now if the load stays low enough, even though the task got to run almost instantly on another cpu. So no, if we need per-cpu iowait time we have to do A. Since we already have atomics in the io_schedule*() paths, please replace those with (seq)locks. Also see if you can place the entire iowait accounting thing in a separate cacheline. I considered option A for a while but, fearing it would be considered overkill, took a different approach: create a shadow copy of -iowait_sleeptime that is always kept monotonic (artificially in some cases) and use that to compute the values exported through /proc. That said, if deemed acceptable, option A is the one I would choose. -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@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 2/4] nohz: Synchronize sleep time stats with seqlock
(2013年08月17日 01:46), Frederic Weisbecker wrote:
On Fri, Aug 16, 2013 at 06:26:54PM +0200, Oleg Nesterov wrote:
On 08/16, Frederic Weisbecker wrote:
On Fri, Aug 16, 2013 at 06:02:01PM +0200, Oleg Nesterov wrote:
+ do {
+ seq = read_seqcount_begin(ts-sleeptime_seq);
+ if (ts-idle_active nr_iowait_cpu(cpu) 0) {
+ ktime_t delta = ktime_sub(now, ts-idle_entrytime);
+ iowait = ktime_add(ts-iowait_sleeptime, delta);
+ } else {
+ iowait = ts-iowait_sleeptime;
+ }
+ } while (read_seqcount_retry(ts-sleeptime_seq, seq));
Unless I missread this patch, this is still racy a bit.
Suppose it is called on CPU_0 and cpu == 1. Suppose that
ts-idle_active == T and nr_iowait_cpu(cpu) == 1.
So we return iowait_sleeptime + delta.
Suppose that we call get_cpu_iowait_time_us() again. By this time
the task which incremented -nr_iowait can be woken up on another
CPU, and it can do atomic_dec(rq-nr_iowait). So the next time
we return iowait_sleeptime, and this is not monotonic again.
Hmm, by the time it decrements nr_iowait, it returned from schedule() and
so idle had flushed the pending iowait sleeptime.
Suppose a task does io_schedule() on CPU_0, and increments the counter.
This CPU becomes idle and nr_iowait_cpu(0) == 1.
Then this task is woken up, but try_to_wake_up() selects another CPU != 0.
It returns from schedule() and decrements the same counter, it doesn't
do raw_rq/etc again. nr_iowait_cpu(0) becomes 0.
In fact the task can even migrate to another CPU right after raw_rq().
Ah I see now. So that indeed yet another race.
I am sorry for chiming in late.
That precisely the race I described here:
https://lkml.org/lkml/2013/7/2/3
I should have been more concise in my explanation. I apologize.
Should we flush that iowait to the src CPU? But then it means we must handle
concurrent updates to iowait_sleeptime, idle_sleeptime from the migration
code and from idle enter / exit.
So I fear we need a seqlock.
Or we can live with that and still account the whole idle time slept until
tick_nohz_stop_idle() to iowait if we called tick_nohz_start_idle() with nr_iowait
0.
All we need is just a new field in ts- that records on which state we entered
idle.
Another approach could be to shadow -iowait_sleeptime as
suggested here: https://lkml.org/lkml/2013/7/2/165
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH] x86, 64bit: do not assume CPU is NX capable when setting early page tables
The kernel sets the NX bit in the early page tables without checking whether the CPU actually supports this feature. If it doesn't the first attempt to use them will cause a kernel hang. Since these are temporary page tables marked as initdata this fix takes the approach of not bothering with the NX bit at all. Noticed when my AMD machine that happened to have the NX feature disabled by the BIOS failed to boot after the update to 3.9. Cc: sta...@vger.kernel.org Signed-off-by: Fernando Luis Vazquez Cao --- diff -urNp linux-3.9/arch/x86/kernel/head64.c linux-3.9-fix/arch/x86/kernel/head64.c --- linux-3.9/arch/x86/kernel/head64.c 2013-04-29 09:36:01.0 +0900 +++ linux-3.9-fix/arch/x86/kernel/head64.c 2013-05-02 14:38:52.589276092 +0900 @@ -99,7 +99,7 @@ again: pmd_p[i] = 0; *pud_p = (pudval_t)pmd_p - __START_KERNEL_map + phys_base + _KERNPG_TABLE; } - pmd = (physaddr & PMD_MASK) + (__PAGE_KERNEL_LARGE & ~_PAGE_GLOBAL); + pmd = (physaddr & PMD_MASK) + (__PAGE_KERNEL_LARGE_EXEC & ~_PAGE_GLOBAL); pmd_p[pmd_index(address)] = pmd; return 0; -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH] x86, 64bit: do not assume CPU is NX capable when setting early page tables
The kernel sets the NX bit in the early page tables without checking whether the CPU actually supports this feature. If it doesn't the first attempt to use them will cause a kernel hang. Since these are temporary page tables marked as initdata this fix takes the approach of not bothering with the NX bit at all. Noticed when my AMD machine that happened to have the NX feature disabled by the BIOS failed to boot after the update to 3.9. Cc: sta...@vger.kernel.org Signed-off-by: Fernando Luis Vazquez Cao ferna...@oss.ntt.co.jp --- diff -urNp linux-3.9/arch/x86/kernel/head64.c linux-3.9-fix/arch/x86/kernel/head64.c --- linux-3.9/arch/x86/kernel/head64.c 2013-04-29 09:36:01.0 +0900 +++ linux-3.9-fix/arch/x86/kernel/head64.c 2013-05-02 14:38:52.589276092 +0900 @@ -99,7 +99,7 @@ again: pmd_p[i] = 0; *pud_p = (pudval_t)pmd_p - __START_KERNEL_map + phys_base + _KERNPG_TABLE; } - pmd = (physaddr PMD_MASK) + (__PAGE_KERNEL_LARGE ~_PAGE_GLOBAL); + pmd = (physaddr PMD_MASK) + (__PAGE_KERNEL_LARGE_EXEC ~_PAGE_GLOBAL); pmd_p[pmd_index(address)] = pmd; return 0; -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH] HID: fix botched tree merge that disabled fix-up for certain Sony RF receivers
It looks like the manual merge 0d69a3c731e120b05b7da9fb976830475a3fbc01 ("Merge
branches 'for-3.9/sony' and 'for-3.9/steelseries' into for-linus") accidentally
removed Sony RF receiver with USB product id 0x0374 from the "have special
driver" list, effectively nullifying a464918419f94a0043d2f549d6defb4c3f69f68a
("HID: add support for Sony RF receiver with USB product id 0x0374"). Add the
device back to the list.
Cc: sta...@vger.kernel.org
Signed-off-by: Fernando Luis Vazquez Cao
---
diff -urNp linux-3.9/drivers/hid/hid-core.c linux-3.9-fix/drivers/hid/hid-core.c
--- linux-3.9/drivers/hid/hid-core.c2013-04-29 09:36:01.0 +0900
+++ linux-3.9-fix/drivers/hid/hid-core.c2013-04-30 21:53:57.596269692
+0900
@@ -1702,6 +1702,7 @@ static const struct hid_device_id hid_ha
{ HID_USB_DEVICE(USB_VENDOR_ID_SONY,
USB_DEVICE_ID_SONY_NAVIGATION_CONTROLLER) },
{ HID_BLUETOOTH_DEVICE(USB_VENDOR_ID_SONY,
USB_DEVICE_ID_SONY_PS3_CONTROLLER) },
{ HID_USB_DEVICE(USB_VENDOR_ID_SONY, USB_DEVICE_ID_SONY_VAIO_VGX_MOUSE)
},
+ { HID_USB_DEVICE(USB_VENDOR_ID_SONY, USB_DEVICE_ID_SONY_VAIO_VGP_MOUSE)
},
{ HID_USB_DEVICE(USB_VENDOR_ID_STEELSERIES,
USB_DEVICE_ID_STEELSERIES_SRWS1) },
{ HID_USB_DEVICE(USB_VENDOR_ID_SUNPLUS, USB_DEVICE_ID_SUNPLUS_WDESKTOP)
},
{ HID_USB_DEVICE(USB_VENDOR_ID_THINGM, USB_DEVICE_ID_BLINK1) },
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@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 v2] HID: add support for Sony RF receiver with USB product id 0x0374
Hi Jiri, On Tue, 2013-01-15 at 17:02 +0100, Jiri Kosina wrote: > On Tue, 15 Jan 2013, Fernando Luis Vázquez Cao wrote: > > > Some Vaio desktop computers, among them the VGC-LN51JGB multimedia PC, have > > a RF receiver, multi-interface USB device 054c:0374, that is used to connect > > a wireless keyboard and a wireless mouse. > > > > The keyboard works flawlessly, but the mouse (VGP-WMS3 in my case) does not > > seem to be generating any pointer events. The problem is that the mouse > > pointer > > is wrongly declared as a constant non-data variable in the report descriptor > > (see lsusb and usbhid-dump output below), with the consequence that it is > > ignored by the HID code. > > > > Add this device to the have-special-driver list and fix up the report > > descriptor in the Sony-specific driver which happens to already have a fixup > > for a similar firmware bug. > > Applied, thanks. It looks like after the merge of the the sony and steelseries branches the hid core hunk was left out, which means that this fix-up is never applied. I will be replying to this email with a fix. Thanks, Fernando -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@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 v2] HID: add support for Sony RF receiver with USB product id 0x0374
Hi Jiri, On Tue, 2013-01-15 at 17:02 +0100, Jiri Kosina wrote: On Tue, 15 Jan 2013, Fernando Luis Vázquez Cao wrote: Some Vaio desktop computers, among them the VGC-LN51JGB multimedia PC, have a RF receiver, multi-interface USB device 054c:0374, that is used to connect a wireless keyboard and a wireless mouse. The keyboard works flawlessly, but the mouse (VGP-WMS3 in my case) does not seem to be generating any pointer events. The problem is that the mouse pointer is wrongly declared as a constant non-data variable in the report descriptor (see lsusb and usbhid-dump output below), with the consequence that it is ignored by the HID code. Add this device to the have-special-driver list and fix up the report descriptor in the Sony-specific driver which happens to already have a fixup for a similar firmware bug. Applied, thanks. It looks like after the merge of the the sony and steelseries branches the hid core hunk was left out, which means that this fix-up is never applied. I will be replying to this email with a fix. Thanks, Fernando -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH] HID: fix botched tree merge that disabled fix-up for certain Sony RF receivers
It looks like the manual merge 0d69a3c731e120b05b7da9fb976830475a3fbc01 (Merge
branches 'for-3.9/sony' and 'for-3.9/steelseries' into for-linus) accidentally
removed Sony RF receiver with USB product id 0x0374 from the have special
driver list, effectively nullifying a464918419f94a0043d2f549d6defb4c3f69f68a
(HID: add support for Sony RF receiver with USB product id 0x0374). Add the
device back to the list.
Cc: sta...@vger.kernel.org
Signed-off-by: Fernando Luis Vazquez Cao ferna...@oss.ntt.co.jp
---
diff -urNp linux-3.9/drivers/hid/hid-core.c linux-3.9-fix/drivers/hid/hid-core.c
--- linux-3.9/drivers/hid/hid-core.c2013-04-29 09:36:01.0 +0900
+++ linux-3.9-fix/drivers/hid/hid-core.c2013-04-30 21:53:57.596269692
+0900
@@ -1702,6 +1702,7 @@ static const struct hid_device_id hid_ha
{ HID_USB_DEVICE(USB_VENDOR_ID_SONY,
USB_DEVICE_ID_SONY_NAVIGATION_CONTROLLER) },
{ HID_BLUETOOTH_DEVICE(USB_VENDOR_ID_SONY,
USB_DEVICE_ID_SONY_PS3_CONTROLLER) },
{ HID_USB_DEVICE(USB_VENDOR_ID_SONY, USB_DEVICE_ID_SONY_VAIO_VGX_MOUSE)
},
+ { HID_USB_DEVICE(USB_VENDOR_ID_SONY, USB_DEVICE_ID_SONY_VAIO_VGP_MOUSE)
},
{ HID_USB_DEVICE(USB_VENDOR_ID_STEELSERIES,
USB_DEVICE_ID_STEELSERIES_SRWS1) },
{ HID_USB_DEVICE(USB_VENDOR_ID_SUNPLUS, USB_DEVICE_ID_SUNPLUS_WDESKTOP)
},
{ HID_USB_DEVICE(USB_VENDOR_ID_THINGM, USB_DEVICE_ID_BLINK1) },
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[RFC] iowait/idle time accounting hiccups in NOHZ kernels
(Moving discussion to LKML)
Hi Thomas, Frederic,
Tetsuo Handa reported that the iowait time obtained through /proc/stat
is not monotonic.
The reason is that get_cpu_iowait_time_us() is inherently racy;
->idle_entrytime and ->iowait_sleeptime can be updated from another
CPU (via update_ts_time_stats()) during the delta and iowait time
calculations and the "now" values used by the racing CPUs are not
necessarily ordered.
The patch below fixes the problem that the delta becomes negative, but
this is not enough. Fixing the whole problem properly may require some
major plumbing so I would like to know your take on this before going
ahead.
Thanks,
Fernando
---
diff -urNp linux-3.9-rc3-orig/kernel/time/tick-sched.c
linux-3.9-rc3/kernel/time/tick-sched.c
--- linux-3.9-rc3-orig/kernel/time/tick-sched.c 2013-03-18 16:58:36.076335000
+0900
+++ linux-3.9-rc3/kernel/time/tick-sched.c 2013-03-19 10:57:32.729247000
+0900
@@ -292,18 +292,20 @@ EXPORT_SYMBOL_GPL(get_cpu_idle_time_us);
u64 get_cpu_iowait_time_us(int cpu, u64 *last_update_time)
{
struct tick_sched *ts = _cpu(tick_cpu_sched, cpu);
- ktime_t now, iowait;
+ ktime_t now, iowait, idle_entrytime;
if (!tick_nohz_enabled)
return -1;
+ idle_entrytime = ts->idle_entrytime;
+ smp_mb();
now = ktime_get();
if (last_update_time) {
update_ts_time_stats(cpu, ts, now, last_update_time);
iowait = ts->iowait_sleeptime;
} else {
if (ts->idle_active && nr_iowait_cpu(cpu) > 0) {
- ktime_t delta = ktime_sub(now, ts->idle_entrytime);
+ ktime_t delta = ktime_sub(now, idle_entrytime);
iowait = ktime_add(ts->iowait_sleeptime, delta);
} else {
On Fri, 2013-01-18 at 17:57 +0900, Tetsuo Handa wrote:
> I forwarded this problem to Fernando.
> I think he will start discussion on how to fix this problem at the LKML.
>
> On Tue, 15 Jan 2013 13:14:38 +0100 (CET)
> Thomas Gleixner wrote:
>
> > On Tue, 15 Jan 2013, Tetsuo Handa wrote:
> >
> > > Hello.
> > >
> > > I can observe that get_cpu_iowait_time_us(cpu, NULL) sometime decreases,
> > > resulting in iowait field of cpu lines in /proc/stat decreasing.
> > > Is this a feature of tick_nohz_enabled == 1 ?
> >
> > It definitely not a feature. Is that simple to observe or does it
> > require any special setup/workload ?
> >
> > Thanks,
> >
> > Thomas
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[RFC] iowait/idle time accounting hiccups in NOHZ kernels
(Moving discussion to LKML)
Hi Thomas, Frederic,
Tetsuo Handa reported that the iowait time obtained through /proc/stat
is not monotonic.
The reason is that get_cpu_iowait_time_us() is inherently racy;
-idle_entrytime and -iowait_sleeptime can be updated from another
CPU (via update_ts_time_stats()) during the delta and iowait time
calculations and the now values used by the racing CPUs are not
necessarily ordered.
The patch below fixes the problem that the delta becomes negative, but
this is not enough. Fixing the whole problem properly may require some
major plumbing so I would like to know your take on this before going
ahead.
Thanks,
Fernando
---
diff -urNp linux-3.9-rc3-orig/kernel/time/tick-sched.c
linux-3.9-rc3/kernel/time/tick-sched.c
--- linux-3.9-rc3-orig/kernel/time/tick-sched.c 2013-03-18 16:58:36.076335000
+0900
+++ linux-3.9-rc3/kernel/time/tick-sched.c 2013-03-19 10:57:32.729247000
+0900
@@ -292,18 +292,20 @@ EXPORT_SYMBOL_GPL(get_cpu_idle_time_us);
u64 get_cpu_iowait_time_us(int cpu, u64 *last_update_time)
{
struct tick_sched *ts = per_cpu(tick_cpu_sched, cpu);
- ktime_t now, iowait;
+ ktime_t now, iowait, idle_entrytime;
if (!tick_nohz_enabled)
return -1;
+ idle_entrytime = ts-idle_entrytime;
+ smp_mb();
now = ktime_get();
if (last_update_time) {
update_ts_time_stats(cpu, ts, now, last_update_time);
iowait = ts-iowait_sleeptime;
} else {
if (ts-idle_active nr_iowait_cpu(cpu) 0) {
- ktime_t delta = ktime_sub(now, ts-idle_entrytime);
+ ktime_t delta = ktime_sub(now, idle_entrytime);
iowait = ktime_add(ts-iowait_sleeptime, delta);
} else {
On Fri, 2013-01-18 at 17:57 +0900, Tetsuo Handa wrote:
I forwarded this problem to Fernando.
I think he will start discussion on how to fix this problem at the LKML.
On Tue, 15 Jan 2013 13:14:38 +0100 (CET)
Thomas Gleixner t...@linutronix.de wrote:
On Tue, 15 Jan 2013, Tetsuo Handa wrote:
Hello.
I can observe that get_cpu_iowait_time_us(cpu, NULL) sometime decreases,
resulting in iowait field of cpu lines in /proc/stat decreasing.
Is this a feature of tick_nohz_enabled == 1 ?
It definitely not a feature. Is that simple to observe or does it
require any special setup/workload ?
Thanks,
Thomas
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH] scripts/mod: add device table offsets file to list of files to clean
From: Fernando Luis Vázquez Cao This file is generated so it does not get cleaned automagically. In other words we need to added to the clean-files list. Signed-off-by: Fernando Luis Vázquez Cao --- diff -urNp linux-3.9-rc1-orig/scripts/mod/Makefile linux-3.9-rc1/scripts/mod/Makefile --- linux-3.9-rc1-orig/scripts/mod/Makefile 2013-03-04 16:09:29.297904000 +0900 +++ linux-3.9-rc1/scripts/mod/Makefile 2013-03-04 18:06:16.359389000 +0900 @@ -49,3 +49,5 @@ $(obj)/elfconfig.h: $(obj)/empty.o $(obj $(call if_changed,elfconfig) targets += elfconfig.h + +clean-files:= $(devicetable-offsets-file) -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH] scripts/mod: add device table offsets file to list of files to clean
From: Fernando Luis Vázquez Cao ferna...@oss.ntt.co.jp This file is generated so it does not get cleaned automagically. In other words we need to added to the clean-files list. Signed-off-by: Fernando Luis Vázquez Cao ferna...@oss.ntt.co.jp --- diff -urNp linux-3.9-rc1-orig/scripts/mod/Makefile linux-3.9-rc1/scripts/mod/Makefile --- linux-3.9-rc1-orig/scripts/mod/Makefile 2013-03-04 16:09:29.297904000 +0900 +++ linux-3.9-rc1/scripts/mod/Makefile 2013-03-04 18:06:16.359389000 +0900 @@ -49,3 +49,5 @@ $(obj)/elfconfig.h: $(obj)/empty.o $(obj $(call if_changed,elfconfig) targets += elfconfig.h + +clean-files:= $(devicetable-offsets-file) -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH 2/3] ALSA: hda - no-primary-hp is a quirk for model ALC889 not ALC882
Substitute ALC889 for ALC882 in macro and function names.
Cc: sta...@vger.kernel.org
Cc: alsa-de...@alsa-project.org
Signed-off-by: Fernando Luis Vazquez Cao
---
diff -urNp linux-3.7.6-orig/sound/pci/hda/patch_realtek.c
linux-3.7.6/sound/pci/hda/patch_realtek.c
--- linux-3.7.6-orig/sound/pci/hda/patch_realtek.c 2013-02-09
22:52:40.301209823 +0900
+++ linux-3.7.6/sound/pci/hda/patch_realtek.c 2013-02-09 22:58:54.695063675
+0900
@@ -5030,7 +5030,7 @@ enum {
ALC889_FIXUP_MBP_VREF,
ALC889_FIXUP_IMAC91_VREF,
ALC882_FIXUP_INV_DMIC,
- ALC882_FIXUP_NO_PRIMARY_HP,
+ ALC889_FIXUP_NO_PRIMARY_HP,
};
static void alc889_fixup_coef(struct hda_codec *codec,
@@ -5156,7 +5156,7 @@ static void alc889_fixup_imac91_vref(str
* Strangely, the speaker output doesn't work on Vaio Z and some Vaio
* all-in-one desktop PCs (for example VGC-LN51JGB) through DAC 0x05
*/
-static void alc882_fixup_no_primary_hp(struct hda_codec *codec,
+static void alc889_fixup_no_primary_hp(struct hda_codec *codec,
const struct alc_fixup *fix, int action)
{
struct alc_spec *spec = codec->spec;
@@ -5350,9 +5350,9 @@ static const struct alc_fixup alc882_fix
.type = ALC_FIXUP_FUNC,
.v.func = alc_fixup_inv_dmic_0x12,
},
- [ALC882_FIXUP_NO_PRIMARY_HP] = {
+ [ALC889_FIXUP_NO_PRIMARY_HP] = {
.type = ALC_FIXUP_FUNC,
- .v.func = alc882_fixup_no_primary_hp,
+ .v.func = alc889_fixup_no_primary_hp,
},
};
@@ -5388,8 +5388,8 @@ static const struct snd_pci_quirk alc882
SND_PCI_QUIRK(0x1043, 0x1971, "Asus W2JC", ALC882_FIXUP_ASUS_W2JC),
SND_PCI_QUIRK(0x1043, 0x835f, "Asus Eee 1601", ALC888_FIXUP_EEE1601),
SND_PCI_QUIRK(0x104d, 0x9047, "Sony Vaio TT", ALC889_FIXUP_VAIO_TT),
- SND_PCI_QUIRK(0x104d, 0x905a, "Sony Vaio Z",
ALC882_FIXUP_NO_PRIMARY_HP),
- SND_PCI_QUIRK(0x104d, 0x9043, "Sony Vaio VGC-LN51JGB",
ALC882_FIXUP_NO_PRIMARY_HP),
+ SND_PCI_QUIRK(0x104d, 0x905a, "Sony Vaio Z",
ALC889_FIXUP_NO_PRIMARY_HP),
+ SND_PCI_QUIRK(0x104d, 0x9043, "Sony Vaio VGC-LN51JGB",
ALC889_FIXUP_NO_PRIMARY_HP),
/* All Apple entries are in codec SSIDs */
SND_PCI_QUIRK(0x106b, 0x00a0, "MacBookPro 3,1", ALC889_FIXUP_MBP_VREF),
@@ -5432,7 +5432,7 @@ static const struct alc_model_fixup alc8
{.id = ALC882_FIXUP_ACER_ASPIRE_8930G, .name = "acer-aspire-8930g"},
{.id = ALC883_FIXUP_ACER_EAPD, .name = "acer-aspire"},
{.id = ALC882_FIXUP_INV_DMIC, .name = "inv-dmic"},
- {.id = ALC882_FIXUP_NO_PRIMARY_HP, .name = "no-primary-hp"},
+ {.id = ALC889_FIXUP_NO_PRIMARY_HP, .name = "no-primary-hp"},
{}
};
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH 2/3] ALSA: hda - update documentation for no-primary-hp fixup
The problem addressed by this fixup is not specific to Vaio Z, affecting some Vaio all-in-one desktop PCs too. Update the code comments accordingly. Cc: sta...@vger.kernel.org Cc: alsa-de...@alsa-project.org Signed-off-by: Fernando Luis Vazquez Cao --- diff -urNp linux-3.7.6-orig/Documentation/sound/alsa/HD-Audio-Models.txt linux-3.7.6/Documentation/sound/alsa/HD-Audio-Models.txt --- linux-3.7.6-orig/Documentation/sound/alsa/HD-Audio-Models.txt 2012-12-11 12:30:57.0 +0900 +++ linux-3.7.6/Documentation/sound/alsa/HD-Audio-Models.txt2013-02-09 22:44:28.614772054 +0900 @@ -53,7 +53,7 @@ ALC882/883/885/888/889 acer-aspire-8930gAcer Aspire 8330G/6935G acer-aspire Acer Aspire others inv-dmic Inverted internal mic workaround - no-primary-hpVAIO Z workaround (for fixed speaker DAC) + no-primary-hpVAIO Z/VGC-LN51JGB workaround (for fixed speaker DAC) ALC861/660 == diff -urNp linux-3.7.6-orig/sound/pci/hda/patch_realtek.c linux-3.7.6/sound/pci/hda/patch_realtek.c --- linux-3.7.6-orig/sound/pci/hda/patch_realtek.c 2013-02-09 22:39:11.745202168 +0900 +++ linux-3.7.6/sound/pci/hda/patch_realtek.c 2013-02-09 22:50:10.924470402 +0900 @@ -5153,7 +5153,8 @@ static void alc889_fixup_imac91_vref(str } /* Don't take HP output as primary - * strangely, the speaker output doesn't work on VAIO Z through DAC 0x05 + * Strangely, the speaker output doesn't work on Vaio Z and some Vaio + * all-in-one desktop PCs (for example VGC-LN51JGB) through DAC 0x05 */ static void alc882_fixup_no_primary_hp(struct hda_codec *codec, const struct alc_fixup *fix, int action) -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH 1/3] ALSA: hda - Workaround for silent output on Sony Vaio VGC-LN51JGB with ALC889
Some Vaio all-in-one desktop PCs (for example VGC-LN51JGB) are affected by the same issue that caused Vaio Z laptops to become silent: the speaker pin must be connected to the first DAC even though the codec itself advertises flexible routing through any of the DACs. Use the no-primary-hp fixup for choosing the speaker pin as the primary so that the right DAC is assigned on this device. Cc: sta...@vger.kernel.org Cc: alsa-de...@alsa-project.org Signed-off-by: Fernando Luis Vazquez Cao --- diff -urNp linux-3.7.6-orig/sound/pci/hda/patch_realtek.c linux-3.7.6/sound/pci/hda/patch_realtek.c --- linux-3.7.6-orig/sound/pci/hda/patch_realtek.c 2013-02-08 23:27:05.949484427 +0900 +++ linux-3.7.6/sound/pci/hda/patch_realtek.c 2013-02-08 23:41:04.897644545 +0900 @@ -5388,6 +5388,7 @@ static const struct snd_pci_quirk alc882 SND_PCI_QUIRK(0x1043, 0x835f, "Asus Eee 1601", ALC888_FIXUP_EEE1601), SND_PCI_QUIRK(0x104d, 0x9047, "Sony Vaio TT", ALC889_FIXUP_VAIO_TT), SND_PCI_QUIRK(0x104d, 0x905a, "Sony Vaio Z", ALC882_FIXUP_NO_PRIMARY_HP), + SND_PCI_QUIRK(0x104d, 0x9043, "Sony Vaio VGC-LN51JGB", ALC882_FIXUP_NO_PRIMARY_HP), /* All Apple entries are in codec SSIDs */ SND_PCI_QUIRK(0x106b, 0x00a0, "MacBookPro 3,1", ALC889_FIXUP_MBP_VREF), -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH 1/3] ALSA: hda - Workaround for silent output on Sony Vaio VGC-LN51JGB with ALC889
Some Vaio all-in-one desktop PCs (for example VGC-LN51JGB) are affected by the same issue that caused Vaio Z laptops to become silent: the speaker pin must be connected to the first DAC even though the codec itself advertises flexible routing through any of the DACs. Use the no-primary-hp fixup for choosing the speaker pin as the primary so that the right DAC is assigned on this device. Cc: sta...@vger.kernel.org Cc: alsa-de...@alsa-project.org Signed-off-by: Fernando Luis Vazquez Cao ferna...@oss.ntt.co.jp --- diff -urNp linux-3.7.6-orig/sound/pci/hda/patch_realtek.c linux-3.7.6/sound/pci/hda/patch_realtek.c --- linux-3.7.6-orig/sound/pci/hda/patch_realtek.c 2013-02-08 23:27:05.949484427 +0900 +++ linux-3.7.6/sound/pci/hda/patch_realtek.c 2013-02-08 23:41:04.897644545 +0900 @@ -5388,6 +5388,7 @@ static const struct snd_pci_quirk alc882 SND_PCI_QUIRK(0x1043, 0x835f, Asus Eee 1601, ALC888_FIXUP_EEE1601), SND_PCI_QUIRK(0x104d, 0x9047, Sony Vaio TT, ALC889_FIXUP_VAIO_TT), SND_PCI_QUIRK(0x104d, 0x905a, Sony Vaio Z, ALC882_FIXUP_NO_PRIMARY_HP), + SND_PCI_QUIRK(0x104d, 0x9043, Sony Vaio VGC-LN51JGB, ALC882_FIXUP_NO_PRIMARY_HP), /* All Apple entries are in codec SSIDs */ SND_PCI_QUIRK(0x106b, 0x00a0, MacBookPro 3,1, ALC889_FIXUP_MBP_VREF), -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH 2/3] ALSA: hda - update documentation for no-primary-hp fixup
The problem addressed by this fixup is not specific to Vaio Z, affecting some Vaio all-in-one desktop PCs too. Update the code comments accordingly. Cc: sta...@vger.kernel.org Cc: alsa-de...@alsa-project.org Signed-off-by: Fernando Luis Vazquez Cao ferna...@oss.ntt.co.jp --- diff -urNp linux-3.7.6-orig/Documentation/sound/alsa/HD-Audio-Models.txt linux-3.7.6/Documentation/sound/alsa/HD-Audio-Models.txt --- linux-3.7.6-orig/Documentation/sound/alsa/HD-Audio-Models.txt 2012-12-11 12:30:57.0 +0900 +++ linux-3.7.6/Documentation/sound/alsa/HD-Audio-Models.txt2013-02-09 22:44:28.614772054 +0900 @@ -53,7 +53,7 @@ ALC882/883/885/888/889 acer-aspire-8930gAcer Aspire 8330G/6935G acer-aspire Acer Aspire others inv-dmic Inverted internal mic workaround - no-primary-hpVAIO Z workaround (for fixed speaker DAC) + no-primary-hpVAIO Z/VGC-LN51JGB workaround (for fixed speaker DAC) ALC861/660 == diff -urNp linux-3.7.6-orig/sound/pci/hda/patch_realtek.c linux-3.7.6/sound/pci/hda/patch_realtek.c --- linux-3.7.6-orig/sound/pci/hda/patch_realtek.c 2013-02-09 22:39:11.745202168 +0900 +++ linux-3.7.6/sound/pci/hda/patch_realtek.c 2013-02-09 22:50:10.924470402 +0900 @@ -5153,7 +5153,8 @@ static void alc889_fixup_imac91_vref(str } /* Don't take HP output as primary - * strangely, the speaker output doesn't work on VAIO Z through DAC 0x05 + * Strangely, the speaker output doesn't work on Vaio Z and some Vaio + * all-in-one desktop PCs (for example VGC-LN51JGB) through DAC 0x05 */ static void alc882_fixup_no_primary_hp(struct hda_codec *codec, const struct alc_fixup *fix, int action) -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH 2/3] ALSA: hda - no-primary-hp is a quirk for model ALC889 not ALC882
Substitute ALC889 for ALC882 in macro and function names.
Cc: sta...@vger.kernel.org
Cc: alsa-de...@alsa-project.org
Signed-off-by: Fernando Luis Vazquez Cao ferna...@oss.ntt.co.jp
---
diff -urNp linux-3.7.6-orig/sound/pci/hda/patch_realtek.c
linux-3.7.6/sound/pci/hda/patch_realtek.c
--- linux-3.7.6-orig/sound/pci/hda/patch_realtek.c 2013-02-09
22:52:40.301209823 +0900
+++ linux-3.7.6/sound/pci/hda/patch_realtek.c 2013-02-09 22:58:54.695063675
+0900
@@ -5030,7 +5030,7 @@ enum {
ALC889_FIXUP_MBP_VREF,
ALC889_FIXUP_IMAC91_VREF,
ALC882_FIXUP_INV_DMIC,
- ALC882_FIXUP_NO_PRIMARY_HP,
+ ALC889_FIXUP_NO_PRIMARY_HP,
};
static void alc889_fixup_coef(struct hda_codec *codec,
@@ -5156,7 +5156,7 @@ static void alc889_fixup_imac91_vref(str
* Strangely, the speaker output doesn't work on Vaio Z and some Vaio
* all-in-one desktop PCs (for example VGC-LN51JGB) through DAC 0x05
*/
-static void alc882_fixup_no_primary_hp(struct hda_codec *codec,
+static void alc889_fixup_no_primary_hp(struct hda_codec *codec,
const struct alc_fixup *fix, int action)
{
struct alc_spec *spec = codec-spec;
@@ -5350,9 +5350,9 @@ static const struct alc_fixup alc882_fix
.type = ALC_FIXUP_FUNC,
.v.func = alc_fixup_inv_dmic_0x12,
},
- [ALC882_FIXUP_NO_PRIMARY_HP] = {
+ [ALC889_FIXUP_NO_PRIMARY_HP] = {
.type = ALC_FIXUP_FUNC,
- .v.func = alc882_fixup_no_primary_hp,
+ .v.func = alc889_fixup_no_primary_hp,
},
};
@@ -5388,8 +5388,8 @@ static const struct snd_pci_quirk alc882
SND_PCI_QUIRK(0x1043, 0x1971, Asus W2JC, ALC882_FIXUP_ASUS_W2JC),
SND_PCI_QUIRK(0x1043, 0x835f, Asus Eee 1601, ALC888_FIXUP_EEE1601),
SND_PCI_QUIRK(0x104d, 0x9047, Sony Vaio TT, ALC889_FIXUP_VAIO_TT),
- SND_PCI_QUIRK(0x104d, 0x905a, Sony Vaio Z,
ALC882_FIXUP_NO_PRIMARY_HP),
- SND_PCI_QUIRK(0x104d, 0x9043, Sony Vaio VGC-LN51JGB,
ALC882_FIXUP_NO_PRIMARY_HP),
+ SND_PCI_QUIRK(0x104d, 0x905a, Sony Vaio Z,
ALC889_FIXUP_NO_PRIMARY_HP),
+ SND_PCI_QUIRK(0x104d, 0x9043, Sony Vaio VGC-LN51JGB,
ALC889_FIXUP_NO_PRIMARY_HP),
/* All Apple entries are in codec SSIDs */
SND_PCI_QUIRK(0x106b, 0x00a0, MacBookPro 3,1, ALC889_FIXUP_MBP_VREF),
@@ -5432,7 +5432,7 @@ static const struct alc_model_fixup alc8
{.id = ALC882_FIXUP_ACER_ASPIRE_8930G, .name = acer-aspire-8930g},
{.id = ALC883_FIXUP_ACER_EAPD, .name = acer-aspire},
{.id = ALC882_FIXUP_INV_DMIC, .name = inv-dmic},
- {.id = ALC882_FIXUP_NO_PRIMARY_HP, .name = no-primary-hp},
+ {.id = ALC889_FIXUP_NO_PRIMARY_HP, .name = no-primary-hp},
{}
};
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
HID: clean up quirk for Sony RF receivers
Document what the fix-up is does and make it more robust by ensuring
that it is only applied to the USB interface that corresponds to the
mouse (sony_report_fixup() is called once per interface during probing).
Cc: linux-in...@vger.kernel.org
Cc: linux-...@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
Signed-off-by: Fernando Luis Vazquez Cao
Signed-off-by: Jiri Kosina
---
diff -urNp linux-3.8-rc4-orig/drivers/hid/hid-sony.c
linux-3.8-rc4/drivers/hid/hid-sony.c
--- linux-3.8-rc4-orig/drivers/hid/hid-sony.c 2013-01-22 14:21:13.380552283
+0900
+++ linux-3.8-rc4/drivers/hid/hid-sony.c2013-01-22 14:41:56.316934002
+0900
@@ -43,9 +43,19 @@ static __u8 *sony_report_fixup(struct hi
{
struct sony_sc *sc = hid_get_drvdata(hdev);
- if ((sc->quirks & VAIO_RDESC_CONSTANT) &&
- *rsize >= 56 && rdesc[54] == 0x81 && rdesc[55] == 0x07)
{
+ /*
+* Some Sony RF receivers wrongly declare the mouse pointer as a
+* a constant non-data variable.
+*/
+ if ((sc->quirks & VAIO_RDESC_CONSTANT) && *rsize >= 56 &&
+ /* usage page: generic desktop controls */
+ /* rdesc[0] == 0x05 && rdesc[1] == 0x01 && */
+ /* usage: mouse */
+ rdesc[2] == 0x09 && rdesc[3] == 0x02 &&
+ /* input (usage page for x,y axes): constant, variable, relative */
+ rdesc[54] == 0x81 && rdesc[55] == 0x07) {
hid_info(hdev, "Fixing up Sony RF Receiver report
descriptor\n");
+ /* input: data, variable, relative */
rdesc[55] = 0x06;
}
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
HID: clean up quirk for Sony RF receivers
Document what the fix-up is does and make it more robust by ensuring
that it is only applied to the USB interface that corresponds to the
mouse (sony_report_fixup() is called once per interface during probing).
Cc: linux-in...@vger.kernel.org
Cc: linux-...@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
Signed-off-by: Fernando Luis Vazquez Cao ferna...@oss.ntt.co.jp
Signed-off-by: Jiri Kosina jkos...@suse.cz
---
diff -urNp linux-3.8-rc4-orig/drivers/hid/hid-sony.c
linux-3.8-rc4/drivers/hid/hid-sony.c
--- linux-3.8-rc4-orig/drivers/hid/hid-sony.c 2013-01-22 14:21:13.380552283
+0900
+++ linux-3.8-rc4/drivers/hid/hid-sony.c2013-01-22 14:41:56.316934002
+0900
@@ -43,9 +43,19 @@ static __u8 *sony_report_fixup(struct hi
{
struct sony_sc *sc = hid_get_drvdata(hdev);
- if ((sc-quirks VAIO_RDESC_CONSTANT)
- *rsize = 56 rdesc[54] == 0x81 rdesc[55] == 0x07)
{
+ /*
+* Some Sony RF receivers wrongly declare the mouse pointer as a
+* a constant non-data variable.
+*/
+ if ((sc-quirks VAIO_RDESC_CONSTANT) *rsize = 56
+ /* usage page: generic desktop controls */
+ /* rdesc[0] == 0x05 rdesc[1] == 0x01 */
+ /* usage: mouse */
+ rdesc[2] == 0x09 rdesc[3] == 0x02
+ /* input (usage page for x,y axes): constant, variable, relative */
+ rdesc[54] == 0x81 rdesc[55] == 0x07) {
hid_info(hdev, Fixing up Sony RF Receiver report
descriptor\n);
+ /* input: data, variable, relative */
rdesc[55] = 0x06;
}
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH v2] HID: add support for Sony RF receiver with USB product id 0x0374
Some Vaio desktop computers, among them the VGC-LN51JGB multimedia PC, have
a RF receiver, multi-interface USB device 054c:0374, that is used to connect
a wireless keyboard and a wireless mouse.
The keyboard works flawlessly, but the mouse (VGP-WMS3 in my case) does not
seem to be generating any pointer events. The problem is that the mouse pointer
is wrongly declared as a constant non-data variable in the report descriptor
(see lsusb and usbhid-dump output below), with the consequence that it is
ignored by the HID code.
Add this device to the have-special-driver list and fix up the report
descriptor in the Sony-specific driver which happens to already have a fixup
for a similar firmware bug.
# lsusb -vd 054C:0374
Bus 003 Device 002: ID 054c:0374 Sony Corp.
Device Descriptor:
bLength18
bDescriptorType 1
bcdUSB 2.00
bDeviceClass0 (Defined at Interface level)
bDeviceSubClass 0
bDeviceProtocol 0
bMaxPacketSize0 8
idVendor 0x054c Sony Corp.
idProduct 0x0374
iSerial 0
[...]
Interface Descriptor:
bLength 9
bDescriptorType 4
bInterfaceNumber1
bAlternateSetting 0
bNumEndpoints 1
bInterfaceClass 3 Human Interface Device
bInterfaceSubClass 1 Boot Interface Subclass
bInterfaceProtocol 2 Mouse
iInterface 2 RF Receiver
[...]
Report Descriptor: (length is 100)
[...]
Item(Global): Usage Page, data= [ 0x01 ] 1
Generic Desktop Controls
Item(Local ): Usage, data= [ 0x30 ] 48
Direction-X
Item(Local ): Usage, data= [ 0x31 ] 49
Direction-Y
Item(Global): Report Count, data= [ 0x02 ] 2
Item(Global): Report Size, data= [ 0x08 ] 8
Item(Global): Logical Minimum, data= [ 0x81 ] 129
Item(Global): Logical Maximum, data= [ 0x7f ] 127
Item(Main ): Input, data= [ 0x07 ] 7
Constant Variable Relative No_Wrap Linear
Preferred_State No_Null_Position Non_Volatile
Bitfield
# usbhid-dump
003:002:001:DESCRIPTOR 1357910009.758544
05 01 09 02 A1 01 05 01 09 02 A1 02 85 01 09 01
A1 00 05 09 19 01 29 05 95 05 75 01 15 00 25 01
81 02 75 03 95 01 81 01 05 01 09 30 09 31 95 02
75 08 15 81 25 7F 81 07 A1 02 85 01 09 38 35 00
45 00 15 81 25 7F 95 01 75 08 81 06 C0 A1 02 85
01 05 0C 15 81 25 7F 95 01 75 08 0A 38 02 81 06
C0 C0 C0 C0
Cc: linux-in...@vger.kernel.org
Cc: linux-...@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
Signed-off-by: Fernando Luis Vazquez Cao
---
diff -urNp linux-3.8-rc3-orig/drivers/hid/hid-core.c
linux-3.8-rc3/drivers/hid/hid-core.c
--- linux-3.8-rc3-orig/drivers/hid/hid-core.c 2013-01-10 11:59:55.0
+0900
+++ linux-3.8-rc3/drivers/hid/hid-core.c2013-01-15 19:32:22.189574034
+0900
@@ -1697,6 +1697,7 @@ static const struct hid_device_id hid_ha
{ HID_USB_DEVICE(USB_VENDOR_ID_SONY,
USB_DEVICE_ID_SONY_NAVIGATION_CONTROLLER) },
{ HID_BLUETOOTH_DEVICE(USB_VENDOR_ID_SONY,
USB_DEVICE_ID_SONY_PS3_CONTROLLER) },
{ HID_USB_DEVICE(USB_VENDOR_ID_SONY, USB_DEVICE_ID_SONY_VAIO_VGX_MOUSE)
},
+ { HID_USB_DEVICE(USB_VENDOR_ID_SONY, USB_DEVICE_ID_SONY_VAIO_VGP_MOUSE)
},
{ HID_USB_DEVICE(USB_VENDOR_ID_SUNPLUS, USB_DEVICE_ID_SUNPLUS_WDESKTOP)
},
{ HID_USB_DEVICE(USB_VENDOR_ID_THRUSTMASTER, 0xb300) },
{ HID_USB_DEVICE(USB_VENDOR_ID_THRUSTMASTER, 0xb304) },
diff -urNp linux-3.8-rc3-orig/drivers/hid/hid-ids.h
linux-3.8-rc3/drivers/hid/hid-ids.h
--- linux-3.8-rc3-orig/drivers/hid/hid-ids.h2013-01-10 11:59:55.0
+0900
+++ linux-3.8-rc3/drivers/hid/hid-ids.h 2013-01-15 19:32:22.189574034 +0900
@@ -706,6 +706,7 @@
#define USB_VENDOR_ID_SONY 0x054c
#define USB_DEVICE_ID_SONY_VAIO_VGX_MOUSE 0x024b
+#define USB_DEVICE_ID_SONY_VAIO_VGP_MOUSE 0x0374
#define USB_DEVICE_ID_SONY_PS3_BDREMOTE0x0306
#define USB_DEVICE_ID_SONY_PS3_CONTROLLER 0x0268
#define USB_DEVICE_ID_SONY_NAVIGATION_CONTROLLER 0x042f
diff -urNp linux-3.8-rc3-orig/drivers/hid/hid-sony.c
linux-3.8-rc3/drivers/hid/hid-sony.c
--- linux-3.8-rc3-orig/drivers/hid/hid-sony.c 2013-01-10 11:59:55.0
+0900
+++ linux-3.8-rc3/drivers/hid/hid-sony.c2013-01-15 19:35:57.858683185
+0900
@@ -45,7 +45,7 @@ static __u8 *sony_report_fixup(struct hi
if ((sc->quirks & VAIO_RDESC_CONSTANT) &&
*rsize >= 56 && rdesc[54] == 0x81 && rdesc[55] == 0x07)
{
- hid_info(hdev, "Fixing up Sony Vaio VGX report descriptor\n");
+ hid_info(hdev, "Fixing up Sony RF Receiver report
descriptor\n");
rdesc[55] = 0x06;
}
@@ -217,6 +217,8 @@ static const
[PATCH v2] HID: add support for Sony RF receiver with USB product id 0x0374
Some Vaio desktop computers, among them the VGC-LN51JGB multimedia PC, have
a RF receiver, multi-interface USB device 054c:0374, that is used to connect
a wireless keyboard and a wireless mouse.
The keyboard works flawlessly, but the mouse (VGP-WMS3 in my case) does not
seem to be generating any pointer events. The problem is that the mouse pointer
is wrongly declared as a constant non-data variable in the report descriptor
(see lsusb and usbhid-dump output below), with the consequence that it is
ignored by the HID code.
Add this device to the have-special-driver list and fix up the report
descriptor in the Sony-specific driver which happens to already have a fixup
for a similar firmware bug.
# lsusb -vd 054C:0374
Bus 003 Device 002: ID 054c:0374 Sony Corp.
Device Descriptor:
bLength18
bDescriptorType 1
bcdUSB 2.00
bDeviceClass0 (Defined at Interface level)
bDeviceSubClass 0
bDeviceProtocol 0
bMaxPacketSize0 8
idVendor 0x054c Sony Corp.
idProduct 0x0374
iSerial 0
[...]
Interface Descriptor:
bLength 9
bDescriptorType 4
bInterfaceNumber1
bAlternateSetting 0
bNumEndpoints 1
bInterfaceClass 3 Human Interface Device
bInterfaceSubClass 1 Boot Interface Subclass
bInterfaceProtocol 2 Mouse
iInterface 2 RF Receiver
[...]
Report Descriptor: (length is 100)
[...]
Item(Global): Usage Page, data= [ 0x01 ] 1
Generic Desktop Controls
Item(Local ): Usage, data= [ 0x30 ] 48
Direction-X
Item(Local ): Usage, data= [ 0x31 ] 49
Direction-Y
Item(Global): Report Count, data= [ 0x02 ] 2
Item(Global): Report Size, data= [ 0x08 ] 8
Item(Global): Logical Minimum, data= [ 0x81 ] 129
Item(Global): Logical Maximum, data= [ 0x7f ] 127
Item(Main ): Input, data= [ 0x07 ] 7
Constant Variable Relative No_Wrap Linear
Preferred_State No_Null_Position Non_Volatile
Bitfield
# usbhid-dump
003:002:001:DESCRIPTOR 1357910009.758544
05 01 09 02 A1 01 05 01 09 02 A1 02 85 01 09 01
A1 00 05 09 19 01 29 05 95 05 75 01 15 00 25 01
81 02 75 03 95 01 81 01 05 01 09 30 09 31 95 02
75 08 15 81 25 7F 81 07 A1 02 85 01 09 38 35 00
45 00 15 81 25 7F 95 01 75 08 81 06 C0 A1 02 85
01 05 0C 15 81 25 7F 95 01 75 08 0A 38 02 81 06
C0 C0 C0 C0
Cc: linux-in...@vger.kernel.org
Cc: linux-...@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
Signed-off-by: Fernando Luis Vazquez Cao ferna...@oss.ntt.co.jp
---
diff -urNp linux-3.8-rc3-orig/drivers/hid/hid-core.c
linux-3.8-rc3/drivers/hid/hid-core.c
--- linux-3.8-rc3-orig/drivers/hid/hid-core.c 2013-01-10 11:59:55.0
+0900
+++ linux-3.8-rc3/drivers/hid/hid-core.c2013-01-15 19:32:22.189574034
+0900
@@ -1697,6 +1697,7 @@ static const struct hid_device_id hid_ha
{ HID_USB_DEVICE(USB_VENDOR_ID_SONY,
USB_DEVICE_ID_SONY_NAVIGATION_CONTROLLER) },
{ HID_BLUETOOTH_DEVICE(USB_VENDOR_ID_SONY,
USB_DEVICE_ID_SONY_PS3_CONTROLLER) },
{ HID_USB_DEVICE(USB_VENDOR_ID_SONY, USB_DEVICE_ID_SONY_VAIO_VGX_MOUSE)
},
+ { HID_USB_DEVICE(USB_VENDOR_ID_SONY, USB_DEVICE_ID_SONY_VAIO_VGP_MOUSE)
},
{ HID_USB_DEVICE(USB_VENDOR_ID_SUNPLUS, USB_DEVICE_ID_SUNPLUS_WDESKTOP)
},
{ HID_USB_DEVICE(USB_VENDOR_ID_THRUSTMASTER, 0xb300) },
{ HID_USB_DEVICE(USB_VENDOR_ID_THRUSTMASTER, 0xb304) },
diff -urNp linux-3.8-rc3-orig/drivers/hid/hid-ids.h
linux-3.8-rc3/drivers/hid/hid-ids.h
--- linux-3.8-rc3-orig/drivers/hid/hid-ids.h2013-01-10 11:59:55.0
+0900
+++ linux-3.8-rc3/drivers/hid/hid-ids.h 2013-01-15 19:32:22.189574034 +0900
@@ -706,6 +706,7 @@
#define USB_VENDOR_ID_SONY 0x054c
#define USB_DEVICE_ID_SONY_VAIO_VGX_MOUSE 0x024b
+#define USB_DEVICE_ID_SONY_VAIO_VGP_MOUSE 0x0374
#define USB_DEVICE_ID_SONY_PS3_BDREMOTE0x0306
#define USB_DEVICE_ID_SONY_PS3_CONTROLLER 0x0268
#define USB_DEVICE_ID_SONY_NAVIGATION_CONTROLLER 0x042f
diff -urNp linux-3.8-rc3-orig/drivers/hid/hid-sony.c
linux-3.8-rc3/drivers/hid/hid-sony.c
--- linux-3.8-rc3-orig/drivers/hid/hid-sony.c 2013-01-10 11:59:55.0
+0900
+++ linux-3.8-rc3/drivers/hid/hid-sony.c2013-01-15 19:35:57.858683185
+0900
@@ -45,7 +45,7 @@ static __u8 *sony_report_fixup(struct hi
if ((sc-quirks VAIO_RDESC_CONSTANT)
*rsize = 56 rdesc[54] == 0x81 rdesc[55] == 0x07)
{
- hid_info(hdev, Fixing up Sony Vaio VGX report descriptor\n);
+ hid_info(hdev, Fixing up Sony RF Receiver report
descriptor\n);
rdesc[55] = 0x06;
}
@@ -217,6 +217,8 @@ static
[PATCH] HID: add support for Sony RF receiver with USB product id 0x0374
Some Vaio desktop computers, among them the VGC-LN51JGB multimedia PC, have
a RF receiver, multi-interface USB device 054c:0374, that is used to connect
a wireless keyboard and a wireless mouse.
The keyboard works flawlessly, but the mouse (VGP-WMS3 in my case) does not
seem to be generating any pointer events. The problem is that the mouse pointer
is wrongly declared as a constant non-data variable in the report descriptor
(see lsusb and usbhid-dump output below), with the consenquence that it is
ignored by the HID code.
Add this device to the have-special-driver list and fix up the report
descriptor in the Sony-specific driver which happens to already have a fixup
for a similar firmware bug.
# lsusb -vd 054C:0374
Bus 003 Device 002: ID 054c:0374 Sony Corp.
Device Descriptor:
bLength18
bDescriptorType 1
bcdUSB 2.00
bDeviceClass0 (Defined at Interface level)
bDeviceSubClass 0
bDeviceProtocol 0
bMaxPacketSize0 8
idVendor 0x054c Sony Corp.
idProduct 0x0374
iSerial 0
[...]
Interface Descriptor:
bLength 9
bDescriptorType 4
bInterfaceNumber1
bAlternateSetting 0
bNumEndpoints 1
bInterfaceClass 3 Human Interface Device
bInterfaceSubClass 1 Boot Interface Subclass
bInterfaceProtocol 2 Mouse
iInterface 2 RF Receiver
[...]
Report Descriptor: (length is 100)
Item(Global): Usage Page, data= [ 0x01 ] 1
Generic Desktop Controls
Item(Local ): Usage, data= [ 0x30 ] 48
Direction-X
Item(Local ): Usage, data= [ 0x31 ] 49
Direction-Y
Item(Global): Report Count, data= [ 0x02 ] 2
Item(Global): Report Size, data= [ 0x08 ] 8
Item(Global): Logical Minimum, data= [ 0x81 ] 129
Item(Global): Logical Maximum, data= [ 0x7f ] 127
Item(Main ): Input, data= [ 0x07 ] 7
Constant Variable Relative No_Wrap Linear
Preferred_State No_Null_Position Non_Volatile
Bitfield
# sudo usbhid-dump
003:002:001:DESCRIPTOR 1357910009.758544
05 01 09 02 A1 01 05 01 09 02 A1 02 85 01 09 01
A1 00 05 09 19 01 29 05 95 05 75 01 15 00 25 01
81 02 75 03 95 01 81 01 05 01 09 30 09 31 95 02
75 08 15 81 25 7F 81 07 A1 02 85 01 09 38 35 00
45 00 15 81 25 7F 95 01 75 08 81 06 C0 A1 02 85
01 05 0C 15 81 25 7F 95 01 75 08 0A 38 02 81 06
C0 C0 C0 C0
Cc: linux-in...@vger.kernel.org
Cc: linux-...@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
Signed-off-by: Fernando Luis Vazquez Cao
---
diff -urNp linux-3.8-rc3-orig/drivers/hid/hid-core.c
linux-3.8-rc3/drivers/hid/hid-core.c
--- linux-3.8-rc3-orig/drivers/hid/hid-core.c 2013-01-13 20:54:36.846952518
+0900
+++ linux-3.8-rc3/drivers/hid/hid-core.c2013-01-13 18:21:19.901347327
+0900
@@ -1697,6 +1697,7 @@ static const struct hid_device_id hid_ha
{ HID_USB_DEVICE(USB_VENDOR_ID_SONY,
USB_DEVICE_ID_SONY_NAVIGATION_CONTROLLER) },
{ HID_BLUETOOTH_DEVICE(USB_VENDOR_ID_SONY,
USB_DEVICE_ID_SONY_PS3_CONTROLLER) },
{ HID_USB_DEVICE(USB_VENDOR_ID_SONY, USB_DEVICE_ID_SONY_VAIO_VGX_MOUSE)
},
+ { HID_USB_DEVICE(USB_VENDOR_ID_SONY, USB_DEVICE_ID_SONY_VAIO_VGP_MOUSE)
},
{ HID_USB_DEVICE(USB_VENDOR_ID_SUNPLUS, USB_DEVICE_ID_SUNPLUS_WDESKTOP)
},
{ HID_USB_DEVICE(USB_VENDOR_ID_THRUSTMASTER, 0xb300) },
{ HID_USB_DEVICE(USB_VENDOR_ID_THRUSTMASTER, 0xb304) },
diff -urNp linux-3.8-rc3-orig/drivers/hid/hid-ids.h
linux-3.8-rc3/drivers/hid/hid-ids.h
--- linux-3.8-rc3-orig/drivers/hid/hid-ids.h2013-01-13 20:54:36.850952537
+0900
+++ linux-3.8-rc3/drivers/hid/hid-ids.h 2013-01-13 18:21:19.901347327 +0900
@@ -706,6 +706,7 @@
#define USB_VENDOR_ID_SONY 0x054c
#define USB_DEVICE_ID_SONY_VAIO_VGX_MOUSE 0x024b
+#define USB_DEVICE_ID_SONY_VAIO_VGP_MOUSE 0x0374
#define USB_DEVICE_ID_SONY_PS3_BDREMOTE0x0306
#define USB_DEVICE_ID_SONY_PS3_CONTROLLER 0x0268
#define USB_DEVICE_ID_SONY_NAVIGATION_CONTROLLER 0x042f
diff -urNp linux-3.8-rc3-orig/drivers/hid/hid-sony.c
linux-3.8-rc3/drivers/hid/hid-sony.c
--- linux-3.8-rc3-orig/drivers/hid/hid-sony.c 2012-12-11 12:30:57.0
+0900
+++ linux-3.8-rc3/drivers/hid/hid-sony.c2013-01-13 18:21:19.901347327
+0900
@@ -44,19 +44,21 @@ static __u8 *sony_report_fixup(struct hi
struct sony_sc *sc = hid_get_drvdata(hdev);
if ((sc->quirks & VAIO_RDESC_CONSTANT) &&
- *rsize >= 56 && rdesc[54] == 0x81 && rdesc[55] == 0x07)
{
- hid_info(hdev, "Fixing up Sony Vaio VGX report descriptor\n");
+ *rsize >= 56 && rdesc[54] == 0x81 && rdesc[55] == 0x07) {
+ hid_info(hdev,
+
[PATCH] HID: add support for Sony RF receiver with USB product id 0x0374
Some Vaio desktop computers, among them the VGC-LN51JGB multimedia PC, have
a RF receiver, multi-interface USB device 054c:0374, that is used to connect
a wireless keyboard and a wireless mouse.
The keyboard works flawlessly, but the mouse (VGP-WMS3 in my case) does not
seem to be generating any pointer events. The problem is that the mouse pointer
is wrongly declared as a constant non-data variable in the report descriptor
(see lsusb and usbhid-dump output below), with the consenquence that it is
ignored by the HID code.
Add this device to the have-special-driver list and fix up the report
descriptor in the Sony-specific driver which happens to already have a fixup
for a similar firmware bug.
# lsusb -vd 054C:0374
Bus 003 Device 002: ID 054c:0374 Sony Corp.
Device Descriptor:
bLength18
bDescriptorType 1
bcdUSB 2.00
bDeviceClass0 (Defined at Interface level)
bDeviceSubClass 0
bDeviceProtocol 0
bMaxPacketSize0 8
idVendor 0x054c Sony Corp.
idProduct 0x0374
iSerial 0
[...]
Interface Descriptor:
bLength 9
bDescriptorType 4
bInterfaceNumber1
bAlternateSetting 0
bNumEndpoints 1
bInterfaceClass 3 Human Interface Device
bInterfaceSubClass 1 Boot Interface Subclass
bInterfaceProtocol 2 Mouse
iInterface 2 RF Receiver
[...]
Report Descriptor: (length is 100)
Item(Global): Usage Page, data= [ 0x01 ] 1
Generic Desktop Controls
Item(Local ): Usage, data= [ 0x30 ] 48
Direction-X
Item(Local ): Usage, data= [ 0x31 ] 49
Direction-Y
Item(Global): Report Count, data= [ 0x02 ] 2
Item(Global): Report Size, data= [ 0x08 ] 8
Item(Global): Logical Minimum, data= [ 0x81 ] 129
Item(Global): Logical Maximum, data= [ 0x7f ] 127
Item(Main ): Input, data= [ 0x07 ] 7
Constant Variable Relative No_Wrap Linear
Preferred_State No_Null_Position Non_Volatile
Bitfield
# sudo usbhid-dump
003:002:001:DESCRIPTOR 1357910009.758544
05 01 09 02 A1 01 05 01 09 02 A1 02 85 01 09 01
A1 00 05 09 19 01 29 05 95 05 75 01 15 00 25 01
81 02 75 03 95 01 81 01 05 01 09 30 09 31 95 02
75 08 15 81 25 7F 81 07 A1 02 85 01 09 38 35 00
45 00 15 81 25 7F 95 01 75 08 81 06 C0 A1 02 85
01 05 0C 15 81 25 7F 95 01 75 08 0A 38 02 81 06
C0 C0 C0 C0
Cc: linux-in...@vger.kernel.org
Cc: linux-...@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
Signed-off-by: Fernando Luis Vazquez Cao ferna...@oss.ntt.co.jp
---
diff -urNp linux-3.8-rc3-orig/drivers/hid/hid-core.c
linux-3.8-rc3/drivers/hid/hid-core.c
--- linux-3.8-rc3-orig/drivers/hid/hid-core.c 2013-01-13 20:54:36.846952518
+0900
+++ linux-3.8-rc3/drivers/hid/hid-core.c2013-01-13 18:21:19.901347327
+0900
@@ -1697,6 +1697,7 @@ static const struct hid_device_id hid_ha
{ HID_USB_DEVICE(USB_VENDOR_ID_SONY,
USB_DEVICE_ID_SONY_NAVIGATION_CONTROLLER) },
{ HID_BLUETOOTH_DEVICE(USB_VENDOR_ID_SONY,
USB_DEVICE_ID_SONY_PS3_CONTROLLER) },
{ HID_USB_DEVICE(USB_VENDOR_ID_SONY, USB_DEVICE_ID_SONY_VAIO_VGX_MOUSE)
},
+ { HID_USB_DEVICE(USB_VENDOR_ID_SONY, USB_DEVICE_ID_SONY_VAIO_VGP_MOUSE)
},
{ HID_USB_DEVICE(USB_VENDOR_ID_SUNPLUS, USB_DEVICE_ID_SUNPLUS_WDESKTOP)
},
{ HID_USB_DEVICE(USB_VENDOR_ID_THRUSTMASTER, 0xb300) },
{ HID_USB_DEVICE(USB_VENDOR_ID_THRUSTMASTER, 0xb304) },
diff -urNp linux-3.8-rc3-orig/drivers/hid/hid-ids.h
linux-3.8-rc3/drivers/hid/hid-ids.h
--- linux-3.8-rc3-orig/drivers/hid/hid-ids.h2013-01-13 20:54:36.850952537
+0900
+++ linux-3.8-rc3/drivers/hid/hid-ids.h 2013-01-13 18:21:19.901347327 +0900
@@ -706,6 +706,7 @@
#define USB_VENDOR_ID_SONY 0x054c
#define USB_DEVICE_ID_SONY_VAIO_VGX_MOUSE 0x024b
+#define USB_DEVICE_ID_SONY_VAIO_VGP_MOUSE 0x0374
#define USB_DEVICE_ID_SONY_PS3_BDREMOTE0x0306
#define USB_DEVICE_ID_SONY_PS3_CONTROLLER 0x0268
#define USB_DEVICE_ID_SONY_NAVIGATION_CONTROLLER 0x042f
diff -urNp linux-3.8-rc3-orig/drivers/hid/hid-sony.c
linux-3.8-rc3/drivers/hid/hid-sony.c
--- linux-3.8-rc3-orig/drivers/hid/hid-sony.c 2012-12-11 12:30:57.0
+0900
+++ linux-3.8-rc3/drivers/hid/hid-sony.c2013-01-13 18:21:19.901347327
+0900
@@ -44,19 +44,21 @@ static __u8 *sony_report_fixup(struct hi
struct sony_sc *sc = hid_get_drvdata(hdev);
if ((sc-quirks VAIO_RDESC_CONSTANT)
- *rsize = 56 rdesc[54] == 0x81 rdesc[55] == 0x07)
{
- hid_info(hdev, Fixing up Sony Vaio VGX report descriptor\n);
+ *rsize = 56 rdesc[54] == 0x81 rdesc[55] == 0x07) {
+ hid_info(hdev,
+
[PATCH -libata] nv_hardreset: update dangling reference to bugzilla entry
Signed-off-by: Fernando Luis Vazquez Cao <[EMAIL PROTECTED]> --- diff -urNp linux-2.6.24-rc2-orig/drivers/ata/sata_nv.c linux-2.6.24-rc2/drivers/ata/sata_nv.c --- linux-2.6.24-rc2-orig/drivers/ata/sata_nv.c 2007-11-07 10:28:41.0 +0900 +++ linux-2.6.24-rc2/drivers/ata/sata_nv.c 2007-11-07 16:29:21.0 +0900 @@ -1629,7 +1629,7 @@ static int nv_hardreset(struct ata_link /* SATA hardreset fails to retrieve proper device signature on * some controllers. Don't classify on hardreset. For more -* info, see http://bugme.osdl.org/show_bug.cgi?id=3352 +* info, see http://bugzilla.kernel.org/show_bug.cgi?id=3352 */ return sata_std_hardreset(link, , deadline); } - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH -libata] nv_hardreset: update dangling reference to bugzilla entry
Signed-off-by: Fernando Luis Vazquez Cao [EMAIL PROTECTED] --- diff -urNp linux-2.6.24-rc2-orig/drivers/ata/sata_nv.c linux-2.6.24-rc2/drivers/ata/sata_nv.c --- linux-2.6.24-rc2-orig/drivers/ata/sata_nv.c 2007-11-07 10:28:41.0 +0900 +++ linux-2.6.24-rc2/drivers/ata/sata_nv.c 2007-11-07 16:29:21.0 +0900 @@ -1629,7 +1629,7 @@ static int nv_hardreset(struct ata_link /* SATA hardreset fails to retrieve proper device signature on * some controllers. Don't classify on hardreset. For more -* info, see http://bugme.osdl.org/show_bug.cgi?id=3352 +* info, see http://bugzilla.kernel.org/show_bug.cgi?id=3352 */ return sata_std_hardreset(link, dummy, deadline); } - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH 2/2 -mm] sisusb: *_ioctl32_conversion functions do not exist in recent kernels
Remove dead code while at it.
Signed-off-by: Fernando Luis Vazquez Cao <[EMAIL PROTECTED]>
---
diff -urNp linux-2.6.24-rc1-git13-orig/drivers/video/sis/sis.h
linux-2.6.24-rc1-git13/drivers/video/sis/sis.h
--- linux-2.6.24-rc1-git13-orig/drivers/video/sis/sis.h 2007-10-10
05:31:38.0 +0900
+++ linux-2.6.24-rc1-git13/drivers/video/sis/sis.h 2007-11-05
15:24:15.0 +0900
@@ -39,12 +39,7 @@
#include
#ifdef CONFIG_COMPAT
-#if LINUX_VERSION_CODE <= KERNEL_VERSION(2,6,10)
-#include
-#define SIS_OLD_CONFIG_COMPAT
-#else
#define SIS_NEW_CONFIG_COMPAT
-#endif
#endif /* CONFIG_COMPAT */
#if LINUX_VERSION_CODE > KERNEL_VERSION(2,6,8)
@@ -607,9 +602,6 @@ struct sis_video_info {
int haveXGIROM;
int registered;
int warncount;
-#ifdef SIS_OLD_CONFIG_COMPAT
- int ioctl32registered;
-#endif
int sisvga_engine;
int hwcursor_size;
diff -urNp linux-2.6.24-rc1-git13-orig/drivers/video/sis/sis_main.c
linux-2.6.24-rc1-git13/drivers/video/sis/sis_main.c
--- linux-2.6.24-rc1-git13-orig/drivers/video/sis/sis_main.c2007-11-05
10:22:15.0 +0900
+++ linux-2.6.24-rc1-git13/drivers/video/sis/sis_main.c 2007-11-05
15:25:47.0 +0900
@@ -5804,9 +5804,6 @@ sisfb_probe(struct pci_dev *pdev, const
ivideo->pcifunc = PCI_FUNC(pdev->devfn);
ivideo->subsysvendor = pdev->subsystem_vendor;
ivideo->subsysdevice = pdev->subsystem_device;
-#ifdef SIS_OLD_CONFIG_COMPAT
- ivideo->ioctl32registered = 0;
-#endif
#ifndef MODULE
if(sisfb_mode_idx == -1) {
@@ -6419,30 +6416,6 @@ error_3: vfree(ivideo->bios_abase);
ivideo->next = card_list;
card_list = ivideo;
-#ifdef SIS_OLD_CONFIG_COMPAT
- {
- int ret;
- /* Our ioctls are all "32/64bit compatible" */
- ret = register_ioctl32_conversion(FBIO_ALLOC,
NULL);
- ret |= register_ioctl32_conversion(FBIO_FREE,
NULL);
- ret |= register_ioctl32_conversion(FBIOGET_VBLANK,
NULL);
- ret |= register_ioctl32_conversion(SISFB_GET_INFO_SIZE,
NULL);
- ret |= register_ioctl32_conversion(SISFB_GET_INFO,
NULL);
- ret |= register_ioctl32_conversion(SISFB_GET_TVPOSOFFSET,
NULL);
- ret |= register_ioctl32_conversion(SISFB_SET_TVPOSOFFSET,
NULL);
- ret |= register_ioctl32_conversion(SISFB_SET_LOCK,
NULL);
- ret |= register_ioctl32_conversion(SISFB_GET_VBRSTATUS,
NULL);
- ret |= register_ioctl32_conversion(SISFB_GET_AUTOMAXIMIZE,
NULL);
- ret |= register_ioctl32_conversion(SISFB_SET_AUTOMAXIMIZE,
NULL);
- ret |= register_ioctl32_conversion(SISFB_COMMAND,
NULL);
- if(ret)
- printk(KERN_ERR
- "sisfb: Error registering ioctl32
translations\n");
- else
- ivideo->ioctl32registered = 1;
- }
-#endif
-
printk(KERN_INFO "sisfb: 2D acceleration is %s, y-panning %s\n",
ivideo->sisfb_accel ? "enabled" : "disabled",
ivideo->sisfb_ypan ?
@@ -6472,27 +6445,6 @@ static void __devexit sisfb_remove(struc
int registered = ivideo->registered;
int modechanged = ivideo->modechanged;
-#ifdef SIS_OLD_CONFIG_COMPAT
- if(ivideo->ioctl32registered) {
- int ret;
- ret = unregister_ioctl32_conversion(FBIO_ALLOC);
- ret |= unregister_ioctl32_conversion(FBIO_FREE);
- ret |= unregister_ioctl32_conversion(FBIOGET_VBLANK);
- ret |= unregister_ioctl32_conversion(SISFB_GET_INFO_SIZE);
- ret |= unregister_ioctl32_conversion(SISFB_GET_INFO);
- ret |= unregister_ioctl32_conversion(SISFB_GET_TVPOSOFFSET);
- ret |= unregister_ioctl32_conversion(SISFB_SET_TVPOSOFFSET);
- ret |= unregister_ioctl32_conversion(SISFB_SET_LOCK);
- ret |= unregister_ioctl32_conversion(SISFB_GET_VBRSTATUS);
- ret |= unregister_ioctl32_conversion(SISFB_GET_AUTOMAXIMIZE);
- ret |= unregister_ioctl32_conversion(SISFB_SET_AUTOMAXIMIZE);
- ret |= unregister_ioctl32_conversion(SISFB_COMMAND);
- if(ret)
- printk(KERN_ERR
-"sisfb: Error unregistering ioctl32
translations\n");
- }
-#endif
-
/* Unmap */
iounmap(ivideo->mmio_vbase);
iounmap(ivideo->video_vbase);
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ
[PATCH 1/2 -mm] sis FB driver: *_ioctl32_conversion functions do not exist in recent kernels
Remove dead code while at it.
Signed-off-by: Fernando Luis Vazquez Cao <[EMAIL PROTECTED]>
---
diff -urNp linux-2.6.24-rc1-git13-orig/drivers/usb/misc/sisusbvga/sisusb.c
linux-2.6.24-rc1-git13/drivers/usb/misc/sisusbvga/sisusb.c
--- linux-2.6.24-rc1-git13-orig/drivers/usb/misc/sisusbvga/sisusb.c
2007-11-05 10:22:15.0 +0900
+++ linux-2.6.24-rc1-git13/drivers/usb/misc/sisusbvga/sisusb.c 2007-11-05
15:25:51.0 +0900
@@ -3195,20 +3195,6 @@ static int sisusb_probe(struct usb_inter
sisusb->present = 1;
-#ifdef SISUSB_OLD_CONFIG_COMPAT
- {
- int ret;
- /* Our ioctls are all "32/64bit compatible" */
- ret = register_ioctl32_conversion(SISUSB_GET_CONFIG_SIZE, NULL);
- ret |= register_ioctl32_conversion(SISUSB_GET_CONFIG, NULL);
- ret |= register_ioctl32_conversion(SISUSB_COMMAND, NULL);
- if (ret)
- dev_err(>sisusb_dev->dev, "Error registering ioctl32
translations\n");
- else
- sisusb->ioctl32registered = 1;
- }
-#endif
-
if (dev->speed == USB_SPEED_HIGH) {
int initscreen = 1;
#ifdef INCL_SISUSB_CON
@@ -3271,19 +3257,6 @@ static void sisusb_disconnect(struct usb
usb_set_intfdata(intf, NULL);
-#ifdef SISUSB_OLD_CONFIG_COMPAT
- if (sisusb->ioctl32registered) {
- int ret;
- sisusb->ioctl32registered = 0;
- ret = unregister_ioctl32_conversion(SISUSB_GET_CONFIG_SIZE);
- ret |= unregister_ioctl32_conversion(SISUSB_GET_CONFIG);
- ret |= unregister_ioctl32_conversion(SISUSB_COMMAND);
- if (ret) {
- dev_err(>sisusb_dev->dev, "Error unregistering
ioctl32 translations\n");
- }
- }
-#endif
-
sisusb->present = 0;
sisusb->ready = 0;
diff -urNp linux-2.6.24-rc1-git13-orig/drivers/usb/misc/sisusbvga/sisusb.h
linux-2.6.24-rc1-git13/drivers/usb/misc/sisusbvga/sisusb.h
--- linux-2.6.24-rc1-git13-orig/drivers/usb/misc/sisusbvga/sisusb.h
2007-11-05 10:22:15.0 +0900
+++ linux-2.6.24-rc1-git13/drivers/usb/misc/sisusbvga/sisusb.h 2007-11-05
15:20:23.0 +0900
@@ -120,9 +120,6 @@ struct sisusb_usb_data {
int isopen; /* !=0 if open */
int present;/* !=0 if device is present on the bus */
int ready; /* !=0 if device is ready for userland */
-#ifdef SISUSB_OLD_CONFIG_COMPAT
- int ioctl32registered;
-#endif
int numobufs; /* number of obufs = number of out urbs */
char *obuf[NUMOBUFS], *ibuf;/* transfer buffers */
int obufsize, ibufsize;
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH 1/2 -mm] sis FB driver: *_ioctl32_conversion functions do not exist in recent kernels
Remove dead code while at it.
Signed-off-by: Fernando Luis Vazquez Cao [EMAIL PROTECTED]
---
diff -urNp linux-2.6.24-rc1-git13-orig/drivers/usb/misc/sisusbvga/sisusb.c
linux-2.6.24-rc1-git13/drivers/usb/misc/sisusbvga/sisusb.c
--- linux-2.6.24-rc1-git13-orig/drivers/usb/misc/sisusbvga/sisusb.c
2007-11-05 10:22:15.0 +0900
+++ linux-2.6.24-rc1-git13/drivers/usb/misc/sisusbvga/sisusb.c 2007-11-05
15:25:51.0 +0900
@@ -3195,20 +3195,6 @@ static int sisusb_probe(struct usb_inter
sisusb-present = 1;
-#ifdef SISUSB_OLD_CONFIG_COMPAT
- {
- int ret;
- /* Our ioctls are all 32/64bit compatible */
- ret = register_ioctl32_conversion(SISUSB_GET_CONFIG_SIZE, NULL);
- ret |= register_ioctl32_conversion(SISUSB_GET_CONFIG, NULL);
- ret |= register_ioctl32_conversion(SISUSB_COMMAND, NULL);
- if (ret)
- dev_err(sisusb-sisusb_dev-dev, Error registering ioctl32
translations\n);
- else
- sisusb-ioctl32registered = 1;
- }
-#endif
-
if (dev-speed == USB_SPEED_HIGH) {
int initscreen = 1;
#ifdef INCL_SISUSB_CON
@@ -3271,19 +3257,6 @@ static void sisusb_disconnect(struct usb
usb_set_intfdata(intf, NULL);
-#ifdef SISUSB_OLD_CONFIG_COMPAT
- if (sisusb-ioctl32registered) {
- int ret;
- sisusb-ioctl32registered = 0;
- ret = unregister_ioctl32_conversion(SISUSB_GET_CONFIG_SIZE);
- ret |= unregister_ioctl32_conversion(SISUSB_GET_CONFIG);
- ret |= unregister_ioctl32_conversion(SISUSB_COMMAND);
- if (ret) {
- dev_err(sisusb-sisusb_dev-dev, Error unregistering
ioctl32 translations\n);
- }
- }
-#endif
-
sisusb-present = 0;
sisusb-ready = 0;
diff -urNp linux-2.6.24-rc1-git13-orig/drivers/usb/misc/sisusbvga/sisusb.h
linux-2.6.24-rc1-git13/drivers/usb/misc/sisusbvga/sisusb.h
--- linux-2.6.24-rc1-git13-orig/drivers/usb/misc/sisusbvga/sisusb.h
2007-11-05 10:22:15.0 +0900
+++ linux-2.6.24-rc1-git13/drivers/usb/misc/sisusbvga/sisusb.h 2007-11-05
15:20:23.0 +0900
@@ -120,9 +120,6 @@ struct sisusb_usb_data {
int isopen; /* !=0 if open */
int present;/* !=0 if device is present on the bus */
int ready; /* !=0 if device is ready for userland */
-#ifdef SISUSB_OLD_CONFIG_COMPAT
- int ioctl32registered;
-#endif
int numobufs; /* number of obufs = number of out urbs */
char *obuf[NUMOBUFS], *ibuf;/* transfer buffers */
int obufsize, ibufsize;
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH 2/2 -mm] sisusb: *_ioctl32_conversion functions do not exist in recent kernels
Remove dead code while at it.
Signed-off-by: Fernando Luis Vazquez Cao [EMAIL PROTECTED]
---
diff -urNp linux-2.6.24-rc1-git13-orig/drivers/video/sis/sis.h
linux-2.6.24-rc1-git13/drivers/video/sis/sis.h
--- linux-2.6.24-rc1-git13-orig/drivers/video/sis/sis.h 2007-10-10
05:31:38.0 +0900
+++ linux-2.6.24-rc1-git13/drivers/video/sis/sis.h 2007-11-05
15:24:15.0 +0900
@@ -39,12 +39,7 @@
#include linux/spinlock.h
#ifdef CONFIG_COMPAT
-#if LINUX_VERSION_CODE = KERNEL_VERSION(2,6,10)
-#include linux/ioctl32.h
-#define SIS_OLD_CONFIG_COMPAT
-#else
#define SIS_NEW_CONFIG_COMPAT
-#endif
#endif /* CONFIG_COMPAT */
#if LINUX_VERSION_CODE KERNEL_VERSION(2,6,8)
@@ -607,9 +602,6 @@ struct sis_video_info {
int haveXGIROM;
int registered;
int warncount;
-#ifdef SIS_OLD_CONFIG_COMPAT
- int ioctl32registered;
-#endif
int sisvga_engine;
int hwcursor_size;
diff -urNp linux-2.6.24-rc1-git13-orig/drivers/video/sis/sis_main.c
linux-2.6.24-rc1-git13/drivers/video/sis/sis_main.c
--- linux-2.6.24-rc1-git13-orig/drivers/video/sis/sis_main.c2007-11-05
10:22:15.0 +0900
+++ linux-2.6.24-rc1-git13/drivers/video/sis/sis_main.c 2007-11-05
15:25:47.0 +0900
@@ -5804,9 +5804,6 @@ sisfb_probe(struct pci_dev *pdev, const
ivideo-pcifunc = PCI_FUNC(pdev-devfn);
ivideo-subsysvendor = pdev-subsystem_vendor;
ivideo-subsysdevice = pdev-subsystem_device;
-#ifdef SIS_OLD_CONFIG_COMPAT
- ivideo-ioctl32registered = 0;
-#endif
#ifndef MODULE
if(sisfb_mode_idx == -1) {
@@ -6419,30 +6416,6 @@ error_3: vfree(ivideo-bios_abase);
ivideo-next = card_list;
card_list = ivideo;
-#ifdef SIS_OLD_CONFIG_COMPAT
- {
- int ret;
- /* Our ioctls are all 32/64bit compatible */
- ret = register_ioctl32_conversion(FBIO_ALLOC,
NULL);
- ret |= register_ioctl32_conversion(FBIO_FREE,
NULL);
- ret |= register_ioctl32_conversion(FBIOGET_VBLANK,
NULL);
- ret |= register_ioctl32_conversion(SISFB_GET_INFO_SIZE,
NULL);
- ret |= register_ioctl32_conversion(SISFB_GET_INFO,
NULL);
- ret |= register_ioctl32_conversion(SISFB_GET_TVPOSOFFSET,
NULL);
- ret |= register_ioctl32_conversion(SISFB_SET_TVPOSOFFSET,
NULL);
- ret |= register_ioctl32_conversion(SISFB_SET_LOCK,
NULL);
- ret |= register_ioctl32_conversion(SISFB_GET_VBRSTATUS,
NULL);
- ret |= register_ioctl32_conversion(SISFB_GET_AUTOMAXIMIZE,
NULL);
- ret |= register_ioctl32_conversion(SISFB_SET_AUTOMAXIMIZE,
NULL);
- ret |= register_ioctl32_conversion(SISFB_COMMAND,
NULL);
- if(ret)
- printk(KERN_ERR
- sisfb: Error registering ioctl32
translations\n);
- else
- ivideo-ioctl32registered = 1;
- }
-#endif
-
printk(KERN_INFO sisfb: 2D acceleration is %s, y-panning %s\n,
ivideo-sisfb_accel ? enabled : disabled,
ivideo-sisfb_ypan ?
@@ -6472,27 +6445,6 @@ static void __devexit sisfb_remove(struc
int registered = ivideo-registered;
int modechanged = ivideo-modechanged;
-#ifdef SIS_OLD_CONFIG_COMPAT
- if(ivideo-ioctl32registered) {
- int ret;
- ret = unregister_ioctl32_conversion(FBIO_ALLOC);
- ret |= unregister_ioctl32_conversion(FBIO_FREE);
- ret |= unregister_ioctl32_conversion(FBIOGET_VBLANK);
- ret |= unregister_ioctl32_conversion(SISFB_GET_INFO_SIZE);
- ret |= unregister_ioctl32_conversion(SISFB_GET_INFO);
- ret |= unregister_ioctl32_conversion(SISFB_GET_TVPOSOFFSET);
- ret |= unregister_ioctl32_conversion(SISFB_SET_TVPOSOFFSET);
- ret |= unregister_ioctl32_conversion(SISFB_SET_LOCK);
- ret |= unregister_ioctl32_conversion(SISFB_GET_VBRSTATUS);
- ret |= unregister_ioctl32_conversion(SISFB_GET_AUTOMAXIMIZE);
- ret |= unregister_ioctl32_conversion(SISFB_SET_AUTOMAXIMIZE);
- ret |= unregister_ioctl32_conversion(SISFB_COMMAND);
- if(ret)
- printk(KERN_ERR
-sisfb: Error unregistering ioctl32
translations\n);
- }
-#endif
-
/* Unmap */
iounmap(ivideo-mmio_vbase);
iounmap(ivideo-video_vbase);
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at
Re: [PATCH] isdn/sc: remove unused REQUEST_IRQ and unnecessary header file
Hi Karsten, What happened to this patch? Should I send it to Andrew or will you take it in your tree before pushing it upstream? Thank you in advance. - Fernando On Sat, 2007-08-25 at 21:30 +0200, Karsten Keil wrote: > [PATCH] isdn/sc: remove unused REQUEST_IRQ and unnecessary > > REQUEST_IRQ is never used, so delete it. In the process get rid of the > macro FREE_IRQ which makes the code unnecessarily difficult to read. > > Signed-off-by: Fernando Luis Vazquez Cao <[EMAIL PROTECTED]> > Acked-by: Karsten Keil <[EMAIL PROTECTED]> > --- > > diff -urNp linux-2.6.23-rc3-orig/drivers/isdn/sc/debug.h > linux-2.6.23-rc3/drivers/isdn/sc/debug.h > --- linux-2.6.23-rc3-orig/drivers/isdn/sc/debug.h 2007-07-09 > 08:32:17.0 +0900 > +++ linux-2.6.23-rc3/drivers/isdn/sc/debug.h 1970-01-01 09:00:00.0 > +0900 > @@ -1,19 +0,0 @@ > -/* $Id: debug.h,v 1.2.8.1 2001/09/23 22:24:59 kai Exp $ > - * > - * Copyright (C) 1996 SpellCaster Telecommunications Inc. > - * > - * This software may be used and distributed according to the terms > - * of the GNU General Public License, incorporated herein by reference. > - * > - * For more information, please contact [EMAIL PROTECTED] or write: > - * > - * SpellCaster Telecommunications Inc. > - * 5621 Finch Avenue East, Unit #3 > - * Scarborough, Ontario Canada > - * M1B 2T9 > - * +1 (416) 297-8565 > - * +1 (416) 297-6433 Facsimile > - */ > - > -#define REQUEST_IRQ(a,b,c,d,e) request_irq(a,b,c,d,e) > -#define FREE_IRQ(a,b) free_irq(a,b) > diff -urNp linux-2.6.23-rc3-orig/drivers/isdn/sc/includes.h > linux-2.6.23-rc3/drivers/isdn/sc/includes.h > --- linux-2.6.23-rc3-orig/drivers/isdn/sc/includes.h 2007-07-09 > 08:32:17.0 +0900 > +++ linux-2.6.23-rc3/drivers/isdn/sc/includes.h 2007-08-24 > 20:55:45.0 +0900 > @@ -14,4 +14,3 @@ > #include > #include > #include > -#include "debug.h" > diff -urNp linux-2.6.23-rc3-orig/drivers/isdn/sc/init.c > linux-2.6.23-rc3/drivers/isdn/sc/init.c > --- linux-2.6.23-rc3-orig/drivers/isdn/sc/init.c 2007-07-09 > 08:32:17.0 +0900 > +++ linux-2.6.23-rc3/drivers/isdn/sc/init.c 2007-08-24 20:31:58.0 > +0900 > @@ -404,7 +404,7 @@ static void __exit sc_exit(void) > /* >* Release the IRQ >*/ > - FREE_IRQ(sc_adapter[i]->interrupt, NULL); > + free_irq(sc_adapter[i]->interrupt, NULL); > > /* >* Reset for a clean start > - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] isdn/sc: remove unused REQUEST_IRQ and unnecessary header file
Hi Karsten, What happened to this patch? Should I send it to Andrew or will you take it in your tree before pushing it upstream? Thank you in advance. - Fernando On Sat, 2007-08-25 at 21:30 +0200, Karsten Keil wrote: [PATCH] isdn/sc: remove unused REQUEST_IRQ and unnecessary REQUEST_IRQ is never used, so delete it. In the process get rid of the macro FREE_IRQ which makes the code unnecessarily difficult to read. Signed-off-by: Fernando Luis Vazquez Cao [EMAIL PROTECTED] Acked-by: Karsten Keil [EMAIL PROTECTED] --- diff -urNp linux-2.6.23-rc3-orig/drivers/isdn/sc/debug.h linux-2.6.23-rc3/drivers/isdn/sc/debug.h --- linux-2.6.23-rc3-orig/drivers/isdn/sc/debug.h 2007-07-09 08:32:17.0 +0900 +++ linux-2.6.23-rc3/drivers/isdn/sc/debug.h 1970-01-01 09:00:00.0 +0900 @@ -1,19 +0,0 @@ -/* $Id: debug.h,v 1.2.8.1 2001/09/23 22:24:59 kai Exp $ - * - * Copyright (C) 1996 SpellCaster Telecommunications Inc. - * - * This software may be used and distributed according to the terms - * of the GNU General Public License, incorporated herein by reference. - * - * For more information, please contact [EMAIL PROTECTED] or write: - * - * SpellCaster Telecommunications Inc. - * 5621 Finch Avenue East, Unit #3 - * Scarborough, Ontario Canada - * M1B 2T9 - * +1 (416) 297-8565 - * +1 (416) 297-6433 Facsimile - */ - -#define REQUEST_IRQ(a,b,c,d,e) request_irq(a,b,c,d,e) -#define FREE_IRQ(a,b) free_irq(a,b) diff -urNp linux-2.6.23-rc3-orig/drivers/isdn/sc/includes.h linux-2.6.23-rc3/drivers/isdn/sc/includes.h --- linux-2.6.23-rc3-orig/drivers/isdn/sc/includes.h 2007-07-09 08:32:17.0 +0900 +++ linux-2.6.23-rc3/drivers/isdn/sc/includes.h 2007-08-24 20:55:45.0 +0900 @@ -14,4 +14,3 @@ #include linux/timer.h #include linux/wait.h #include linux/isdnif.h -#include debug.h diff -urNp linux-2.6.23-rc3-orig/drivers/isdn/sc/init.c linux-2.6.23-rc3/drivers/isdn/sc/init.c --- linux-2.6.23-rc3-orig/drivers/isdn/sc/init.c 2007-07-09 08:32:17.0 +0900 +++ linux-2.6.23-rc3/drivers/isdn/sc/init.c 2007-08-24 20:31:58.0 +0900 @@ -404,7 +404,7 @@ static void __exit sc_exit(void) /* * Release the IRQ */ - FREE_IRQ(sc_adapter[i]-interrupt, NULL); + free_irq(sc_adapter[i]-interrupt, NULL); /* * Reset for a clean start - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH -mm] Make Smart Battery System depend on ACPI_SBS ACPI_PROC_EVENT
Playing with the latest -mm kernel I stumbled upon the following compile error: CHK include/linux/version.h CHK include/linux/utsrelease.h CALLscripts/checksyscalls.sh :1389:2: warning: #warning syscall revokeat not implemented :1393:2: warning: #warning syscall frevoke not implemented CHK include/linux/compile.h LD drivers/acpi/built-in.o LD [M] drivers/acpi/processor.o CC [M] drivers/acpi/sbs.o drivers/acpi/sbs.c: In function 'acpi_battery_add': drivers/acpi/sbs.c:825: warning: ignoring return value of 'device_create_file',\ declared with attribute warn_unused_result drivers/acpi/sbs.c: In function 'acpi_sbs_callback': drivers/acpi/sbs.c:898: error: implicit declaration of function 'acpi_bus_gener\ ate_proc_event4' make[2]: *** [drivers/acpi/sbs.o] Error 1 make[1]: *** [drivers/acpi] Error 2 make: *** [drivers] Error 2 The problem turned out to be that the sbs module is still using acpi_bus_generate_proc_event4(), which is part of the deprecated /proc/acpi/event interface. This patch makes this dependency explicit. Signed-off-by: Fernando Luis Vazquez Cao <[EMAIL PROTECTED]> --- diff -urNp linux-2.6.23-rc8-mm2-orig/drivers/acpi/Kconfig linux-2.6.23-rc8-mm2/drivers/acpi/Kconfig --- linux-2.6.23-rc8-mm2-orig/drivers/acpi/Kconfig 2007-10-09 17:12:55.0 +0900 +++ linux-2.6.23-rc8-mm2/drivers/acpi/Kconfig 2007-10-09 16:36:07.0 +0900 @@ -352,6 +352,7 @@ config ACPI_HOTPLUG_MEMORY config ACPI_SBS tristate "Smart Battery System (EXPERIMENTAL)" depends on X86 + depends on ACPI_PROC_EVENT depends on EXPERIMENTAL help This driver adds support for the Smart Battery System. - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH -mm] Make Smart Battery System depend on ACPI_SBS ACPI_PROC_EVENT
Playing with the latest -mm kernel I stumbled upon the following compile error: CHK include/linux/version.h CHK include/linux/utsrelease.h CALLscripts/checksyscalls.sh stdin:1389:2: warning: #warning syscall revokeat not implemented stdin:1393:2: warning: #warning syscall frevoke not implemented CHK include/linux/compile.h LD drivers/acpi/built-in.o LD [M] drivers/acpi/processor.o CC [M] drivers/acpi/sbs.o drivers/acpi/sbs.c: In function 'acpi_battery_add': drivers/acpi/sbs.c:825: warning: ignoring return value of 'device_create_file',\ declared with attribute warn_unused_result drivers/acpi/sbs.c: In function 'acpi_sbs_callback': drivers/acpi/sbs.c:898: error: implicit declaration of function 'acpi_bus_gener\ ate_proc_event4' make[2]: *** [drivers/acpi/sbs.o] Error 1 make[1]: *** [drivers/acpi] Error 2 make: *** [drivers] Error 2 The problem turned out to be that the sbs module is still using acpi_bus_generate_proc_event4(), which is part of the deprecated /proc/acpi/event interface. This patch makes this dependency explicit. Signed-off-by: Fernando Luis Vazquez Cao [EMAIL PROTECTED] --- diff -urNp linux-2.6.23-rc8-mm2-orig/drivers/acpi/Kconfig linux-2.6.23-rc8-mm2/drivers/acpi/Kconfig --- linux-2.6.23-rc8-mm2-orig/drivers/acpi/Kconfig 2007-10-09 17:12:55.0 +0900 +++ linux-2.6.23-rc8-mm2/drivers/acpi/Kconfig 2007-10-09 16:36:07.0 +0900 @@ -352,6 +352,7 @@ config ACPI_HOTPLUG_MEMORY config ACPI_SBS tristate Smart Battery System (EXPERIMENTAL) depends on X86 + depends on ACPI_PROC_EVENT depends on EXPERIMENTAL help This driver adds support for the Smart Battery System. - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH] isdn/sc: remove unused REQUEST_IRQ and unnecessary header file
REQUEST_IRQ is never used, so delete it. In the process get rid of the macro FREE_IRQ which makes the code unnecessarily difficult to read. Signed-off-by: Fernando Luis Vazquez Cao <[EMAIL PROTECTED]> --- diff -urNp linux-2.6.23-rc3-orig/drivers/isdn/sc/debug.h linux-2.6.23-rc3/drivers/isdn/sc/debug.h --- linux-2.6.23-rc3-orig/drivers/isdn/sc/debug.h 2007-07-09 08:32:17.0 +0900 +++ linux-2.6.23-rc3/drivers/isdn/sc/debug.h1970-01-01 09:00:00.0 +0900 @@ -1,19 +0,0 @@ -/* $Id: debug.h,v 1.2.8.1 2001/09/23 22:24:59 kai Exp $ - * - * Copyright (C) 1996 SpellCaster Telecommunications Inc. - * - * This software may be used and distributed according to the terms - * of the GNU General Public License, incorporated herein by reference. - * - * For more information, please contact [EMAIL PROTECTED] or write: - * - * SpellCaster Telecommunications Inc. - * 5621 Finch Avenue East, Unit #3 - * Scarborough, Ontario Canada - * M1B 2T9 - * +1 (416) 297-8565 - * +1 (416) 297-6433 Facsimile - */ - -#define REQUEST_IRQ(a,b,c,d,e) request_irq(a,b,c,d,e) -#define FREE_IRQ(a,b) free_irq(a,b) diff -urNp linux-2.6.23-rc3-orig/drivers/isdn/sc/init.c linux-2.6.23-rc3/drivers/isdn/sc/init.c --- linux-2.6.23-rc3-orig/drivers/isdn/sc/init.c2007-07-09 08:32:17.0 +0900 +++ linux-2.6.23-rc3/drivers/isdn/sc/init.c 2007-08-24 18:12:27.0 +0900 @@ -404,7 +404,7 @@ static void __exit sc_exit(void) /* * Release the IRQ */ - FREE_IRQ(sc_adapter[i]->interrupt, NULL); + free_irq(sc_adapter[i]->interrupt, NULL); /* * Reset for a clean start - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH] isdn/sc: remove unused REQUEST_IRQ and unnecessary header file
REQUEST_IRQ is never used, so delete it. In the process get rid of the macro FREE_IRQ which makes the code unnecessarily difficult to read. Signed-off-by: Fernando Luis Vazquez Cao [EMAIL PROTECTED] --- diff -urNp linux-2.6.23-rc3-orig/drivers/isdn/sc/debug.h linux-2.6.23-rc3/drivers/isdn/sc/debug.h --- linux-2.6.23-rc3-orig/drivers/isdn/sc/debug.h 2007-07-09 08:32:17.0 +0900 +++ linux-2.6.23-rc3/drivers/isdn/sc/debug.h1970-01-01 09:00:00.0 +0900 @@ -1,19 +0,0 @@ -/* $Id: debug.h,v 1.2.8.1 2001/09/23 22:24:59 kai Exp $ - * - * Copyright (C) 1996 SpellCaster Telecommunications Inc. - * - * This software may be used and distributed according to the terms - * of the GNU General Public License, incorporated herein by reference. - * - * For more information, please contact [EMAIL PROTECTED] or write: - * - * SpellCaster Telecommunications Inc. - * 5621 Finch Avenue East, Unit #3 - * Scarborough, Ontario Canada - * M1B 2T9 - * +1 (416) 297-8565 - * +1 (416) 297-6433 Facsimile - */ - -#define REQUEST_IRQ(a,b,c,d,e) request_irq(a,b,c,d,e) -#define FREE_IRQ(a,b) free_irq(a,b) diff -urNp linux-2.6.23-rc3-orig/drivers/isdn/sc/init.c linux-2.6.23-rc3/drivers/isdn/sc/init.c --- linux-2.6.23-rc3-orig/drivers/isdn/sc/init.c2007-07-09 08:32:17.0 +0900 +++ linux-2.6.23-rc3/drivers/isdn/sc/init.c 2007-08-24 18:12:27.0 +0900 @@ -404,7 +404,7 @@ static void __exit sc_exit(void) /* * Release the IRQ */ - FREE_IRQ(sc_adapter[i]-interrupt, NULL); + free_irq(sc_adapter[i]-interrupt, NULL); /* * Reset for a clean start - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] ppc: remove unused amiga_request_irq and mach_request_irq
On Wed, 2007-08-22 at 13:28 -0500, Linas Vepstas wrote: > On Wed, Aug 22, 2007 at 07:44:41PM +1000, Paul Mackerras wrote: > > Fernando Luis Vázquez Cao writes: > > > > > amiga_request_irq and mach_request_irq are never used, so delete them. > > > > OK, but is there a particular reason you want to do this? > > > > The whole of arch/ppc is going away eventually, so I don't think we > > need to remove it piece by piece. > > Its often easier to port/move stuff if you clean it up first. Agreed. It would make things easier for me too. Fernando - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] ppc: remove unused amiga_request_irq and mach_request_irq
On Wed, 2007-08-22 at 19:44 +1000, Paul Mackerras wrote: > Fernando Luis Vázquez Cao writes: > > > amiga_request_irq and mach_request_irq are never used, so delete them. > > OK, but is there a particular reason you want to do this? Hi Paul, Thank you for your reply. I am currently auditing all the interrupt handlers and related code (request_irq(), free_irq(), and others of that ilk) and, in the process, I found some dead code. Getting rid of this cruft would just make my life easier. The final goal of this audit is to determine whether the Linux interrupt handlers are kdump-ready. With the advent of kdump it is now possible that device drivers have to handle pending interrupts generated in the context of a previous kernel. Any pending interrupts will come in as soon as the IRQ is allocated, but, unfortunately, not all the device drivers handle this situation properly. Besides, I am also trying to determine if applying this patch: http://lkml.org/lkml/2007/7/19/687 is a sane thing to do. > The whole of arch/ppc is going away eventually, so I don't think we > need to remove it piece by piece. As Linas mentioned in his reply, cleaning things up first will probably make things easier for you too. Please consider merging the four patches I sent to the PPC mailing list. Fernando - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH] ppc: remove unused sys_free_irq
sys_free_irq is never used, so delete it.
Signed-off-by: Fernando Luis Vazquez Cao <[EMAIL PROTECTED]>
---
diff -urNp linux-2.6.23-rc3-orig/arch/ppc/amiga/ints.c
linux-2.6.23-rc3/arch/ppc/amiga/ints.c
--- linux-2.6.23-rc3-orig/arch/ppc/amiga/ints.c 2007-08-22 17:14:32.0
+0900
+++ linux-2.6.23-rc3/arch/ppc/amiga/ints.c 2007-08-22 17:15:27.0
+0900
@@ -75,23 +75,6 @@ irq_node_t *new_irq_node(void)
return NULL;
}
-void sys_free_irq(unsigned int irq, void *dev_id)
-{
- if (irq < IRQ1 || irq > IRQ7) {
- printk("%s: Incorrect IRQ %d\n", __FUNCTION__, irq);
- return;
- }
-
- if (irq_list[irq].dev_id != dev_id)
- printk("%s: Removing probably wrong IRQ %d from %s\n",
- __FUNCTION__, irq, irq_list[irq].devname);
-
- irq_list[irq].handler = (*mach_default_handler)[irq];
- irq_list[irq].flags = 0;
- irq_list[irq].dev_id = NULL;
- irq_list[irq].devname = default_names[irq];
-}
-
asmlinkage void process_int(unsigned long vec, struct pt_regs *fp)
{
if (vec >= VEC_INT1 && vec <= VEC_INT7 && !MACH_IS_BVME6000) {
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH] ppc: remove unused sys_request_irq
sys_request_irq is never used, so delete it.
Signed-off-by: Fernando Luis Vazquez Cao <[EMAIL PROTECTED]>
---
diff -urNp linux-2.6.23-rc3-orig/arch/ppc/amiga/ints.c
linux-2.6.23-rc3/arch/ppc/amiga/ints.c
--- linux-2.6.23-rc3-orig/arch/ppc/amiga/ints.c 2007-07-09 08:32:17.0
+0900
+++ linux-2.6.23-rc3/arch/ppc/amiga/ints.c 2007-08-22 17:12:48.0
+0900
@@ -75,38 +75,6 @@ irq_node_t *new_irq_node(void)
return NULL;
}
-int sys_request_irq(unsigned int irq,
-void (*handler)(int, void *, struct pt_regs *),
-unsigned long flags, const char *devname, void *dev_id)
-{
- if (irq < IRQ1 || irq > IRQ7) {
- printk("%s: Incorrect IRQ %d from %s\n",
- __FUNCTION__, irq, devname);
- return -ENXIO;
- }
-
-#if 0
- if (!(irq_list[irq].flags & IRQ_FLG_STD)) {
- if (irq_list[irq].flags & IRQ_FLG_LOCK) {
- printk("%s: IRQ %d from %s is not replaceable\n",
- __FUNCTION__, irq, irq_list[irq].devname);
- return -EBUSY;
- }
- if (!(flags & IRQ_FLG_REPLACE)) {
- printk("%s: %s can't replace IRQ %d from %s\n",
- __FUNCTION__, devname, irq,
irq_list[irq].devname);
- return -EBUSY;
- }
- }
-#endif
-
- irq_list[irq].handler = handler;
- irq_list[irq].flags = flags;
- irq_list[irq].dev_id = dev_id;
- irq_list[irq].devname = devname;
- return 0;
-}
-
void sys_free_irq(unsigned int irq, void *dev_id)
{
if (irq < IRQ1 || irq > IRQ7) {
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH] ppc: remove unused amiga_free_irq and mach_free_irq
amiga_free_irq and mach_free_irq are never used, so delete them. Signed-off-by: Fernando Luis Vazquez Cao <[EMAIL PROTECTED]> --- diff -urNp linux-2.6.23-rc3-orig/arch/ppc/amiga/config.c linux-2.6.23-rc3/arch/ppc/amiga/config.c --- linux-2.6.23-rc3-orig/arch/ppc/amiga/config.c 2007-08-22 15:53:20.0 +0900 +++ linux-2.6.23-rc3/arch/ppc/amiga/config.c2007-08-22 15:56:15.0 +0900 @@ -72,7 +72,6 @@ static void amiga_sched_init(irqreturn_t /* amiga specific irq functions */ extern void amiga_init_IRQ (void); extern void (*amiga_default_handler[]) (int, void *, struct pt_regs *); -extern void amiga_free_irq (unsigned int irq, void *dev_id); extern void amiga_enable_irq (unsigned int); extern void amiga_disable_irq (unsigned int); static void amiga_get_model(char *model); @@ -378,7 +377,6 @@ void __init config_amiga(void) mach_init_IRQ= amiga_init_IRQ; #ifndef CONFIG_APUS mach_default_handler = _default_handler; - mach_free_irq= amiga_free_irq; enable_irq = amiga_enable_irq; disable_irq = amiga_disable_irq; #endif - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH] ppc: remove unused amiga_request_irq and mach_request_irq
amiga_request_irq and mach_request_irq are never used, so delete them. Signed-off-by: Fernando Luis Vazquez Cao <[EMAIL PROTECTED]> --- diff -urNp linux-2.6.23-rc3-orig/arch/ppc/amiga/config.c linux-2.6.23-rc3/arch/ppc/amiga/config.c --- linux-2.6.23-rc3-orig/arch/ppc/amiga/config.c 2007-07-09 08:32:17.0 +0900 +++ linux-2.6.23-rc3/arch/ppc/amiga/config.c2007-08-22 15:45:51.0 +0900 @@ -72,10 +72,6 @@ static void amiga_sched_init(irqreturn_t /* amiga specific irq functions */ extern void amiga_init_IRQ (void); extern void (*amiga_default_handler[]) (int, void *, struct pt_regs *); -extern int amiga_request_irq (unsigned int irq, - void (*handler)(int, void *, struct pt_regs *), - unsigned long flags, const char *devname, - void *dev_id); extern void amiga_free_irq (unsigned int irq, void *dev_id); extern void amiga_enable_irq (unsigned int); extern void amiga_disable_irq (unsigned int); @@ -382,7 +378,6 @@ void __init config_amiga(void) mach_init_IRQ= amiga_init_IRQ; #ifndef CONFIG_APUS mach_default_handler = _default_handler; - mach_request_irq = amiga_request_irq; mach_free_irq= amiga_free_irq; enable_irq = amiga_enable_irq; disable_irq = amiga_disable_irq; - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH] ppc: remove unused amiga_request_irq and mach_request_irq
amiga_request_irq and mach_request_irq are never used, so delete them. Signed-off-by: Fernando Luis Vazquez Cao [EMAIL PROTECTED] --- diff -urNp linux-2.6.23-rc3-orig/arch/ppc/amiga/config.c linux-2.6.23-rc3/arch/ppc/amiga/config.c --- linux-2.6.23-rc3-orig/arch/ppc/amiga/config.c 2007-07-09 08:32:17.0 +0900 +++ linux-2.6.23-rc3/arch/ppc/amiga/config.c2007-08-22 15:45:51.0 +0900 @@ -72,10 +72,6 @@ static void amiga_sched_init(irqreturn_t /* amiga specific irq functions */ extern void amiga_init_IRQ (void); extern void (*amiga_default_handler[]) (int, void *, struct pt_regs *); -extern int amiga_request_irq (unsigned int irq, - void (*handler)(int, void *, struct pt_regs *), - unsigned long flags, const char *devname, - void *dev_id); extern void amiga_free_irq (unsigned int irq, void *dev_id); extern void amiga_enable_irq (unsigned int); extern void amiga_disable_irq (unsigned int); @@ -382,7 +378,6 @@ void __init config_amiga(void) mach_init_IRQ= amiga_init_IRQ; #ifndef CONFIG_APUS mach_default_handler = amiga_default_handler; - mach_request_irq = amiga_request_irq; mach_free_irq= amiga_free_irq; enable_irq = amiga_enable_irq; disable_irq = amiga_disable_irq; - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH] ppc: remove unused amiga_free_irq and mach_free_irq
amiga_free_irq and mach_free_irq are never used, so delete them. Signed-off-by: Fernando Luis Vazquez Cao [EMAIL PROTECTED] --- diff -urNp linux-2.6.23-rc3-orig/arch/ppc/amiga/config.c linux-2.6.23-rc3/arch/ppc/amiga/config.c --- linux-2.6.23-rc3-orig/arch/ppc/amiga/config.c 2007-08-22 15:53:20.0 +0900 +++ linux-2.6.23-rc3/arch/ppc/amiga/config.c2007-08-22 15:56:15.0 +0900 @@ -72,7 +72,6 @@ static void amiga_sched_init(irqreturn_t /* amiga specific irq functions */ extern void amiga_init_IRQ (void); extern void (*amiga_default_handler[]) (int, void *, struct pt_regs *); -extern void amiga_free_irq (unsigned int irq, void *dev_id); extern void amiga_enable_irq (unsigned int); extern void amiga_disable_irq (unsigned int); static void amiga_get_model(char *model); @@ -378,7 +377,6 @@ void __init config_amiga(void) mach_init_IRQ= amiga_init_IRQ; #ifndef CONFIG_APUS mach_default_handler = amiga_default_handler; - mach_free_irq= amiga_free_irq; enable_irq = amiga_enable_irq; disable_irq = amiga_disable_irq; #endif - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH] ppc: remove unused sys_request_irq
sys_request_irq is never used, so delete it.
Signed-off-by: Fernando Luis Vazquez Cao [EMAIL PROTECTED]
---
diff -urNp linux-2.6.23-rc3-orig/arch/ppc/amiga/ints.c
linux-2.6.23-rc3/arch/ppc/amiga/ints.c
--- linux-2.6.23-rc3-orig/arch/ppc/amiga/ints.c 2007-07-09 08:32:17.0
+0900
+++ linux-2.6.23-rc3/arch/ppc/amiga/ints.c 2007-08-22 17:12:48.0
+0900
@@ -75,38 +75,6 @@ irq_node_t *new_irq_node(void)
return NULL;
}
-int sys_request_irq(unsigned int irq,
-void (*handler)(int, void *, struct pt_regs *),
-unsigned long flags, const char *devname, void *dev_id)
-{
- if (irq IRQ1 || irq IRQ7) {
- printk(%s: Incorrect IRQ %d from %s\n,
- __FUNCTION__, irq, devname);
- return -ENXIO;
- }
-
-#if 0
- if (!(irq_list[irq].flags IRQ_FLG_STD)) {
- if (irq_list[irq].flags IRQ_FLG_LOCK) {
- printk(%s: IRQ %d from %s is not replaceable\n,
- __FUNCTION__, irq, irq_list[irq].devname);
- return -EBUSY;
- }
- if (!(flags IRQ_FLG_REPLACE)) {
- printk(%s: %s can't replace IRQ %d from %s\n,
- __FUNCTION__, devname, irq,
irq_list[irq].devname);
- return -EBUSY;
- }
- }
-#endif
-
- irq_list[irq].handler = handler;
- irq_list[irq].flags = flags;
- irq_list[irq].dev_id = dev_id;
- irq_list[irq].devname = devname;
- return 0;
-}
-
void sys_free_irq(unsigned int irq, void *dev_id)
{
if (irq IRQ1 || irq IRQ7) {
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH] ppc: remove unused sys_free_irq
sys_free_irq is never used, so delete it.
Signed-off-by: Fernando Luis Vazquez Cao [EMAIL PROTECTED]
---
diff -urNp linux-2.6.23-rc3-orig/arch/ppc/amiga/ints.c
linux-2.6.23-rc3/arch/ppc/amiga/ints.c
--- linux-2.6.23-rc3-orig/arch/ppc/amiga/ints.c 2007-08-22 17:14:32.0
+0900
+++ linux-2.6.23-rc3/arch/ppc/amiga/ints.c 2007-08-22 17:15:27.0
+0900
@@ -75,23 +75,6 @@ irq_node_t *new_irq_node(void)
return NULL;
}
-void sys_free_irq(unsigned int irq, void *dev_id)
-{
- if (irq IRQ1 || irq IRQ7) {
- printk(%s: Incorrect IRQ %d\n, __FUNCTION__, irq);
- return;
- }
-
- if (irq_list[irq].dev_id != dev_id)
- printk(%s: Removing probably wrong IRQ %d from %s\n,
- __FUNCTION__, irq, irq_list[irq].devname);
-
- irq_list[irq].handler = (*mach_default_handler)[irq];
- irq_list[irq].flags = 0;
- irq_list[irq].dev_id = NULL;
- irq_list[irq].devname = default_names[irq];
-}
-
asmlinkage void process_int(unsigned long vec, struct pt_regs *fp)
{
if (vec = VEC_INT1 vec = VEC_INT7 !MACH_IS_BVME6000) {
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] ppc: remove unused amiga_request_irq and mach_request_irq
On Wed, 2007-08-22 at 19:44 +1000, Paul Mackerras wrote: Fernando Luis Vázquez Cao writes: amiga_request_irq and mach_request_irq are never used, so delete them. OK, but is there a particular reason you want to do this? Hi Paul, Thank you for your reply. I am currently auditing all the interrupt handlers and related code (request_irq(), free_irq(), and others of that ilk) and, in the process, I found some dead code. Getting rid of this cruft would just make my life easier. The final goal of this audit is to determine whether the Linux interrupt handlers are kdump-ready. With the advent of kdump it is now possible that device drivers have to handle pending interrupts generated in the context of a previous kernel. Any pending interrupts will come in as soon as the IRQ is allocated, but, unfortunately, not all the device drivers handle this situation properly. Besides, I am also trying to determine if applying this patch: http://lkml.org/lkml/2007/7/19/687 is a sane thing to do. The whole of arch/ppc is going away eventually, so I don't think we need to remove it piece by piece. As Linas mentioned in his reply, cleaning things up first will probably make things easier for you too. Please consider merging the four patches I sent to the PPC mailing list. Fernando - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] ppc: remove unused amiga_request_irq and mach_request_irq
On Wed, 2007-08-22 at 13:28 -0500, Linas Vepstas wrote: On Wed, Aug 22, 2007 at 07:44:41PM +1000, Paul Mackerras wrote: Fernando Luis Vázquez Cao writes: amiga_request_irq and mach_request_irq are never used, so delete them. OK, but is there a particular reason you want to do this? The whole of arch/ppc is going away eventually, so I don't think we need to remove it piece by piece. Its often easier to port/move stuff if you clean it up first. Agreed. It would make things easier for me too. Fernando - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 2/2] Debug handling of early spurious interrupts
On Mon, 2007-07-30 at 11:22 -0700, Andrew Morton wrote: > On Mon, 30 Jul 2007 18:58:14 +0900 > Fernando Luis V__zquez Cao <[EMAIL PROTECTED]> wrote: > > > > > > > So bad things might happen because of this change. And if they do, they > > > will take a lng time to be discovered, because non-shared interrupt > > > handlers tend to dwell in crufty old drivers which not many people use. > > > > > > So I'm wondering if it would be more prudent to only do all this for > > > shared > > > handlers? > > > > I have been testing this patches on all my machines, which spans three > > different architectures: i386, x86_64 (EM64T and Opteron), and ia64. > > > > The good news is that nothing catastrophic happened and, in the process, > > I managed to find some somewhat buggy interrupt handlers. > > > > I will present a brief summary of my findings here. > > That's quite a lot of breakage for such a small sample of machines. I do > suspect that if we were to merge this lot into mainline, all sorts of bad > stuff would happen. Yup. > otoh, as you point out, pretty much everthing which goes wrong is due to > incorrect or dubious driver behaviour, and there is value in weeding these > things out. > > But the problem with this process is that we're weeding things out at > runtime. Some drivers don't get used by many people and users of some > architectures (esp embedded) tend to lag kernel.org by a long time. So it > could be years before all the fallout from this change is finally wrapped > up. Yes, that is a big concern. However, the same embedded people is starting to use both kexec and kdump, so they may suffer the issues we are trying to weed out anyway, even if these patches are not applied. The difference is that with this new functionality it is possible to catch potential problems relatively easily, because any incorrect behaviour this may cause will be easily reproducible and, in most cases, will reveal itself early at boot time. As things stand now, I guess we will keep seeing occasional crashes and strange behaviour in kexec-booted kernels, which in some cases will be due to incorrect handling of spurious interrupts. Besides, such problems are really difficult to reproduce because, commonly, we would need to hit an obscure corner case. > > If we find drivers that are not fixable we should probably consider this > > new behaviour on a per-driver basis, as Andrew suggested. > > We haven't found any such yet, have we? Not yet, fortunately. > > This would > > probably require passing a new flag into request_irq. Besides, when such > > a driver is detected we should emit a warning that it may not work > > properly in a kdump kernel. > > > > I would appreciate your comments on this. > > Oh well, let's just keep maintaining it in -mm for now, gather more > information so that we can make a more informed decision later on. Makes sense. I will look into all the issues I have found so far, do some more testing (I think a new approach is needed to speed up testing of these issues...), and get back to you. Thank you. Fernando - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 2/2] Debug handling of early spurious interrupts
On Fri, 2007-07-20 at 14:43 -0700, Andrew Morton wrote:
> On Fri, 20 Jul 2007 11:20:43 +0900
> Fernando Luis V__zquez Cao <[EMAIL PROTECTED]> wrote:
>
> > With the advent of kdump it is possible that device drivers receive
> > interrupts generated in the context of a previous kernel. Ideally
> > quiescing the underlying devices should suffice but not all drivers do
> > this, either because it is not possible or because they did not
> > contemplate this case. Thus drivers ought to be able to handle
> > interrupts coming in as soon as the interrupt handler is registered.
> >
> > Signed-off-by: Fernando Luis Vazquez Cao <[EMAIL PROTECTED]>
> > ---
> >
> > diff -urNp linux-2.6.22-orig/kernel/irq/manage.c
> > linux-2.6.22-pendirq/kernel/irq/manage.c
> > --- linux-2.6.22-orig/kernel/irq/manage.c 2007-07-19 18:18:53.0
> > +0900
> > +++ linux-2.6.22-pendirq/kernel/irq/manage.c2007-07-19
> > 19:43:41.0 +0900
> > @@ -533,21 +533,32 @@ int request_irq(unsigned int irq, irq_ha
> >
> > select_smp_affinity(irq);
> >
> > - if (irqflags & IRQF_SHARED) {
> > - /*
> > -* It's a shared IRQ -- the driver ought to be prepared for it
> > -* to happen immediately, so let's make sure
> > -* We do this before actually registering it, to make sure that
> > -* a 'real' IRQ doesn't run in parallel with our fake
> > -*/
> > - if (irqflags & IRQF_DISABLED) {
> > - unsigned long flags;
> > + /*
> > +* With the advent of kdump it possible that device drivers receive
> > +* interrupts generated in the context of a previous kernel. Ideally
> > +* quiescing the underlying devices should suffice but not all drivers
> > +* do this, either because it is not possible or because they did not
> > +* contemplate this case. Thus drivers ought to be able to handle
> > +* interrupts coming in as soon as the interrupt handler is registered.
> > +*
> > +* Besides, if it is a shared IRQ the driver ought to be prepared for
> > +* it to happen immediately too.
> > +*
> > +* We do this before actually registering it, to make sure that
> > +* a 'real' IRQ doesn't run in parallel with our fake.
> > +*/
> > + if (irqflags & IRQF_DISABLED) {
> > + unsigned long flags;
> >
> > - local_irq_save(flags);
> > - handler(irq, dev_id);
> > - local_irq_restore(flags);
> > - } else
> > - handler(irq, dev_id);
> > + local_irq_save(flags);
> > + retval = handler(irq, dev_id);
> > + local_irq_restore(flags);
> > + } else
> > + retval = handler(irq, dev_id);
> > + if (retval == IRQ_HANDLED) {
> > + printk(KERN_WARNING
> > + "%s (IRQ %d) handled a spurious interrupt\n",
> > + devname, irq);
> > }
> >
>
> This change means that we'll run the irq handler at request_irq()-time even
> for non-shared interrupts.
>
> This is a bit of a worry. See, shared-interrupt handlers are required to
> be able to cope with being called when their device isn't interrupting.
> But nobody ever said that non-shared interrupt handlers need to be able to
> cope with that.
>
> Hence these non-shared handlers are within their rights to emit warning
> printks, go BUG or accuse the operator of having busted hardware.
>
> So bad things might happen because of this change. And if they do, they
> will take a lng time to be discovered, because non-shared interrupt
> handlers tend to dwell in crufty old drivers which not many people use.
>
> So I'm wondering if it would be more prudent to only do all this for shared
> handlers?
I have been testing this patches on all my machines, which spans three
different architectures: i386, x86_64 (EM64T and Opteron), and ia64.
The good news is that nothing catastrophic happened and, in the process,
I managed to find some somewhat buggy interrupt handlers.
I will present a brief summary of my findings here. That said, I have to
admit that these are still preliminary results and have not had time to
dig deep into all the issues. I would like hear from you first.
- Test environment
-- x86_64 [EM64T]
CPU: Intel(R) Xeon(TM) CPU 3.20GHz (2 cpus)
SCSI: Adaptec AIC-7902B U320
IDE: Intel Corporation 82801EB/ER (ICH5/ICH5R) IDE Controller
-- x86_64 [Opteron]
CPU: AMD Opteron(tm) Processor 240 (2 cpus)
IDE: Advanced Micro Devices [AMD] AMD-8111 IDE (rev 03)
-- ia64
CPU: Itanium 2 Madison (2 cpus)
SCSI: LSI Logic / Symbios Logic 53c1030 PCI-X Fusion-MPT Dual
Ultra320 SCSI
IDE: Intel Corporation 82801DB (ICH4) IDE Controller
-- i386
CPU: Intel(R) Xeon(TM) CPU 2.40GHz (2 cpus)
IDE: Intel Corporation 82801EB/ER (ICH5/ICH5R) IDE Controller (rev
02)
- Test results for i386
--i8042 (IRQ 12) handled a spurious interrupt -> i8042
The culprit here was the
Re: [PATCH 2/2] Debug handling of early spurious interrupts
On Fri, 2007-07-20 at 14:43 -0700, Andrew Morton wrote:
On Fri, 20 Jul 2007 11:20:43 +0900
Fernando Luis V__zquez Cao [EMAIL PROTECTED] wrote:
With the advent of kdump it is possible that device drivers receive
interrupts generated in the context of a previous kernel. Ideally
quiescing the underlying devices should suffice but not all drivers do
this, either because it is not possible or because they did not
contemplate this case. Thus drivers ought to be able to handle
interrupts coming in as soon as the interrupt handler is registered.
Signed-off-by: Fernando Luis Vazquez Cao [EMAIL PROTECTED]
---
diff -urNp linux-2.6.22-orig/kernel/irq/manage.c
linux-2.6.22-pendirq/kernel/irq/manage.c
--- linux-2.6.22-orig/kernel/irq/manage.c 2007-07-19 18:18:53.0
+0900
+++ linux-2.6.22-pendirq/kernel/irq/manage.c2007-07-19
19:43:41.0 +0900
@@ -533,21 +533,32 @@ int request_irq(unsigned int irq, irq_ha
select_smp_affinity(irq);
- if (irqflags IRQF_SHARED) {
- /*
-* It's a shared IRQ -- the driver ought to be prepared for it
-* to happen immediately, so let's make sure
-* We do this before actually registering it, to make sure that
-* a 'real' IRQ doesn't run in parallel with our fake
-*/
- if (irqflags IRQF_DISABLED) {
- unsigned long flags;
+ /*
+* With the advent of kdump it possible that device drivers receive
+* interrupts generated in the context of a previous kernel. Ideally
+* quiescing the underlying devices should suffice but not all drivers
+* do this, either because it is not possible or because they did not
+* contemplate this case. Thus drivers ought to be able to handle
+* interrupts coming in as soon as the interrupt handler is registered.
+*
+* Besides, if it is a shared IRQ the driver ought to be prepared for
+* it to happen immediately too.
+*
+* We do this before actually registering it, to make sure that
+* a 'real' IRQ doesn't run in parallel with our fake.
+*/
+ if (irqflags IRQF_DISABLED) {
+ unsigned long flags;
- local_irq_save(flags);
- handler(irq, dev_id);
- local_irq_restore(flags);
- } else
- handler(irq, dev_id);
+ local_irq_save(flags);
+ retval = handler(irq, dev_id);
+ local_irq_restore(flags);
+ } else
+ retval = handler(irq, dev_id);
+ if (retval == IRQ_HANDLED) {
+ printk(KERN_WARNING
+ %s (IRQ %d) handled a spurious interrupt\n,
+ devname, irq);
}
This change means that we'll run the irq handler at request_irq()-time even
for non-shared interrupts.
This is a bit of a worry. See, shared-interrupt handlers are required to
be able to cope with being called when their device isn't interrupting.
But nobody ever said that non-shared interrupt handlers need to be able to
cope with that.
Hence these non-shared handlers are within their rights to emit warning
printks, go BUG or accuse the operator of having busted hardware.
So bad things might happen because of this change. And if they do, they
will take a lng time to be discovered, because non-shared interrupt
handlers tend to dwell in crufty old drivers which not many people use.
So I'm wondering if it would be more prudent to only do all this for shared
handlers?
I have been testing this patches on all my machines, which spans three
different architectures: i386, x86_64 (EM64T and Opteron), and ia64.
The good news is that nothing catastrophic happened and, in the process,
I managed to find some somewhat buggy interrupt handlers.
I will present a brief summary of my findings here. That said, I have to
admit that these are still preliminary results and have not had time to
dig deep into all the issues. I would like hear from you first.
- Test environment
-- x86_64 [EM64T]
CPU: Intel(R) Xeon(TM) CPU 3.20GHz (2 cpus)
SCSI: Adaptec AIC-7902B U320
IDE: Intel Corporation 82801EB/ER (ICH5/ICH5R) IDE Controller
-- x86_64 [Opteron]
CPU: AMD Opteron(tm) Processor 240 (2 cpus)
IDE: Advanced Micro Devices [AMD] AMD-8111 IDE (rev 03)
-- ia64
CPU: Itanium 2 Madison (2 cpus)
SCSI: LSI Logic / Symbios Logic 53c1030 PCI-X Fusion-MPT Dual
Ultra320 SCSI
IDE: Intel Corporation 82801DB (ICH4) IDE Controller
-- i386
CPU: Intel(R) Xeon(TM) CPU 2.40GHz (2 cpus)
IDE: Intel Corporation 82801EB/ER (ICH5/ICH5R) IDE Controller (rev
02)
- Test results for i386
--i8042 (IRQ 12) handled a spurious interrupt - i8042
The culprit here was the i8042_aux_test_irq interrupt handler
unconditionally returning IRQ_HANDLED despite being marked as
IRQF_SHARED. I submitted a fix and it is currently
Re: [PATCH 2/2] Debug handling of early spurious interrupts
On Mon, 2007-07-30 at 11:22 -0700, Andrew Morton wrote: On Mon, 30 Jul 2007 18:58:14 +0900 Fernando Luis V__zquez Cao [EMAIL PROTECTED] wrote: So bad things might happen because of this change. And if they do, they will take a lng time to be discovered, because non-shared interrupt handlers tend to dwell in crufty old drivers which not many people use. So I'm wondering if it would be more prudent to only do all this for shared handlers? I have been testing this patches on all my machines, which spans three different architectures: i386, x86_64 (EM64T and Opteron), and ia64. The good news is that nothing catastrophic happened and, in the process, I managed to find some somewhat buggy interrupt handlers. I will present a brief summary of my findings here. That's quite a lot of breakage for such a small sample of machines. I do suspect that if we were to merge this lot into mainline, all sorts of bad stuff would happen. Yup. otoh, as you point out, pretty much everthing which goes wrong is due to incorrect or dubious driver behaviour, and there is value in weeding these things out. But the problem with this process is that we're weeding things out at runtime. Some drivers don't get used by many people and users of some architectures (esp embedded) tend to lag kernel.org by a long time. So it could be years before all the fallout from this change is finally wrapped up. Yes, that is a big concern. However, the same embedded people is starting to use both kexec and kdump, so they may suffer the issues we are trying to weed out anyway, even if these patches are not applied. The difference is that with this new functionality it is possible to catch potential problems relatively easily, because any incorrect behaviour this may cause will be easily reproducible and, in most cases, will reveal itself early at boot time. As things stand now, I guess we will keep seeing occasional crashes and strange behaviour in kexec-booted kernels, which in some cases will be due to incorrect handling of spurious interrupts. Besides, such problems are really difficult to reproduce because, commonly, we would need to hit an obscure corner case. If we find drivers that are not fixable we should probably consider this new behaviour on a per-driver basis, as Andrew suggested. We haven't found any such yet, have we? Not yet, fortunately. This would probably require passing a new flag into request_irq. Besides, when such a driver is detected we should emit a warning that it may not work properly in a kdump kernel. I would appreciate your comments on this. Oh well, let's just keep maintaining it in -mm for now, gather more information so that we can make a more informed decision later on. Makes sense. I will look into all the issues I have found so far, do some more testing (I think a new approach is needed to speed up testing of these issues...), and get back to you. Thank you. Fernando - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH] fix return value of i8042_aux_test_irq
I made an interesting finding while testing the two patches below.
http://lkml.org/lkml/2007/7/19/685
http://lkml.org/lkml/2007/7/19/687
These patches modify the traditional CONFIG_DEBUG_KERNEL in such a way
that the request_irq prints a warning if after calling the handler it
returned IRQ_HANDLED .
The code looks like this:
int request_irq(unsigned int irq, irq_handler_t handler,
unsigned long irqflags, const char *devname, void
*dev_id)
.
if (irqflags & IRQF_DISABLED) {
unsigned long flags;
local_irq_save(flags);
retval = handler(irq, dev_id);
local_irq_restore(flags);
} else
retval = handler(irq, dev_id);
if (retval == IRQ_HANDLED) {
printk(KERN_WARNING
"%s (IRQ %d) handled a spurious interrupt\n",
devname, irq);
}
.
I discovered that i8042_aux_test_irq handles the "fake" interrupt,
which, in principle, is not correct because it obviously isn't a real
interrupt and it could have been a spurious interrupt as well.
The problem is that the interrupt handler unconditionally returns IRQ
handled, which does not seem correct. Anyway I am not very familiar with
this code so I may be missing the whole point. I would appreciate your
comments on this.
Thank you in advance.
Fernando
--
Do not return IRQ_HANDLED when we haven't handle the interrupt.
Signed-off-by: Fernando Luis Vazquez Cao <[EMAIL PROTECTED]>
---
diff -urNp linux-2.6.23-rc1-mm1-orig/drivers/input/serio/i8042.c
linux-2.6.23-rc1-mm1/drivers/input/serio/i8042.c
--- linux-2.6.23-rc1-mm1-orig/drivers/input/serio/i8042.c 2007-07-26
16:10:11.0 +0900
+++ linux-2.6.23-rc1-mm1/drivers/input/serio/i8042.c2007-07-26
16:05:19.0 +0900
@@ -516,6 +516,7 @@ static irqreturn_t __devinit i8042_aux_t
{
unsigned long flags;
unsigned char str, data;
+ int ret;
spin_lock_irqsave(_lock, flags);
str = i8042_read_status();
@@ -524,10 +525,13 @@ static irqreturn_t __devinit i8042_aux_t
if (i8042_irq_being_tested &&
data == 0xa5 && (str & I8042_STR_AUXDATA))
complete(_aux_irq_delivered);
+ ret = 1;
}
+ else
+ ret = 0;
spin_unlock_irqrestore(_lock, flags);
- return IRQ_HANDLED;
+ return IRQ_RETVAL(ret);
}
/*
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH] fix return value of i8042_aux_test_irq
I made an interesting finding while testing the two patches below.
http://lkml.org/lkml/2007/7/19/685
http://lkml.org/lkml/2007/7/19/687
These patches modify the traditional CONFIG_DEBUG_KERNEL in such a way
that the request_irq prints a warning if after calling the handler it
returned IRQ_HANDLED .
The code looks like this:
int request_irq(unsigned int irq, irq_handler_t handler,
unsigned long irqflags, const char *devname, void
*dev_id)
.
if (irqflags IRQF_DISABLED) {
unsigned long flags;
local_irq_save(flags);
retval = handler(irq, dev_id);
local_irq_restore(flags);
} else
retval = handler(irq, dev_id);
if (retval == IRQ_HANDLED) {
printk(KERN_WARNING
%s (IRQ %d) handled a spurious interrupt\n,
devname, irq);
}
.
I discovered that i8042_aux_test_irq handles the fake interrupt,
which, in principle, is not correct because it obviously isn't a real
interrupt and it could have been a spurious interrupt as well.
The problem is that the interrupt handler unconditionally returns IRQ
handled, which does not seem correct. Anyway I am not very familiar with
this code so I may be missing the whole point. I would appreciate your
comments on this.
Thank you in advance.
Fernando
--
Do not return IRQ_HANDLED when we haven't handle the interrupt.
Signed-off-by: Fernando Luis Vazquez Cao [EMAIL PROTECTED]
---
diff -urNp linux-2.6.23-rc1-mm1-orig/drivers/input/serio/i8042.c
linux-2.6.23-rc1-mm1/drivers/input/serio/i8042.c
--- linux-2.6.23-rc1-mm1-orig/drivers/input/serio/i8042.c 2007-07-26
16:10:11.0 +0900
+++ linux-2.6.23-rc1-mm1/drivers/input/serio/i8042.c2007-07-26
16:05:19.0 +0900
@@ -516,6 +516,7 @@ static irqreturn_t __devinit i8042_aux_t
{
unsigned long flags;
unsigned char str, data;
+ int ret;
spin_lock_irqsave(i8042_lock, flags);
str = i8042_read_status();
@@ -524,10 +525,13 @@ static irqreturn_t __devinit i8042_aux_t
if (i8042_irq_being_tested
data == 0xa5 (str I8042_STR_AUXDATA))
complete(i8042_aux_irq_delivered);
+ ret = 1;
}
+ else
+ ret = 0;
spin_unlock_irqrestore(i8042_lock, flags);
- return IRQ_HANDLED;
+ return IRQ_RETVAL(ret);
}
/*
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH RFC] e1000: clear ICR before requesting an IRQ line
On Wed, 2007-07-25 at 08:27 -0700, Kok, Auke wrote:
> Fernando Luis Vázquez Cao wrote:
> > I made an interesting finding while testing the two patches below.
> >
> > http://lkml.org/lkml/2007/7/19/685
> > http://lkml.org/lkml/2007/7/19/687
> >
> > These patches modify the traditional CONFIG_DEBUG_KERNEL in such a way
> > that the request_irq prints a warning if after calling the handler it
> > returned IRQ_HANDLED .
> >
> > The code looks like this:
> >
> > int request_irq(unsigned int irq, irq_handler_t handler,
> > unsigned long irqflags, const char *devname, void *dev_id)
> > .
> > if (irqflags & IRQF_DISABLED) {
> > unsigned long flags;
> >
> > local_irq_save(flags);
> > retval = handler(irq, dev_id);
> > local_irq_restore(flags);
> > } else
> > retval = handler(irq, dev_id);
> > if (retval == IRQ_HANDLED) {
> > printk(KERN_WARNING
> >"%s (IRQ %d) handled a spurious interrupt\n",
> >devname, irq);
> > }
> > .
> >
> > I discovered that the e1000 driver handles the "fake" interrupt, which,
> > in principle, is not correct because it obviously isn't a real interrupt
> > and it could have been an interrupt coming from another device that is
> > sharing the IRQ line.
> >
> > The problem is that the interrupt handler assumes that if ICR!=0 it was
> > its device who generated the interrupt and, consequently, it should be
> > handled. But, unfortunately, that is not always the case. If the network
> > link is active when we open the device (e1000_open) the ICR will have
> > the E1000_ICR_LSC bit set (by the way, is this the expected behavior?).
>
> yes. is it really a problem though?
It seems we may end up handling spurious interrupts or interrupts coming
from another devices.
> > This means that _any_ interrupt coming in after allocating our interrupt
> > (e1000_request_irq) will be handled, no matter where it came from.
>
> we actually generate this LSC interrupt ourselves in the driver, to make sure
> that we cascade into the watchdog which then enables or disables the link
> code
> based on the link status change. This allows us to _not_ do any link checking
> in
> _open and makes things a bit more simple.
I am not referring to the LSC interrupt the driver itself generates in
e1000_open just before returning. The ICR is masked (ICR==0) after
executing the driver probe (e1000_probe), but when we enter e1000_open
the E1000_ICR_LSC bit will already be set, before the function even
starts executing. I also observed that when the link is active the line
/* fire a link status change interrupt to start the watchdog */
E1000_WRITE_REG(>hw, ICS, E1000_ICS_LSC);
is redundant because the E1000_ICS_LSC bit is already set. In fact, the
irq handler gets invoked twice in a row with the interrupt cause being a
link status change.
> > The solution I came up with is clearing the ICR before calling
> > request_irq. I have to admit that I am not familiar enough with this
> > driver, so it is quite likely that this is not the right fix. I would
> > appreciate your comments on this.
>
> Clearing the ICR before requesting an irq might not work - at the same time
> the
> device could generate another LSC irq...
Is it not possible to prevent the device from generating interrupts until we
call request_irq?
Thank you for your feedback!
- Fernando
> Of course, we probably should just schedule some delayed work to run our
> watchdog in e1000_open, but I haven't checked if that actually works.
>
>
> Auke
>
> > Signed-off-by: Fernando Luis Vazquez Cao <[EMAIL PROTECTED]>
> > ---
> >
> > diff -urNp linux-2.6.22-orig/drivers/net/e1000/e1000_main.c
> > linux-2.6.22-pendirq/drivers/net/e1000/e1000_main.c
> > --- linux-2.6.22-orig/drivers/net/e1000/e1000_main.c2007-07-19
> > 18:18:53.0 +0900
> > +++ linux-2.6.22-pendirq/drivers/net/e1000/e1000_main.c 2007-07-25
> > 17:22:54.0 +0900
> > @@ -1378,6 +1378,17 @@ e1000_alloc_queues(struct e1000_adapter
> > }
> >
> > /**
> > + * e1000_clear_interrupts
> > + * @adapter: address of board private structure
> > + *
> > + * Mask interrupts
> > + **/
> > +static void
> > +e1000_clear_interrupts(struct e1000_adapter *adapter) {
> > + E1000_READ_REG(>hw, ICR);
> > +}
> > +
> > +/**
> > * e1000_open - Called when a network i
[PATCH RFC] e1000: clear ICR before requesting an IRQ line
I made an interesting finding while testing the two patches below.
http://lkml.org/lkml/2007/7/19/685
http://lkml.org/lkml/2007/7/19/687
These patches modify the traditional CONFIG_DEBUG_KERNEL in such a way
that the request_irq prints a warning if after calling the handler it
returned IRQ_HANDLED .
The code looks like this:
int request_irq(unsigned int irq, irq_handler_t handler,
unsigned long irqflags, const char *devname, void *dev_id)
.
if (irqflags & IRQF_DISABLED) {
unsigned long flags;
local_irq_save(flags);
retval = handler(irq, dev_id);
local_irq_restore(flags);
} else
retval = handler(irq, dev_id);
if (retval == IRQ_HANDLED) {
printk(KERN_WARNING
"%s (IRQ %d) handled a spurious interrupt\n",
devname, irq);
}
.
I discovered that the e1000 driver handles the "fake" interrupt, which,
in principle, is not correct because it obviously isn't a real interrupt
and it could have been an interrupt coming from another device that is
sharing the IRQ line.
The problem is that the interrupt handler assumes that if ICR!=0 it was
its device who generated the interrupt and, consequently, it should be
handled. But, unfortunately, that is not always the case. If the network
link is active when we open the device (e1000_open) the ICR will have
the E1000_ICR_LSC bit set (by the way, is this the expected behavior?).
This means that _any_ interrupt coming in after allocating our interrupt
(e1000_request_irq) will be handled, no matter where it came from.
The solution I came up with is clearing the ICR before calling
request_irq. I have to admit that I am not familiar enough with this
driver, so it is quite likely that this is not the right fix. I would
appreciate your comments on this.
Signed-off-by: Fernando Luis Vazquez Cao <[EMAIL PROTECTED]>
---
diff -urNp linux-2.6.22-orig/drivers/net/e1000/e1000_main.c
linux-2.6.22-pendirq/drivers/net/e1000/e1000_main.c
--- linux-2.6.22-orig/drivers/net/e1000/e1000_main.c2007-07-19
18:18:53.0 +0900
+++ linux-2.6.22-pendirq/drivers/net/e1000/e1000_main.c 2007-07-25
17:22:54.0 +0900
@@ -1378,6 +1378,17 @@ e1000_alloc_queues(struct e1000_adapter
}
/**
+ * e1000_clear_interrupts
+ * @adapter: address of board private structure
+ *
+ * Mask interrupts
+ **/
+static void
+e1000_clear_interrupts(struct e1000_adapter *adapter) {
+ E1000_READ_REG(>hw, ICR);
+}
+
+/**
* e1000_open - Called when a network interface is made active
* @netdev: network interface device structure
*
@@ -1431,6 +1442,9 @@ e1000_open(struct net_device *netdev)
* so we have to setup our clean_rx handler before we do so. */
e1000_configure(adapter);
+ /* Discard any possible pending interrupts. */
+ e1000_clear_interrupts(adapter);
+
err = e1000_request_irq(adapter);
if (err)
goto err_req_irq;
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH RFC] e1000: clear ICR before requesting an IRQ line
I made an interesting finding while testing the two patches below.
http://lkml.org/lkml/2007/7/19/685
http://lkml.org/lkml/2007/7/19/687
These patches modify the traditional CONFIG_DEBUG_KERNEL in such a way
that the request_irq prints a warning if after calling the handler it
returned IRQ_HANDLED .
The code looks like this:
int request_irq(unsigned int irq, irq_handler_t handler,
unsigned long irqflags, const char *devname, void *dev_id)
.
if (irqflags IRQF_DISABLED) {
unsigned long flags;
local_irq_save(flags);
retval = handler(irq, dev_id);
local_irq_restore(flags);
} else
retval = handler(irq, dev_id);
if (retval == IRQ_HANDLED) {
printk(KERN_WARNING
%s (IRQ %d) handled a spurious interrupt\n,
devname, irq);
}
.
I discovered that the e1000 driver handles the fake interrupt, which,
in principle, is not correct because it obviously isn't a real interrupt
and it could have been an interrupt coming from another device that is
sharing the IRQ line.
The problem is that the interrupt handler assumes that if ICR!=0 it was
its device who generated the interrupt and, consequently, it should be
handled. But, unfortunately, that is not always the case. If the network
link is active when we open the device (e1000_open) the ICR will have
the E1000_ICR_LSC bit set (by the way, is this the expected behavior?).
This means that _any_ interrupt coming in after allocating our interrupt
(e1000_request_irq) will be handled, no matter where it came from.
The solution I came up with is clearing the ICR before calling
request_irq. I have to admit that I am not familiar enough with this
driver, so it is quite likely that this is not the right fix. I would
appreciate your comments on this.
Signed-off-by: Fernando Luis Vazquez Cao [EMAIL PROTECTED]
---
diff -urNp linux-2.6.22-orig/drivers/net/e1000/e1000_main.c
linux-2.6.22-pendirq/drivers/net/e1000/e1000_main.c
--- linux-2.6.22-orig/drivers/net/e1000/e1000_main.c2007-07-19
18:18:53.0 +0900
+++ linux-2.6.22-pendirq/drivers/net/e1000/e1000_main.c 2007-07-25
17:22:54.0 +0900
@@ -1378,6 +1378,17 @@ e1000_alloc_queues(struct e1000_adapter
}
/**
+ * e1000_clear_interrupts
+ * @adapter: address of board private structure
+ *
+ * Mask interrupts
+ **/
+static void
+e1000_clear_interrupts(struct e1000_adapter *adapter) {
+ E1000_READ_REG(adapter-hw, ICR);
+}
+
+/**
* e1000_open - Called when a network interface is made active
* @netdev: network interface device structure
*
@@ -1431,6 +1442,9 @@ e1000_open(struct net_device *netdev)
* so we have to setup our clean_rx handler before we do so. */
e1000_configure(adapter);
+ /* Discard any possible pending interrupts. */
+ e1000_clear_interrupts(adapter);
+
err = e1000_request_irq(adapter);
if (err)
goto err_req_irq;
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH RFC] e1000: clear ICR before requesting an IRQ line
On Wed, 2007-07-25 at 08:27 -0700, Kok, Auke wrote:
Fernando Luis Vázquez Cao wrote:
I made an interesting finding while testing the two patches below.
http://lkml.org/lkml/2007/7/19/685
http://lkml.org/lkml/2007/7/19/687
These patches modify the traditional CONFIG_DEBUG_KERNEL in such a way
that the request_irq prints a warning if after calling the handler it
returned IRQ_HANDLED .
The code looks like this:
int request_irq(unsigned int irq, irq_handler_t handler,
unsigned long irqflags, const char *devname, void *dev_id)
.
if (irqflags IRQF_DISABLED) {
unsigned long flags;
local_irq_save(flags);
retval = handler(irq, dev_id);
local_irq_restore(flags);
} else
retval = handler(irq, dev_id);
if (retval == IRQ_HANDLED) {
printk(KERN_WARNING
%s (IRQ %d) handled a spurious interrupt\n,
devname, irq);
}
.
I discovered that the e1000 driver handles the fake interrupt, which,
in principle, is not correct because it obviously isn't a real interrupt
and it could have been an interrupt coming from another device that is
sharing the IRQ line.
The problem is that the interrupt handler assumes that if ICR!=0 it was
its device who generated the interrupt and, consequently, it should be
handled. But, unfortunately, that is not always the case. If the network
link is active when we open the device (e1000_open) the ICR will have
the E1000_ICR_LSC bit set (by the way, is this the expected behavior?).
yes. is it really a problem though?
It seems we may end up handling spurious interrupts or interrupts coming
from another devices.
This means that _any_ interrupt coming in after allocating our interrupt
(e1000_request_irq) will be handled, no matter where it came from.
we actually generate this LSC interrupt ourselves in the driver, to make sure
that we cascade into the watchdog which then enables or disables the link
code
based on the link status change. This allows us to _not_ do any link checking
in
_open and makes things a bit more simple.
I am not referring to the LSC interrupt the driver itself generates in
e1000_open just before returning. The ICR is masked (ICR==0) after
executing the driver probe (e1000_probe), but when we enter e1000_open
the E1000_ICR_LSC bit will already be set, before the function even
starts executing. I also observed that when the link is active the line
/* fire a link status change interrupt to start the watchdog */
E1000_WRITE_REG(adapter-hw, ICS, E1000_ICS_LSC);
is redundant because the E1000_ICS_LSC bit is already set. In fact, the
irq handler gets invoked twice in a row with the interrupt cause being a
link status change.
The solution I came up with is clearing the ICR before calling
request_irq. I have to admit that I am not familiar enough with this
driver, so it is quite likely that this is not the right fix. I would
appreciate your comments on this.
Clearing the ICR before requesting an irq might not work - at the same time
the
device could generate another LSC irq...
Is it not possible to prevent the device from generating interrupts until we
call request_irq?
Thank you for your feedback!
- Fernando
Of course, we probably should just schedule some delayed work to run our
watchdog in e1000_open, but I haven't checked if that actually works.
Auke
Signed-off-by: Fernando Luis Vazquez Cao [EMAIL PROTECTED]
---
diff -urNp linux-2.6.22-orig/drivers/net/e1000/e1000_main.c
linux-2.6.22-pendirq/drivers/net/e1000/e1000_main.c
--- linux-2.6.22-orig/drivers/net/e1000/e1000_main.c2007-07-19
18:18:53.0 +0900
+++ linux-2.6.22-pendirq/drivers/net/e1000/e1000_main.c 2007-07-25
17:22:54.0 +0900
@@ -1378,6 +1378,17 @@ e1000_alloc_queues(struct e1000_adapter
}
/**
+ * e1000_clear_interrupts
+ * @adapter: address of board private structure
+ *
+ * Mask interrupts
+ **/
+static void
+e1000_clear_interrupts(struct e1000_adapter *adapter) {
+ E1000_READ_REG(adapter-hw, ICR);
+}
+
+/**
* e1000_open - Called when a network interface is made active
* @netdev: network interface device structure
*
@@ -1431,6 +1442,9 @@ e1000_open(struct net_device *netdev)
* so we have to setup our clean_rx handler before we do so. */
e1000_configure(adapter);
+ /* Discard any possible pending interrupts. */
+ e1000_clear_interrupts(adapter);
+
err = e1000_request_irq(adapter);
if (err)
goto err_req_irq;
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH 2/2] Debug handling of early spurious interrupts
With the advent of kdump it is possible that device drivers receive
interrupts generated in the context of a previous kernel. Ideally
quiescing the underlying devices should suffice but not all drivers do
this, either because it is not possible or because they did not
contemplate this case. Thus drivers ought to be able to handle
interrupts coming in as soon as the interrupt handler is registered.
Signed-off-by: Fernando Luis Vazquez Cao <[EMAIL PROTECTED]>
---
diff -urNp linux-2.6.22-orig/kernel/irq/manage.c
linux-2.6.22-pendirq/kernel/irq/manage.c
--- linux-2.6.22-orig/kernel/irq/manage.c 2007-07-19 18:18:53.0
+0900
+++ linux-2.6.22-pendirq/kernel/irq/manage.c2007-07-19 19:43:41.0
+0900
@@ -533,21 +533,32 @@ int request_irq(unsigned int irq, irq_ha
select_smp_affinity(irq);
- if (irqflags & IRQF_SHARED) {
- /*
-* It's a shared IRQ -- the driver ought to be prepared for it
-* to happen immediately, so let's make sure
-* We do this before actually registering it, to make sure that
-* a 'real' IRQ doesn't run in parallel with our fake
-*/
- if (irqflags & IRQF_DISABLED) {
- unsigned long flags;
+ /*
+* With the advent of kdump it possible that device drivers receive
+* interrupts generated in the context of a previous kernel. Ideally
+* quiescing the underlying devices should suffice but not all drivers
+* do this, either because it is not possible or because they did not
+* contemplate this case. Thus drivers ought to be able to handle
+* interrupts coming in as soon as the interrupt handler is registered.
+*
+* Besides, if it is a shared IRQ the driver ought to be prepared for
+* it to happen immediately too.
+*
+* We do this before actually registering it, to make sure that
+* a 'real' IRQ doesn't run in parallel with our fake.
+*/
+ if (irqflags & IRQF_DISABLED) {
+ unsigned long flags;
- local_irq_save(flags);
- handler(irq, dev_id);
- local_irq_restore(flags);
- } else
- handler(irq, dev_id);
+ local_irq_save(flags);
+ retval = handler(irq, dev_id);
+ local_irq_restore(flags);
+ } else
+ retval = handler(irq, dev_id);
+ if (retval == IRQ_HANDLED) {
+ printk(KERN_WARNING
+ "%s (IRQ %d) handled a spurious interrupt\n",
+ devname, irq);
}
retval = setup_irq(irq, action);
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH 1/2] Remove Kconfig setting CONFIG_DEBUG_SHIRQ
request_irq() and setup_irq() are not fast paths and free_irq() much less so. In fact, by enabling this feature unconditionally we would have _everyone_ (unknowingly) testing devices drivers, which hopefully will result in more bug-reports and, in turn, better drivers. Signed-off-by: Fernando Luis Vazquez Cao <[EMAIL PROTECTED]> --- diff -urNp linux-2.6.22-orig/arch/arm/configs/at91sam9rlek_defconfig linux-2.6.22-pendirq/arch/arm/configs/at91sam9rlek_defconfig --- linux-2.6.22-orig/arch/arm/configs/at91sam9rlek_defconfig 2007-07-09 08:32:17.0 +0900 +++ linux-2.6.22-pendirq/arch/arm/configs/at91sam9rlek_defconfig 2007-07-19 16:49:54.0 +0900 @@ -906,7 +906,6 @@ CONFIG_ENABLE_MUST_CHECK=y # CONFIG_DEBUG_FS is not set # CONFIG_HEADERS_CHECK is not set CONFIG_DEBUG_KERNEL=y -# CONFIG_DEBUG_SHIRQ is not set CONFIG_LOG_BUF_SHIFT=14 CONFIG_DETECT_SOFTLOCKUP=y # CONFIG_SCHEDSTATS is not set diff -urNp linux-2.6.22-orig/arch/arm/configs/picotux200_defconfig linux-2.6.22-pendirq/arch/arm/configs/picotux200_defconfig --- linux-2.6.22-orig/arch/arm/configs/picotux200_defconfig 2007-07-09 08:32:17.0 +0900 +++ linux-2.6.22-pendirq/arch/arm/configs/picotux200_defconfig 2007-07-19 16:50:00.0 +0900 @@ -1293,7 +1293,6 @@ CONFIG_ENABLE_MUST_CHECK=y # CONFIG_DEBUG_FS is not set # CONFIG_HEADERS_CHECK is not set CONFIG_DEBUG_KERNEL=y -# CONFIG_DEBUG_SHIRQ is not set CONFIG_LOG_BUF_SHIFT=14 CONFIG_DETECT_SOFTLOCKUP=y # CONFIG_SCHEDSTATS is not set diff -urNp linux-2.6.22-orig/arch/arm/configs/s3c2410_defconfig linux-2.6.22-pendirq/arch/arm/configs/s3c2410_defconfig --- linux-2.6.22-orig/arch/arm/configs/s3c2410_defconfig2007-07-09 08:32:17.0 +0900 +++ linux-2.6.22-pendirq/arch/arm/configs/s3c2410_defconfig 2007-07-19 16:50:06.0 +0900 @@ -1368,7 +1368,6 @@ CONFIG_MAGIC_SYSRQ=y # CONFIG_DEBUG_FS is not set # CONFIG_HEADERS_CHECK is not set CONFIG_DEBUG_KERNEL=y -# CONFIG_DEBUG_SHIRQ is not set CONFIG_LOG_BUF_SHIFT=16 CONFIG_DETECT_SOFTLOCKUP=y # CONFIG_SCHEDSTATS is not set diff -urNp linux-2.6.22-orig/arch/avr32/configs/atngw100_defconfig linux-2.6.22-pendirq/arch/avr32/configs/atngw100_defconfig --- linux-2.6.22-orig/arch/avr32/configs/atngw100_defconfig 2007-07-09 08:32:17.0 +0900 +++ linux-2.6.22-pendirq/arch/avr32/configs/atngw100_defconfig 2007-07-19 16:50:11.0 +0900 @@ -925,7 +925,6 @@ CONFIG_MAGIC_SYSRQ=y # CONFIG_DEBUG_FS is not set # CONFIG_HEADERS_CHECK is not set CONFIG_DEBUG_KERNEL=y -# CONFIG_DEBUG_SHIRQ is not set CONFIG_DETECT_SOFTLOCKUP=y # CONFIG_SCHEDSTATS is not set # CONFIG_TIMER_STATS is not set diff -urNp linux-2.6.22-orig/arch/avr32/configs/atstk1002_defconfig linux-2.6.22-pendirq/arch/avr32/configs/atstk1002_defconfig --- linux-2.6.22-orig/arch/avr32/configs/atstk1002_defconfig2007-07-09 08:32:17.0 +0900 +++ linux-2.6.22-pendirq/arch/avr32/configs/atstk1002_defconfig 2007-07-19 16:50:16.0 +0900 @@ -756,7 +756,6 @@ CONFIG_MAGIC_SYSRQ=y CONFIG_DEBUG_FS=y # CONFIG_HEADERS_CHECK is not set CONFIG_DEBUG_KERNEL=y -# CONFIG_DEBUG_SHIRQ is not set CONFIG_DETECT_SOFTLOCKUP=y # CONFIG_SCHEDSTATS is not set # CONFIG_TIMER_STATS is not set diff -urNp linux-2.6.22-orig/arch/i386/defconfig linux-2.6.22-pendirq/arch/i386/defconfig --- linux-2.6.22-orig/arch/i386/defconfig 2007-07-09 08:32:17.0 +0900 +++ linux-2.6.22-pendirq/arch/i386/defconfig2007-07-19 16:50:21.0 +0900 @@ -1424,7 +1424,6 @@ CONFIG_UNUSED_SYMBOLS=y # CONFIG_DEBUG_FS is not set # CONFIG_HEADERS_CHECK is not set CONFIG_DEBUG_KERNEL=y -# CONFIG_DEBUG_SHIRQ is not set CONFIG_DETECT_SOFTLOCKUP=y # CONFIG_SCHEDSTATS is not set # CONFIG_TIMER_STATS is not set diff -urNp linux-2.6.22-orig/arch/ia64/configs/tiger_defconfig linux-2.6.22-pendirq/arch/ia64/configs/tiger_defconfig --- linux-2.6.22-orig/arch/ia64/configs/tiger_defconfig 2007-07-09 08:32:17.0 +0900 +++ linux-2.6.22-pendirq/arch/ia64/configs/tiger_defconfig 2007-07-19 16:50:26.0 +0900 @@ -1318,7 +1318,6 @@ CONFIG_MAGIC_SYSRQ=y # CONFIG_DEBUG_FS is not set # CONFIG_HEADERS_CHECK is not set CONFIG_DEBUG_KERNEL=y -# CONFIG_DEBUG_SHIRQ is not set CONFIG_LOG_BUF_SHIFT=20 CONFIG_DETECT_SOFTLOCKUP=y # CONFIG_SCHEDSTATS is not set diff -urNp linux-2.6.22-orig/arch/ia64/configs/zx1_defconfig linux-2.6.22-pendirq/arch/ia64/configs/zx1_defconfig --- linux-2.6.22-orig/arch/ia64/configs/zx1_defconfig 2007-07-09 08:32:17.0 +0900 +++ linux-2.6.22-pendirq/arch/ia64/configs/zx1_defconfig2007-07-19 16:50:31.0 +0900 @@ -1533,7 +1533,6 @@ CONFIG_MAGIC_SYSRQ=y # CONFIG_DEBUG_FS is not set # CONFIG_HEADERS_CHECK is not set CONFIG_DEBUG_KERNEL=y -# CONFIG_DEBUG_SHIRQ is not set CONFIG_LOG_BUF_SHIFT=17 CONFIG_DETECT_SOFTLOCKUP=y # CONFIG_SCHEDSTATS is not set diff -urNp linux-2.6.22-orig/arch/ia64/defconfig linux-2.6.22-pendirq/arch/ia64/defconfig ---
Re: [PATCH RFC] Debug handling of early spurious interrupts
On Wed, 2007-07-18 at 15:46 -0700, Andrew Morton wrote:
> On Tue, 17 Jul 2007 19:09:57 +0900
> Fernando Luis V__zquez Cao <[EMAIL PROTECTED]> wrote:
>
> > With the advent of kdump it is possible that device drivers receive
> > interrupts generated in the context of a previous kernel. Ideally
> > quiescing the underlying devices should suffice but not all drivers
> > do this, either because it is not possible or because they did not
> > contemplate this case. Thus drivers ought to be able to handle
> > interrupts coming in as soon as the interrupt handler is registered.
> >
> > Signed-off-by: Fernando Luis Vazquez Cao <[EMAIL PROTECTED]>
> > ---
> >
> > diff -urNp linux-2.6.22-orig/kernel/irq/manage.c
> > linux-2.6.22/kernel/irq/manage.c
> > --- linux-2.6.22-orig/kernel/irq/manage.c 2007-07-09 08:32:17.0
> > +0900
> > +++ linux-2.6.22/kernel/irq/manage.c2007-07-17 18:37:24.0
> > +0900
> > @@ -537,6 +537,29 @@ int request_irq(unsigned int irq, irq_ha
> >
> > select_smp_affinity(irq);
> >
> > +#if defined(CONFIG_DEBUG_PENDING_IRQ) || defined(CONFIG_DEBUG_SHIRQ)
> > +#ifndef CONFIG_DEBUG_PENDING_IRQ
> > + if (irqflags & IRQF_SHARED) {
> > + /*
> > +* It's a shared IRQ -- the driver ought to be prepared for it
> > +* to happen immediately, so let's make sure
> > +* We do this before actually registering it, to make sure that
> > +* a 'real' IRQ doesn't run in parallel with our fake.
> > +*/
> > +#endif /* !CONFIG_DEBUG_PENDING_IRQ */
> > + if (irqflags & IRQF_DISABLED) {
> > + unsigned long flags;
> > +
> > + local_irq_save(flags);
> > + handler(irq, dev_id);
> > + local_irq_restore(flags);
> > + } else
> > + handler(irq, dev_id);
> > +#ifndef CONFIG_DEBUG_PENDING_IRQ
> > + }
> > +#endif /* !CONFIG_DEBUG_PENDING_IRQ */
> > +#endif /* CONFIG_DEBUG_PENDING_IRQ || CONFIG_DEBUG_SHIRQ */
>
> Even if we were going to merge this functionality as-is, I'd ask for some
> sort of refactoring to fix up that ifdef maze.
I a absolutely agree. My first impulse was to get rid of all the cpp
kludge including the Kconfig setting CONFIG_DEBUG_SHIRQ, since, as you
pointed out, request_irq() is not really performance critical.
Unfortunately for the RFC I decided to be conservative and ended up with
an "ifdef maze". Thank you for the heads-up.
> But more substantial issues:
>
> - This is presented as a "debug" feature, but it isn't a debug feature at
> all - it is new functionality which is unrelated to kernel development.
Yup.
> Also, it is a "debug" feature which provides no debugging! At the very
> least, one would expect to see it emit a printk to tell people that we
> have some driver which needs fixing.
I am afraid that in some occasions the kernel may panic inside the
interrupt handler, but I agree that we need to print a meaningful
message for the general case (i.e. something goes wrong but we can
recover). I will do that.
> Also, this not-really-a-debug-feature is undesirably coupled with a
> real debugging feature: CONFIG_DEBUG_PENDING_IRQ.
In the new version of the patch I will remove both
CONFIG_DEBUG_PENDING_IRQ and CONFIG_DEBUG_SHIRQ. request_irq() and
setup_irq() are not fast paths and free_irq() much less so.
> - Does this new feature really need its own Kconfig setting? Why not enable
> it unconditionally? request_irq() isn't exactly performance-critical.
I guess the Kconfig setting is not needed. In fact, by enabling this
feature unconditionally we would have _everyone_ (unknowing) testing an
area which is a major pain point for kdump. I am not sure this is an
acceptable default for all systems though. Opinions welcome.
> - If poss, we really do want to find some way of emitting a warning when
> we detect such a device driver. Like, call the handler and if it
> returned IRQ_HANDLED, start shouting.
I will do that and submit updated patches.
Thank you for your feedback, Andrew!
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH RFC] Debug handling of early spurious interrupts
On Wed, 2007-07-18 at 15:46 -0700, Andrew Morton wrote:
On Tue, 17 Jul 2007 19:09:57 +0900
Fernando Luis V__zquez Cao [EMAIL PROTECTED] wrote:
With the advent of kdump it is possible that device drivers receive
interrupts generated in the context of a previous kernel. Ideally
quiescing the underlying devices should suffice but not all drivers
do this, either because it is not possible or because they did not
contemplate this case. Thus drivers ought to be able to handle
interrupts coming in as soon as the interrupt handler is registered.
Signed-off-by: Fernando Luis Vazquez Cao [EMAIL PROTECTED]
---
diff -urNp linux-2.6.22-orig/kernel/irq/manage.c
linux-2.6.22/kernel/irq/manage.c
--- linux-2.6.22-orig/kernel/irq/manage.c 2007-07-09 08:32:17.0
+0900
+++ linux-2.6.22/kernel/irq/manage.c2007-07-17 18:37:24.0
+0900
@@ -537,6 +537,29 @@ int request_irq(unsigned int irq, irq_ha
select_smp_affinity(irq);
+#if defined(CONFIG_DEBUG_PENDING_IRQ) || defined(CONFIG_DEBUG_SHIRQ)
+#ifndef CONFIG_DEBUG_PENDING_IRQ
+ if (irqflags IRQF_SHARED) {
+ /*
+* It's a shared IRQ -- the driver ought to be prepared for it
+* to happen immediately, so let's make sure
+* We do this before actually registering it, to make sure that
+* a 'real' IRQ doesn't run in parallel with our fake.
+*/
+#endif /* !CONFIG_DEBUG_PENDING_IRQ */
+ if (irqflags IRQF_DISABLED) {
+ unsigned long flags;
+
+ local_irq_save(flags);
+ handler(irq, dev_id);
+ local_irq_restore(flags);
+ } else
+ handler(irq, dev_id);
+#ifndef CONFIG_DEBUG_PENDING_IRQ
+ }
+#endif /* !CONFIG_DEBUG_PENDING_IRQ */
+#endif /* CONFIG_DEBUG_PENDING_IRQ || CONFIG_DEBUG_SHIRQ */
Even if we were going to merge this functionality as-is, I'd ask for some
sort of refactoring to fix up that ifdef maze.
I a absolutely agree. My first impulse was to get rid of all the cpp
kludge including the Kconfig setting CONFIG_DEBUG_SHIRQ, since, as you
pointed out, request_irq() is not really performance critical.
Unfortunately for the RFC I decided to be conservative and ended up with
an ifdef maze. Thank you for the heads-up.
But more substantial issues:
- This is presented as a debug feature, but it isn't a debug feature at
all - it is new functionality which is unrelated to kernel development.
Yup.
Also, it is a debug feature which provides no debugging! At the very
least, one would expect to see it emit a printk to tell people that we
have some driver which needs fixing.
I am afraid that in some occasions the kernel may panic inside the
interrupt handler, but I agree that we need to print a meaningful
message for the general case (i.e. something goes wrong but we can
recover). I will do that.
Also, this not-really-a-debug-feature is undesirably coupled with a
real debugging feature: CONFIG_DEBUG_PENDING_IRQ.
In the new version of the patch I will remove both
CONFIG_DEBUG_PENDING_IRQ and CONFIG_DEBUG_SHIRQ. request_irq() and
setup_irq() are not fast paths and free_irq() much less so.
- Does this new feature really need its own Kconfig setting? Why not enable
it unconditionally? request_irq() isn't exactly performance-critical.
I guess the Kconfig setting is not needed. In fact, by enabling this
feature unconditionally we would have _everyone_ (unknowing) testing an
area which is a major pain point for kdump. I am not sure this is an
acceptable default for all systems though. Opinions welcome.
- If poss, we really do want to find some way of emitting a warning when
we detect such a device driver. Like, call the handler and if it
returned IRQ_HANDLED, start shouting.
I will do that and submit updated patches.
Thank you for your feedback, Andrew!
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH 1/2] Remove Kconfig setting CONFIG_DEBUG_SHIRQ
request_irq() and setup_irq() are not fast paths and free_irq() much less so. In fact, by enabling this feature unconditionally we would have _everyone_ (unknowingly) testing devices drivers, which hopefully will result in more bug-reports and, in turn, better drivers. Signed-off-by: Fernando Luis Vazquez Cao [EMAIL PROTECTED] --- diff -urNp linux-2.6.22-orig/arch/arm/configs/at91sam9rlek_defconfig linux-2.6.22-pendirq/arch/arm/configs/at91sam9rlek_defconfig --- linux-2.6.22-orig/arch/arm/configs/at91sam9rlek_defconfig 2007-07-09 08:32:17.0 +0900 +++ linux-2.6.22-pendirq/arch/arm/configs/at91sam9rlek_defconfig 2007-07-19 16:49:54.0 +0900 @@ -906,7 +906,6 @@ CONFIG_ENABLE_MUST_CHECK=y # CONFIG_DEBUG_FS is not set # CONFIG_HEADERS_CHECK is not set CONFIG_DEBUG_KERNEL=y -# CONFIG_DEBUG_SHIRQ is not set CONFIG_LOG_BUF_SHIFT=14 CONFIG_DETECT_SOFTLOCKUP=y # CONFIG_SCHEDSTATS is not set diff -urNp linux-2.6.22-orig/arch/arm/configs/picotux200_defconfig linux-2.6.22-pendirq/arch/arm/configs/picotux200_defconfig --- linux-2.6.22-orig/arch/arm/configs/picotux200_defconfig 2007-07-09 08:32:17.0 +0900 +++ linux-2.6.22-pendirq/arch/arm/configs/picotux200_defconfig 2007-07-19 16:50:00.0 +0900 @@ -1293,7 +1293,6 @@ CONFIG_ENABLE_MUST_CHECK=y # CONFIG_DEBUG_FS is not set # CONFIG_HEADERS_CHECK is not set CONFIG_DEBUG_KERNEL=y -# CONFIG_DEBUG_SHIRQ is not set CONFIG_LOG_BUF_SHIFT=14 CONFIG_DETECT_SOFTLOCKUP=y # CONFIG_SCHEDSTATS is not set diff -urNp linux-2.6.22-orig/arch/arm/configs/s3c2410_defconfig linux-2.6.22-pendirq/arch/arm/configs/s3c2410_defconfig --- linux-2.6.22-orig/arch/arm/configs/s3c2410_defconfig2007-07-09 08:32:17.0 +0900 +++ linux-2.6.22-pendirq/arch/arm/configs/s3c2410_defconfig 2007-07-19 16:50:06.0 +0900 @@ -1368,7 +1368,6 @@ CONFIG_MAGIC_SYSRQ=y # CONFIG_DEBUG_FS is not set # CONFIG_HEADERS_CHECK is not set CONFIG_DEBUG_KERNEL=y -# CONFIG_DEBUG_SHIRQ is not set CONFIG_LOG_BUF_SHIFT=16 CONFIG_DETECT_SOFTLOCKUP=y # CONFIG_SCHEDSTATS is not set diff -urNp linux-2.6.22-orig/arch/avr32/configs/atngw100_defconfig linux-2.6.22-pendirq/arch/avr32/configs/atngw100_defconfig --- linux-2.6.22-orig/arch/avr32/configs/atngw100_defconfig 2007-07-09 08:32:17.0 +0900 +++ linux-2.6.22-pendirq/arch/avr32/configs/atngw100_defconfig 2007-07-19 16:50:11.0 +0900 @@ -925,7 +925,6 @@ CONFIG_MAGIC_SYSRQ=y # CONFIG_DEBUG_FS is not set # CONFIG_HEADERS_CHECK is not set CONFIG_DEBUG_KERNEL=y -# CONFIG_DEBUG_SHIRQ is not set CONFIG_DETECT_SOFTLOCKUP=y # CONFIG_SCHEDSTATS is not set # CONFIG_TIMER_STATS is not set diff -urNp linux-2.6.22-orig/arch/avr32/configs/atstk1002_defconfig linux-2.6.22-pendirq/arch/avr32/configs/atstk1002_defconfig --- linux-2.6.22-orig/arch/avr32/configs/atstk1002_defconfig2007-07-09 08:32:17.0 +0900 +++ linux-2.6.22-pendirq/arch/avr32/configs/atstk1002_defconfig 2007-07-19 16:50:16.0 +0900 @@ -756,7 +756,6 @@ CONFIG_MAGIC_SYSRQ=y CONFIG_DEBUG_FS=y # CONFIG_HEADERS_CHECK is not set CONFIG_DEBUG_KERNEL=y -# CONFIG_DEBUG_SHIRQ is not set CONFIG_DETECT_SOFTLOCKUP=y # CONFIG_SCHEDSTATS is not set # CONFIG_TIMER_STATS is not set diff -urNp linux-2.6.22-orig/arch/i386/defconfig linux-2.6.22-pendirq/arch/i386/defconfig --- linux-2.6.22-orig/arch/i386/defconfig 2007-07-09 08:32:17.0 +0900 +++ linux-2.6.22-pendirq/arch/i386/defconfig2007-07-19 16:50:21.0 +0900 @@ -1424,7 +1424,6 @@ CONFIG_UNUSED_SYMBOLS=y # CONFIG_DEBUG_FS is not set # CONFIG_HEADERS_CHECK is not set CONFIG_DEBUG_KERNEL=y -# CONFIG_DEBUG_SHIRQ is not set CONFIG_DETECT_SOFTLOCKUP=y # CONFIG_SCHEDSTATS is not set # CONFIG_TIMER_STATS is not set diff -urNp linux-2.6.22-orig/arch/ia64/configs/tiger_defconfig linux-2.6.22-pendirq/arch/ia64/configs/tiger_defconfig --- linux-2.6.22-orig/arch/ia64/configs/tiger_defconfig 2007-07-09 08:32:17.0 +0900 +++ linux-2.6.22-pendirq/arch/ia64/configs/tiger_defconfig 2007-07-19 16:50:26.0 +0900 @@ -1318,7 +1318,6 @@ CONFIG_MAGIC_SYSRQ=y # CONFIG_DEBUG_FS is not set # CONFIG_HEADERS_CHECK is not set CONFIG_DEBUG_KERNEL=y -# CONFIG_DEBUG_SHIRQ is not set CONFIG_LOG_BUF_SHIFT=20 CONFIG_DETECT_SOFTLOCKUP=y # CONFIG_SCHEDSTATS is not set diff -urNp linux-2.6.22-orig/arch/ia64/configs/zx1_defconfig linux-2.6.22-pendirq/arch/ia64/configs/zx1_defconfig --- linux-2.6.22-orig/arch/ia64/configs/zx1_defconfig 2007-07-09 08:32:17.0 +0900 +++ linux-2.6.22-pendirq/arch/ia64/configs/zx1_defconfig2007-07-19 16:50:31.0 +0900 @@ -1533,7 +1533,6 @@ CONFIG_MAGIC_SYSRQ=y # CONFIG_DEBUG_FS is not set # CONFIG_HEADERS_CHECK is not set CONFIG_DEBUG_KERNEL=y -# CONFIG_DEBUG_SHIRQ is not set CONFIG_LOG_BUF_SHIFT=17 CONFIG_DETECT_SOFTLOCKUP=y # CONFIG_SCHEDSTATS is not set diff -urNp linux-2.6.22-orig/arch/ia64/defconfig linux-2.6.22-pendirq/arch/ia64/defconfig ---
[PATCH 2/2] Debug handling of early spurious interrupts
With the advent of kdump it is possible that device drivers receive
interrupts generated in the context of a previous kernel. Ideally
quiescing the underlying devices should suffice but not all drivers do
this, either because it is not possible or because they did not
contemplate this case. Thus drivers ought to be able to handle
interrupts coming in as soon as the interrupt handler is registered.
Signed-off-by: Fernando Luis Vazquez Cao [EMAIL PROTECTED]
---
diff -urNp linux-2.6.22-orig/kernel/irq/manage.c
linux-2.6.22-pendirq/kernel/irq/manage.c
--- linux-2.6.22-orig/kernel/irq/manage.c 2007-07-19 18:18:53.0
+0900
+++ linux-2.6.22-pendirq/kernel/irq/manage.c2007-07-19 19:43:41.0
+0900
@@ -533,21 +533,32 @@ int request_irq(unsigned int irq, irq_ha
select_smp_affinity(irq);
- if (irqflags IRQF_SHARED) {
- /*
-* It's a shared IRQ -- the driver ought to be prepared for it
-* to happen immediately, so let's make sure
-* We do this before actually registering it, to make sure that
-* a 'real' IRQ doesn't run in parallel with our fake
-*/
- if (irqflags IRQF_DISABLED) {
- unsigned long flags;
+ /*
+* With the advent of kdump it possible that device drivers receive
+* interrupts generated in the context of a previous kernel. Ideally
+* quiescing the underlying devices should suffice but not all drivers
+* do this, either because it is not possible or because they did not
+* contemplate this case. Thus drivers ought to be able to handle
+* interrupts coming in as soon as the interrupt handler is registered.
+*
+* Besides, if it is a shared IRQ the driver ought to be prepared for
+* it to happen immediately too.
+*
+* We do this before actually registering it, to make sure that
+* a 'real' IRQ doesn't run in parallel with our fake.
+*/
+ if (irqflags IRQF_DISABLED) {
+ unsigned long flags;
- local_irq_save(flags);
- handler(irq, dev_id);
- local_irq_restore(flags);
- } else
- handler(irq, dev_id);
+ local_irq_save(flags);
+ retval = handler(irq, dev_id);
+ local_irq_restore(flags);
+ } else
+ retval = handler(irq, dev_id);
+ if (retval == IRQ_HANDLED) {
+ printk(KERN_WARNING
+ %s (IRQ %d) handled a spurious interrupt\n,
+ devname, irq);
}
retval = setup_irq(irq, action);
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH RFC] Debug handling of early spurious interrupts
With the advent of kdump it is possible that device drivers receive
interrupts generated in the context of a previous kernel. Ideally
quiescing the underlying devices should suffice but not all drivers
do this, either because it is not possible or because they did not
contemplate this case. Thus drivers ought to be able to handle
interrupts coming in as soon as the interrupt handler is registered.
Signed-off-by: Fernando Luis Vazquez Cao <[EMAIL PROTECTED]>
---
diff -urNp linux-2.6.22-orig/kernel/irq/manage.c
linux-2.6.22/kernel/irq/manage.c
--- linux-2.6.22-orig/kernel/irq/manage.c 2007-07-09 08:32:17.0
+0900
+++ linux-2.6.22/kernel/irq/manage.c2007-07-17 18:37:24.0 +0900
@@ -537,6 +537,29 @@ int request_irq(unsigned int irq, irq_ha
select_smp_affinity(irq);
+#if defined(CONFIG_DEBUG_PENDING_IRQ) || defined(CONFIG_DEBUG_SHIRQ)
+#ifndef CONFIG_DEBUG_PENDING_IRQ
+ if (irqflags & IRQF_SHARED) {
+ /*
+* It's a shared IRQ -- the driver ought to be prepared for it
+* to happen immediately, so let's make sure
+* We do this before actually registering it, to make sure that
+* a 'real' IRQ doesn't run in parallel with our fake.
+*/
+#endif /* !CONFIG_DEBUG_PENDING_IRQ */
+ if (irqflags & IRQF_DISABLED) {
+ unsigned long flags;
+
+ local_irq_save(flags);
+ handler(irq, dev_id);
+ local_irq_restore(flags);
+ } else
+ handler(irq, dev_id);
+#ifndef CONFIG_DEBUG_PENDING_IRQ
+ }
+#endif /* !CONFIG_DEBUG_PENDING_IRQ */
+#endif /* CONFIG_DEBUG_PENDING_IRQ || CONFIG_DEBUG_SHIRQ */
+
#ifdef CONFIG_DEBUG_SHIRQ
if (irqflags & IRQF_SHARED) {
/*
diff -urNp linux-2.6.22-orig/lib/Kconfig.debug linux-2.6.22/lib/Kconfig.debug
--- linux-2.6.22-orig/lib/Kconfig.debug 2007-07-09 08:32:17.0 +0900
+++ linux-2.6.22/lib/Kconfig.debug 2007-07-17 18:52:00.0 +0900
@@ -77,6 +77,17 @@ config DEBUG_KERNEL
Say Y here if you are developing drivers or trying to debug and
identify kernel problems.
+config DEBUG_PENDING_IRQ
+ bool "Debug handling of early spurious interrupts"
+ depends on DEBUG_KERNEL && GENERIC_HARDIRQS
+ help
+ With the advent of kdump it is possible that device drivers receive
+ interrupts generated in the context of a previous kernel. Ideally
+ quiescing the underlying devices should suffice but not all drivers
+ do this, either because it is not possible or because they did not
+ contemplate this case. Thus drivers ought to be able to handle
+ interrupts coming in as soon as the interrupt handler is registered.
+
config DEBUG_SHIRQ
bool "Debug shared IRQ handlers"
depends on DEBUG_KERNEL && GENERIC_HARDIRQS
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH RFC] Debug handling of early spurious interrupts
With the advent of kdump it is possible that device drivers receive
interrupts generated in the context of a previous kernel. Ideally
quiescing the underlying devices should suffice but not all drivers
do this, either because it is not possible or because they did not
contemplate this case. Thus drivers ought to be able to handle
interrupts coming in as soon as the interrupt handler is registered.
Signed-off-by: Fernando Luis Vazquez Cao [EMAIL PROTECTED]
---
diff -urNp linux-2.6.22-orig/kernel/irq/manage.c
linux-2.6.22/kernel/irq/manage.c
--- linux-2.6.22-orig/kernel/irq/manage.c 2007-07-09 08:32:17.0
+0900
+++ linux-2.6.22/kernel/irq/manage.c2007-07-17 18:37:24.0 +0900
@@ -537,6 +537,29 @@ int request_irq(unsigned int irq, irq_ha
select_smp_affinity(irq);
+#if defined(CONFIG_DEBUG_PENDING_IRQ) || defined(CONFIG_DEBUG_SHIRQ)
+#ifndef CONFIG_DEBUG_PENDING_IRQ
+ if (irqflags IRQF_SHARED) {
+ /*
+* It's a shared IRQ -- the driver ought to be prepared for it
+* to happen immediately, so let's make sure
+* We do this before actually registering it, to make sure that
+* a 'real' IRQ doesn't run in parallel with our fake.
+*/
+#endif /* !CONFIG_DEBUG_PENDING_IRQ */
+ if (irqflags IRQF_DISABLED) {
+ unsigned long flags;
+
+ local_irq_save(flags);
+ handler(irq, dev_id);
+ local_irq_restore(flags);
+ } else
+ handler(irq, dev_id);
+#ifndef CONFIG_DEBUG_PENDING_IRQ
+ }
+#endif /* !CONFIG_DEBUG_PENDING_IRQ */
+#endif /* CONFIG_DEBUG_PENDING_IRQ || CONFIG_DEBUG_SHIRQ */
+
#ifdef CONFIG_DEBUG_SHIRQ
if (irqflags IRQF_SHARED) {
/*
diff -urNp linux-2.6.22-orig/lib/Kconfig.debug linux-2.6.22/lib/Kconfig.debug
--- linux-2.6.22-orig/lib/Kconfig.debug 2007-07-09 08:32:17.0 +0900
+++ linux-2.6.22/lib/Kconfig.debug 2007-07-17 18:52:00.0 +0900
@@ -77,6 +77,17 @@ config DEBUG_KERNEL
Say Y here if you are developing drivers or trying to debug and
identify kernel problems.
+config DEBUG_PENDING_IRQ
+ bool Debug handling of early spurious interrupts
+ depends on DEBUG_KERNEL GENERIC_HARDIRQS
+ help
+ With the advent of kdump it is possible that device drivers receive
+ interrupts generated in the context of a previous kernel. Ideally
+ quiescing the underlying devices should suffice but not all drivers
+ do this, either because it is not possible or because they did not
+ contemplate this case. Thus drivers ought to be able to handle
+ interrupts coming in as soon as the interrupt handler is registered.
+
config DEBUG_SHIRQ
bool Debug shared IRQ handlers
depends on DEBUG_KERNEL GENERIC_HARDIRQS
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH RFC] try_module_get usage
I keep seeing uses of try_module_get(THIS_MODULE) which seem to mimic
the behavior of the former MOD_INC_USE_COUNT. The UBI driver is one
example:
int ubi_get_device_info(int ubi_num, struct ubi_device_info *di)
{
const struct ubi_device *ubi;
if (!try_module_get(THIS_MODULE))
return -ENODEV;
if (ubi_num < 0 || ubi_num >= UBI_MAX_DEVICES ||
!ubi_devices[ubi_num]) {
module_put(THIS_MODULE);
return -ENODEV;
}
ubi = ubi_devices[ubi_num];
di->ubi_num = ubi->ubi_num;
di->leb_size = ubi->leb_size;
di->min_io_size = ubi->min_io_size;
di->ro_mode = ubi->ro_mode;
di->cdev = MKDEV(ubi->major, 0);
module_put(THIS_MODULE);
return 0;
}
My understanding is that this is not completely safe (we could be
preempted before try_modules_get gets executed) and that it is the
caller who should manipulate the refcounts. Am I missing something here?
Thank you in advance.
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH] UBI: cleanup usage of try_module_get
The use of try_module_get(THIS_MODULE) in ubi_get_device_info does not
offer real protection against unexpected driver unloads, since we could
be preempted before try_modules_get gets executed. It is the caller who
should manipulate the refcounts. Besides, ubi_get_device_info is an
exported symbol which guarantees protection when accessed through
symbol_get.
Signed-off-by: Fernando Luis Vazquez Cao <[EMAIL PROTECTED]>
---
diff -urNp linux-2.6.22-orig/drivers/mtd/ubi/kapi.c
linux-2.6.22/drivers/mtd/ubi/kapi.c
--- linux-2.6.22-orig/drivers/mtd/ubi/kapi.c2007-07-09 08:32:17.0
+0900
+++ linux-2.6.22/drivers/mtd/ubi/kapi.c 2007-07-11 16:34:10.0 +0900
@@ -37,12 +37,8 @@ int ubi_get_device_info(int ubi_num, str
{
const struct ubi_device *ubi;
- if (!try_module_get(THIS_MODULE))
- return -ENODEV;
-
if (ubi_num < 0 || ubi_num >= UBI_MAX_DEVICES ||
!ubi_devices[ubi_num]) {
- module_put(THIS_MODULE);
return -ENODEV;
}
@@ -52,7 +48,6 @@ int ubi_get_device_info(int ubi_num, str
di->min_io_size = ubi->min_io_size;
di->ro_mode = ubi->ro_mode;
di->cdev = MKDEV(ubi->major, 0);
- module_put(THIS_MODULE);
return 0;
}
EXPORT_SYMBOL_GPL(ubi_get_device_info);
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH] UBI: cleanup usage of try_module_get
The use of try_module_get(THIS_MODULE) in ubi_get_device_info does not
offer real protection against unexpected driver unloads, since we could
be preempted before try_modules_get gets executed. It is the caller who
should manipulate the refcounts. Besides, ubi_get_device_info is an
exported symbol which guarantees protection when accessed through
symbol_get.
Signed-off-by: Fernando Luis Vazquez Cao [EMAIL PROTECTED]
---
diff -urNp linux-2.6.22-orig/drivers/mtd/ubi/kapi.c
linux-2.6.22/drivers/mtd/ubi/kapi.c
--- linux-2.6.22-orig/drivers/mtd/ubi/kapi.c2007-07-09 08:32:17.0
+0900
+++ linux-2.6.22/drivers/mtd/ubi/kapi.c 2007-07-11 16:34:10.0 +0900
@@ -37,12 +37,8 @@ int ubi_get_device_info(int ubi_num, str
{
const struct ubi_device *ubi;
- if (!try_module_get(THIS_MODULE))
- return -ENODEV;
-
if (ubi_num 0 || ubi_num = UBI_MAX_DEVICES ||
!ubi_devices[ubi_num]) {
- module_put(THIS_MODULE);
return -ENODEV;
}
@@ -52,7 +48,6 @@ int ubi_get_device_info(int ubi_num, str
di-min_io_size = ubi-min_io_size;
di-ro_mode = ubi-ro_mode;
di-cdev = MKDEV(ubi-major, 0);
- module_put(THIS_MODULE);
return 0;
}
EXPORT_SYMBOL_GPL(ubi_get_device_info);
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH RFC] try_module_get usage
I keep seeing uses of try_module_get(THIS_MODULE) which seem to mimic
the behavior of the former MOD_INC_USE_COUNT. The UBI driver is one
example:
int ubi_get_device_info(int ubi_num, struct ubi_device_info *di)
{
const struct ubi_device *ubi;
if (!try_module_get(THIS_MODULE))
return -ENODEV;
if (ubi_num 0 || ubi_num = UBI_MAX_DEVICES ||
!ubi_devices[ubi_num]) {
module_put(THIS_MODULE);
return -ENODEV;
}
ubi = ubi_devices[ubi_num];
di-ubi_num = ubi-ubi_num;
di-leb_size = ubi-leb_size;
di-min_io_size = ubi-min_io_size;
di-ro_mode = ubi-ro_mode;
di-cdev = MKDEV(ubi-major, 0);
module_put(THIS_MODULE);
return 0;
}
My understanding is that this is not completely safe (we could be
preempted before try_modules_get gets executed) and that it is the
caller who should manipulate the refcounts. Am I missing something here?
Thank you in advance.
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [ARM] Fix hard_smp_processor_id compile error
On Tue, 2007-05-15 at 11:18 +0900, Simon Horman wrote: > "Remove hardcoding of hard_smp_processor_id on UP systems", > 2f4dfe206a2fc07099dfad77a8ea2f4b4ae2140f in Linus' tree, moved > the definition of hard_smp_processor_id linux/smp.h to asm/smp.h > for UP systems. This causes a regression on ARM as the definition > was not added to asm-arm/smp.h. Hi Simon! Thank you for catching and fixing these compile errors. I should install a cross-compiler ASAP. - Fernando > Cc: Fernando Luis Vazquez Cao <[EMAIL PROTECTED]> > Signed-off-by: Simon Horman <[EMAIL PROTECTED]> > > --- > arch/arm/mach-integrator/core.c |1 + > include/asm-arm/smp.h | 10 ++ > 2 files changed, 7 insertions(+), 4 deletions(-) > > arm-unknown-linux-gnu-gcc (GCC) 4.1.1 > Copyright (C) 2006 Free Software Foundation, Inc. > This is free software; see the source for copying conditions. There is NO > warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. > > CC arch/arm/mach-integrator/core.o > arch/arm/mach-integrator/core.c: In function 'integrator_timer_interrupt': > arch/arm/mach-integrator/core.c:264: warning: implicit declaration of > function 'hard_smp_processor_id' > [snip] > LD .tmp_vmlinux1 > arch/arm/mach-integrator/built-in.o: In function `integrator_timer_interrupt': > cpu.c:(.text+0x398): undefined reference to `hard_smp_processor_id' > make: *** [.tmp_vmlinux1] error 1 > > This was produced by running the following in a crosstool cross-compiling > environment: > cp arch/arm/configs/integrator_defconfig .config > yes "" | make modconfig > make > > Index: linux-2.6/arch/arm/mach-integrator/core.c > === > --- linux-2.6.orig/arch/arm/mach-integrator/core.c2007-05-15 > 10:56:06.0 +0900 > +++ linux-2.6/arch/arm/mach-integrator/core.c 2007-05-15 10:56:16.0 > +0900 > @@ -28,6 +28,7 @@ > #include > #include > #include > +#include > > #include "common.h" > > Index: linux-2.6/include/asm-arm/smp.h > === > --- linux-2.6.orig/include/asm-arm/smp.h 2007-05-15 10:51:54.0 > +0900 > +++ linux-2.6/include/asm-arm/smp.h 2007-05-15 10:56:34.0 +0900 > @@ -10,16 +10,16 @@ > #ifndef __ASM_ARM_SMP_H > #define __ASM_ARM_SMP_H > > +#ifndef CONFIG_SMP > +#define hard_smp_processor_id() 0 > +#else > + > #include > #include > #include > > #include > > -#ifndef CONFIG_SMP > -# error " included in non-SMP build" > -#endif > - > #define raw_smp_processor_id() (current_thread_info()->cpu) > > /* > @@ -134,4 +134,6 @@ extern void show_local_irqs(struct seq_f > */ > asmlinkage void do_local_timer(struct pt_regs *); > > +#endif /* CONFIG_SMP */ > + > #endif /* ifndef __ASM_ARM_SMP_H */ - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [ARM] Fix hard_smp_processor_id compile error
On Tue, 2007-05-15 at 11:18 +0900, Simon Horman wrote: Remove hardcoding of hard_smp_processor_id on UP systems, 2f4dfe206a2fc07099dfad77a8ea2f4b4ae2140f in Linus' tree, moved the definition of hard_smp_processor_id linux/smp.h to asm/smp.h for UP systems. This causes a regression on ARM as the definition was not added to asm-arm/smp.h. Hi Simon! Thank you for catching and fixing these compile errors. I should install a cross-compiler ASAP. - Fernando Cc: Fernando Luis Vazquez Cao [EMAIL PROTECTED] Signed-off-by: Simon Horman [EMAIL PROTECTED] --- arch/arm/mach-integrator/core.c |1 + include/asm-arm/smp.h | 10 ++ 2 files changed, 7 insertions(+), 4 deletions(-) arm-unknown-linux-gnu-gcc (GCC) 4.1.1 Copyright (C) 2006 Free Software Foundation, Inc. This is free software; see the source for copying conditions. There is NO warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. CC arch/arm/mach-integrator/core.o arch/arm/mach-integrator/core.c: In function 'integrator_timer_interrupt': arch/arm/mach-integrator/core.c:264: warning: implicit declaration of function 'hard_smp_processor_id' [snip] LD .tmp_vmlinux1 arch/arm/mach-integrator/built-in.o: In function `integrator_timer_interrupt': cpu.c:(.text+0x398): undefined reference to `hard_smp_processor_id' make: *** [.tmp_vmlinux1] error 1 This was produced by running the following in a crosstool cross-compiling environment: cp arch/arm/configs/integrator_defconfig .config yes | make modconfig make Index: linux-2.6/arch/arm/mach-integrator/core.c === --- linux-2.6.orig/arch/arm/mach-integrator/core.c2007-05-15 10:56:06.0 +0900 +++ linux-2.6/arch/arm/mach-integrator/core.c 2007-05-15 10:56:16.0 +0900 @@ -28,6 +28,7 @@ #include asm/system.h #include asm/leds.h #include asm/mach/time.h +#include asm/smp.h #include common.h Index: linux-2.6/include/asm-arm/smp.h === --- linux-2.6.orig/include/asm-arm/smp.h 2007-05-15 10:51:54.0 +0900 +++ linux-2.6/include/asm-arm/smp.h 2007-05-15 10:56:34.0 +0900 @@ -10,16 +10,16 @@ #ifndef __ASM_ARM_SMP_H #define __ASM_ARM_SMP_H +#ifndef CONFIG_SMP +#define hard_smp_processor_id() 0 +#else + #include linux/threads.h #include linux/cpumask.h #include linux/thread_info.h #include asm/arch/smp.h -#ifndef CONFIG_SMP -# error asm-arm/smp.h included in non-SMP build -#endif - #define raw_smp_processor_id() (current_thread_info()-cpu) /* @@ -134,4 +134,6 @@ extern void show_local_irqs(struct seq_f */ asmlinkage void do_local_timer(struct pt_regs *); +#endif /* CONFIG_SMP */ + #endif /* ifndef __ASM_ARM_SMP_H */ - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC PATCH 0/10] apic_wait_icr_idle issues and possible solutions
On Thu, 2007-04-26 at 12:18 +0530, Vivek Goyal wrote:
> On Wed, Apr 25, 2007 at 08:03:04PM +0900, Fernando Luis Vázquez Cao wrote:
> >
> > static __inline__ void apic_wait_icr_idle(void)
> > {
> > while (apic_read(APIC_ICR) & APIC_ICR_BUSY)
> > cpu_relax();
> > }
> >
> > The busy loop in this function would not be problematic if the
> > corresponding status bit in the ICR were always updated, but that does
> > not seem to be the case under certain crash scenarios. As an example,
> > when the other CPUs are locked-up inside the NMI handler the CPU that
> > sends the IPI will end up looping forever in the ICR check, effectively
> > hard-locking the whole system.
> >
> > Quoting from Intel's "MultiProcessor Specification" (Version 1.4), B-3:
> >
> > "A local APIC unit indicates successful dispatch of an IPI by
> > resetting the Delivery Status bit in the Interrupt Command
> > Register (ICR). The operating system polls the delivery status
> > bit after sending an INIT or STARTUP IPI until the command has
> > been dispatched.
> >
> > A period of 20 microseconds should be sufficient for IPI dispatch
> > to complete under normal operating conditions. If the IPI is not
> > successfully dispatched, the operating system can abort the
> > command. Alternatively, the operating system can retry the IPI by
> > writing the lower 32-bit double word of the ICR. This “time-out”
> > mechanism can be implemented through an external interrupt, if
> > interrupts are enabled on the processor, or through execution of
> > an instruction or time-stamp counter spin loop."
> >
> > Intel's documentation suggests the implementation of a time-out
> > mechanism, which, by the way, is already being open-coded in some parts
> > of the kernel that tinker with ICR.
> >
> > --- Possible solutions
> >
> > * Solution A: Implement the time-out mechanism in apic_wait_icr_idle.
> >
> > The problem with this approach is that introduces a performance penalty
> > that may not be acceptable for some callers of apic_wait_icr_idle.
> > Besides, during normal operation delivery errors should not occur. This
> > brings us to solution B.
> >
>
> Hi Fernando,
Hi Vivek,
Thank you for your feedback!
> How much is the performance penalty? Is it really significant. My point
> is that, to me changing apic_wait_icr_dle() itself seems to be the simple
> approach instead of introducing another function.
That was what my gut feel said at first, too. But...
> Original implementation is:
>
> static __inline__ void apic_wait_icr_idle(void)
> {
> while (apic_read(APIC_ICR) & APIC_ICR_BUSY)
> cpu_relax();
> }
>
> And new one will look something like.
>
> do {
> send_status = apic_read(APIC_ICR) & APIC_ICR_BUSY;
> if (!send_status)
> break;
> udelay(100);
> } while (timeout++ < 1000);
>
> There will be at max 100 microsecond delay before you realize that IPI has
> been dispatched. To optimize it further we can change it to 10 microsecond
> delay
... I noticed that the maximum theoretical delay you mention has to be
multiplied by the number of CPUs on big systems, because on those
machines send_IPI_mask is implemented as a series of unicasts to each
CPU. It is for this reason that I thought this approach may be
considered unacceptable (I have to admit that I have not performed any
micro-benchmarks, though).
> do {
> send_status = apic_read(APIC_ICR) & APIC_ICR_BUSY;
> if (!send_status)
> break;
> udelay(10);
> } while (timeout++ < 1);
>
> or may be
>
> do {
> send_status = apic_read(APIC_ICR) & APIC_ICR_BUSY;
> if (!send_status)
> break;
> udelay(1);
> } while (timeout++ < 10);
>
> I don't know if 1 micro second delay is supported. I do see it being
> used in kernel/hpet.c
Please notice that such high-precision timers are not available in all
machines.
> Is it too much of performance overhead? Somebody who knows more about it
> needs to tell. To me changing apic_wait_icr_idle() seems simple instead
> of introducing a new function and then making a special case for NMI.
As I mentioned above the performance overhead depends on several
factors. I hope it makes sense.
- Fernando
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC PATCH 0/10] apic_wait_icr_idle issues and possible solutions
On Thu, 2007-04-26 at 12:18 +0530, Vivek Goyal wrote:
On Wed, Apr 25, 2007 at 08:03:04PM +0900, Fernando Luis Vázquez Cao wrote:
static __inline__ void apic_wait_icr_idle(void)
{
while (apic_read(APIC_ICR) APIC_ICR_BUSY)
cpu_relax();
}
The busy loop in this function would not be problematic if the
corresponding status bit in the ICR were always updated, but that does
not seem to be the case under certain crash scenarios. As an example,
when the other CPUs are locked-up inside the NMI handler the CPU that
sends the IPI will end up looping forever in the ICR check, effectively
hard-locking the whole system.
Quoting from Intel's MultiProcessor Specification (Version 1.4), B-3:
A local APIC unit indicates successful dispatch of an IPI by
resetting the Delivery Status bit in the Interrupt Command
Register (ICR). The operating system polls the delivery status
bit after sending an INIT or STARTUP IPI until the command has
been dispatched.
A period of 20 microseconds should be sufficient for IPI dispatch
to complete under normal operating conditions. If the IPI is not
successfully dispatched, the operating system can abort the
command. Alternatively, the operating system can retry the IPI by
writing the lower 32-bit double word of the ICR. This “time-out”
mechanism can be implemented through an external interrupt, if
interrupts are enabled on the processor, or through execution of
an instruction or time-stamp counter spin loop.
Intel's documentation suggests the implementation of a time-out
mechanism, which, by the way, is already being open-coded in some parts
of the kernel that tinker with ICR.
--- Possible solutions
* Solution A: Implement the time-out mechanism in apic_wait_icr_idle.
The problem with this approach is that introduces a performance penalty
that may not be acceptable for some callers of apic_wait_icr_idle.
Besides, during normal operation delivery errors should not occur. This
brings us to solution B.
Hi Fernando,
Hi Vivek,
Thank you for your feedback!
How much is the performance penalty? Is it really significant. My point
is that, to me changing apic_wait_icr_dle() itself seems to be the simple
approach instead of introducing another function.
That was what my gut feel said at first, too. But...
Original implementation is:
static __inline__ void apic_wait_icr_idle(void)
{
while (apic_read(APIC_ICR) APIC_ICR_BUSY)
cpu_relax();
}
And new one will look something like.
do {
send_status = apic_read(APIC_ICR) APIC_ICR_BUSY;
if (!send_status)
break;
udelay(100);
} while (timeout++ 1000);
There will be at max 100 microsecond delay before you realize that IPI has
been dispatched. To optimize it further we can change it to 10 microsecond
delay
... I noticed that the maximum theoretical delay you mention has to be
multiplied by the number of CPUs on big systems, because on those
machines send_IPI_mask is implemented as a series of unicasts to each
CPU. It is for this reason that I thought this approach may be
considered unacceptable (I have to admit that I have not performed any
micro-benchmarks, though).
do {
send_status = apic_read(APIC_ICR) APIC_ICR_BUSY;
if (!send_status)
break;
udelay(10);
} while (timeout++ 1);
or may be
do {
send_status = apic_read(APIC_ICR) APIC_ICR_BUSY;
if (!send_status)
break;
udelay(1);
} while (timeout++ 10);
I don't know if 1 micro second delay is supported. I do see it being
used in kernel/hpet.c
Please notice that such high-precision timers are not available in all
machines.
Is it too much of performance overhead? Somebody who knows more about it
needs to tell. To me changing apic_wait_icr_idle() seems simple instead
of introducing a new function and then making a special case for NMI.
As I mentioned above the performance overhead depends on several
factors. I hope it makes sense.
- Fernando
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 2/10] safe_apic_wait_icr_idle - x86_64
On Wed, 2007-04-25 at 15:11 +0200, Andi Kleen wrote:
> On Wednesday 25 April 2007 14:55:12 Fernando Luis Vázquez Cao wrote:
> > On Wed, 2007-04-25 at 14:26 +0200, Andi Kleen wrote:
> > > > static __inline__ void apic_wait_icr_idle(void)
> > > > {
> > > > - while (apic_read( APIC_ICR ) & APIC_ICR_BUSY)
> > > > + while (apic_read(APIC_ICR) & APIC_ICR_BUSY)
> > > > cpu_relax();
> > > > }
> > > >
> > > > +static __inline__ unsigned int safe_apic_wait_icr_idle(void)
> > >
> > > This should be probably not inline -- too large
> > Hello Andi,
> >
> > Thank you for reviewing the patches. Do you want me to resend the whole
> > patch or should I cook a new one that un-inlines the function instead?
>
> I already did that.
>
> Also will apply Keith's suggestion.
Thank you Andi. Sorry for the trouble.
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/10] safe_apic_wait_icr_idle - i386
On Wed, 2007-04-25 at 22:55 +1000, Keith Owens wrote:
> Fernando Luis =?ISO-8859-1?Q?V=E1zquez?= Cao (on Wed, 25 Apr 2007 20:13:28
> +0900) wrote:
> >+static __inline__ unsigned long safe_apic_wait_icr_idle(void)
> >+{
> >+unsigned long send_status;
> >+int timeout;
> >+
> >+timeout = 0;
> >+do {
> >+udelay(100);
> >+send_status = apic_read(APIC_ICR) & APIC_ICR_BUSY;
> >+} while (send_status && (timeout++ < 1000));
> >+
> >+return send_status;
> >+}
> >+
>
> safe_apic_wait_icr_idle() as coded guarantees a minimum 100 usec delay
> before sending the IPI, this extra delay is unnecessary. Change it to
Hi Keith,
Thank you for the feedback!
> do {
> send_status = apic_read(APIC_ICR) & APIC_ICR_BUSY;
> if (send_status)
> break;
> udelay(100);
> } while (timeout++ < 1000);
Oops, that is a good point. I will resend this patch either tonight or
tomorrow morning.
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 2/10] safe_apic_wait_icr_idle - x86_64
On Wed, 2007-04-25 at 14:26 +0200, Andi Kleen wrote:
> > static __inline__ void apic_wait_icr_idle(void)
> > {
> > - while (apic_read( APIC_ICR ) & APIC_ICR_BUSY)
> > + while (apic_read(APIC_ICR) & APIC_ICR_BUSY)
> > cpu_relax();
> > }
> >
> > +static __inline__ unsigned int safe_apic_wait_icr_idle(void)
>
> This should be probably not inline -- too large
Hello Andi,
Thank you for reviewing the patches. Do you want me to resend the whole
patch or should I cook a new one that un-inlines the function instead?
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 10/10] Use safe_apic_wait_icr_idle in __send_IPI_dest_field - x86_64
On Wed, 2007-04-25 at 14:33 +0200, Andi Kleen wrote: > On Wednesday 25 April 2007 13:51:12 Fernando Luis Vázquez Cao wrote: > > Use safe_apic_wait_icr_idle to check ICR idle bit if the vector is > > NMI_VECTOR to avoid potential hangups in the event of crash when kdump > > tries to stop the other CPUs. > > But what happens then when this fails? Won't this give another hang? > Have you tested this? In kdump the crashing CPU (i.e. the CPU that called crash_kexec) is the one in charge of rebooting into and executing the dump capture kernel. But before doing this it attempts to stop the other CPUs sending a IPI using NMI_VECTOR as the vector. The problem is that sometimes delivery seems to fail and the crashing CPU gets stuck waiting for the ICR status bit to be cleared, which will never happen. With this patch, when safe_apic_wait_icr_idle times out the CPU will continue executing and try to hand over control to the dump capture kernel as usual. After applying this patch I have not seen hangs in the reboot path to second kernel showing the symptoms mentioned before, but perhaps I am just being lucky and there is something else going on. - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH 10/10] Use safe_apic_wait_icr_idle in __send_IPI_dest_field - x86_64
Use safe_apic_wait_icr_idle to check ICR idle bit if the vector is NMI_VECTOR to avoid potential hangups in the event of crash when kdump tries to stop the other CPUs. Signed-off-by: Fernando Luis Vazquez Cao <[EMAIL PROTECTED]> --- diff -urNp linux-2.6.21-rc7-orig/include/asm-x86_64/ipi.h linux-2.6.21-rc7/include/asm-x86_64/ipi.h --- linux-2.6.21-rc7-orig/include/asm-x86_64/ipi.h 2007-04-23 17:34:56.0 +0900 +++ linux-2.6.21-rc7/include/asm-x86_64/ipi.h 2007-04-25 19:15:01.0 +0900 @@ -87,7 +87,10 @@ static inline void __send_IPI_dest_field /* * Wait for idle. */ - apic_wait_icr_idle(); + if (unlikely(vector == NMI_VECTOR)) + safe_apic_wait_icr_idle(); + else + apic_wait_icr_idle(); /* * prepare target chip field - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH 9/10] Use safe_apic_wait_icr_idle in safe_apic_wait_icr_idle - i386
Use safe_apic_wait_icr_idle to check ICR idle bit if the vector is NMI_VECTOR to avoid potential hangups in the event of crash when kdump tries to stop the other CPUs. Signed-off-by: Fernando Luis Vazquez Cao <[EMAIL PROTECTED]> --- diff -urNp linux-2.6.21-rc7-orig/arch/i386/kernel/smp.c linux-2.6.21-rc7/arch/i386/kernel/smp.c --- linux-2.6.21-rc7-orig/arch/i386/kernel/smp.c2007-04-23 16:32:58.0 +0900 +++ linux-2.6.21-rc7/arch/i386/kernel/smp.c 2007-04-25 19:06:40.0 +0900 @@ -175,7 +175,10 @@ static inline void __send_IPI_dest_field /* * Wait for idle. */ - apic_wait_icr_idle(); + if (unlikely(vector == NMI_VECTOR)) + safe_apic_wait_icr_idle(); + else + apic_wait_icr_idle(); /* * prepare target chip field - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH 8/10] __send_IPI_dest_field - x86_64
Implement __send_IPI_dest_field which can be used to send IPIs when the
"destination shorthand" field of the ICR is set to 00 (destination
field). Use it whenever possible.
Signed-off-by: Fernando Luis Vazquez Cao <[EMAIL PROTECTED]>
---
diff -urNp linux-2.6.21-rc7-orig/arch/x86_64/kernel/genapic_flat.c
linux-2.6.21-rc7/arch/x86_64/kernel/genapic_flat.c
--- linux-2.6.21-rc7-orig/arch/x86_64/kernel/genapic_flat.c 2007-02-05
03:44:54.0 +0900
+++ linux-2.6.21-rc7/arch/x86_64/kernel/genapic_flat.c 2007-04-23
17:19:14.0 +0900
@@ -60,31 +60,10 @@ static void flat_init_apic_ldr(void)
static void flat_send_IPI_mask(cpumask_t cpumask, int vector)
{
unsigned long mask = cpus_addr(cpumask)[0];
- unsigned long cfg;
unsigned long flags;
local_irq_save(flags);
-
- /*
-* Wait for idle.
-*/
- apic_wait_icr_idle();
-
- /*
-* prepare target chip field
-*/
- cfg = __prepare_ICR2(mask);
- apic_write(APIC_ICR2, cfg);
-
- /*
-* program the ICR
-*/
- cfg = __prepare_ICR(0, vector, APIC_DEST_LOGICAL);
-
- /*
-* Send the IPI. The write to APIC_ICR fires this off.
-*/
- apic_write(APIC_ICR, cfg);
+ __send_IPI_dest_field(mask, vector, APIC_DEST_LOGICAL);
local_irq_restore(flags);
}
diff -urNp linux-2.6.21-rc7-orig/include/asm-x86_64/ipi.h
linux-2.6.21-rc7/include/asm-x86_64/ipi.h
--- linux-2.6.21-rc7-orig/include/asm-x86_64/ipi.h 2007-02-05
03:44:54.0 +0900
+++ linux-2.6.21-rc7/include/asm-x86_64/ipi.h 2007-04-23 17:11:04.0
+0900
@@ -76,10 +76,39 @@ static inline void __send_IPI_shortcut(u
apic_write(APIC_ICR, cfg);
}
+/*
+ * This is used to send an IPI with no shorthand notation (the destination is
+ * specified in bits 56 to 63 of the ICR).
+ */
+static inline void __send_IPI_dest_field(unsigned int mask, int vector,
unsigned int dest)
+{
+ unsigned long cfg;
+
+ /*
+* Wait for idle.
+*/
+ apic_wait_icr_idle();
+
+ /*
+* prepare target chip field
+*/
+ cfg = __prepare_ICR2(mask);
+ apic_write(APIC_ICR2, cfg);
+
+ /*
+* program the ICR
+*/
+ cfg = __prepare_ICR(0, vector, dest);
+
+ /*
+* Send the IPI. The write to APIC_ICR fires this off.
+*/
+ apic_write(APIC_ICR, cfg);
+}
static inline void send_IPI_mask_sequence(cpumask_t mask, int vector)
{
- unsigned long cfg, flags;
+ unsigned long flags;
unsigned long query_cpu;
/*
@@ -88,28 +117,9 @@ static inline void send_IPI_mask_sequenc
* - mbligh
*/
local_irq_save(flags);
-
for_each_cpu_mask(query_cpu, mask) {
- /*
-* Wait for idle.
-*/
- apic_wait_icr_idle();
-
- /*
-* prepare target chip field
-*/
- cfg = __prepare_ICR2(x86_cpu_to_apicid[query_cpu]);
- apic_write(APIC_ICR2, cfg);
-
- /*
-* program the ICR
-*/
- cfg = __prepare_ICR(0, vector, APIC_DEST_PHYSICAL);
-
- /*
-* Send the IPI. The write to APIC_ICR fires this off.
-*/
- apic_write(APIC_ICR, cfg);
+ __send_IPI_dest_field(x86_cpu_to_apicid[query_cpu],
+ vector, APIC_DEST_PHYSICAL);
}
local_irq_restore(flags);
}
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH 7/10] __send_IPI_dest_field - i386
Implement __send_IPI_dest_field which can be used to send IPIs when the
"destination shorthand" field of the ICR is set to 00 (destination
field). Use it whenever possible.
Signed-off-by: Fernando Luis Vazquez Cao <[EMAIL PROTECTED]>
---
diff -urNp linux-2.6.21-rc7-orig/arch/i386/kernel/smp.c
linux-2.6.21-rc7/arch/i386/kernel/smp.c
--- linux-2.6.21-rc7-orig/arch/i386/kernel/smp.c2007-04-18
15:27:48.0 +0900
+++ linux-2.6.21-rc7/arch/i386/kernel/smp.c 2007-04-23 16:29:24.0
+0900
@@ -165,16 +165,13 @@ void fastcall send_IPI_self(int vector)
}
/*
- * This is only used on smaller machines.
+ * This is used to send an IPI with no shorthand notation (the destination is
+ * specified in bits 56 to 63 of the ICR).
*/
-void send_IPI_mask_bitmask(cpumask_t cpumask, int vector)
+static inline void __send_IPI_dest_field(unsigned long mask, int vector)
{
- unsigned long mask = cpus_addr(cpumask)[0];
unsigned long cfg;
- unsigned long flags;
- local_irq_save(flags);
- WARN_ON(mask & ~cpus_addr(cpu_online_map)[0]);
/*
* Wait for idle.
*/
@@ -195,13 +192,25 @@ void send_IPI_mask_bitmask(cpumask_t cpu
* Send the IPI. The write to APIC_ICR fires this off.
*/
apic_write_around(APIC_ICR, cfg);
+}
+/*
+ * This is only used on smaller machines.
+ */
+void send_IPI_mask_bitmask(cpumask_t cpumask, int vector)
+{
+ unsigned long mask = cpus_addr(cpumask)[0];
+ unsigned long flags;
+
+ local_irq_save(flags);
+ WARN_ON(mask & ~cpus_addr(cpu_online_map)[0]);
+ __send_IPI_dest_field(mask, vector);
local_irq_restore(flags);
}
void send_IPI_mask_sequence(cpumask_t mask, int vector)
{
- unsigned long cfg, flags;
+ unsigned long flags;
unsigned int query_cpu;
/*
@@ -211,30 +220,10 @@ void send_IPI_mask_sequence(cpumask_t ma
*/
local_irq_save(flags);
-
for (query_cpu = 0; query_cpu < NR_CPUS; ++query_cpu) {
if (cpu_isset(query_cpu, mask)) {
-
- /*
-* Wait for idle.
-*/
- apic_wait_icr_idle();
-
- /*
-* prepare target chip field
-*/
- cfg = __prepare_ICR2(cpu_to_logical_apicid(query_cpu));
- apic_write_around(APIC_ICR2, cfg);
-
- /*
-* program the ICR
-*/
- cfg = __prepare_ICR(0, vector);
-
- /*
-* Send the IPI. The write to APIC_ICR fires this off.
-*/
- apic_write_around(APIC_ICR, cfg);
+ __send_IPI_dest_field(cpu_to_logical_apicid(query_cpu),
+ vector);
}
}
local_irq_restore(flags);
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH 6/10] inquire_remote_apic: use safe_apic_wait_icr_idle - x86_64
inquire_remote_apic is used for APIC debugging, so use
safe_apic_wait_icr_idle instead of apic_wait_icr_idle to avoid possible
lockups when APIC delivery fails.
Signed-off-by: Fernando Luis Vazquez Cao <[EMAIL PROTECTED]>
---
diff -urNp linux-2.6.21-rc7-orig/arch/x86_64/kernel/smpboot.c
linux-2.6.21-rc7/arch/x86_64/kernel/smpboot.c
--- linux-2.6.21-rc7-orig/arch/x86_64/kernel/smpboot.c 2007-04-18
15:53:11.0 +0900
+++ linux-2.6.21-rc7/arch/x86_64/kernel/smpboot.c 2007-04-18
16:39:20.0 +0900
@@ -392,7 +392,8 @@ static void inquire_remote_apic(int apic
{
unsigned i, regs[] = { APIC_ID >> 4, APIC_LVR >> 4, APIC_SPIV >> 4 };
char *names[] = { "ID", "VERSION", "SPIV" };
- int timeout, status;
+ int timeout;
+ unsigned int status;
printk(KERN_INFO "Inquiring remote APIC #%d...\n", apicid);
@@ -402,7 +403,9 @@ static void inquire_remote_apic(int apic
/*
* Wait for idle.
*/
- apic_wait_icr_idle();
+ status = safe_apic_wait_icr_idle();
+ if (status)
+ printk("a previous APIC delivery may have failed\n");
apic_write(APIC_ICR2, SET_APIC_DEST_FIELD(apicid));
apic_write(APIC_ICR, APIC_DM_REMRD | regs[i]);
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH 5/10] __inquire_remote_apic: use safe_apic_wait_icr_idle - i386
__inquire_remote_apic is used for APIC debugging, so use
safe_apic_wait_icr_idle instead of apic_wait_icr_idle to avoid possible
lockups when APIC delivery fails.
Signed-off-by: Fernando Luis Vazquez Cao <[EMAIL PROTECTED]>
---
diff -urNp linux-2.6.21-rc7-orig/arch/i386/kernel/smpboot.c
linux-2.6.21-rc7/arch/i386/kernel/smpboot.c
--- linux-2.6.21-rc7-orig/arch/i386/kernel/smpboot.c2007-04-18
15:49:33.0 +0900
+++ linux-2.6.21-rc7/arch/i386/kernel/smpboot.c 2007-04-18 16:36:27.0
+0900
@@ -526,7 +526,8 @@ static inline void __inquire_remote_apic
{
int i, regs[] = { APIC_ID >> 4, APIC_LVR >> 4, APIC_SPIV >> 4 };
char *names[] = { "ID", "VERSION", "SPIV" };
- int timeout, status;
+ int timeout;
+ unsigned long status;
printk("Inquiring remote APIC #%d...\n", apicid);
@@ -536,7 +537,9 @@ static inline void __inquire_remote_apic
/*
* Wait for idle.
*/
- apic_wait_icr_idle();
+ status = safe_apic_wait_icr_idle();
+ if (status)
+ printk("a previous APIC delivery may have failed\n");
apic_write_around(APIC_ICR2, SET_APIC_DEST_FIELD(apicid));
apic_write_around(APIC_ICR, APIC_DM_REMRD | regs[i]);
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH 4/10] smpboot: use safe_apic_wait_icr_idle - x86_64
The functionality provided by the new safe_apic_wait_icr_idle is being
open-coded all over "kernel/smpboot.c". Use safe_apic_wait_icr_idle
instead to consolidate code and ease maintenance.
Signed-off-by: Fernando Luis Vazquez Cao <[EMAIL PROTECTED]>
---
diff -urNp linux-2.6.21-rc7-orig/arch/x86_64/kernel/smpboot.c
linux-2.6.21-rc7/arch/x86_64/kernel/smpboot.c
--- linux-2.6.21-rc7-orig/arch/x86_64/kernel/smpboot.c 2007-04-18
15:27:48.0 +0900
+++ linux-2.6.21-rc7/arch/x86_64/kernel/smpboot.c 2007-04-18
15:52:08.0 +0900
@@ -430,8 +430,8 @@ static void inquire_remote_apic(int apic
*/
static int __cpuinit wakeup_secondary_via_INIT(int phys_apicid, unsigned int
start_rip)
{
- unsigned long send_status = 0, accept_status = 0;
- int maxlvt, timeout, num_starts, j;
+ unsigned long send_status, accept_status = 0;
+ int maxlvt, num_starts, j;
Dprintk("Asserting INIT.\n");
@@ -447,12 +447,7 @@ static int __cpuinit wakeup_secondary_vi
| APIC_DM_INIT);
Dprintk("Waiting for send to finish...\n");
- timeout = 0;
- do {
- Dprintk("+");
- udelay(100);
- send_status = apic_read(APIC_ICR) & APIC_ICR_BUSY;
- } while (send_status && (timeout++ < 1000));
+ send_status = safe_apic_wait_icr_idle();
mdelay(10);
@@ -465,12 +460,7 @@ static int __cpuinit wakeup_secondary_vi
apic_write(APIC_ICR, APIC_INT_LEVELTRIG | APIC_DM_INIT);
Dprintk("Waiting for send to finish...\n");
- timeout = 0;
- do {
- Dprintk("+");
- udelay(100);
- send_status = apic_read(APIC_ICR) & APIC_ICR_BUSY;
- } while (send_status && (timeout++ < 1000));
+ send_status = safe_apic_wait_icr_idle();
mb();
atomic_set(_deasserted, 1);
@@ -509,12 +499,7 @@ static int __cpuinit wakeup_secondary_vi
Dprintk("Startup point 1.\n");
Dprintk("Waiting for send to finish...\n");
- timeout = 0;
- do {
- Dprintk("+");
- udelay(100);
- send_status = apic_read(APIC_ICR) & APIC_ICR_BUSY;
- } while (send_status && (timeout++ < 1000));
+ send_status = safe_apic_wait_icr_idle();
/*
* Give the other CPU some time to accept the IPI.
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH 3/10] smpboot: use safe_apic_wait_icr_idle - i386
The functionality provided by the new safe_apic_wait_icr_idle is being
open-coded all over "kernel/smpboot.c". Use safe_apic_wait_icr_idle
instead to consolidate code and ease maintenance.
Signed-off-by: Fernando Luis Vazquez Cao <[EMAIL PROTECTED]>
---
diff -urNp linux-2.6.21-rc7-orig/arch/i386/kernel/smpboot.c
linux-2.6.21-rc7/arch/i386/kernel/smpboot.c
--- linux-2.6.21-rc7-orig/arch/i386/kernel/smpboot.c2007-04-18
15:27:48.0 +0900
+++ linux-2.6.21-rc7/arch/i386/kernel/smpboot.c 2007-04-18 15:44:41.0
+0900
@@ -568,8 +568,8 @@ static inline void __inquire_remote_apic
static int __devinit
wakeup_secondary_cpu(int logical_apicid, unsigned long start_eip)
{
- unsigned long send_status = 0, accept_status = 0;
- int timeout, maxlvt;
+ unsigned long send_status, accept_status = 0;
+ int maxlvt;
/* Target chip */
apic_write_around(APIC_ICR2, SET_APIC_DEST_FIELD(logical_apicid));
@@ -579,12 +579,7 @@ wakeup_secondary_cpu(int logical_apicid,
apic_write_around(APIC_ICR, APIC_DM_NMI | APIC_DEST_LOGICAL);
Dprintk("Waiting for send to finish...\n");
- timeout = 0;
- do {
- Dprintk("+");
- udelay(100);
- send_status = apic_read(APIC_ICR) & APIC_ICR_BUSY;
- } while (send_status && (timeout++ < 1000));
+ send_status = safe_apic_wait_icr_idle();
/*
* Give the other CPU some time to accept the IPI.
@@ -614,8 +609,8 @@ wakeup_secondary_cpu(int logical_apicid,
static int __devinit
wakeup_secondary_cpu(int phys_apicid, unsigned long start_eip)
{
- unsigned long send_status = 0, accept_status = 0;
- int maxlvt, timeout, num_starts, j;
+ unsigned long send_status, accept_status = 0;
+ int maxlvt, num_starts, j;
/*
* Be paranoid about clearing APIC errors.
@@ -640,12 +635,7 @@ wakeup_secondary_cpu(int phys_apicid, un
| APIC_DM_INIT);
Dprintk("Waiting for send to finish...\n");
- timeout = 0;
- do {
- Dprintk("+");
- udelay(100);
- send_status = apic_read(APIC_ICR) & APIC_ICR_BUSY;
- } while (send_status && (timeout++ < 1000));
+ send_status = safe_apic_wait_icr_idle();
mdelay(10);
@@ -658,12 +648,7 @@ wakeup_secondary_cpu(int phys_apicid, un
apic_write_around(APIC_ICR, APIC_INT_LEVELTRIG | APIC_DM_INIT);
Dprintk("Waiting for send to finish...\n");
- timeout = 0;
- do {
- Dprintk("+");
- udelay(100);
- send_status = apic_read(APIC_ICR) & APIC_ICR_BUSY;
- } while (send_status && (timeout++ < 1000));
+ send_status = safe_apic_wait_icr_idle();
atomic_set(_deasserted, 1);
@@ -719,12 +704,7 @@ wakeup_secondary_cpu(int phys_apicid, un
Dprintk("Startup point 1.\n");
Dprintk("Waiting for send to finish...\n");
- timeout = 0;
- do {
- Dprintk("+");
- udelay(100);
- send_status = apic_read(APIC_ICR) & APIC_ICR_BUSY;
- } while (send_status && (timeout++ < 1000));
+ send_status = safe_apic_wait_icr_idle();
/*
* Give the other CPU some time to accept the IPI.
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH 2/10] safe_apic_wait_icr_idle - x86_64
apic_wait_icr_idle looks like this:
static __inline__ void apic_wait_icr_idle(void)
{
while (apic_read(APIC_ICR) & APIC_ICR_BUSY)
cpu_relax();
}
The busy loop in this function would not be problematic if the
corresponding status bit in the ICR were always updated, but that does
not seem to be the case under certain crash scenarios. Kdump uses an IPI
to stop the other CPUs in the event of a crash, but when any of the
other CPUs are locked-up inside the NMI handler the CPU that sends the
IPI will end up looping forever in the ICR check, effectively
hard-locking the whole system.
Quoting from Intel's "MultiProcessor Specification" (Version 1.4), B-3:
"A local APIC unit indicates successful dispatch of an IPI by
resetting the Delivery Status bit in the Interrupt Command
Register (ICR). The operating system polls the delivery status
bit after sending an INIT or STARTUP IPI until the command has
been dispatched.
A period of 20 microseconds should be sufficient for IPI dispatch
to complete under normal operating conditions. If the IPI is not
successfully dispatched, the operating system can abort the
command. Alternatively, the operating system can retry the IPI by
writing the lower 32-bit double word of the ICR. This “time-out”
mechanism can be implemented through an external interrupt, if
interrupts are enabled on the processor, or through execution of
an instruction or time-stamp counter spin loop."
Intel's documentation suggests the implementation of a time-out
mechanism, which, by the way, is already being open-coded in some parts
of the kernel that tinker with ICR.
Create a apic_wait_icr_idle replacement that implements the time-out
mechanism and that can be used to solve the aforementioned problem.
Signed-off-by: Fernando Luis Vazquez Cao <[EMAIL PROTECTED]>
---
diff -urNp linux-2.6.21-rc7-orig/include/asm-x86_64/apic.h
linux-2.6.21-rc7/include/asm-x86_64/apic.h
--- linux-2.6.21-rc7-orig/include/asm-x86_64/apic.h 2007-04-18
13:47:06.0 +0900
+++ linux-2.6.21-rc7/include/asm-x86_64/apic.h 2007-04-18 15:24:30.0
+0900
@@ -2,6 +2,7 @@
#define __ASM_APIC_H
#include
+#include
#include
#include
#include
@@ -49,10 +50,24 @@ static __inline unsigned int apic_read(u
static __inline__ void apic_wait_icr_idle(void)
{
- while (apic_read( APIC_ICR ) & APIC_ICR_BUSY)
+ while (apic_read(APIC_ICR) & APIC_ICR_BUSY)
cpu_relax();
}
+static __inline__ unsigned int safe_apic_wait_icr_idle(void)
+{
+ unsigned int send_status;
+ int timeout;
+
+ timeout = 0;
+ do {
+ udelay(100);
+ send_status = apic_read(APIC_ICR) & APIC_ICR_BUSY;
+ } while (send_status && (timeout++ < 1000));
+
+ return send_status;
+}
+
static inline void ack_APIC_irq(void)
{
/*
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH 1/10] safe_apic_wait_icr_idle - i386
apic_wait_icr_idle looks like this:
static __inline__ void apic_wait_icr_idle(void)
{
while (apic_read(APIC_ICR) & APIC_ICR_BUSY)
cpu_relax();
}
The busy loop in this function would not be problematic if the
corresponding status bit in the ICR were always updated, but that does
not seem to be the case under certain crash scenarios. Kdump uses an IPI
to stop the other CPUs in the event of a crash, but when any of the
other CPUs are locked-up inside the NMI handler the CPU that sends the
IPI will end up looping forever in the ICR check, effectively
hard-locking the whole system.
Quoting from Intel's "MultiProcessor Specification" (Version 1.4), B-3:
"A local APIC unit indicates successful dispatch of an IPI by
resetting the Delivery Status bit in the Interrupt Command
Register (ICR). The operating system polls the delivery status
bit after sending an INIT or STARTUP IPI until the command has
been dispatched.
A period of 20 microseconds should be sufficient for IPI dispatch
to complete under normal operating conditions. If the IPI is not
successfully dispatched, the operating system can abort the
command. Alternatively, the operating system can retry the IPI by
writing the lower 32-bit double word of the ICR. This “time-out”
mechanism can be implemented through an external interrupt, if
interrupts are enabled on the processor, or through execution of
an instruction or time-stamp counter spin loop."
Intel's documentation suggests the implementation of a time-out
mechanism, which, by the way, is already being open-coded in some parts
of the kernel that tinker with ICR.
Create a apic_wait_icr_idle replacement that implements the time-out
mechanism and that can be used to solve the aforementioned problem.
Signed-off-by: Fernando Luis Vazquez Cao <[EMAIL PROTECTED]>
---
diff -urNp linux-2.6.21-rc7-orig/include/asm-i386/apic.h
linux-2.6.21-rc7/include/asm-i386/apic.h
--- linux-2.6.21-rc7-orig/include/asm-i386/apic.h 2007-04-18
15:27:50.0 +0900
+++ linux-2.6.21-rc7/include/asm-i386/apic.h2007-04-18 15:28:36.0
+0900
@@ -2,6 +2,7 @@
#define __ASM_APIC_H
#include
+#include
#include
#include
#include
@@ -66,10 +67,24 @@ static __inline fastcall unsigned long n
static __inline__ void apic_wait_icr_idle(void)
{
- while ( apic_read( APIC_ICR ) & APIC_ICR_BUSY )
+ while (apic_read(APIC_ICR) & APIC_ICR_BUSY)
cpu_relax();
}
+static __inline__ unsigned long safe_apic_wait_icr_idle(void)
+{
+ unsigned long send_status;
+ int timeout;
+
+ timeout = 0;
+ do {
+ udelay(100);
+ send_status = apic_read(APIC_ICR) & APIC_ICR_BUSY;
+ } while (send_status && (timeout++ < 1000));
+
+ return send_status;
+}
+
int get_physical_broadcast(void);
#ifdef CONFIG_X86_GOOD_APIC
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[RFC PATCH 0/10] apic_wait_icr_idle issues and possible solutions
--- Background
On i386 and x86_64, in the event of a crash, kdump attempts to stop all
other CPUs before proceeding to boot into the dump capture kernel.
Depending on the sub-architecture several implementations exist as
detailed below.
- i386
smp_send_nmi_allbutself
send_IPI_mask(mask, NMI_VECTOR)
* mach-default
send_IPI_mask
-->send_IPI_mask_bitmask
apic_wait_icr_idle
* mach-bigsmp, mach-es7000, mach-numaq, mach-summit
send_IPI_mask
-->send_IPI_mask_sequence
apic_wait_icr_idle
-- x86_64
smp_send_nmi_allbutself
send_IPI_allbutself(NMI_VECTOR)
* flat APIC
send_IPI_allbutself=flat_send_IPI_allbutself
-->flat_send_IPI_mask
apic_wait_icr_idle
* clustered APIC, physflat APIC
send_IPI_allbutself=(cluster_send_IPI_allbutself/physflat_send_IPI_allbutself)
(cluster_send_IPI_mask/physflat_send_IPI_mask)
-->send_IPI_mask_sequence
apic_wait_icr_idle
As the pseudo-code above shows all the sub-architectures end up calling
apic_wait_icr_idle, which looks like this:
static __inline__ void apic_wait_icr_idle(void)
{
while (apic_read(APIC_ICR) & APIC_ICR_BUSY)
cpu_relax();
}
The busy loop in this function would not be problematic if the
corresponding status bit in the ICR were always updated, but that does
not seem to be the case under certain crash scenarios. As an example,
when the other CPUs are locked-up inside the NMI handler the CPU that
sends the IPI will end up looping forever in the ICR check, effectively
hard-locking the whole system.
Quoting from Intel's "MultiProcessor Specification" (Version 1.4), B-3:
"A local APIC unit indicates successful dispatch of an IPI by
resetting the Delivery Status bit in the Interrupt Command
Register (ICR). The operating system polls the delivery status
bit after sending an INIT or STARTUP IPI until the command has
been dispatched.
A period of 20 microseconds should be sufficient for IPI dispatch
to complete under normal operating conditions. If the IPI is not
successfully dispatched, the operating system can abort the
command. Alternatively, the operating system can retry the IPI by
writing the lower 32-bit double word of the ICR. This “time-out”
mechanism can be implemented through an external interrupt, if
interrupts are enabled on the processor, or through execution of
an instruction or time-stamp counter spin loop."
Intel's documentation suggests the implementation of a time-out
mechanism, which, by the way, is already being open-coded in some parts
of the kernel that tinker with ICR.
--- Possible solutions
* Solution A: Implement the time-out mechanism in apic_wait_icr_idle.
The problem with this approach is that introduces a performance penalty
that may not be acceptable for some callers of apic_wait_icr_idle.
Besides, during normal operation delivery errors should not occur. This
brings us to solution B.
* Solution B: Create a new function that implements the time-out
mechanism.
Create a new function (what about safe_apic_wait_icr_idle?) that
provides the apic_wait_icr_idle functionality but also implements a
time-out mechanism. This function would look like this:
static __inline__ int safe_apic_wait_icr_idle(void)
{
unsigned long send_status;
int timeout;
timeout = 0;
do {
udelay(100);
send_status = apic_read(APIC_ICR) & APIC_ICR_BUSY;
} while (send_status && (timeout++ < 1000));
return send_status;
}
Once we have this function we still have to decide how to use it:
Solution B-1: Create send_IPI_mask and send_IPI_allbutself replacements
that use the new safe_apic_wait_icr_idle instead of apic_wait_icr_idle.
The problem with this approach is that it may be overkill, since they
probably would not have many users besides kdump.
Solution B-2: Use safe_apic_wait_icr_idle when the vector is NMI_VECTOR
and apic_wait_icr_icr_idle otherwise. This solution has the advantage
that it is very simple, but it probably isn't very elegant.
--- About this patch set
The kdump-related stuff is included in patches 9 and 10, but even this
part is rejected I think that patches 1 through 8 include some necessary
cleanups.
Patches 1 and 2: implement safe_apic_wait_icr_idle for i386 and x86_64
respectively.
Patches 3 through 6: remove some ugly cut and pasting from the smp code.
Patches 7 and 8: Cleanup *send_IPI*.
Patches 9 and 10: Implement solution B-2.
I would appreciate your feedback on these patches.
- Fernando
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] mm: PageLRU can be non-atomic bit operation
On Tue, 2007-04-24 at 16:22 +0200, Andi Kleen wrote: > > Why would you need any kind of lock when just changing a single bit, > > if it didn't affect other bits of the same word? Just as you don't > > need a lock when simply assigning a word, setting a bit to 0 or 1 > > is simple in itself (it doesn't matter if it was 0 or 1 before). > > > > > But, I think that concurrent bit operation on different bits > > > is just like OR operation , so lock prefix is not needed. > > > > I firmly believe that it is; but I'm not a processor expert. > > Think of the CPU cache like the page cache. The VFS cannot change > anything directly on disk - it always has to read a page (or block); > change it there even if it's only a single bit and back. > > Now imagine multiple independent kernels to that disk doing this in parallel > without any locking. You will lose data on simple changes because > of data races. > > The CPU is similar. The memory is the disk; the protocol talking to > the memory only knows cache lines. There are multiple CPUs talking > to that memory. The CPUs cannot change anything > without reading a full cache line first and then later writing it back. > There is "just do OR operation" when talking to memory, just like > disks don't have such a operation; just read and write. > > [there are some special cases with uncached mappings, but they are not > relevant here] > > With lock the CPU ensures the read-modify-write cycle is atomic, > without it doesn't. > > The CPU also guarantees you that multiple writes don't get lost > even when they hit the same cache line (otherwise you would need lock > for anything in the same cache line), but it doesn't guarantee that > for a word. The details for that vary by architecture; on x86 it is > any memory access, on others it can be only guaranteed for long stores. > > The exact rules for all this can be quite complicated (and also vary > by architecture), this is a simplification but should be enough as a rough > guide. Intel 80386 Reference Programmer's Manual reads: " When accessing a bit in memory, the processor may access four bytes starting from the memory address given by: Effective Address + (4 * (BitOffset DIV 32)) for a 32-bit operand size, or two bytes starting from the memory address given by: Effective Address + (2 * (BitOffset DIV 16)) for a 16-bit operand size. It may do this even when only a single byte needs to be accessed in order to get at the given bit. " This seems to imply that we need the lock prefix even when updating different bits within the same long (not just the same byte). Is this interpretation correct? - Fernando - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] mm: PageLRU can be non-atomic bit operation
On Tue, 2007-04-24 at 16:22 +0200, Andi Kleen wrote: Why would you need any kind of lock when just changing a single bit, if it didn't affect other bits of the same word? Just as you don't need a lock when simply assigning a word, setting a bit to 0 or 1 is simple in itself (it doesn't matter if it was 0 or 1 before). But, I think that concurrent bit operation on different bits is just like OR operation , so lock prefix is not needed. I firmly believe that it is; but I'm not a processor expert. Think of the CPU cache like the page cache. The VFS cannot change anything directly on disk - it always has to read a page (or block); change it there even if it's only a single bit and back. Now imagine multiple independent kernels to that disk doing this in parallel without any locking. You will lose data on simple changes because of data races. The CPU is similar. The memory is the disk; the protocol talking to the memory only knows cache lines. There are multiple CPUs talking to that memory. The CPUs cannot change anything without reading a full cache line first and then later writing it back. There is just do OR operation when talking to memory, just like disks don't have such a operation; just read and write. [there are some special cases with uncached mappings, but they are not relevant here] With lock the CPU ensures the read-modify-write cycle is atomic, without it doesn't. The CPU also guarantees you that multiple writes don't get lost even when they hit the same cache line (otherwise you would need lock for anything in the same cache line), but it doesn't guarantee that for a word. The details for that vary by architecture; on x86 it is any memory access, on others it can be only guaranteed for long stores. The exact rules for all this can be quite complicated (and also vary by architecture), this is a simplification but should be enough as a rough guide. Intel 80386 Reference Programmer's Manual reads: When accessing a bit in memory, the processor may access four bytes starting from the memory address given by: Effective Address + (4 * (BitOffset DIV 32)) for a 32-bit operand size, or two bytes starting from the memory address given by: Effective Address + (2 * (BitOffset DIV 16)) for a 16-bit operand size. It may do this even when only a single byte needs to be accessed in order to get at the given bit. This seems to imply that we need the lock prefix even when updating different bits within the same long (not just the same byte). Is this interpretation correct? - Fernando - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[RFC PATCH 0/10] apic_wait_icr_idle issues and possible solutions
--- Background
On i386 and x86_64, in the event of a crash, kdump attempts to stop all
other CPUs before proceeding to boot into the dump capture kernel.
Depending on the sub-architecture several implementations exist as
detailed below.
- i386
smp_send_nmi_allbutself
send_IPI_mask(mask, NMI_VECTOR)
* mach-default
send_IPI_mask
--send_IPI_mask_bitmask
apic_wait_icr_idle
* mach-bigsmp, mach-es7000, mach-numaq, mach-summit
send_IPI_mask
--send_IPI_mask_sequence
apic_wait_icr_idle
-- x86_64
smp_send_nmi_allbutself
send_IPI_allbutself(NMI_VECTOR)
* flat APIC
send_IPI_allbutself=flat_send_IPI_allbutself
--flat_send_IPI_mask
apic_wait_icr_idle
* clustered APIC, physflat APIC
send_IPI_allbutself=(cluster_send_IPI_allbutself/physflat_send_IPI_allbutself)
(cluster_send_IPI_mask/physflat_send_IPI_mask)
--send_IPI_mask_sequence
apic_wait_icr_idle
As the pseudo-code above shows all the sub-architectures end up calling
apic_wait_icr_idle, which looks like this:
static __inline__ void apic_wait_icr_idle(void)
{
while (apic_read(APIC_ICR) APIC_ICR_BUSY)
cpu_relax();
}
The busy loop in this function would not be problematic if the
corresponding status bit in the ICR were always updated, but that does
not seem to be the case under certain crash scenarios. As an example,
when the other CPUs are locked-up inside the NMI handler the CPU that
sends the IPI will end up looping forever in the ICR check, effectively
hard-locking the whole system.
Quoting from Intel's MultiProcessor Specification (Version 1.4), B-3:
A local APIC unit indicates successful dispatch of an IPI by
resetting the Delivery Status bit in the Interrupt Command
Register (ICR). The operating system polls the delivery status
bit after sending an INIT or STARTUP IPI until the command has
been dispatched.
A period of 20 microseconds should be sufficient for IPI dispatch
to complete under normal operating conditions. If the IPI is not
successfully dispatched, the operating system can abort the
command. Alternatively, the operating system can retry the IPI by
writing the lower 32-bit double word of the ICR. This “time-out”
mechanism can be implemented through an external interrupt, if
interrupts are enabled on the processor, or through execution of
an instruction or time-stamp counter spin loop.
Intel's documentation suggests the implementation of a time-out
mechanism, which, by the way, is already being open-coded in some parts
of the kernel that tinker with ICR.
--- Possible solutions
* Solution A: Implement the time-out mechanism in apic_wait_icr_idle.
The problem with this approach is that introduces a performance penalty
that may not be acceptable for some callers of apic_wait_icr_idle.
Besides, during normal operation delivery errors should not occur. This
brings us to solution B.
* Solution B: Create a new function that implements the time-out
mechanism.
Create a new function (what about safe_apic_wait_icr_idle?) that
provides the apic_wait_icr_idle functionality but also implements a
time-out mechanism. This function would look like this:
static __inline__ int safe_apic_wait_icr_idle(void)
{
unsigned long send_status;
int timeout;
timeout = 0;
do {
udelay(100);
send_status = apic_read(APIC_ICR) APIC_ICR_BUSY;
} while (send_status (timeout++ 1000));
return send_status;
}
Once we have this function we still have to decide how to use it:
Solution B-1: Create send_IPI_mask and send_IPI_allbutself replacements
that use the new safe_apic_wait_icr_idle instead of apic_wait_icr_idle.
The problem with this approach is that it may be overkill, since they
probably would not have many users besides kdump.
Solution B-2: Use safe_apic_wait_icr_idle when the vector is NMI_VECTOR
and apic_wait_icr_icr_idle otherwise. This solution has the advantage
that it is very simple, but it probably isn't very elegant.
--- About this patch set
The kdump-related stuff is included in patches 9 and 10, but even this
part is rejected I think that patches 1 through 8 include some necessary
cleanups.
Patches 1 and 2: implement safe_apic_wait_icr_idle for i386 and x86_64
respectively.
Patches 3 through 6: remove some ugly cut and pasting from the smp code.
Patches 7 and 8: Cleanup *send_IPI*.
Patches 9 and 10: Implement solution B-2.
I would appreciate your feedback on these patches.
- Fernando
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH 1/10] safe_apic_wait_icr_idle - i386
apic_wait_icr_idle looks like this:
static __inline__ void apic_wait_icr_idle(void)
{
while (apic_read(APIC_ICR) APIC_ICR_BUSY)
cpu_relax();
}
The busy loop in this function would not be problematic if the
corresponding status bit in the ICR were always updated, but that does
not seem to be the case under certain crash scenarios. Kdump uses an IPI
to stop the other CPUs in the event of a crash, but when any of the
other CPUs are locked-up inside the NMI handler the CPU that sends the
IPI will end up looping forever in the ICR check, effectively
hard-locking the whole system.
Quoting from Intel's MultiProcessor Specification (Version 1.4), B-3:
A local APIC unit indicates successful dispatch of an IPI by
resetting the Delivery Status bit in the Interrupt Command
Register (ICR). The operating system polls the delivery status
bit after sending an INIT or STARTUP IPI until the command has
been dispatched.
A period of 20 microseconds should be sufficient for IPI dispatch
to complete under normal operating conditions. If the IPI is not
successfully dispatched, the operating system can abort the
command. Alternatively, the operating system can retry the IPI by
writing the lower 32-bit double word of the ICR. This “time-out”
mechanism can be implemented through an external interrupt, if
interrupts are enabled on the processor, or through execution of
an instruction or time-stamp counter spin loop.
Intel's documentation suggests the implementation of a time-out
mechanism, which, by the way, is already being open-coded in some parts
of the kernel that tinker with ICR.
Create a apic_wait_icr_idle replacement that implements the time-out
mechanism and that can be used to solve the aforementioned problem.
Signed-off-by: Fernando Luis Vazquez Cao [EMAIL PROTECTED]
---
diff -urNp linux-2.6.21-rc7-orig/include/asm-i386/apic.h
linux-2.6.21-rc7/include/asm-i386/apic.h
--- linux-2.6.21-rc7-orig/include/asm-i386/apic.h 2007-04-18
15:27:50.0 +0900
+++ linux-2.6.21-rc7/include/asm-i386/apic.h2007-04-18 15:28:36.0
+0900
@@ -2,6 +2,7 @@
#define __ASM_APIC_H
#include linux/pm.h
+#include linux/delay.h
#include asm/fixmap.h
#include asm/apicdef.h
#include asm/processor.h
@@ -66,10 +67,24 @@ static __inline fastcall unsigned long n
static __inline__ void apic_wait_icr_idle(void)
{
- while ( apic_read( APIC_ICR ) APIC_ICR_BUSY )
+ while (apic_read(APIC_ICR) APIC_ICR_BUSY)
cpu_relax();
}
+static __inline__ unsigned long safe_apic_wait_icr_idle(void)
+{
+ unsigned long send_status;
+ int timeout;
+
+ timeout = 0;
+ do {
+ udelay(100);
+ send_status = apic_read(APIC_ICR) APIC_ICR_BUSY;
+ } while (send_status (timeout++ 1000));
+
+ return send_status;
+}
+
int get_physical_broadcast(void);
#ifdef CONFIG_X86_GOOD_APIC
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH 2/10] safe_apic_wait_icr_idle - x86_64
apic_wait_icr_idle looks like this:
static __inline__ void apic_wait_icr_idle(void)
{
while (apic_read(APIC_ICR) APIC_ICR_BUSY)
cpu_relax();
}
The busy loop in this function would not be problematic if the
corresponding status bit in the ICR were always updated, but that does
not seem to be the case under certain crash scenarios. Kdump uses an IPI
to stop the other CPUs in the event of a crash, but when any of the
other CPUs are locked-up inside the NMI handler the CPU that sends the
IPI will end up looping forever in the ICR check, effectively
hard-locking the whole system.
Quoting from Intel's MultiProcessor Specification (Version 1.4), B-3:
A local APIC unit indicates successful dispatch of an IPI by
resetting the Delivery Status bit in the Interrupt Command
Register (ICR). The operating system polls the delivery status
bit after sending an INIT or STARTUP IPI until the command has
been dispatched.
A period of 20 microseconds should be sufficient for IPI dispatch
to complete under normal operating conditions. If the IPI is not
successfully dispatched, the operating system can abort the
command. Alternatively, the operating system can retry the IPI by
writing the lower 32-bit double word of the ICR. This “time-out”
mechanism can be implemented through an external interrupt, if
interrupts are enabled on the processor, or through execution of
an instruction or time-stamp counter spin loop.
Intel's documentation suggests the implementation of a time-out
mechanism, which, by the way, is already being open-coded in some parts
of the kernel that tinker with ICR.
Create a apic_wait_icr_idle replacement that implements the time-out
mechanism and that can be used to solve the aforementioned problem.
Signed-off-by: Fernando Luis Vazquez Cao [EMAIL PROTECTED]
---
diff -urNp linux-2.6.21-rc7-orig/include/asm-x86_64/apic.h
linux-2.6.21-rc7/include/asm-x86_64/apic.h
--- linux-2.6.21-rc7-orig/include/asm-x86_64/apic.h 2007-04-18
13:47:06.0 +0900
+++ linux-2.6.21-rc7/include/asm-x86_64/apic.h 2007-04-18 15:24:30.0
+0900
@@ -2,6 +2,7 @@
#define __ASM_APIC_H
#include linux/pm.h
+#include linux/delay.h
#include asm/fixmap.h
#include asm/apicdef.h
#include asm/system.h
@@ -49,10 +50,24 @@ static __inline unsigned int apic_read(u
static __inline__ void apic_wait_icr_idle(void)
{
- while (apic_read( APIC_ICR ) APIC_ICR_BUSY)
+ while (apic_read(APIC_ICR) APIC_ICR_BUSY)
cpu_relax();
}
+static __inline__ unsigned int safe_apic_wait_icr_idle(void)
+{
+ unsigned int send_status;
+ int timeout;
+
+ timeout = 0;
+ do {
+ udelay(100);
+ send_status = apic_read(APIC_ICR) APIC_ICR_BUSY;
+ } while (send_status (timeout++ 1000));
+
+ return send_status;
+}
+
static inline void ack_APIC_irq(void)
{
/*
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
