* Re: [PATCH 3/4] [XFRM]: Kill some bloat
From: Ilpo Järvinen @ 2008-01-07 8:21 UTC (permalink / raw)
To: Herbert Xu
Cc: David Miller, Netdev, Arnaldo Carvalho de Melo, paul.moore,
latten
In-Reply-To: <E1JBJOR-0000jO-00@gondolin.me.apana.org.au>
On Sun, 6 Jan 2008, Herbert Xu wrote:
> is definitely not a fast path. If a function ends up being called
> just once the compiler will most likely inline it anyway, making the
> use of the keyword inline redundant.
Unexpected enough, even this logic seems to fail in a way with my gcc, I'm
yet to study it closer but it seems to me that e.g., uninlining only once
called tcp_fastretrans_alert is worth of at least 100 bytes (note that
it's not inlined by us, gcc did it all by itself)! Otherwise I'd fail to
understand why I got -270 bytes from uninlining tcp_moderate_cwnd which is
only 57 bytes as unlined with three call sites.
net/ipv4/tcp_input.c:
tcp_undo_cwr | -48
tcp_try_undo_recovery | -55
tcp_ack | -2941
3 functions changed, 3044 bytes removed, diff: -3044
net/ipv4/tcp_input.c:
tcp_moderate_cwnd | +57
tcp_fastretrans_alert | +2717
2 functions changed, 2774 bytes added, diff: +2774
net/ipv4/tcp_input.o:
5 functions changed, 2774 bytes added, 3044 bytes removed, diff: -270
I'll probably force uninlining of it without tcp_moderate_cwnd noise and
try a number of gcc versions.
--
i.
^ permalink raw reply
* Re: NAPI poll behavior in various Intel drivers
From: Jarek Poplawski @ 2008-01-07 8:24 UTC (permalink / raw)
To: David Miller; +Cc: netdev, auke-jan.h.kok
In-Reply-To: <20080104.034036.160194618.davem@davemloft.net>
On 04-01-2008 12:40, David Miller wrote:
...
> That "tx_cleaned" thing clouds the logic in all of these driver's
> poll routines.
>
> The one necessary precondition is that when work_done < budget
> we exit polling and return a value less than budget.
>
> If the ->poll() returns a value less than budget, net_rx_action()
> assumes that the device has been removed from the poll() list.
>
> /* Drivers must not modify the NAPI state if they
> * consume the entire weight. In such cases this code
> * still "owns" the NAPI instance and therefore can
> * move the instance around on the list at-will.
> */
> if (unlikely(work == weight))
> list_move_tail(&n->poll_list, list);
>
Probably it's because of this clouded logic, but IMHO: "Drivers must
not modify the NAPI state if..." doesn't imply: drivers must modify
the NAPI state otherwise. (But it seems I must have missed the real
reason for this quote here.)
Regards,
Jarek P.
^ permalink raw reply
* Re: Module for simulating network default
From: Paul Rolland @ 2008-01-07 7:45 UTC (permalink / raw)
To: David Miller; +Cc: linux-kernel, netdev
In-Reply-To: <20080106.234207.113177790.davem@davemloft.net>
Hello,
On Sun, 06 Jan 2008 23:42:07 -0800 (PST)
David Miller <davem@davemloft.net> wrote:
> From: Paul Rolland <rol@as2917.net>
> Date: Mon, 7 Jan 2008 08:37:04 +0100
>
> > I remember reading some time ago about a network driver to "simulate"
> > network default, for example packet loss...
> > Unfortunately, I can't find the post, neither in my mailbox nor in
> > archives...
> >
> > Does anyone has an URL that you could send me ?
>
> You want the "netem" packet scheduler, and you'll get better
> answers to networking questions on the networking list
> netdev@vger.kernel.org
Thanks David, I'll have a look at this.
For netdev, as I'm not subscribed, please don't forget to copy me on reply ;)
Regards,
Paul
^ permalink raw reply
* Re: Module for simulating network default
From: David Miller @ 2008-01-07 7:42 UTC (permalink / raw)
To: rol; +Cc: linux-kernel, netdev
In-Reply-To: <20080107083704.6527306e@tux.DEF.witbe.net>
From: Paul Rolland <rol@as2917.net>
Date: Mon, 7 Jan 2008 08:37:04 +0100
> I remember reading some time ago about a network driver to "simulate"
> network default, for example packet loss...
> Unfortunately, I can't find the post, neither in my mailbox nor in
> archives...
>
> Does anyone has an URL that you could send me ?
You want the "netem" packet scheduler, and you'll get better
answers to networking questions on the networking list
netdev@vger.kernel.org
^ permalink raw reply
* Re: [PATCH 3/4] [XFRM]: Kill some bloat
From: Herbert Xu @ 2008-01-07 7:32 UTC (permalink / raw)
To: Ilpo Järvinen
Cc: David Miller, Netdev, Arnaldo Carvalho de Melo, paul.moore,
latten
In-Reply-To: <Pine.LNX.4.64.0801070908001.17676@kivilampi-30.cs.helsinki.fi>
On Mon, Jan 07, 2008 at 09:13:22AM +0200, Ilpo Järvinen wrote:
>
> There still seems to be good candidates for inline in *.c files, in worst
> case I had +172 due to inline removed and ~60 are on +10-+90 range with
> my gcc, later gccs might do better but I definately would just blindly
> remove them all. Here's the other end of the list:
Right. If the inline helps the compiler perform code size optimisations
that it otherwise couldn't then it might be worthwhile.
So when in doubt, check the assembly :)
Cheers,
--
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
^ permalink raw reply
* Re: [PATCH 3/4] [XFRM]: Kill some bloat
From: Ilpo Järvinen @ 2008-01-07 7:13 UTC (permalink / raw)
To: David Miller
Cc: Herbert Xu, Netdev, Arnaldo Carvalho de Melo, paul.moore, latten
In-Reply-To: <20080105.231658.168081302.davem@davemloft.net>
On Sat, 5 Jan 2008, David Miller wrote:
> From: Herbert Xu <herbert@gondor.apana.org.au>
> Date: Sun, 06 Jan 2008 11:29:35 +1100
>
> > We should never use inline except when it's on the fast path and this
> > is definitely not a fast path. If a function ends up being called
> > just once the compiler will most likely inline it anyway, making the
> > use of the keyword inline redundant.
>
> Similarly I question just about any inline usage at all in *.c files
> these days.
>
> I even would discourage it's use for fast-path cases as well.
There still seems to be good candidates for inline in *.c files, in worst
case I had +172 due to inline removed and ~60 are on +10-+90 range with
my gcc, later gccs might do better but I definately would just blindly
remove them all. Here's the other end of the list:
+35 static inline hci_encrypt_change_evt net/bluetooth/hci_event.c
+36 static __inline__ tcp_in_window net/ipv4/tcp_minisocks.c
+38 static inline hci_conn_complete_evt net/bluetooth/hci_event.c
+38 static inline hci_conn_request_evt net/bluetooth/hci_event.c
+42 static inline gred_wred_mode net/sched/sch_gred.c
+45 static inline secpath_has_nontransport net/xfrm/xfrm_policy.c
+52 static inline bool port_match net/netfilter/xt_tcpudp.c
+67 static inline dn_check_idf net/decnet/dn_nsp_in.c
+90 static inline __ieee80211_queue_stopped net/mac80211/tx.c
+172 static inline sctp_chunk_length_valid net/sctp/sm_statefuns.c
--
i.
^ permalink raw reply
* Re: 2.6.24-rc6 oops in net_tx_action
From: linux @ 2008-01-07 6:43 UTC (permalink / raw)
To: linux, romieu; +Cc: jgarzik, linux-kernel, netdev, shemminger
In-Reply-To: <20080106235719.GA10588@electric-eye.fr.zoreil.com>
> linux@horizon.com <linux@horizon.com> :
>> Kernel is 2.6.24-rc6 + linuxpps patches, which are all to the serial
>> port driver.
>>
>> 2.6.23 was known stable. I haven't tested earlier 2.6.24 releases.
>> I think it happened once before; I got a black-screen lockup with
>> keyboard LEDs blinking, but that was with X running so I couldn't see a
>> console oops. But given that I installed 2.6.24-rc6 about 24 hours ago,
>> that's a disturbing pattern.
> It is probably this one:
>
> http://marc.info/?t=119782794000003&r=1&w=2
Thanks! I got the patch from
http://marc.info/?l=linux-netdev&m=119756785219214
(Which didn't make it into -rc7; please fix!)
and am recompiling now.
Actually, I grabbed the hardware mitigation followon patch while I was
at it. I notice that the comment explaining the format of CSR11 and
what 0x80F10000 means got lost; perhaps it would be nice to resurrect it?
0x80F10000
80000000 = Cycle size (timer control)
78000000 = TX timer in 16 * Cycle size
07000000 = No. pkts before Int. (0 = interrupt per packet)
00F00000 = Rx timer in Cycle size
000E0000 = No. pkts before Int.
00010000 = Continues mode (CM)
(Boy, that tulip driver could use a whitespace overhaul.)
^ permalink raw reply
* Re: 2.6.24-rc6-mm1
From: FUJITA Tomonori @ 2008-01-07 6:16 UTC (permalink / raw)
To: just.for.lkml
Cc: tomof, akpm, jarkao2, herbert, linux-kernel, neilb, bfields,
netdev, tom, fujita.tomonori
In-Reply-To: <64bb37e0801061203l503f29f0hd922a1347f8169ac@mail.gmail.com>
On Sun, 6 Jan 2008 21:03:42 +0100
"Torsten Kaiser" <just.for.lkml@googlemail.com> wrote:
> On Jan 6, 2008 2:33 PM, FUJITA Tomonori <tomof@acm.org> wrote:
> > On Sun, 6 Jan 2008 12:35:35 +0100
> > "Torsten Kaiser" <just.for.lkml@googlemail.com> wrote:
> > > On Jan 6, 2008 12:23 PM, FUJITA Tomonori <tomof@acm.org> wrote:
> > > > On Sun, 6 Jan 2008 11:41:10 +0100
> > > > "Torsten Kaiser" <just.for.lkml@googlemail.com> wrote:
> > > > > I will applie your patch and see if this hunk from
> > > > > find_next_zero_area() makes a difference:
> > > > >
> > > > > end = index + nr;
> > > > > - if (end > size)
> > > > > + if (end >= size)
> > > > > return -1;
>
> -> that might still have made a difference, but ...
>
> > > > > - for (i = index + 1; i < end; i++) {
> > > > > + for (i = index; i < end; i++) {
>
> ... as you say below, the test for the index position is only needed
> if index is modified after find_next_zero_bit().
>
> > > > > if (test_bit(i, map)) {
> > > >
> > > > The patch should not make a difference for X86_64.
> > >
> > > Hmm...
> > > arch/x86/kernel/pci-gart_64.c:
> > > alloc_iommu() calls iommu_area_alloc()
> > > lib/iommu-helper.c:
> > > iommu_area_alloc() calls find_next_zero_area()
> > > -> so the above code should be called even on X86_64
> >
> > Oops, I meant that the patch fixes the align allocation (non zero
> > align_mask case). X86_64 doesn't use the align allocation.
> >
> >
> > > And the change in the for loop means that 'index' will now be tested,
> > > but with the old code it was not.
> >
> > With the old code, 'index' is tested by find_next_zero_bit.
> >
> > With the new code and non zero align_mask case, 'index' is not tested
> > by find_next_zero_bit. So test_bit needs to start with 'index'.
> >
> > So If I understand the correctly, this patch should not make a
> > difference for x86_64 though I might miss something.
>
> You did not miss anything.
> After 18 packages my system crashed again.
>
> > > And double using something does fit with the errors I'm seeing...
> > >
> > > > Can you try the patch to revert my IOMMU changes?
> > > >
> > > > http://www.mail-archive.com/linux-scsi@vger.kernel.org/msg12694.html
> > >
> > > Testing for this bug is a little bit slow, as I'm compiling ~100
> > > packages trying to trigger it.
> > > If my current testrun with the patch from
> > > http://www.mail-archive.com/linux-scsi@vger.kernel.org/msg12702.html
> > > crashes, I will revert the hole IOMMU changes with above patch and try again.
> >
> > Thanks for testing,
>
> OK, I'm still testing this, but after 95 completed packages I'm rather
> certain that reverting the IOMMU changes with this patch fixes my
> problem.
> I didn't have time to look more into this, so I can't offer any
> concrete ideas where the bug is.
>
> If you send more patches, I'm willing to test them, but it might take
> some more time during the next week.
Can you try 2.6.24-rc7 + the IOMMU changes?
The patches are available at:
http://www.kernel.org/pub/linux/kernel/people/tomo/iommu/
Or if you prefer the git tree:
git://git.kernel.org/pub/scm/linux/kernel/git/tomo/linux-2.6-misc.git iommu-sg-fixes
I've looked at the changes to GART but they are straightforward and
don't look wrong...
Thanks,
^ permalink raw reply
* [PATCH] bluetooth : rfcomm tty BUG_ON() code fix
From: Dave Young @ 2008-01-07 5:41 UTC (permalink / raw)
To: marcel; +Cc: netdev, bluez-devel, davem, linux-kernel
1) In tty.c the BUG_ON at line 115 will never be called, because the the before list_del_init in this same function.
115 BUG_ON(!list_empty(&dev->list));
So move the list_del_init to rfcomm_dev_del
2) The rfcomm_dev_del could be called from diffrent path (rfcomm_tty_hangup/rfcomm_dev_state_change/rfcomm_release_dev),
So add another BUG_ON when the rfcomm_dev_del is called more than one time.
Signed-off-by: Dave Young <hidave.darkstar@gmail.com>
---
net/bluetooth/rfcomm/tty.c | 22 +++++++++++++---------
1 file changed, 13 insertions(+), 9 deletions(-)
diff -upr linux/net/bluetooth/rfcomm/tty.c linux.new/net/bluetooth/rfcomm/tty.c
--- linux/net/bluetooth/rfcomm/tty.c 2008-01-07 13:20:17.000000000 +0800
+++ linux.new/net/bluetooth/rfcomm/tty.c 2008-01-07 13:25:17.000000000 +0800
@@ -95,9 +95,10 @@ static void rfcomm_dev_destruct(struct r
BT_DBG("dev %p dlc %p", dev, dlc);
- write_lock_bh(&rfcomm_dev_lock);
- list_del_init(&dev->list);
- write_unlock_bh(&rfcomm_dev_lock);
+ /* Refcount should only hit zero when called from rfcomm_dev_del()
+ which will have taken us off the list. Everything else are
+ refcounting bugs. */
+ BUG_ON(!list_empty(&dev->list));
rfcomm_dlc_lock(dlc);
/* Detach DLC if it's owned by this dev */
@@ -109,11 +110,6 @@ static void rfcomm_dev_destruct(struct r
tty_unregister_device(rfcomm_tty_driver, dev->id);
- /* Refcount should only hit zero when called from rfcomm_dev_del()
- which will have taken us off the list. Everything else are
- refcounting bugs. */
- BUG_ON(!list_empty(&dev->list));
-
kfree(dev);
/* It's safe to call module_put() here because socket still
@@ -313,7 +309,15 @@ static void rfcomm_dev_del(struct rfcomm
{
BT_DBG("dev %p", dev);
- set_bit(RFCOMM_TTY_RELEASED, &dev->flags);
+ if (test_bit(RFCOMM_TTY_RELEASED, &dev->flags))
+ BUG_ON(1);
+ else
+ set_bit(RFCOMM_TTY_RELEASED, &dev->flags);
+
+ write_lock_bh(&rfcomm_dev_lock);
+ list_del_init(&dev->list);
+ write_unlock_bh(&rfcomm_dev_lock);
+
rfcomm_dev_put(dev);
}
-------------------------------------------------------------------------
This SF.net email is sponsored by: Microsoft
Defy all challenges. Microsoft(R) Visual Studio 2005.
http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/
^ permalink raw reply
* Re: Request to include ESFQ patch
From: Denys Fedoryshchenko @ 2008-01-07 4:46 UTC (permalink / raw)
To: lists; +Cc: netdev, bugfood-c
In-Reply-To: <478133A4.4020903@andyfurniss.entadsl.com>
On Sun, 06 Jan 2008 20:01:40 +0000, Andy Furniss wrote
> Denys Fedoryshchenko wrote:
> > Hi
> >
> > I took risk and installed ESFQ on my main backbone QoS. I found it highly
> > useful, and very need in setup's where is more than 128 flows passing and
> > especially where is nat available.
>
> I agree it will be good when it's in.
>
> >
> > Here is results with overloaded class for low-priority P2P traffic customers:
> >
>
> *sfq was never meant for interactive traffic as such. If you really
> want to do QOS for them you would need to (somehow) classify
> interactive and give it prio over bulk. I know this may not be
> practical for your setup, but what ping times users get will vary
> depending how many other active users there are/queue length/how
> many tcps the user has open etc.
In this specific setup i have a lot of classes just for type of traffic and
with priority, with many levels of hierarchy (root, realtime low bw
apps(inside also separation for few levels, and next step is specific
applications), realtime high bw apps, p2p low priority high bw and etc), plus
i have at root separation for types of customers.
But because even with this, i have few thousands of IP's, and after all if i
will create classes for each customer i will not fit in 65k of classes, plus i
will face performance issues, even if i will use hashes in filters. Instead
doing a thousands of classes, i group traffic and customers types and inside
distribute traffic fairly by ESFQ. ESFQ do the job perfectly.
SFQ for example cause of limit 128 flows will mess up with VoIP and other
applications for whom is critical packet ordering.
ESFQ does good job to prevent one dst IP inside group (for example) taking
prevailing part of bandwidth. Or to prevent one source to do same thing. Or
whatever - depends on hash type. It is not corrupting packet ordering on
non-classic hash type.
Btw in test i used ping just to get statistics how traffic passing class, how
bandwidth of single user affected with different qdiscs, and etc. If i see
some problem with specific type of bandwidth, i can forward ping with specific
ToS to that class by filters and i can compare regular ping with ToS'ed ping,
to see if issue caused by shaper, router,backbone provider or whatever.
>
> Andy.
> --
> To unsubscribe from this list: send the line "unsubscribe netdev" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
--
Denys Fedoryshchenko
Technical Manager
Virtual ISP S.A.L.
^ permalink raw reply
* Re: [PATCH] Fix forcedeth reversing the MAC address on suspend
From: Björn Steinbrink @ 2008-01-07 1:47 UTC (permalink / raw)
To: Richard Jonsson
Cc: linux-kernel, Adrian Bunk, Andreas Mohr, Ayaz Abdulla, jgarzik,
netdev
In-Reply-To: <20080104222633.GA21133@atjola.homenet>
On 2008.01.04 23:26:33 +0100, Björn Steinbrink wrote:
> For cards that initially have the MAC address stored in reverse order,
> the forcedeth driver uses a flag to signal whether the address was
> already corrected, so that it is not reversed again on a subsequent
> probe.
>
> Unfortunately this flag, which is stored in a register of the card,
> seems to get lost during suspend, resulting in the MAC address being
> reversed again. To fix that, the MAC address needs to be written back in
> reversed order before we suspend and the flag needs to be reset.
>
> The flag is still required because at least kexec will never write back
> the reversed address and thus needs to know what state the card is in.
>
> Signed-off-by: Björn Steinbrink <B.Steinbrink@gmx.de>
> ---
> On 2008.01.04 15:08:42 +0100, Richard Jonsson wrote:
> > Björn Steinbrink skrev:
> >> Richard, could you give this a spin? And then we'd likely need someone
> >> to test that with kexec...
> >
> > The patch you sent does the trick, works fine now, thanks!
> > I cannot test this with kexec as I barely know what it is, I'll leave that
> > to someone else.
>
> Thanks.
>
> Ayaz, you originally wrote the kexec fix (IIRC), was my analysis of the
> problem correct? If so, I'm quite sure that the patch DTRT. Still it
> should be tested for the rmmod+modprobe and the kexec case. I'll try to
> get my box free for some testing, but that's unlikely in the next few
> days. Plus, I've never used kexec myself either. So I'd be grateful if
> someone else would step up.
Found a few minutes to test, but kexec would just hang for me. So I'm
unable to test that atm. :-(
Björn
^ permalink raw reply
* Re: forcedeth: MAC-address reversed on resume from suspend
From: Björn Steinbrink @ 2008-01-07 1:46 UTC (permalink / raw)
To: Adolfo R. Brandes
Cc: Adrian Bunk, Andreas Mohr, Richard Jonsson, linux-kernel,
Ayaz Abdulla, jgarzik, netdev
In-Reply-To: <b4292e150801061349v3b54ffc0vfed75dbcb83daf9b@mail.gmail.com>
On 2008.01.06 19:49:49 -0200, Adolfo R. Brandes wrote:
> I have this forcedeth MAC address reversal problem when suspending
> on 2 distinct boxes. I can confirm Steinbrink's patch fixes the
> problem on only one of them:
>
> 00:0a.0 Bridge: nVidia Corporation CK804 Ethernet Controller (rev f3)
>
> On the other one the problem persists:
>
> 00:14.0 Bridge: nVidia Corporation MCP51 Ethernet Controller (rev a1)
Thanks. Leaves me pretty clueless though. Especially since it worked for
Richard, who also has a MCP51. In a private mail, you said that you had
hardcoded the mac address in the source. Any chance that you applied the
patch on your modified sources and didn't get it right?
thanks,
Björn
^ permalink raw reply
* Re: [PATCH] METH: fix MAC address handling
From: David Miller @ 2008-01-07 1:20 UTC (permalink / raw)
To: tsbogend; +Cc: netdev, linux-mips, ralf, jgarzik
In-Reply-To: <20080106113815.GA6140@alpha.franken.de>
From: tsbogend@alpha.franken.de (Thomas Bogendoerfer)
Date: Sun, 6 Jan 2008 12:38:16 +0100
> On Sun, Jan 06, 2008 at 12:23:05AM -0800, David Miller wrote:
> > I know that this whole driver is full of assumptions about
> > the endianness of the system this chip is found on, so
> > I'm only interested in if the transformation is equivalent
> > and the driver will keep working properly.
>
> I've tested the driver and it's still working :-)
Great :)
^ permalink raw reply
* [PATCH] IRDA: irda_create() nuke user triggable printk
From: maximilian attems @ 2008-01-07 0:34 UTC (permalink / raw)
To: davem; +Cc: netdev, samuel, maximilian attems
easy to trigger as user with sfuzz.
irda_create() is quiet on unknown sock->type,
match this behaviour for SOCK_DGRAM unknown protocol
Signed-off-by: maximilian attems <max@stro.at>
---
net/irda/af_irda.c | 2 --
1 files changed, 0 insertions(+), 2 deletions(-)
diff --git a/net/irda/af_irda.c b/net/irda/af_irda.c
index 48ce59a..d5e4dd7 100644
--- a/net/irda/af_irda.c
+++ b/net/irda/af_irda.c
@@ -1118,8 +1118,6 @@ static int irda_create(struct net *net, struct socket *sock, int protocol)
self->max_sdu_size_rx = TTP_SAR_UNBOUND;
break;
default:
- IRDA_ERROR("%s: protocol not supported!\n",
- __FUNCTION__);
return -ESOCKTNOSUPPORT;
}
break;
--
debian.1.5.3.7.1-dirty
^ permalink raw reply related
* Re: forcedeth: MAC-address reversed on resume from suspend
From: Adolfo R. Brandes @ 2008-01-06 21:49 UTC (permalink / raw)
To: Björn Steinbrink
Cc: Adrian Bunk, Andreas Mohr, Richard Jonsson, linux-kernel,
Ayaz Abdulla, jgarzik, netdev
In-Reply-To: <20080104034357.GA2113@atjola.homenet>
I have this forcedeth MAC address reversal problem when suspending
on 2 distinct boxes. I can confirm Steinbrink's patch fixes the
problem on only one of them:
00:0a.0 Bridge: nVidia Corporation CK804 Ethernet Controller (rev f3)
On the other one the problem persists:
00:14.0 Bridge: nVidia Corporation MCP51 Ethernet Controller (rev a1)
Thanks,
Adolfo
^ permalink raw reply
* Re: Atheros driver
From: John W. Linville @ 2008-01-06 21:21 UTC (permalink / raw)
To: Ralph Spitzner; +Cc: netdev, linux-wireless
In-Reply-To: <47813432.2010208@spitzner.org>
On Sun, Jan 06, 2008 at 09:04:02PM +0100, Ralph Spitzner wrote:
> With Kernel 2.6.23.12 && ath5k I get this:
> ------------------------------------------------------------------
>
> Jan 6 20:42:44 meepmeep kernel: CPU: 0
> Jan 6 20:42:44 meepmeep kernel: EIP: 0060:[<d0d98e61>] Not tainted
> VLI
> Jan 6 20:42:44 meepmeep kernel: EFLAGS: 00010246 (2.6.23.12 #1)
> Jan 6 20:42:44 meepmeep kernel: EIP is at
> ieee80211_generic_frame_duration+0x21/0x70 [mac80211]
> Jan 6 20:42:44 meepmeep kernel: eax: c2c68180 ebx: c2c68180 ecx:
> 0000000a edx: 00000000
> Jan 6 20:42:44 meepmeep kernel: esi: 00000000 edi: 0000000a ebp:
> 000003e8 esp: c3a03dd8
> Jan 6 20:42:44 meepmeep kernel: ds: 007b es: 007b fs: 0000 gs: 0033
> ss: 0068
> Jan 6 20:42:44 meepmeep kernel: Process ifconfig (pid: 5119, ti=c3a02000
> task=c363c570 task.ti=c3a02000)
> Jan 6 20:42:44 meepmeep kernel: Stack: d0e7539b 00000002 0000876c 00000000
> c4555000 d0e71c67 0000000a 1fb748ed
> Jan 6 20:42:44 meepmeep kernel: 0000006b c2c68180 00000000 d0e76e44
> c2c68eec 00000000 00000000 00000003
> Jan 6 20:42:44 meepmeep kernel: 00000002 00000002 00000014 00000000
> d0e76e20 c2c68de0 00000001 00000003
> Jan 6 20:42:44 meepmeep kernel: Call Trace:
> Jan 6 20:42:44 meepmeep kernel: [<d0e7539b>] ath5k_hw_rfgain+0x4b/0x80
> [ath5k]
> Jan 6 20:42:44 meepmeep kernel: [<d0e71c67>] ath5k_hw_reset+0xaa7/0xeb0
> [ath5k]
> Jan 6 20:42:44 meepmeep kernel: [<d0e6a766>] ath5k_init+0x46/0x110 [ath5k]
> Jan 6 20:42:44 meepmeep kernel: [<d0d86b7c>] ieee80211_open+0x19c/0x4c0
> [mac80211]
> Jan 6 20:42:44 meepmeep kernel: [<c0142235>] filemap_fault+0x1c5/0x3e0
> Jan 6 20:42:44 meepmeep kernel: [<c06a1353>] dev_open+0x33/0x80
> Jan 6 20:42:44 meepmeep kernel: [<c069f1a9>] dev_change_flags+0x149/0x1c0
> Jan 6 20:42:44 meepmeep kernel: [<c06e7261>] devinet_ioctl+0x521/0x6c0
> Jan 6 20:42:44 meepmeep kernel: [<c069efbf>] __dev_get_by_name+0x6f/0x90
> Jan 6 20:42:44 meepmeep kernel: [<c0693f4f>] sock_ioctl+0xbf/0x230
> Jan 6 20:42:44 meepmeep kernel: [<c0113b2b>] do_page_fault+0x18b/0x6d0
> Jan 6 20:42:44 meepmeep kernel: [<c0693e90>] sock_ioctl+0x0/0x230
> Jan 6 20:42:44 meepmeep kernel: [<c01682cf>] do_ioctl+0x1f/0x70
> Jan 6 20:42:44 meepmeep kernel: [<c016837c>] vfs_ioctl+0x5c/0x260
> Jan 6 20:42:44 meepmeep kernel: [<c01685f2>] sys_ioctl+0x72/0x90
> Jan 6 20:42:44 meepmeep kernel: [<c0104012>] syscall_call+0x7/0xb
> Jan 6 20:42:44 meepmeep kernel: [<c0710000>] __svc_create+0x1c0/0x1d0
> Jan 6 20:42:44 meepmeep kernel: =======================
> Jan 6 20:42:44 meepmeep kernel: Code: 5d c3 90 8d b4 26 00 00 00 00 83 ec
> 14 89 5c 24 08 89 c3 89 74 24 0c 89 d6 89 7c 24 10
> 89 cf 8b 4c 24 18 83 78 0c 02 74 31 31 d2 <8b> 86 d0 fd ff ff 89 14 24 89
> fa 83 e0 08 89 44 24 04 89 d8 e8
> Jan 6 20:42:44 meepmeep kernel: EIP: [<d0d98e61>]
> ieee80211_generic_frame_duration+0x21/0x70 [mac80211] SS:ESP 0068:c3a03dd8
> Jan 6 20:47:27 meepmeep kernel: ACPI: RTC can wake from S4
>
> ------------------------------------------------------------------
>
>
> Just in case someone is interested in this......
I'm sure someone is interested, although you might have slightly
better luck on linux-wireless@vger.kernel.org.
Also, I'm curious how you are running ath5k on 2.6.23.anything,
since the driver has not been merged upstream even now. Is this a
distro kernel? Your own hacking? Or...? It might be helpful to
know the exact origins of the kernel in use.
Thanks,
John
--
John W. Linville
linville@tuxdriver.com
^ permalink raw reply
* Atheros driver
From: Ralph Spitzner @ 2008-01-06 20:04 UTC (permalink / raw)
To: netdev
With Kernel 2.6.23.12 && ath5k I get this:
------------------------------------------------------------------
Jan 6 20:42:44 meepmeep kernel: CPU: 0
Jan 6 20:42:44 meepmeep kernel: EIP: 0060:[<d0d98e61>] Not
tainted VLI
Jan 6 20:42:44 meepmeep kernel: EFLAGS: 00010246 (2.6.23.12 #1)
Jan 6 20:42:44 meepmeep kernel: EIP is at
ieee80211_generic_frame_duration+0x21/0x70 [mac80211]
Jan 6 20:42:44 meepmeep kernel: eax: c2c68180 ebx: c2c68180 ecx:
0000000a edx: 00000000
Jan 6 20:42:44 meepmeep kernel: esi: 00000000 edi: 0000000a ebp:
000003e8 esp: c3a03dd8
Jan 6 20:42:44 meepmeep kernel: ds: 007b es: 007b fs: 0000 gs:
0033 ss: 0068
Jan 6 20:42:44 meepmeep kernel: Process ifconfig (pid: 5119,
ti=c3a02000 task=c363c570 task.ti=c3a02000)
Jan 6 20:42:44 meepmeep kernel: Stack: d0e7539b 00000002 0000876c
00000000 c4555000 d0e71c67 0000000a 1fb748ed
Jan 6 20:42:44 meepmeep kernel: 0000006b c2c68180 00000000
d0e76e44 c2c68eec 00000000 00000000 00000003
Jan 6 20:42:44 meepmeep kernel: 00000002 00000002 00000014
00000000 d0e76e20 c2c68de0 00000001 00000003
Jan 6 20:42:44 meepmeep kernel: Call Trace:
Jan 6 20:42:44 meepmeep kernel: [<d0e7539b>] ath5k_hw_rfgain+0x4b/0x80
[ath5k]
Jan 6 20:42:44 meepmeep kernel: [<d0e71c67>]
ath5k_hw_reset+0xaa7/0xeb0 [ath5k]
Jan 6 20:42:44 meepmeep kernel: [<d0e6a766>] ath5k_init+0x46/0x110 [ath5k]
Jan 6 20:42:44 meepmeep kernel: [<d0d86b7c>]
ieee80211_open+0x19c/0x4c0 [mac80211]
Jan 6 20:42:44 meepmeep kernel: [<c0142235>] filemap_fault+0x1c5/0x3e0
Jan 6 20:42:44 meepmeep kernel: [<c06a1353>] dev_open+0x33/0x80
Jan 6 20:42:44 meepmeep kernel: [<c069f1a9>] dev_change_flags+0x149/0x1c0
Jan 6 20:42:44 meepmeep kernel: [<c06e7261>] devinet_ioctl+0x521/0x6c0
Jan 6 20:42:44 meepmeep kernel: [<c069efbf>] __dev_get_by_name+0x6f/0x90
Jan 6 20:42:44 meepmeep kernel: [<c0693f4f>] sock_ioctl+0xbf/0x230
Jan 6 20:42:44 meepmeep kernel: [<c0113b2b>] do_page_fault+0x18b/0x6d0
Jan 6 20:42:44 meepmeep kernel: [<c0693e90>] sock_ioctl+0x0/0x230
Jan 6 20:42:44 meepmeep kernel: [<c01682cf>] do_ioctl+0x1f/0x70
Jan 6 20:42:44 meepmeep kernel: [<c016837c>] vfs_ioctl+0x5c/0x260
Jan 6 20:42:44 meepmeep kernel: [<c01685f2>] sys_ioctl+0x72/0x90
Jan 6 20:42:44 meepmeep kernel: [<c0104012>] syscall_call+0x7/0xb
Jan 6 20:42:44 meepmeep kernel: [<c0710000>] __svc_create+0x1c0/0x1d0
Jan 6 20:42:44 meepmeep kernel: =======================
Jan 6 20:42:44 meepmeep kernel: Code: 5d c3 90 8d b4 26 00 00 00 00 83
ec 14 89 5c 24 08 89 c3 89 74 24 0c 89 d6 89 7c 24 10
89 cf 8b 4c 24 18 83 78 0c 02 74 31 31 d2 <8b> 86 d0 fd ff ff 89 14 24
89 fa 83 e0 08 89 44 24 04 89 d8 e8
Jan 6 20:42:44 meepmeep kernel: EIP: [<d0d98e61>]
ieee80211_generic_frame_duration+0x21/0x70 [mac80211] SS:ESP 0068:c3a03dd8
Jan 6 20:47:27 meepmeep kernel: ACPI: RTC can wake from S4
------------------------------------------------------------------
Just in case someone is interested in this......
-rasp
--
"If I were you, I'd take a permanent vacation"
-Angels
^ permalink raw reply
* Re: Request to include ESFQ patch
From: Andy Furniss @ 2008-01-06 20:01 UTC (permalink / raw)
To: Denys Fedoryshchenko; +Cc: netdev, bugfood-c
In-Reply-To: <20080102155903.M66678@visp.net.lb>
Denys Fedoryshchenko wrote:
> Hi
>
> I took risk and installed ESFQ on my main backbone QoS. I found it highly
> useful, and very need in setup's where is more than 128 flows passing and
> especially where is nat available.
I agree it will be good when it's in.
>
> Here is results with overloaded class for low-priority P2P traffic customers:
>
*sfq was never meant for interactive traffic as such. If you really want
to do QOS for them you would need to (somehow) classify interactive and
give it prio over bulk. I know this may not be practical for your setup,
but what ping times users get will vary depending how many other active
users there are/queue length/how many tcps the user has open etc.
Andy.
^ permalink raw reply
* Re: 2.6.24-rc6-mm1
From: Torsten Kaiser @ 2008-01-06 20:03 UTC (permalink / raw)
To: FUJITA Tomonori
Cc: akpm, jarkao2, herbert, linux-kernel, neilb, bfields, netdev, tom,
fujita.tomonori
In-Reply-To: <20080106223650E.tomof@acm.org>
On Jan 6, 2008 2:33 PM, FUJITA Tomonori <tomof@acm.org> wrote:
> On Sun, 6 Jan 2008 12:35:35 +0100
> "Torsten Kaiser" <just.for.lkml@googlemail.com> wrote:
> > On Jan 6, 2008 12:23 PM, FUJITA Tomonori <tomof@acm.org> wrote:
> > > On Sun, 6 Jan 2008 11:41:10 +0100
> > > "Torsten Kaiser" <just.for.lkml@googlemail.com> wrote:
> > > > I will applie your patch and see if this hunk from
> > > > find_next_zero_area() makes a difference:
> > > >
> > > > end = index + nr;
> > > > - if (end > size)
> > > > + if (end >= size)
> > > > return -1;
-> that might still have made a difference, but ...
> > > > - for (i = index + 1; i < end; i++) {
> > > > + for (i = index; i < end; i++) {
... as you say below, the test for the index position is only needed
if index is modified after find_next_zero_bit().
> > > > if (test_bit(i, map)) {
> > >
> > > The patch should not make a difference for X86_64.
> >
> > Hmm...
> > arch/x86/kernel/pci-gart_64.c:
> > alloc_iommu() calls iommu_area_alloc()
> > lib/iommu-helper.c:
> > iommu_area_alloc() calls find_next_zero_area()
> > -> so the above code should be called even on X86_64
>
> Oops, I meant that the patch fixes the align allocation (non zero
> align_mask case). X86_64 doesn't use the align allocation.
>
>
> > And the change in the for loop means that 'index' will now be tested,
> > but with the old code it was not.
>
> With the old code, 'index' is tested by find_next_zero_bit.
>
> With the new code and non zero align_mask case, 'index' is not tested
> by find_next_zero_bit. So test_bit needs to start with 'index'.
>
> So If I understand the correctly, this patch should not make a
> difference for x86_64 though I might miss something.
You did not miss anything.
After 18 packages my system crashed again.
> > And double using something does fit with the errors I'm seeing...
> >
> > > Can you try the patch to revert my IOMMU changes?
> > >
> > > http://www.mail-archive.com/linux-scsi@vger.kernel.org/msg12694.html
> >
> > Testing for this bug is a little bit slow, as I'm compiling ~100
> > packages trying to trigger it.
> > If my current testrun with the patch from
> > http://www.mail-archive.com/linux-scsi@vger.kernel.org/msg12702.html
> > crashes, I will revert the hole IOMMU changes with above patch and try again.
>
> Thanks for testing,
OK, I'm still testing this, but after 95 completed packages I'm rather
certain that reverting the IOMMU changes with this patch fixes my
problem.
I didn't have time to look more into this, so I can't offer any
concrete ideas where the bug is.
If you send more patches, I'm willing to test them, but it might take
some more time during the next week.
Thanks for looking into this.
Torsten
^ permalink raw reply
* Re: 2.6.24-rc6-mm1
From: Jarek Poplawski @ 2008-01-06 14:52 UTC (permalink / raw)
To: Torsten Kaiser
Cc: Herbert Xu, Andrew Morton, linux-kernel, Neil Brown,
J. Bruce Fields, netdev, Tom Tucker
In-Reply-To: <64bb37e0801060230x6b392542la9556d72a184f306@mail.gmail.com>
On Sun, Jan 06, 2008 at 11:30:48AM +0100, Torsten Kaiser wrote:
...
> I think this bug is highly timing dependent. Its not always the same
> package that dies and as this is a SMP system I would guess two CPUs
> using the same data will trigger this.
> And using the poison-option will definitily slow the system down and
> mess up the timings.
Of course it looks like using the same data, but it seems there is no
reason to think it needs the same time: e.g. some timer or workqueue
could retrigger after it's supposed to be killed. Any additional
debugging/poisonning might help to see it earlier, so this should be
safer for your system, but, most probably this would show data from
the damaged side, so not necessarily very helpful.
> What also speaks against the 'safer' offsets is, that after adding my
> notfreed-byte to skbuff the bug still triggered in the same way.
We are not even sure skbuffs were directly affected by this or they
were incorrectly freed because of other structures beeing damaged?
IMHO, e.g. starting your system with limited memory should cause
faster memory reclaiming, and thus more often triggering of these bugs,
but of course I can be wrong.
Jarek P.
^ permalink raw reply
* Re: 2.6.24-rc6-mm1
From: FUJITA Tomonori @ 2008-01-06 13:33 UTC (permalink / raw)
To: just.for.lkml
Cc: tomof, akpm, jarkao2, herbert, linux-kernel, neilb, bfields,
netdev, tom, fujita.tomonori, fujita.tomonori
In-Reply-To: <64bb37e0801060335k4afb3134u9c6fadb57d525dc5@mail.gmail.com>
On Sun, 6 Jan 2008 12:35:35 +0100
"Torsten Kaiser" <just.for.lkml@googlemail.com> wrote:
> On Jan 6, 2008 12:23 PM, FUJITA Tomonori <tomof@acm.org> wrote:
> > On Sun, 6 Jan 2008 11:41:10 +0100
> > "Torsten Kaiser" <just.for.lkml@googlemail.com> wrote:
> > > I will applie your patch and see if this hunk from
> > > find_next_zero_area() makes a difference:
> > >
> > > end = index + nr;
> > > - if (end > size)
> > > + if (end >= size)
> > > return -1;
> > > - for (i = index + 1; i < end; i++) {
> > > + for (i = index; i < end; i++) {
> > > if (test_bit(i, map)) {
> >
> > The patch should not make a difference for X86_64.
>
> Hmm...
> arch/x86/kernel/pci-gart_64.c:
> alloc_iommu() calls iommu_area_alloc()
> lib/iommu-helper.c:
> iommu_area_alloc() calls find_next_zero_area()
> -> so the above code should be called even on X86_64
Oops, I meant that the patch fixes the align allocation (non zero
align_mask case). X86_64 doesn't use the align allocation.
> And the change in the for loop means that 'index' will now be tested,
> but with the old code it was not.
With the old code, 'index' is tested by find_next_zero_bit.
With the new code and non zero align_mask case, 'index' is not tested
by find_next_zero_bit. So test_bit needs to start with 'index'.
So If I understand the correctly, this patch should not make a
difference for x86_64 though I might miss something.
> And double using something does fit with the errors I'm seeing...
>
> > Can you try the patch to revert my IOMMU changes?
> >
> > http://www.mail-archive.com/linux-scsi@vger.kernel.org/msg12694.html
>
> Testing for this bug is a little bit slow, as I'm compiling ~100
> packages trying to trigger it.
> If my current testrun with the patch from
> http://www.mail-archive.com/linux-scsi@vger.kernel.org/msg12702.html
> crashes, I will revert the hole IOMMU changes with above patch and try again.
Thanks for testing,
^ permalink raw reply
* Re: [PATCH] METH: fix MAC address handling
From: Thomas Bogendoerfer @ 2008-01-06 11:38 UTC (permalink / raw)
To: David Miller; +Cc: netdev, linux-mips, ralf, jgarzik
In-Reply-To: <20080106.002305.99653155.davem@davemloft.net>
On Sun, Jan 06, 2008 at 12:23:05AM -0800, David Miller wrote:
> > + u64 macaddr;
> >
> > - for (i = 0; i < 6; i++)
> > - dev->dev_addr[i] = o2meth_eaddr[i];
> > DPRINTK("Loading MAC Address: %s\n", print_mac(mac, dev->dev_addr));
> > - mace->eth.mac_addr = (*(unsigned long*)o2meth_eaddr) >> 16;
> > + macaddr = 0;
> > + for (i = 0; i < 6; i++)
> > + macaddr |= dev->dev_addr[i] << ((5 - i) * 8);
> > +
> > + mace->eth.mac_addr = macaddr;
> > }
> >
> > /*
>
> Can you double-check that this conversion is equivalent.
yes, I did.
> I know that this whole driver is full of assumptions about
> the endianness of the system this chip is found on, so
> I'm only interested in if the transformation is equivalent
> and the driver will keep working properly.
I've tested the driver and it's still working :-)
Thomas.
--
Crap can work. Given enough thrust pigs will fly, but it's not necessary a
good idea. [ RFC1925, 2.3 ]
^ permalink raw reply
* Re: 2.6.24-rc6-mm1
From: Torsten Kaiser @ 2008-01-06 11:35 UTC (permalink / raw)
To: FUJITA Tomonori
Cc: akpm, jarkao2, herbert, linux-kernel, neilb, bfields, netdev, tom,
fujita.tomonori
In-Reply-To: <20080106202616Z.tomof@acm.org>
On Jan 6, 2008 12:23 PM, FUJITA Tomonori <tomof@acm.org> wrote:
> On Sun, 6 Jan 2008 11:41:10 +0100
> "Torsten Kaiser" <just.for.lkml@googlemail.com> wrote:
> > I will applie your patch and see if this hunk from
> > find_next_zero_area() makes a difference:
> >
> > end = index + nr;
> > - if (end > size)
> > + if (end >= size)
> > return -1;
> > - for (i = index + 1; i < end; i++) {
> > + for (i = index; i < end; i++) {
> > if (test_bit(i, map)) {
>
> The patch should not make a difference for X86_64.
Hmm...
arch/x86/kernel/pci-gart_64.c:
alloc_iommu() calls iommu_area_alloc()
lib/iommu-helper.c:
iommu_area_alloc() calls find_next_zero_area()
-> so the above code should be called even on X86_64
And the change in the for loop means that 'index' will now be tested,
but with the old code it was not.
And double using something does fit with the errors I'm seeing...
> Can you try the patch to revert my IOMMU changes?
>
> http://www.mail-archive.com/linux-scsi@vger.kernel.org/msg12694.html
Testing for this bug is a little bit slow, as I'm compiling ~100
packages trying to trigger it.
If my current testrun with the patch from
http://www.mail-archive.com/linux-scsi@vger.kernel.org/msg12702.html
crashes, I will revert the hole IOMMU changes with above patch and try again.
Torsten
^ permalink raw reply
* Re: 2.6.24-rc6-mm1
From: FUJITA Tomonori @ 2008-01-06 11:23 UTC (permalink / raw)
To: just.for.lkml
Cc: tomof, akpm, jarkao2, herbert, linux-kernel, neilb, bfields,
netdev, tom, fujita.tomonori, fujita.tomonori
In-Reply-To: <64bb37e0801060241o2c6d8172r73b69291fce76ce1@mail.gmail.com>
On Sun, 6 Jan 2008 11:41:10 +0100
"Torsten Kaiser" <just.for.lkml@googlemail.com> wrote:
> On Jan 6, 2008 4:28 AM, FUJITA Tomonori <tomof@acm.org> wrote:
> > On Sat, 5 Jan 2008 17:25:24 -0800
> > Andrew Morton <akpm@linux-foundation.org> wrote:
> > > On Sat, 5 Jan 2008 23:10:17 +0100 "Torsten Kaiser" <just.for.lkml@googlemail.com> wrote:
> > > > But the cause of my mail is the following question:
> > > > Regarding my "iommu-sg-merging-patches are new in -rc3-mm and could be
> > > > the cause"-suspicion I looked at these patches and came across these
> > > > hunks:
> > > >
> > > > This is removed from arch/x86/lib/bitstr_64.c:
> > > > -/* Find string of zero bits in a bitmap */
> > > > -unsigned long
> > > > -find_next_zero_string(unsigned long *bitmap, long start, long nbits, int len)
> > > > -{
> > > > - unsigned long n, end, i;
> > > > -
> > > > - again:
> > > > - n = find_next_zero_bit(bitmap, nbits, start);
> > > > - if (n == -1)
> > > > - return -1;
> > > > -
> > > > - /* could test bitsliced, but it's hardly worth it */
> > > > - end = n+len;
> > > > - if (end > nbits)
> > > > - return -1;
> > > > - for (i = n+1; i < end; i++) {
> > > > - if (test_bit(i, bitmap)) {
> > > > - start = i+1;
> > > > - goto again;
> > > > - }
> > > > - }
> > > > - return n;
> > > > -}
> > > >
> > > > This is added to lib/iommu-helper.c:
> > > > +static unsigned long find_next_zero_area(unsigned long *map,
> > > > + unsigned long size,
> > > > + unsigned long start,
> > > > + unsigned int nr)
> > > > +{
> > > > + unsigned long index, end, i;
> > > > +again:
> > > > + index = find_next_zero_bit(map, size, start);
> > > > + end = index + nr;
> > > > + if (end > size)
> > > > + return -1;
> > > > + for (i = index + 1; i < end; i++) {
> > > > + if (test_bit(i, map)) {
> > > > + start = i+1;
> > > > + goto again;
> > > > + }
> > > > + }
> > > > + return index;
> > > > +}
> > > >
> > > > The old version checks, if find_next_zero_bit returns -1, the new
> > > > version doesn't do this.
> > > > Is this intended and can find_next_zero_bit never fail?
> > > > Hmm... but in the worst case it should only loop forever if I'm
> > > > reading this right (index = -1 => for-loop counts from 0 to nr, if any
> > > > bit is set it will jump to "again:" and if the next call to
> > > > find_next_zero_bit also fails, its an endless loop)
> >
> > find_next_zero_bit returns -1?
> >
> > It seems that x86_64 doesn't.
>
> I'm sorry. I didn't look into find_next_zero_bit, I only noted that
> the old version did check for -1 and the new one didn't.
> Obviously the old check was superfluous.
>
> > POWER and SPARC64 IOMMUs use
> > find_next_zero_bit too but both doesn't check if find_next_zero_bit
> > returns -1. If find_next_zero_bit fails, it returns size. So it
> > doesn't leads to an endless loop.
>
> Yes, this can't happen. It was a wrong assumption on my part.
>
> > But this patch has other bugs that break POWER IOMMUs.
> >
> > If you use the IOMMUs on POWER, please try the following patch:
>
> I'm using CONFIG_GART_IOMMU=y on x86_64.
>
> > http://www.mail-archive.com/linux-scsi@vger.kernel.org/msg12702.html
>
> I also noted the line "index = (index + align_mask) & ~align_mask;" in
> iommu_area_alloc() and didn't understand what this was trying to do
> and how this should work, but as arch/x86/kernel/pci-gart_64.c always
> uses 0 as align_mask I just ignored it.
Yeah, it's for only POWER IOMMUs. It's meaningless for gart and
calgary IOMMUs.
> I will applie your patch and see if this hunk from
> find_next_zero_area() makes a difference:
>
> end = index + nr;
> - if (end > size)
> + if (end >= size)
> return -1;
> - for (i = index + 1; i < end; i++) {
> + for (i = index; i < end; i++) {
> if (test_bit(i, map)) {
The patch should not make a difference for X86_64.
Can you try the patch to revert my IOMMU changes?
http://www.mail-archive.com/linux-scsi@vger.kernel.org/msg12694.html
^ permalink raw reply
* Re: 2.6.24-rc6-mm1
From: Torsten Kaiser @ 2008-01-06 10:41 UTC (permalink / raw)
To: FUJITA Tomonori
Cc: akpm, jarkao2, herbert, linux-kernel, neilb, bfields, netdev, tom,
fujita.tomonori
In-Reply-To: <20080106123103K.tomof@acm.org>
On Jan 6, 2008 4:28 AM, FUJITA Tomonori <tomof@acm.org> wrote:
> On Sat, 5 Jan 2008 17:25:24 -0800
> Andrew Morton <akpm@linux-foundation.org> wrote:
> > On Sat, 5 Jan 2008 23:10:17 +0100 "Torsten Kaiser" <just.for.lkml@googlemail.com> wrote:
> > > But the cause of my mail is the following question:
> > > Regarding my "iommu-sg-merging-patches are new in -rc3-mm and could be
> > > the cause"-suspicion I looked at these patches and came across these
> > > hunks:
> > >
> > > This is removed from arch/x86/lib/bitstr_64.c:
> > > -/* Find string of zero bits in a bitmap */
> > > -unsigned long
> > > -find_next_zero_string(unsigned long *bitmap, long start, long nbits, int len)
> > > -{
> > > - unsigned long n, end, i;
> > > -
> > > - again:
> > > - n = find_next_zero_bit(bitmap, nbits, start);
> > > - if (n == -1)
> > > - return -1;
> > > -
> > > - /* could test bitsliced, but it's hardly worth it */
> > > - end = n+len;
> > > - if (end > nbits)
> > > - return -1;
> > > - for (i = n+1; i < end; i++) {
> > > - if (test_bit(i, bitmap)) {
> > > - start = i+1;
> > > - goto again;
> > > - }
> > > - }
> > > - return n;
> > > -}
> > >
> > > This is added to lib/iommu-helper.c:
> > > +static unsigned long find_next_zero_area(unsigned long *map,
> > > + unsigned long size,
> > > + unsigned long start,
> > > + unsigned int nr)
> > > +{
> > > + unsigned long index, end, i;
> > > +again:
> > > + index = find_next_zero_bit(map, size, start);
> > > + end = index + nr;
> > > + if (end > size)
> > > + return -1;
> > > + for (i = index + 1; i < end; i++) {
> > > + if (test_bit(i, map)) {
> > > + start = i+1;
> > > + goto again;
> > > + }
> > > + }
> > > + return index;
> > > +}
> > >
> > > The old version checks, if find_next_zero_bit returns -1, the new
> > > version doesn't do this.
> > > Is this intended and can find_next_zero_bit never fail?
> > > Hmm... but in the worst case it should only loop forever if I'm
> > > reading this right (index = -1 => for-loop counts from 0 to nr, if any
> > > bit is set it will jump to "again:" and if the next call to
> > > find_next_zero_bit also fails, its an endless loop)
>
> find_next_zero_bit returns -1?
>
> It seems that x86_64 doesn't.
I'm sorry. I didn't look into find_next_zero_bit, I only noted that
the old version did check for -1 and the new one didn't.
Obviously the old check was superfluous.
> POWER and SPARC64 IOMMUs use
> find_next_zero_bit too but both doesn't check if find_next_zero_bit
> returns -1. If find_next_zero_bit fails, it returns size. So it
> doesn't leads to an endless loop.
Yes, this can't happen. It was a wrong assumption on my part.
> But this patch has other bugs that break POWER IOMMUs.
>
> If you use the IOMMUs on POWER, please try the following patch:
I'm using CONFIG_GART_IOMMU=y on x86_64.
> http://www.mail-archive.com/linux-scsi@vger.kernel.org/msg12702.html
I also noted the line "index = (index + align_mask) & ~align_mask;" in
iommu_area_alloc() and didn't understand what this was trying to do
and how this should work, but as arch/x86/kernel/pci-gart_64.c always
uses 0 as align_mask I just ignored it.
I will applie your patch and see if this hunk from
find_next_zero_area() makes a difference:
end = index + nr;
- if (end > size)
+ if (end >= size)
return -1;
- for (i = index + 1; i < end; i++) {
+ for (i = index; i < end; i++) {
if (test_bit(i, map)) {
Torsten
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox