* Wireless IBSS on Linux-2.6.34 broken by commit 3bbb9ec946428b96657126768f65487a48dd090c
@ 2010-05-23 4:58 Jeff Chua
2010-05-23 6:21 ` Arjan van de Ven
2010-05-23 9:49 ` Johannes Berg
0 siblings, 2 replies; 9+ messages in thread
From: Jeff Chua @ 2010-05-23 4:58 UTC (permalink / raw
To: Arjan van de Ven, johnstul, Ingo Molnar, Andrew Morton,
Thomas Gleixner, Intel Linux Wireless, John W. Linville,
Frans Pop, Johannes Berg, Reinette Chatre, Linus Torvalds, lkml
Wireless IBSS on Linux-2.6.34 is broken. Reverting commit
3bbb9ec946428b96657126768f65487a48dd090c makes it work again. This was
tested by bisecting the kernel.
I'm using 6200 AGN ...
cfg80211: Calling CRDA to update world regulatory domain
iwlagn: Intel(R) Wireless WiFi Link AGN driver for Linux, in-tree:
iwlagn: Copyright(c) 2003-2010 Intel Corporation
iwlagn 0000:02:00.0: PCI INT A -> GSI 16 (level, low) -> IRQ 16
iwlagn 0000:02:00.0: setting latency timer to 64
iwlagn 0000:02:00.0: Detected Intel(R) Centrino(R) Advanced-N 6200 AGN, REV=0x74
iwlagn 0000:02:00.0: Tunable channels: 13 802.11bg, 24 802.11a channels
iwlagn 0000:02:00.0: irq 46 for MSI/MSI-X
iwlagn 0000:02:00.0: loaded firmware version 9.193.4.1 build 19710
So, where should the fix be?
Here's the commit ...
3bbb9ec946428b96657126768f65487a48dd090c is the first bad commit
commit 3bbb9ec946428b96657126768f65487a48dd090c
Author: Arjan van de Ven <arjan@linux.intel.com>
Date: Thu Mar 11 14:04:36 2010 -0800
timers: Introduce the concept of timer slack for legacy timers
While HR timers have had the concept of timer slack for quite some time
now, the legacy timers lacked this concept, and had to make do with
round_jiffies() and friends.
Timer slack is important for power management; grouping timers reduces the
number of wakeups which in turn reduces power consumption.
This patch introduces timer slack to the legacy timers using the following
pieces:
* A slack field in the timer struct
* An api (set_timer_slack) that callers can use to set explicit timer slack
* A default slack of 0.4% of the requested delay for callers that do not set
any explicit slack
* Rounding code that is part of mod_timer() that tries to
group timers around jiffies values every 'power of two'
(so quick timers will group around every 2, but longer timers
will group around every 4, 8, 16, 32 etc)
Signed-off-by: Arjan van de Ven <arjan@linux.intel.com>
Cc: johnstul@us.ibm.com
Cc: Ingo Molnar <mingo@elte.hu>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Thanks,
Jeff.
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: Wireless IBSS on Linux-2.6.34 broken by commit 3bbb9ec946428b96657126768f65487a48dd090c
2010-05-23 4:58 Wireless IBSS on Linux-2.6.34 broken by commit 3bbb9ec946428b96657126768f65487a48dd090c Jeff Chua
@ 2010-05-23 6:21 ` Arjan van de Ven
2010-05-23 9:49 ` Johannes Berg
1 sibling, 0 replies; 9+ messages in thread
From: Arjan van de Ven @ 2010-05-23 6:21 UTC (permalink / raw
To: Jeff Chua
Cc: johnstul, Ingo Molnar, Andrew Morton, Thomas Gleixner,
Intel Linux Wireless, John W. Linville, Frans Pop, Johannes Berg,
Reinette Chatre, Linus Torvalds, lkml
On 5/22/2010 21:58, Jeff Chua wrote:
> Wireless IBSS on Linux-2.6.34 is broken. Reverting commit
> 3bbb9ec946428b96657126768f65487a48dd090c makes it work again. This was
> tested by bisecting the kernel.
>
> I'm using 6200 AGN ...
>
> cfg80211: Calling CRDA to update world regulatory domain
> iwlagn: Intel(R) Wireless WiFi Link AGN driver for Linux, in-tree:
> iwlagn: Copyright(c) 2003-2010 Intel Corporation
> iwlagn 0000:02:00.0: PCI INT A -> GSI 16 (level, low) -> IRQ 16
> iwlagn 0000:02:00.0: setting latency timer to 64
> iwlagn 0000:02:00.0: Detected Intel(R) Centrino(R) Advanced-N 6200 AGN, REV=0x74
> iwlagn 0000:02:00.0: Tunable channels: 13 802.11bg, 24 802.11a channels
> iwlagn 0000:02:00.0: irq 46 for MSI/MSI-X
> iwlagn 0000:02:00.0: loaded firmware version 9.193.4.1 build 19710
can you get us whole dmesg of two cases?
(just to know if there's something in surrounding messages of interest)
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: Wireless IBSS on Linux-2.6.34 broken by commit 3bbb9ec946428b96657126768f65487a48dd090c
2010-05-23 4:58 Wireless IBSS on Linux-2.6.34 broken by commit 3bbb9ec946428b96657126768f65487a48dd090c Jeff Chua
2010-05-23 6:21 ` Arjan van de Ven
@ 2010-05-23 9:49 ` Johannes Berg
2010-05-23 13:19 ` Jeff Chua
1 sibling, 1 reply; 9+ messages in thread
From: Johannes Berg @ 2010-05-23 9:49 UTC (permalink / raw
To: Jeff Chua
Cc: Arjan van de Ven, johnstul@us.ibm.com, Ingo Molnar, Andrew Morton,
Thomas Gleixner, Intel Linux Wireless, John W. Linville,
Frans Pop, Chatre, Reinette, Linus Torvalds, lkml
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset="utf-8", Size: 2228 bytes --]
On Sun, 2010-05-23 at 05:58 +0100, Jeff Chua wrote:> Wireless IBSS on Linux-2.6.34 is broken. Reverting commit> 3bbb9ec946428b96657126768f65487a48dd090c makes it work again. This was> tested by bisecting the kernel.> > I'm using 6200 AGN ...> > cfg80211: Calling CRDA to update world regulatory domain> iwlagn: Intel(R) Wireless WiFi Link AGN driver for Linux, in-tree:> iwlagn: Copyright(c) 2003-2010 Intel Corporation> iwlagn 0000:02:00.0: PCI INT A -> GSI 16 (level, low) -> IRQ 16> iwlagn 0000:02:00.0: setting latency timer to 64> iwlagn 0000:02:00.0: Detected Intel(R) Centrino(R) Advanced-N 6200 AGN, REV=0x74> iwlagn 0000:02:00.0: Tunable channels: 13 802.11bg, 24 802.11a channels> iwlagn 0000:02:00.0: irq 46 for MSI/MSI-X> iwlagn 0000:02:00.0: loaded firmware version 9.193.4.1 build 19710> > > > So, where should the fix be?> > > Here's the commit ...> > 3bbb9ec946428b96657126768f65487a48dd090c is the first bad commit> commit 3bbb9ec946428b96657126768f65487a48dd090c> Author: Arjan van de Ven <arjan@linux.intel.com>> Date: Thu Mar 11 14:04:36 2010 -0800> > timers: Introduce the concept of timer slack for legacy timers
This is odd -- as Arjan said more logging would be good, if you canenable mac80211 IBSS debug (though in one case we had recently that_fixed_ the issue unfortunately) -- it's under the mac80211 debug menuKconfig.
FWIW, I don't know about anything in mac80211 that would be timersensitive?
johannes
---------------------------------------------------------------------Intel GmbHDornacher Strasse 185622 Feldkirchen/Muenchen GermanySitz der Gesellschaft: Feldkirchen bei MuenchenGeschaeftsfuehrer: Douglas Lusk, Peter Gleissner, Hannes SchwadererRegistergericht: Muenchen HRB 47456 Ust.-IdNr.VAT Registration No.: DE129385895Citibank Frankfurt (BLZ 502 109 00) 600119052
This e-mail and any attachments may contain confidential material forthe sole use of the intended recipient(s). Any review or distributionby others is strictly prohibited. If you are not the intendedrecipient, please contact the sender and delete all copies.ÿôèº{.nÇ+‰·Ÿ®‰†+%ŠËÿ±éݶ\x17¥Šwÿº{.nÇ+‰·¥Š{±þG«éÿŠ{ayº\x1dʇڙë,j\a¢f£¢·hšïêÿ‘êçz_è®\x03(階ŠÝ¢j"ú\x1a¶^[m§ÿÿ¾\a«þG«éÿ¢¸?™¨èÚ&£ø§~á¶iO•æ¬z·švØ^\x14\x04\x1a¶^[m§ÿÿÃ\fÿ¶ìÿ¢¸?–I¥
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: Wireless IBSS on Linux-2.6.34 broken by commit 3bbb9ec946428b96657126768f65487a48dd090c
2010-05-23 9:49 ` Johannes Berg
@ 2010-05-23 13:19 ` Jeff Chua
0 siblings, 0 replies; 9+ messages in thread
From: Jeff Chua @ 2010-05-23 13:19 UTC (permalink / raw
To: Johannes Berg
Cc: Arjan van de Ven, johnstul@us.ibm.com, Ingo Molnar, Andrew Morton,
Thomas Gleixner, Intel Linux Wireless, John W. Linville,
Frans Pop, Chatre, Reinette, Linus Torvalds, lkml
> On Sun, 2010-05-23 at 05:58 +0100, Jeff Chua wrote:
> Wireless IBSS on Linux-2.6.34 is broken. Reverting commit
> 3bbb9ec946428b96657126768f65487a48dd090c makes it work again. This was
> tested by bisecting the kernel.
On Sun, May 23, 2010 at 2:21 PM, Arjan van de Ven <arjan@linux.intel.com> wrote:
> can you get us whole dmesg of two cases?
> (just to know if there's something in surrounding messages of interest)
On Sun, May 23, 2010 at 5:49 PM, Johannes Berg <johannes.berg@intel.com> wrote:
> This is odd -- as Arjan said more logging would be good, if you can
> enable mac80211 IBSS debug (though in one case we had recently that
> _fixed_ the issue unfortunately) -- it's under the mac80211 debug menu
> Kconfig.
>
> FWIW, I don't know about anything in mac80211 that would be timer
> sensitive?
Arjan, Johannes,
Here's the dmesg I see. The only difference between the non-working
and working ones are the last 4 lines below with the bad commit
reverted. With the non-working one, I don't see the last 4 lines no
matter how long I waited.
And I'm guessing it's something to do with mod_timer() that modified
the timer's timeout. mod_timer() is used by by both iwlwifi amd
mac80211. I'll get the full mac80211 debug soon.
2010-05-22T11:42:55.129641+08:00 boston kernel: cfg80211: Calling CRDA
to update world regulatory domain
2010-05-22T11:42:55.162713+08:00 boston kernel: iwlagn: Intel(R)
Wireless WiFi Link AGN driver for Linux, in-tree:
2010-05-22T11:42:55.162782+08:00 boston kernel: iwlagn: Copyright(c)
2003-2010 Intel Corporation
2010-05-22T11:42:55.162785+08:00 boston kernel: iwlagn 0000:02:00.0:
PCI INT A -> GSI 16 (level, low) -> IRQ 16
2010-05-22T11:42:55.162788+08:00 boston kernel: iwlagn 0000:02:00.0:
setting latency timer to 64
2010-05-22T11:42:55.162814+08:00 boston kernel: iwlagn 0000:02:00.0:
Detected Intel Wireless WiFi Link 6000 Series 2x2 AGN REV=0x74
2010-05-22T11:42:55.182139+08:00 boston kernel: iwlagn 0000:02:00.0:
Tunable channels: 13 802.11bg, 24 802.11a channels
2010-05-22T11:42:55.182164+08:00 boston kernel: iwlagn 0000:02:00.0:
irq 44 for MSI/MSI-X
2010-05-22T11:42:55.182169+08:00 boston kernel: iwlagn 0000:02:00.0:
firmware: requesting iwlwifi-6000-4.ucode
2010-05-22T11:42:55.198654+08:00 boston kernel: iwlagn 0000:02:00.0:
loaded firmware version 9.193.4.1
2010-05-22T11:42:55.201637+08:00 boston kernel: phy0: Selected rate
control algorithm 'iwl-agn-rs'
2010-05-22T11:42:57.538644+08:00 boston kernel: wlan0: Trigger new
scan to find an IBSS to join
(*** These shows up for the working kernel with bad commit removed ***)
2010-05-22T11:43:04.797649+08:00 boston kernel: wlan0: Trigger new
scan to find an IBSS to join
2010-05-22T11:43:08.149646+08:00 boston kernel: wlan0: Creating new
IBSS network, BSSID 46:9a:67:c7:5d:30
2010-05-22T11:43:08.207216+08:00 boston kernel: iwlagn 0000:02:00.0:
Unable to find TIM Element in beacon
2010-05-22T11:43:08.207232+08:00 boston kernel: iwlagn 0000:02:00.0:
Unable to find TIM Element in beacon
Thanks,
Jeff.
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: Wireless IBSS on Linux-2.6.34 broken by commit 3bbb9ec946428b96657126768f65487a48dd090c
@ 2010-05-23 17:07 Jeff Chua
0 siblings, 0 replies; 9+ messages in thread
From: Jeff Chua @ 2010-05-23 17:07 UTC (permalink / raw
To: Johannes Berg, Arjan van de Ven, johnstul@us.ibm.com, Ingo Molnar,
Andrew Morton, Thomas Gleixner, Intel Linux Wireless,
John W. Linville, Frans Pop, , Chatre, Reinette, Linus Torvalds,
lkml
On Sun, May 23, 2010 at 5:49 PM, Johannes Berg <johannes.berg@intel.com>
wrote:
> On Sun, 2010-05-23 at 05:58 +0100, Jeff Chua wrote:
>> Wireless IBSS on Linux-2.6.34 is broken. Reverting commit
>> 3bbb9ec946428b96657126768f65487a48dd090c makes it work again. This was
>> tested by bisecting the kernel.
Arjan, Johannes,
Here's the demsg with debug enabled ...
1) with bad commit reverted ...
2010-05-23T21:24:23.656671+08:00 boston kernel: cfg80211: Calling CRDA to
update world regulatory domain
2010-05-23T21:24:23.720182+08:00 boston kernel: iwlagn: Intel(R) Wireless
WiFi Link AGN driver for Linux, in-tree:
2010-05-23T21:24:23.720211+08:00 boston kernel: iwlagn: Copyright(c)
2003-2010 Intel Corporation
2010-05-23T21:24:23.720215+08:00 boston kernel: iwlagn 0000:02:00.0: PCI
INT A -> GSI 16 (level, low) -> IRQ 16
2010-05-23T21:24:23.720218+08:00 boston kernel: iwlagn 0000:02:00.0:
setting latency timer to 64
2010-05-23T21:24:23.720222+08:00 boston kernel: iwlagn 0000:02:00.0:
Detected Intel(R) Centrino(R) Advanced-N 6200 AGN, REV=0x74
2010-05-23T21:24:23.732171+08:00 boston kernel: iwlagn 0000:02:00.0:
Tunable channels: 13 802.11bg, 24 802.11a channels
2010-05-23T21:24:23.732181+08:00 boston kernel: iwlagn 0000:02:00.0: irq
46 for MSI/MSI-X
2010-05-23T21:24:23.820182+08:00 boston kernel: iwlagn 0000:02:00.0:
loaded firmware version 9.193.4.1 build 19710
2010-05-23T21:24:23.848169+08:00 boston kernel: phy0: Selected rate
control algorithm 'iwl-agn-rs'
2010-05-23T21:24:24.335667+08:00 boston kernel: phy0: device now idle
2010-05-23T21:24:24.340162+08:00 boston kernel: phy0: device no longer
idle - in use
2010-05-23T21:24:24.384668+08:00 boston kernel: wlan0: sta_find_ibss
(active_ibss=0)
2010-05-23T21:24:24.384678+08:00 boston kernel: did not try to join ibss
2010-05-23T21:24:24.384682+08:00 boston kernel: wlan0: Trigger new scan to
find an IBSS to join
2010-05-23T21:24:24.400170+08:00 boston kernel: wlan0: sta_find_ibss
(active_ibss=0)
2010-05-23T21:24:24.400187+08:00 boston kernel: did not try to join ibss
2010-05-23T21:24:26.687669+08:00 boston kernel: wlan0: sta_find_ibss
(active_ibss=0)
2010-05-23T21:24:26.687691+08:00 boston kernel: did not try to join ibss
2010-05-23T21:24:26.687695+08:00 boston kernel: wlan0: Trigger new scan to
find an IBSS to join
2010-05-23T21:24:26.707658+08:00 boston kernel: wlan0: sta_find_ibss
(active_ibss=0)
2010-05-23T21:24:26.707666+08:00 boston kernel: did not try to join ibss
2010-05-23T21:24:28.703660+08:00 boston kernel: wlan0: sta_find_ibss
(active_ibss=0)
2010-05-23T21:24:28.703668+08:00 boston kernel: did not try to join ibss
2010-05-23T21:24:30.703668+08:00 boston kernel: wlan0: sta_find_ibss
(active_ibss=0)
2010-05-23T21:24:30.703687+08:00 boston kernel: did not try to join ibss
2010-05-23T21:24:30.703691+08:00 boston kernel: wlan0: Trigger new scan to
find an IBSS to join
2010-05-23T21:24:30.723659+08:00 boston kernel: wlan0: sta_find_ibss
(active_ibss=0)
2010-05-23T21:24:30.723666+08:00 boston kernel: did not try to join ibss
2010-05-23T21:24:32.703664+08:00 boston kernel: wlan0: sta_find_ibss
(active_ibss=0)
2010-05-23T21:24:32.703675+08:00 boston kernel: did not try to join ibss
2010-05-23T21:24:32.703679+08:00 boston kernel: wlan0: Creating new IBSS
network, BSSID 06:a5:c6:42:20:80
2010-05-23T21:24:32.712163+08:00 boston kernel: iwlagn 0000:02:00.0:
Unable to find TIM Element in beacon
2010-05-23T21:24:32.712172+08:00 boston kernel: iwlagn 0000:02:00.0:
Unable to find TIM Element in beacon
2) Still with bad-commit in the kernel
2010-05-23T21:47:28.498041+08:00 boston kernel: cfg80211: Calling CRDA to
update world regulatory domain
2010-05-23T21:47:28.535052+08:00 boston kernel: iwlagn: Intel(R) Wireless
WiFi Link AGN driver for Linux, in-tree:
2010-05-23T21:47:28.535078+08:00 boston kernel: iwlagn: Copyright(c)
2003-2010 Intel Corporation
2010-05-23T21:47:28.535082+08:00 boston kernel: iwlagn 0000:02:00.0: PCI
INT A -> GSI 16 (level, low) -> IRQ 16
2010-05-23T21:47:28.535085+08:00 boston kernel: iwlagn 0000:02:00.0:
setting latency timer to 64
2010-05-23T21:47:28.535089+08:00 boston kernel: iwlagn 0000:02:00.0:
Detected Intel(R) Centrino(R) Advanced-N 6200 AGN, REV=0x74
2010-05-23T21:47:28.551036+08:00 boston kernel: iwlagn 0000:02:00.0:
Tunable channels: 13 802.11bg, 24 802.11a channels
2010-05-23T21:47:28.551048+08:00 boston kernel: iwlagn 0000:02:00.0: irq
46 for MSI/MSI-X
2010-05-23T21:47:28.571054+08:00 boston kernel: iwlagn 0000:02:00.0:
loaded firmware version 9.193.4.1 build 19710
2010-05-23T21:47:28.574286+08:00 boston kernel: phy0: Selected rate
control algorithm 'iwl-agn-rs'
2010-05-23T21:47:29.406045+08:00 boston kernel: phy0: device now idle
2010-05-23T21:47:29.413564+08:00 boston kernel: phy0: device no longer
idle - in use
2010-05-23T21:47:29.486081+08:00 boston kernel: wlan0: sta_find_ibss
(active_ibss=0)
2010-05-23T21:47:29.486099+08:00 boston kernel: did not try to join ibss
2010-05-23T21:47:29.486103+08:00 boston kernel: wlan0: Trigger new scan to
find an IBSS to join
So, the difference are these messages when it's working ...
wlan0: sta_find_ibss (active_ibss=0)
did not try to join ibss
wlan0: Trigger new scan to find an IBSS to join
wlan0: sta_find_ibss (active_ibss=0)
did not try to join ibss
wlan0: sta_find_ibss (active_ibss=0)
did not try to join ibss
wlan0: Trigger new scan to find an IBSS to join
wlan0: sta_find_ibss (active_ibss=0)
did not try to join ibss
wlan0: sta_find_ibss (active_ibss=0)
did not try to join ibss
wlan0: sta_find_ibss (active_ibss=0)
did not try to join ibss
wlan0: Trigger new scan to find an IBSS to join
wlan0: sta_find_ibss (active_ibss=0)
did not try to join ibss
wlan0: sta_find_ibss (active_ibss=0)
did not try to join ibss
wlan0: Creating new IBSS network, BSSID 62:c8:0e:ff:7e:69
iwlagn 0000:02:00.0: Unable to find TIM Element in beacon
iwlagn 0000:02:00.0: Unable to find TIM Element in beacon
I modified one line in the bad commit to set the default "timer->slack =
0" and that fixed the problem of IBSS not working. Below is the patch.
I don't know how this impacts others, but it seems better to default to
the previous behavior rather than introducing "slacks" that other
subsystems are not just ready to handle.
Thanks,
Jeff
diff --git a/kernel/timer.c b/kernel/timer.c
--- a/kernel/timer.c
+++ b/kernel/timer.c
@@ -549,7 +567,7 @@ static void __init_timer(struct timer_list *timer,
{
timer->entry.next = NULL;
timer->base = __raw_get_cpu_var(tvec_bases);
- timer->slack = -1;
+ timer->slack = 0;
#ifdef CONFIG_TIMER_STATS
timer->start_site = NULL;
timer->start_pid = -1;
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: Wireless IBSS on Linux-2.6.34 broken by commit 3bbb9ec946428b96657126768f65487a48dd090c
@ 2010-05-23 23:16 Jeff Chua
2010-05-24 14:57 ` Linus Torvalds
0 siblings, 1 reply; 9+ messages in thread
From: Jeff Chua @ 2010-05-23 23:16 UTC (permalink / raw
To: Johannes Berg, Arjan van de Ven, johnstul@us.ibm.com, Ingo Molnar,
Andrew Morton, Thomas Gleixner, Intel Linux Wireless,
John W. Linville, Frans Pop, Chatre, Reinette, Linus Torvalds,
lkml
Mon, May 24, 2010 at 5:25 AM, Arjan van de Ven <arjan@linux.intel.com>
wrote:
>>> can you try, instead do the following, in the apply_slack() function:
>>>
>>> just before the return expires_limit; do
>>>
>>> if (expires_limit< expires)
>>> # # #expires_limit = expires;
>>
>> This doesn't work.
>>
>>
>>> if that does not fix it, a second thought:
>>> add, after the first if (timer->slack< 0)
>>>
>>> if (timer->slack< 0&& expires< jiffies)
>>> # # # #expires_limit = expires;
>>
>> This works.
>
> hmm ok so the wireless stack sets a timer way back in the past
> ok that's technically legal.
>
> how about
>
> expires_limit = expires;
>
> if (timer->slack > -1)
> expires_limit = expires + timer->slack;
> else if (time_after(expires, jiffies))
> expires_limit = expires + (expires - jiffies)/256;
>
> can you test such a thing and send a patch?
> (it has my Acked-By: either way)
Arjan,
Tested and working. Here's the patch. Thanks for the fix!
Jeff
--- a/kernel/timer.c.org 2010-05-24 07:09:04.000000000 +0800
+++ a/kernel/timer.c 2010-05-24 07:05:22.000000000 +0800
@@ -750,9 +750,11 @@
unsigned long expires_limit, mask;
int bit;
- expires_limit = expires + timer->slack;
+ expires_limit = expires;
- if (timer->slack < 0) /* auto slack: use 0.4% */
+ if (timer->slack > -1)
+ expires_limit = expires + timer->slack;
+ else if (time_after(expires, jiffies)) /* auto slack: use 0.4% */
expires_limit = expires + (expires - jiffies)/256;
mask = expires ^ expires_limit;
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: Wireless IBSS on Linux-2.6.34 broken by commit 3bbb9ec946428b96657126768f65487a48dd090c
2010-05-23 23:16 Jeff Chua
@ 2010-05-24 14:57 ` Linus Torvalds
2010-05-25 18:37 ` Thomas Gleixner
0 siblings, 1 reply; 9+ messages in thread
From: Linus Torvalds @ 2010-05-24 14:57 UTC (permalink / raw
To: Jeff Chua
Cc: Johannes Berg, Arjan van de Ven, johnstul@us.ibm.com, Ingo Molnar,
Andrew Morton, Thomas Gleixner, Intel Linux Wireless,
John W. Linville, Frans Pop, Chatre, Reinette, lkml
On Mon, 24 May 2010, Jeff Chua wrote:
>
> - expires_limit = expires + timer->slack;
> + expires_limit = expires;
>
> - if (timer->slack < 0) /* auto slack: use 0.4% */
> + if (timer->slack > -1)
> + expires_limit = expires + timer->slack;
> + else if (time_after(expires, jiffies)) /* auto slack: use 0.4% */
> expires_limit = expires + (expires - jiffies)/256;
Please don't reload 'jiffies' twice. It's volatile, and the compiler will
do a crap job of it.
Also, '0' is a normal number, but '-1' is a rather odd value. Please just
test for non-negative by doing ">= 0" rather than "> -1"
Linus
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: Wireless IBSS on Linux-2.6.34 broken by commit 3bbb9ec946428b96657126768f65487a48dd090c
@ 2010-05-24 22:48 Jeff Chua
0 siblings, 0 replies; 9+ messages in thread
From: Jeff Chua @ 2010-05-24 22:48 UTC (permalink / raw
To: Linus Torvalds
Cc: Johannes Berg, Arjan van de Ven, johnstul@us.ibm.com, Ingo Molnar,
Andrew Morton, Thomas Gleixner, Intel Linux Wireless,
John W. Linville, Frans Pop, Chatre, Reinette, lkml
On Mon, May 24, 2010 at 10:57 PM, Linus Torvalds <torvalds@linux-foundation.org> wrote:>
> On Mon, 24 May 2010, Jeff Chua wrote:
>> + if (timer->slack > -1)
>> + else if (time_after(expires, jiffies)) /* auto slack: use 0.4% */
>> expires_limit = expires + (expires - jiffies)/256;
>
> Please don't reload 'jiffies' twice. It's volatile, and the compiler will
> do a crap job of it.
Linus,
Sorry, didn't know what a jiffies was before, so didn't know about the
crap it caused. Now I learned something new.
> Also, '0' is a normal number, but '-1' is a rather odd value. Please just
> test for non-negative by doing ">= 0" rather than "> -1"
Fixed.
Below is the patch to fix jiffies and checking for non-negative.
Thanks,
Jeff
--- a/kernel/timer.c.org 2010-05-25 06:05:46.000000000 +0800
+++ a/kernel/timer.c 2010-05-25 06:17:44.000000000 +0800
@@ -748,14 +748,15 @@
unsigned long apply_slack(struct timer_list *timer, unsigned long expires)
{
unsigned long expires_limit, mask;
+ unsigned long now = jiffies;
int bit;
expires_limit = expires;
- if (timer->slack > -1)
+ if (timer->slack >= 0)
expires_limit = expires + timer->slack;
- else if (time_after(expires, jiffies)) /* auto slack: use 0.4% */
- expires_limit = expires + (expires - jiffies)/256;
+ else if (time_after(expires, now)) /* auto slack: use 0.4% */
+ expires_limit = expires + (expires - now)/256;
mask = expires ^ expires_limit;
if (mask == 0)
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: Wireless IBSS on Linux-2.6.34 broken by commit 3bbb9ec946428b96657126768f65487a48dd090c
2010-05-24 14:57 ` Linus Torvalds
@ 2010-05-25 18:37 ` Thomas Gleixner
0 siblings, 0 replies; 9+ messages in thread
From: Thomas Gleixner @ 2010-05-25 18:37 UTC (permalink / raw
To: Linus Torvalds
Cc: Jeff Chua, Johannes Berg, Arjan van de Ven, johnstul@us.ibm.com,
Ingo Molnar, Andrew Morton, Intel Linux Wireless,
John W. Linville, Frans Pop, Chatre, Reinette, lkml
On Mon, 24 May 2010, Linus Torvalds wrote:
>
> On Mon, 24 May 2010, Jeff Chua wrote:
> >
> > - expires_limit = expires + timer->slack;
> > + expires_limit = expires;
> >
> > - if (timer->slack < 0) /* auto slack: use 0.4% */
> > + if (timer->slack > -1)
> > + expires_limit = expires + timer->slack;
> > + else if (time_after(expires, jiffies)) /* auto slack: use 0.4% */
> > expires_limit = expires + (expires - jiffies)/256;
>
> Please don't reload 'jiffies' twice. It's volatile, and the compiler will
> do a crap job of it.
Grr, it's even worse. If jiffies increment between the time_after()
check and (expires - jiffies) we might run into the same problem as
before. Sigh, did not notice when I picked it up. ENOTENOUGHCOFFEE.
> Also, '0' is a normal number, but '-1' is a rather odd value. Please just
> test for non-negative by doing ">= 0" rather than "> -1"
Will fix, thanks,
tglx
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2010-05-25 18:39 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2010-05-23 4:58 Wireless IBSS on Linux-2.6.34 broken by commit 3bbb9ec946428b96657126768f65487a48dd090c Jeff Chua
2010-05-23 6:21 ` Arjan van de Ven
2010-05-23 9:49 ` Johannes Berg
2010-05-23 13:19 ` Jeff Chua
-- strict thread matches above, loose matches on Subject: below --
2010-05-23 17:07 Jeff Chua
2010-05-23 23:16 Jeff Chua
2010-05-24 14:57 ` Linus Torvalds
2010-05-25 18:37 ` Thomas Gleixner
2010-05-24 22:48 Jeff Chua
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).