* Re: [PATCH] fix long-standing bug in 2.6/2.4 skb_copy/skb_copy_expand
From: David S. Miller @ 2005-05-11 20:56 UTC (permalink / raw)
To: pizza; +Cc: herbert, linux-kernel, netdev
In-Reply-To: <20050511203845.GA10770@shaftnet.org>
From: Stuffed Crust <pizza@shaftnet.org>
Subject: Re: [PATCH] fix long-standing bug in 2.6/2.4 skb_copy/skb_copy_expand
Date: Wed, 11 May 2005 16:38:45 -0400
> As I said in my original message, it was a crude hack which has since
> been relegated to the great bitbucket of the sky. All that's left is
> that "bugfix" patch.
If alloc_skb() does not do an skb_reserve(), then the bug fix
patch is unnecessary. The "bug" only exists if you happen to
change alloc_skb() to do an skb_reserve().
^ permalink raw reply
* Re: [PATCH] fix long-standing bug in 2.6/2.4 skb_copy/skb_copy_expand
From: Stuffed Crust @ 2005-05-11 20:38 UTC (permalink / raw)
To: Herbert Xu; +Cc: davem, linux-kernel, netdev
In-Reply-To: <E1DUyZO-0004Fp-00@gondolin.me.apana.org.au>
[-- Attachment #1: Type: text/plain, Size: 1078 bytes --]
On Mon, May 09, 2005 at 01:04:34PM +1000, Herbert Xu wrote:
> > This is, fortunately, generally true. But if the alloc_skb function
> > allocates extra head room (ie calls skb_reserve() on the skb before it
> > passes it to the callee, this doesn't quite work. Instead, it should be
> > rewritten as:
>
> As far as I know the alloc_skb funciton in the kernel tree doesn't do
> that so your patch is not necessary unless we decide to change the way
> alloc_skb works. If that's what you want then please provide a patch
> to alloc_skb and a rationale as to why we should do that.
It does not, and I have no intention of submitting a patch to change it.
As I said in my original message, it was a crude hack which has since
been relegated to the great bitbucket of the sky. All that's left is
that "bugfix" patch.
I've performed my due-diligence in airing it to the powers that be, so
I'll go way now.
- Solomon
--
Solomon Peachy ICQ: 1318344
Melbourne, FL JID: pitha@myjabber.net
Quidquid latine dictum sit, altum viditur
[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]
^ permalink raw reply
* Interdomain/interpartition communication without a checksum
From: Jon Mason @ 2005-05-11 18:35 UTC (permalink / raw)
To: netdev; +Cc: Nivedita Singhvi, Andrew Theurer
I have been working on a Xen project to remove unnecessary TCP/UDP
checksums for local (interdomain) communication, while still having the
checksum for all external network communication.
background:
Xen uses a controlling partition (domain 0 in Xen terms) to connect all
guest partitions to physical devices (disk, ethernet, etc). For network
traffic, the guest partitions connect to the controlling partition via
virtual ethernet devices. These virtual ethernet devices are either
bridged or routed to the physical ethernet to connect to the external
network.
In its current implimentation, Xen guest partitions checksum packets
regardless of their intended destination (local or remote) and
regardless of adapter hardware checksum offload capabilities for the
physical adapter. I have a working prototype which corrects both of
these issues in Xen, but it requires minor changes to two files
(net/core/dev.c and include/linux/netdevice.h).
I have not submitted this patch to the Xen mailing list, as I wanted all
of you to comment on it first. The changes are abstract enough to where
they can be used by anyone wanting interdomain/interpartition
communication without a checksum (e.g., not Xen specific).
I have only included the Linux files, but if anyone is interested I can
also include the modified Xen files.
Thanks,
Jon
--- ../xen-unstable-pristine/linux-2.6.11-xen0/net/core/dev.c 2005-03-02 01:38:09.000000000 -0600
+++ linux-2.6.11-xen0/net/core/dev.c 2005-05-11 08:00:28.234919880 -0500
@@ -98,6 +98,7 @@
#include <linux/stat.h>
#include <linux/if_bridge.h>
#include <linux/divert.h>
+#include <net/ip.h>
#include <net/dst.h>
#include <net/pkt_sched.h>
#include <net/checksum.h>
@@ -1236,6 +1237,15 @@ int dev_queue_xmit(struct sk_buff *skb)
__skb_linearize(skb, GFP_ATOMIC))
goto out_kfree_skb;
+ /* If packet is forwarded to a device that needs a checksum and not
+ * checksummed, correct the pointers and enable checksumming in the
+ * next function.
+ */
+ if (!(dev->features & NETIF_F_FWD_NO_CSUM) && skb->csum) {
+ skb->ip_summed = CHECKSUM_HW;
+ skb->h.raw = (void *)skb->nh.iph + (skb->nh.iph->ihl * 4);
+ }
+
/* If packet is not checksummed and device does not support
* checksumming for this protocol, complete checksumming here.
*/
--- ../xen-unstable-pristine/linux-2.6.11-xen0/include/linux/netdevice.h 2005-03-02 01:38:26.000000000 -0600
+++ linux-2.6.11-xen0/include/linux/netdevice.h 2005-05-10 11:03:25.000000000 -0500
@@ -416,6 +416,7 @@ struct net_device
#define NETIF_F_VLAN_CHALLENGED 1024 /* Device cannot handle VLAN packets */
#define NETIF_F_TSO 2048 /* Can offload TCP/IP segmentation */
#define NETIF_F_LLTX 4096 /* LockLess TX */
+#define NETIF_F_FWD_NO_CSUM 8192 /* Forwards unchecksumed packets */
/* Called after device is detached from network. */
void (*uninit)(struct net_device *dev);
^ permalink raw reply
* Re: tg3 support broken on PPC, a workaround
From: Michael Chan @ 2005-05-11 15:24 UTC (permalink / raw)
To: Manuel Perez Ayala; +Cc: netdev
In-Reply-To: <20050511080446.fnstrvtzq8gk8884@158.49.151.11>
On Wed, 2005-05-11 at 08:04 +0200, Manuel Perez Ayala wrote:
> Working driver:
>
> On init of driver:
>
> tg3.c:v3.10 (September 14, 2004)
> PCI: Enabling device 0001:10:13.0 (0014 -> 0016)
>
> tg3: tg3_test_dma #1: 76FF120F
>
This tells me that your cache line size is 32 bytes.
Let's try some experiments and see what works and what doesn't. Please
hardcode the following values in tp->dma_rwctrl before it is written to
the register:
1. DMA read/write boundaries set to 256:
tp->dma_rwctrl = 0x76ff2d0f;
2. DMA read boundary 256, write boundary 32:
tp->dma_rwctrl = 0x76ff150f;
3. DMA read boundary 32, write boundary 256
tp->dma_rwctrl = 0x76ff2a0f;
4. Let's also try without asserting all byte enables:
tp->dma_rwctrl = 0x763f2d0f;
Thanks.
^ permalink raw reply
* Re: assertion (!atomic_read(&sk->sk_rmem_alloc)) failed at net/netlink/af_netlink.c (122)
From: Ken-ichirou MATSUZAWA @ 2005-05-11 15:20 UTC (permalink / raw)
To: herbert; +Cc: netdev, davem
In-Reply-To: <20050511005836.GA1674@gondor.apana.org.au>
Hello.
From: Herbert Xu <herbert@gondor.apana.org.au>
Subject: Re: assertion (!atomic_read(&sk->sk_rmem_alloc)) failed at net/netlink/af_netlink.c (122)
Date: Wed, 11 May 2005 10:58:36 +1000
> Please let me know whether this works for you or not.
It worked, thank you.
^ permalink raw reply
* Re: [1/1] connector/CBUS: new messaging subsystem. Revision number next.
From: Alan Cox @ 2005-05-11 14:09 UTC (permalink / raw)
To: Dmitry Torokhov
Cc: Evgeniy Polyakov, netdev, Greg KH, Jamal Hadi Salim, Kay Sievers,
Herbert Xu, James Morris, Guillaume Thouvenin,
Linux Kernel Mailing List, Andrew Morton, Thomas Graf, Jay Lan
In-Reply-To: <200505100118.48027.dtor_core@ameritech.net>
> BTW, I do not think that "All rights reserved" statement is applicable/
> compatible with GPL this software is supposedly released under, I have
> taken liberty of removing this statement.
Its perfectly compatible with the GPL and BSD license. All rights
reserved is simply asserting all your rights (moral and other). It then
goes on to say you can use it under the GPL
Alan
^ permalink raw reply
* Re: 2.4.30-hf1 do_IRQ stack overflows
From: Marcelo Tosatti @ 2005-05-11 12:46 UTC (permalink / raw)
To: Manfred Schwarb; +Cc: linux-kernel, davem, netdev
In-Reply-To: <5.2.1.1.2.20050511093252.01c887b0@mail.meteodat.ch>
[-- Attachment #1: Type: text/plain, Size: 1371 bytes --]
Hi Manfred,
On Wed, May 11, 2005 at 10:15:02AM +0200, Manfred Schwarb wrote:
> Hi,
> with recent versions of the 2.4 kernel (Vanilla), I get an increasing amount of do_IRQ stack overflows.
> This night, I got 3 of them.
> With 2.4.28 I got an overflow about twice a year, with 2.4.29 nearly once a month and with
> 2.4.30 nearly every day 8-((
The system is getting dangerously close to an actual stack overflow, which would
crash the system.
"do_IRQ: stack overflow: " indicates how many bytes are still available.
The traces show huge networking execution paths.
It seems you are using some packet scheduler (CONFIG_NET_SCHED)? Pretty much all
traces show functions from sch_generic.c. Can you disable that for a test?
> My layout: Pentium4 HT SMP, raid1 on a promise card, driven by libata_promise,
> everything using reiserfs, heavy nfs and network traffic, with a Linksys (tulip) and a Realtek (8139too) NIC.
> The used 2.4.30-hf1 is pure Vanilla.
>
> Below my three overflow messages. Would the stack reduction patches of Badari Pulavarty
> help in my case? If so, I would strongly vote for inclusion into 2.4 series!!
It has been decided that the stack reduction patches were too intrusive to be merged
at this stage of v2.4 life.
We did not receive any report either - v2.4's 8KB kernel stack seems to be enough
for vast majority of situations.
[-- Attachment #2: stack_ovf_2430.log --]
[-- Type: text/plain, Size: 21830 bytes --]
May 11 04:22:09 server kernel: do_IRQ: stack overflow: 628
May 11 04:22:09 server kernel: f3668824 00000274 e9a40380 00000000 00000003 f7e5d280 f3668000 0000000e
May 11 04:22:09 server kernel: c010d948 00000003 f7e5d318 f7e5d308 f7e5d280 f3668000 0000000e 00000006
May 11 04:22:09 server kernel: f1960018 f1a00018 ffffff14 c024893e 00000010 00000202 f7e5d280 f196cc00
May 11 04:22:09 server kernel: Call Trace: [call_do_IRQ+5/13] [pfifo_fast_dequeue+62/80] [qdisc_restart+31/432] [dev_queue_xmit+383/880] [neigh_resolve_output+321/624]
May 11 04:22:09 server kernel: Call Trace: [<c010d948>] [<c024893e>] [<c02483df>] [<c023ccaf>] [<c0243af1>]
May 11 04:22:09 server kernel: [ip_finish_output2+184/336] [ip_finish_output2+0/336] [ip_finish_output2+0/336] [nf_hook_slow+478/528] [ip_finish_output2+0/336] [ip_output+334/480]
May 11 04:22:09 server kernel: [<c025a2d8>] [<c025a220>] [<c025a220>] [<c024763e>] [<c025a220>] [<c0258a4e>]
May 11 04:22:09 server kernel: [ip_finish_output2+0/336] [ip_queue_xmit2+213/671] [ip_queue_xmit2+0/671] [ip_queue_xmit2+0/671] [nf_hook_slow+478/528] [ip_queue_xmit2+0/671]
May 11 04:22:09 server kernel: [<c025a220>] [<c025a445>] [<c025a370>] [<c025a370>] [<c024763e>] [<c025a370>]
May 11 04:22:09 server kernel: [ip_queue_xmit+845/1536] [ip_queue_xmit2+0/671] [dev_queue_xmit+383/880] [tcp_v4_send_check+160/240] [tcp_transmit_skb+1001/1792] [tcp_send_ack+132/208]
May 11 04:22:09 server kernel: [<c0258e2d>] [<c025a370>] [<c023ccaf>] [<c0271c20>] [<c026bdc9>] [<c026e824>]
May 11 04:22:09 server kernel: [tcp_rfree+0/32] [tcp_rfree+0/32] [tcp_rcv_established+2042/2640] [tcp_v4_do_rcv+314/352] [tcp_v4_rcv+1726/1952] [ip_local_deliver_finish+351/416]
May 11 04:22:09 server kernel: [<c025d4a0>] [<c025d4a0>] [<c026a2fa>] [<c0272eda>] [<c02735be>] [<c025581f>]
May 11 04:22:09 server kernel: [ip_local_deliver_finish+0/416] [nf_hook_slow+478/528] [ip_local_deliver_finish+0/416] [ip_local_deliver+399/576] [ip_local_deliver_finish+0/416] [ip_rcv_finish+553/700]
May 11 04:22:09 server kernel: [<c02556c0>] [<c024763e>] [<c02556c0>] [<c025514f>] [<c02556c0>] [<c0255a89>]
May 11 04:22:09 server kernel: [ip_rcv_finish+0/700] [ip_rcv_finish+0/700] [nf_hook_slow+478/528] [ip_rcv_finish+0/700] [ip_rcv+905/1216] [ip_rcv_finish+0/700]
May 11 04:22:09 server kernel: [<c0255860>] [<c0255860>] [<c024763e>] [<c0255860>] [<c0255589>] [<c0255860>]
May 11 04:22:09 server kernel: [netif_receive_skb+485/544] [process_backlog+147/304] [net_rx_action+250/368] [do_softirq+118/224] [do_IRQ+244/304] [call_do_IRQ+5/13]
May 11 04:22:09 server kernel: [<c023d505>] [<c023d5d3>] [<c023d76a>] [<c01254c6>] [<c010b094>] [<c010d948>]
May 11 04:22:09 server kernel: [__switch_to+82/256] [schedule+738/1344] [schedule_timeout+84/160] [process_timeout+0/96] [st:__insmod_st_O/lib/modules/2.4.30-hf1/kernel/drivers/scsi/st+4294702743/96] [st:__insmod_st_O/lib/modules/2.4.30-hf1/kernel/drivers/scsi/st+4294703097/96]
May 11 04:22:09 server kernel: [<c0107722>] [<c011cee2>] [<c011cb14>] [<c011ca60>] [<f90f0697>] [<f90f07f9>]
May 11 04:22:09 server kernel: [st:__insmod_st_O/lib/modules/2.4.30-hf1/kernel/drivers/scsi/st+4294703275/96] [st:__insmod_st_O/lib/modules/2.4.30-hf1/kernel/drivers/scsi/st+4294709446/96] [<f914d588>] [<f914f255>] [st:__insmod_st_O/lib/modules/2.4.30-hf1/kernel/drivers/scsi/st+4288825300/96] [st:__insmod_st_O/lib/modules/2.4.30-hf1/kernel/drivers/scsi/st+4288879050/96]
May 11 04:22:09 server kernel: [<f90f08ab>] [<f90f20c6>] [<f914d588>] [<f914f255>] [<f8b557d4>] [<f8b629ca>]
May 11 04:22:09 server kernel: [st:__insmod_st_O/lib/modules/2.4.30-hf1/kernel/drivers/scsi/st+4288713668/96] [st:__insmod_st_O/lib/modules/2.4.30-hf1/kernel/drivers/scsi/st+4288789620/96] [st:__insmod_st_O/lib/modules/2.4.30-hf1/kernel/drivers/scsi/st+4288713668/96] [st:__insmod_st_O/lib/modules/2.4.30-hf1/kernel/drivers/scsi/st+4288712232/96] [st:__insmod_st_O/lib/modules/2.4.30-hf1/kernel/drivers/scsi/st+4288755907/96] [st:__insmod_st_O/lib/modules/2.4.30-hf1/kernel/drivers/scsi/st+4288791678/96]
May 11 04:22:09 server kernel: [<f8b3a3c4>] [<f8b4cc74>] [<f8b3a3c4>] [<f8b39e28>] [<f8b448c3>] [<f8b4d47e>]
May 11 04:22:09 server kernel: [st:__insmod_st_O/lib/modules/2.4.30-hf1/kernel/drivers/scsi/st+4288800963/96] [st:__insmod_st_O/lib/modules/2.4.30-hf1/kernel/drivers/scsi/st+4288763515/96] [st:__insmod_st_O/lib/modules/2.4.30-hf1/kernel/drivers/scsi/st+4288713480/96] [st:__insmod_st_O/lib/modules/2.4.30-hf1/kernel/drivers/scsi/st+4288763309/96] [st:__insmod_st_O/lib/modules/2.4.30-hf1/kernel/drivers/scsi/st+4288815612/96] [__kfree_skb+254/352]
May 11 04:22:09 server kernel: [<f8b4f8c3>] [<f8b4667b>] [<f8b3a308>] [<f8b465ad>] [<f8b531fc>] [<c02387be>]
May 11 04:22:09 server kernel: [__kfree_skb+254/352] [st:__insmod_st_O/lib/modules/2.4.30-hf1/kernel/drivers/scsi/st+4294624756/96] [qdisc_restart+31/432] [dev_queue_xmit+383/880] [neigh_resolve_output+321/624] [__kfree_skb+254/352]
May 11 04:22:09 server kernel: [<c02387be>] [<f90dd5f4>] [<c02483df>] [<c023ccaf>] [<c0243af1>] [<c02387be>]
May 11 04:22:09 server kernel: [st:__insmod_st_O/lib/modules/2.4.30-hf1/kernel/drivers/scsi/st+4294624756/96] [qdisc_restart+31/432] [dev_queue_xmit+383/880] [neigh_resolve_output+321/624] [ip_finish_output2+184/336] [ip_finish_output2+0/336]
May 11 04:22:09 server kernel: [<f90dd5f4>] [<c02483df>] [<c023ccaf>] [<c0243af1>] [<c025a2d8>] [<c025a220>]
May 11 04:22:09 server kernel: [ip_finish_output2+0/336] [nf_hook_slow+478/528] [ip_finish_output2+0/336] [ip_output+334/480] [ip_finish_output2+0/336] [ip_queue_xmit2+213/671]
May 11 04:22:09 server kernel: [<c025a220>] [<c024763e>] [<c025a220>] [<c0258a4e>] [<c025a220>] [<c025a445>]
May 11 04:22:09 server kernel: [ip_queue_xmit2+0/671] [ip_queue_xmit2+0/671] [nf_hook_slow+478/528] [ip_queue_xmit2+0/671] [ip_queue_xmit+845/1536] [ip_queue_xmit2+0/671]
May 11 04:22:09 server kernel: [<c025a370>] [<c025a370>] [<c024763e>] [<c025a370>] [<c0258e2d>] [<c025a370>]
May 11 04:22:09 server kernel: [skb_checksum+92/800] [sock_def_readable+99/128] [tcp_rfree+0/32] [tcp_rfree+0/32] [tcp_rcv_established+1981/2640] [tcp_v4_do_rcv+314/352]
May 11 04:22:09 server kernel: [<c02399dc>] [<c0237f63>] [<c025d4a0>] [<c025d4a0>] [<c026a2bd>] [<c0272eda>]
May 11 04:22:09 server kernel: [tcp_v4_rcv+1726/1952] [locate_hd_struct+56/160] [st:__insmod_st_O/lib/modules/2.4.30-hf1/kernel/drivers/scsi/st+4288606771/96] [st:__insmod_st_O/lib/modules/2.4.30-hf1/kernel/drivers/scsi/st+4288606771/96] [st:__insmod_st_O/lib/modules/2.4.30-hf1/kernel/drivers/scsi/st+4288607559/96] [st:__insmod_st_O/lib/modules/2.4.30-hf1/kernel/drivers/scsi/st+4288679225/96]
May 11 04:22:09 server kernel: [<c02735be>] [<c01e4308>] [<f8b20233>] [<f8b20233>] [<f8b20547>] [<f8b31d39>]
May 11 04:22:09 server kernel: [st:__insmod_st_O/lib/modules/2.4.30-hf1/kernel/drivers/scsi/st+4288574821/96] [st:__insmod_st_O/lib/modules/2.4.30-hf1/kernel/drivers/scsi/st+4288906935/96] [st:__insmod_st_O/lib/modules/2.4.30-hf1/kernel/drivers/scsi/st+4288935563/96] [bh_action+106/112] [tasklet_hi_action+83/160] [sys_ioctl+677/785]
May 11 04:22:09 server kernel: [<f8b18565>] [<f8b696b7>] [<f8b7068b>] [<c01258ca>] [<c0125753>] [<c0158905>]
May 11 04:22:09 server kernel: [system_call+51/56]
May 11 04:22:09 server kernel: [<c0108fa7>]
#######################################################
May 11 04:26:28 server kernel: do_IRQ: stack overflow: 1000
May 11 04:26:28 server kernel: f3668998 000003e8 00000000 00000000 f3668000 f3668ac0 f3668a34 00000000
May 11 04:26:28 server kernel: c010d948 f3668000 0001961a 00000944 f3668ac0 f3668a34 00000000 00000944
May 11 04:26:28 server kernel: 00000018 f1960018 ffffff14 f90820a7 00000010 00000246 f3668a34 f40a9ec0
May 11 04:26:28 server kernel: Call Trace: [call_do_IRQ+5/13] [st:__insmod_st_O/lib/modules/2.4.30-hf1/kernel/drivers/scsi/st+4294250663/96] [st:__insmod_st_O/lib/modules/2.4.30-hf1/kernel/drivers/scsi/st+4294268736/96] [st:__insmod_st_O/lib/modules/2.4.30-hf1/kernel/drivers/scsi/st+4294250842/96] [st:__insmod_st_O/lib/modules/2.4.30-hf1/kernel/drivers/scsi/st+4294254302/96]
May 11 04:26:28 server kernel: Call Trace: [<c010d948>] [<f90820a7>] [<f9086740>] [<f908215a>] [<f9082ede>]
May 11 04:26:28 server kernel: [ip_queue_xmit2+0/671] [nf_iterate+85/160] [ip_queue_xmit2+0/671] [ip_queue_xmit2+0/671] [nf_hook_slow+247/528] [ip_queue_xmit2+0/671]
May 11 04:26:28 server kernel: [<c025a370>] [<c0247185>] [<c025a370>] [<c025a370>] [<c0247557>] [<c025a370>]
May 11 04:26:28 server kernel: [st:__insmod_st_O/lib/modules/2.4.30-hf1/kernel/drivers/scsi/st+4294266808/96] [ip_queue_xmit+845/1536] [ip_queue_xmit2+0/671] [tcp_v4_rcv+1726/1952] [tcp_v4_send_check+160/240] [tcp_transmit_skb+1001/1792]
May 11 04:26:28 server kernel: [<f9085fb8>] [<c0258e2d>] [<c025a370>] [<c02735be>] [<c0271c20>] [<c026bdc9>]
May 11 04:26:28 server kernel: [tcp_send_ack+132/208] [tcp_rfree+0/32] [tcp_rfree+0/32] [tcp_rcv_established+2042/2640] [tcp_v4_do_rcv+314/352] [tcp_v4_rcv+1726/1952]
May 11 04:26:28 server kernel: [<c026e824>] [<c025d4a0>] [<c025d4a0>] [<c026a2fa>] [<c0272eda>] [<c02735be>]
May 11 04:26:28 server kernel: [ip_local_deliver_finish+351/416] [ip_local_deliver_finish+0/416] [nf_hook_slow+478/528] [ip_local_deliver_finish+0/416] [ip_local_deliver+399/576] [ip_local_deliver_finish+0/416]
May 11 04:26:28 server kernel: [<c025581f>] [<c02556c0>] [<c024763e>] [<c02556c0>] [<c025514f>] [<c02556c0>]
May 11 04:26:28 server kernel: [ip_rcv_finish+553/700] [ip_rcv_finish+0/700] [ip_rcv_finish+0/700] [nf_hook_slow+478/528] [ip_rcv_finish+0/700] [ip_rcv+905/1216]
May 11 04:26:28 server kernel: [<c0255a89>] [<c0255860>] [<c0255860>] [<c024763e>] [<c0255860>] [<c0255589>]
May 11 04:26:28 server kernel: [ip_rcv_finish+0/700] [netif_receive_skb+485/544] [process_backlog+147/304] [net_rx_action+250/368] [do_softirq+118/224] [do_IRQ+244/304]
May 11 04:26:28 server kernel: [<c0255860>] [<c023d505>] [<c023d5d3>] [<c023d76a>] [<c01254c6>] [<c010b094>]
May 11 04:26:28 server kernel: [call_do_IRQ+5/13] [st:__insmod_st_O/lib/modules/2.4.30-hf1/kernel/drivers/scsi/st+4294701080/96] [<f914e620>] [<f914e7b6>] [<f914d473>] [<f914d5a6>]
May 11 04:26:28 server kernel: [<c010d948>] [<f90f0018>] [<f914e620>] [<f914e7b6>] [<f914d473>] [<f914d5a6>]
May 11 04:26:28 server kernel: [isofs_read_super+1131/2048] [<f914f255>] [st:__insmod_st_O/lib/modules/2.4.30-hf1/kernel/drivers/scsi/st+4288825300/96] [st:__insmod_st_O/lib/modules/2.4.30-hf1/kernel/drivers/scsi/st+4288879050/96] [st:__insmod_st_O/lib/modules/2.4.30-hf1/kernel/drivers/scsi/st+4288713668/96] [st:__insmod_st_O/lib/modules/2.4.30-hf1/kernel/drivers/scsi/st+4288789620/96]
May 11 04:26:28 server kernel: [<c018480b>] [<f914f255>] [<f8b557d4>] [<f8b629ca>] [<f8b3a3c4>] [<f8b4cc74>]
May 11 04:26:28 server kernel: [st:__insmod_st_O/lib/modules/2.4.30-hf1/kernel/drivers/scsi/st+4288713668/96] [st:__insmod_st_O/lib/modules/2.4.30-hf1/kernel/drivers/scsi/st+4288712232/96] [st:__insmod_st_O/lib/modules/2.4.30-hf1/kernel/drivers/scsi/st+4288755907/96] [st:__insmod_st_O/lib/modules/2.4.30-hf1/kernel/drivers/scsi/st+4288791678/96] [st:__insmod_st_O/lib/modules/2.4.30-hf1/kernel/drivers/scsi/st+4288800963/96] [st:__insmod_st_O/lib/modules/2.4.30-hf1/kernel/drivers/scsi/st+4288763515/96]
May 11 04:26:28 server kernel: [<f8b3a3c4>] [<f8b39e28>] [<f8b448c3>] [<f8b4d47e>] [<f8b4f8c3>] [<f8b4667b>]
May 11 04:26:28 server kernel: [st:__insmod_st_O/lib/modules/2.4.30-hf1/kernel/drivers/scsi/st+4288713480/96] [st:__insmod_st_O/lib/modules/2.4.30-hf1/kernel/drivers/scsi/st+4288763309/96] [st:__insmod_st_O/lib/modules/2.4.30-hf1/kernel/drivers/scsi/st+4288815612/96] [__kfree_skb+254/352] [__kfree_skb+254/352] [__kfree_skb+254/352]
May 11 04:26:28 server kernel: [<f8b3a308>] [<f8b465ad>] [<f8b531fc>] [<c02387be>] [<c02387be>] [<c02387be>]
May 11 04:26:28 server kernel: [st:__insmod_st_O/lib/modules/2.4.30-hf1/kernel/drivers/scsi/st+4294624756/96] [qdisc_restart+31/432] [dev_queue_xmit+383/880] [neigh_resolve_output+321/624] [ip_finish_output2+184/336] [ip_finish_output2+0/336]
May 11 04:26:28 server kernel: [<f90dd5f4>] [<c02483df>] [<c023ccaf>] [<c0243af1>] [<c025a2d8>] [<c025a220>]
May 11 04:26:28 server kernel: [ip_finish_output2+0/336] [nf_hook_slow+478/528] [ip_finish_output2+0/336] [ip_output+334/480] [ip_finish_output2+0/336] [ip_queue_xmit2+213/671]
May 11 04:26:28 server kernel: [<c025a220>] [<c024763e>] [<c025a220>] [<c0258a4e>] [<c025a220>] [<c025a445>]
May 11 04:26:28 server kernel: [ip_queue_xmit2+0/671] [ip_queue_xmit2+0/671] [nf_hook_slow+478/528] [ip_queue_xmit2+0/671] [ip_queue_xmit+845/1536] [ip_queue_xmit2+0/671]
May 11 04:26:28 server kernel: [<c025a370>] [<c025a370>] [<c024763e>] [<c025a370>] [<c0258e2d>] [<c025a370>]
May 11 04:26:28 server kernel: [nf_hook_slow+478/528] [tcp_v4_send_check+160/240] [tcp_transmit_skb+1001/1792] [sock_def_readable+99/128] [tcp_rfree+0/32] [tcp_rfree+0/32]
May 11 04:26:28 server kernel: [<c024763e>] [<c0271c20>] [<c026bdc9>] [<c0237f63>] [<c025d4a0>] [<c025d4a0>]
May 11 04:26:28 server kernel: [tcp_rcv_established+1981/2640] [tcp_v4_do_rcv+314/352] [tcp_v4_rcv+1726/1952] [ip_local_deliver_finish+351/416] [ip_local_deliver_finish+0/416] [nf_hook_slow+478/528]
May 11 04:26:28 server kernel: [<c026a2bd>] [<c0272eda>] [<c02735be>] [<c025581f>] [<c02556c0>] [<c024763e>]
May 11 04:26:28 server kernel: [ip_local_deliver_finish+0/416] [ip_local_deliver+399/576] [ip_local_deliver_finish+0/416] [ip_rcv_finish+553/700] [ip_rcv_finish+0/700] [ip_rcv_finish+0/700]
May 11 04:26:28 server kernel: [<c02556c0>] [<c025514f>] [<c02556c0>] [<c0255a89>] [<c0255860>] [<c0255860>]
May 11 04:26:28 server kernel: [nf_hook_slow+478/528] [schedule+738/1344] [schedule_timeout+94/160] [poll_freewait+68/80] [do_select+569/592] [sys_select+660/1248]
May 11 04:26:28 server kernel: [<c024763e>] [<c011cee2>] [<c011cb1e>] [<c01594d4>] [<c01598d9>] [<c0159bb4>]
May 11 04:26:28 server kernel: [sys_ioctl+677/785] [sys_time+21/80] [system_call+51/56]
May 11 04:26:28 server kernel: [<c0158905>] [<c0124bb5>] [<c0108fa7>]
#############################################################
May 11 04:30:06 server kernel: do_IRQ: stack overflow: 640
May 11 04:30:06 server kernel: f3668830 00000280 f2c73080 00000000 f196cd80 d2029180 d2029180 00000042
May 11 04:30:06 server kernel: c010d948 f196cd80 f008a0ec d2029180 d2029180 d2029180 00000042 f008a080
May 11 04:30:06 server kernel: f1960018 f5fd0018 ffffff14 c0238723 00000010 00000202 d2029180 d2029180
May 11 04:30:06 server kernel: Call Trace: [call_do_IRQ+5/13] [__kfree_skb+99/352] [st:__insmod_st_O/lib/modules/2.4.30-hf1/kernel/drivers/scsi/st+4294624756/96] [qdisc_restart+114/432] [dev_queue_xmit+383/880]
May 11 04:30:06 server kernel: Call Trace: [<c010d948>] [<c0238723>] [<f90dd5f4>] [<c0248432>] [<c023ccaf>]
May 11 04:30:06 server kernel: [ip_finish_output2+184/336] [ip_finish_output2+0/336] [ip_finish_output2+0/336] [nf_hook_slow+478/528] [ip_finish_output2+0/336] [ip_output+334/480]
May 11 04:30:06 server kernel: [<c025a2d8>] [<c025a220>] [<c025a220>] [<c024763e>] [<c025a220>] [<c0258a4e>]
May 11 04:30:06 server kernel: [ip_finish_output2+0/336] [ip_queue_xmit2+213/671] [ip_queue_xmit2+0/671] [ip_queue_xmit2+0/671] [nf_hook_slow+478/528] [ip_queue_xmit2+0/671]
May 11 04:30:06 server kernel: [<c025a220>] [<c025a445>] [<c025a370>] [<c025a370>] [<c024763e>] [<c025a370>]
May 11 04:30:06 server kernel: [ip_queue_xmit+845/1536] [ip_queue_xmit2+0/671] [tcp_v4_send_check+160/240] [tcp_transmit_skb+1001/1792] [tcp_send_ack+132/208] [tcp_rfree+0/32]
May 11 04:30:06 server kernel: [<c0258e2d>] [<c025a370>] [<c0271c20>] [<c026bdc9>] [<c026e824>] [<c025d4a0>]
May 11 04:30:06 server kernel: [tcp_rfree+0/32] [tcp_rcv_established+2042/2640] [tcp_v4_do_rcv+314/352] [tcp_v4_rcv+1726/1952] [ip_local_deliver_finish+351/416] [ip_local_deliver_finish+0/416]
May 11 04:30:06 server kernel: [<c025d4a0>] [<c026a2fa>] [<c0272eda>] [<c02735be>] [<c025581f>] [<c02556c0>]
May 11 04:30:06 server kernel: [nf_hook_slow+478/528] [ip_local_deliver_finish+0/416] [ip_local_deliver+399/576] [ip_local_deliver_finish+0/416] [ip_rcv_finish+553/700] [ip_rcv_finish+0/700]
May 11 04:30:06 server kernel: [<c024763e>] [<c02556c0>] [<c025514f>] [<c02556c0>] [<c0255a89>] [<c0255860>]
May 11 04:30:06 server kernel: [ip_rcv_finish+0/700] [nf_hook_slow+478/528] [ip_rcv_finish+0/700] [ip_rcv+905/1216] [ip_rcv_finish+0/700] [netif_receive_skb+485/544]
May 11 04:30:06 server kernel: [<c0255860>] [<c024763e>] [<c0255860>] [<c0255589>] [<c0255860>] [<c023d505>]
May 11 04:30:06 server kernel: [process_backlog+147/304] [net_rx_action+250/368] [do_softirq+118/224] [do_IRQ+244/304] [call_do_IRQ+5/13] [st:__insmod_st_O/lib/modules/2.4.30-hf1/kernel/drivers/scsi/st+4294806243/96]
May 11 04:30:06 server kernel: [<c023d5d3>] [<c023d76a>] [<c01254c6>] [<c010b094>] [<c010d948>] [<f9109ae3>]
May 11 04:30:06 server kernel: [st:__insmod_st_O/lib/modules/2.4.30-hf1/kernel/drivers/scsi/st+4294808580/96] [st:__insmod_st_O/lib/modules/2.4.30-hf1/kernel/drivers/scsi/st+4294815256/96] [st:__insmod_st_O/lib/modules/2.4.30-hf1/kernel/drivers/scsi/st+4294702477/96] [st:__insmod_st_O/lib/modules/2.4.30-hf1/kernel/drivers/scsi/st+4294702701/96] [st:__insmod_st_O/lib/modules/2.4.30-hf1/kernel/drivers/scsi/st+4294703097/96] [st:__insmod_st_O/lib/modules/2.4.30-hf1/kernel/drivers/scsi/st+4294703275/96]
May 11 04:30:06 server kernel: [<f910a404>] [<f910be18>] [<f90f058d>] [<f90f066d>] [<f90f07f9>] [<f90f08ab>]
May 11 04:30:06 server kernel: [st:__insmod_st_O/lib/modules/2.4.30-hf1/kernel/drivers/scsi/st+4294709446/96] [<f914d588>] [isofs_read_super+1131/2048] [<f914f255>] [st:__insmod_st_O/lib/modules/2.4.30-hf1/kernel/drivers/scsi/st+4288825300/96] [st:__insmod_st_O/lib/modules/2.4.30-hf1/kernel/drivers/scsi/st+4288879050/96]
May 11 04:30:06 server kernel: [<f90f20c6>] [<f914d588>] [<c018480b>] [<f914f255>] [<f8b557d4>] [<f8b629ca>]
May 11 04:30:06 server kernel: [st:__insmod_st_O/lib/modules/2.4.30-hf1/kernel/drivers/scsi/st+4288713668/96] [st:__insmod_st_O/lib/modules/2.4.30-hf1/kernel/drivers/scsi/st+4288789620/96] [st:__insmod_st_O/lib/modules/2.4.30-hf1/kernel/drivers/scsi/st+4288713668/96] [st:__insmod_st_O/lib/modules/2.4.30-hf1/kernel/drivers/scsi/st+4288712232/96] [st:__insmod_st_O/lib/modules/2.4.30-hf1/kernel/drivers/scsi/st+4288755907/96] [st:__insmod_st_O/lib/modules/2.4.30-hf1/kernel/drivers/scsi/st+4288791678/96]
May 11 04:30:06 server kernel: [<f8b3a3c4>] [<f8b4cc74>] [<f8b3a3c4>] [<f8b39e28>] [<f8b448c3>] [<f8b4d47e>]
May 11 04:30:06 server kernel: [st:__insmod_st_O/lib/modules/2.4.30-hf1/kernel/drivers/scsi/st+4288800963/96] [st:__insmod_st_O/lib/modules/2.4.30-hf1/kernel/drivers/scsi/st+4288763515/96] [st:__insmod_st_O/lib/modules/2.4.30-hf1/kernel/drivers/scsi/st+4288713480/96] [st:__insmod_st_O/lib/modules/2.4.30-hf1/kernel/drivers/scsi/st+4288763309/96] [st:__insmod_st_O/lib/modules/2.4.30-hf1/kernel/drivers/scsi/st+4288815612/96] [__kfree_skb+254/352]
May 11 04:30:06 server kernel: [<f8b4f8c3>] [<f8b4667b>] [<f8b3a308>] [<f8b465ad>] [<f8b531fc>] [<c02387be>]
May 11 04:30:06 server kernel: [netif_rx+147/496] [st:__insmod_st_O/lib/modules/2.4.30-hf1/kernel/drivers/scsi/st+4294626081/96] [st:__insmod_st_O/lib/modules/2.4.30-hf1/kernel/drivers/scsi/st+4294624756/96] [__kfree_skb+254/352] [st:__insmod_st_O/lib/modules/2.4.30-hf1/kernel/drivers/scsi/st+4294624756/96] [__kfree_skb+254/352]
May 11 04:30:06 server kernel: [<c023cfb3>] [<f90ddb21>] [<f90dd5f4>] [<c02387be>] [<f90dd5f4>] [<c02387be>]
May 11 04:30:06 server kernel: [__kfree_skb+254/352] [st:__insmod_st_O/lib/modules/2.4.30-hf1/kernel/drivers/scsi/st+4294624756/96] [qdisc_restart+31/432] [dev_queue_xmit+383/880] [ip_finish_output2+184/336] [ip_finish_output2+0/336]
May 11 04:30:06 server kernel: [<c02387be>] [<f90dd5f4>] [<c02483df>] [<c023ccaf>] [<c025a2d8>] [<c025a220>]
May 11 04:30:06 server kernel: [ip_finish_output2+0/336] [nf_hook_slow+478/528] [ip_finish_output2+0/336] [ip_output+334/480] [ip_finish_output2+0/336] [ip_queue_xmit2+213/671]
May 11 04:30:06 server kernel: [<c025a220>] [<c024763e>] [<c025a220>] [<c0258a4e>] [<c025a220>] [<c025a445>]
May 11 04:30:06 server kernel: [ip_queue_xmit2+0/671] [ip_queue_xmit2+0/671] [nf_hook_slow+478/528] [ip_queue_xmit2+0/671] [ip_queue_xmit+845/1536] [ip_queue_xmit2+0/671]
May 11 04:30:06 server kernel: [<c025a370>] [<c025a370>] [<c024763e>] [<c025a370>] [<c0258e2d>] [<c025a370>]
May 11 04:30:06 server kernel: [sock_def_readable+99/128] [tcp_rfree+0/32] [tcp_rfree+0/32] [tcp_rcv_established+1981/2640] [tcp_v4_do_rcv+314/352] [tcp_v4_rcv+1726/1952]
May 11 04:30:06 server kernel: [<c0237f63>] [<c025d4a0>] [<c025d4a0>] [<c026a2bd>] [<c0272eda>] [<c02735be>]
May 11 04:30:06 server kernel: [ip_local_deliver_finish+351/416] [ip_local_deliver_finish+0/416] [nf_hook_slow+478/528] [ip_local_deliver_finish+0/416] [ip_local_deliver+399/576] [ip_local_deliver_finish+0/416]
May 11 04:30:06 server kernel: [<c025581f>] [<c02556c0>] [<c024763e>] [<c02556c0>] [<c025514f>] [<c02556c0>]
May 11 04:30:06 server kernel: [ip_rcv_finish+553/700] [__alloc_pages+100/640] [poll_freewait+68/80] [do_select+569/592] [sys_select+660/1248] [sys_ioctl+677/785]
May 11 04:30:06 server kernel: [<c0255a89>] [<c013e624>] [<c01594d4>] [<c01598d9>] [<c0159bb4>] [<c0158905>]
May 11 04:30:06 server kernel: [sys_time+21/80] [system_call+51/56]
May 11 04:30:06 server kernel: [<c0124bb5>] [<c0108fa7>]
^ permalink raw reply
* Re: [1/1] connector/CBUS: new messaging subsystem. Revision number next.
From: Evgeniy Polyakov @ 2005-05-11 6:48 UTC (permalink / raw)
To: Dmitry Torokhov
Cc: dmitry.torokhov, netdev, Greg KH, Jamal Hadi Salim, Kay Sievers,
Herbert Xu, James Morris, Guillaume Thouvenin, linux-kernel,
Andrew Morton, Thomas Graf, Jay Lan
In-Reply-To: <200505110046.28797.dtor_core@ameritech.net>
[-- Attachment #1: Type: text/plain, Size: 12328 bytes --]
On Wed, 2005-05-11 at 00:46 -0500, Dmitry Torokhov wrote:
> > > > > >
> > > > > > > - drop cn_dev - there is only one connector;
> > > > > >
> > > > > > There may be several connectors with different socket types.
> > > > > > I use it in my environment for tests.
> > > > > >
> > > > >
> > > > > Why do you need that? u32 ID space is not enough?
> > > >
> > > > They use different netlink sockets.
> > > > One of them can fail during initialisation, for example...
> > >
> > > Are you saying that there more chances successfully allocating 2
> > > netlink sockets than one?
> >
> > Of course - one may be in use by ipt_ULOG for example.
>
> And what does it have to do with multiple connectors and different sockets?
> You just need to find one free socket, once you found it you have only one
> instance of connector running.
Because one connector may be added for general-purpose socket like
existing one,
any other connectors can be added for different transport layer for
example.
> > Although probably it could be better to get new unit from David Miller.
> > I want to create some kind of unused-in-kernel socket number -
> > each netlink user outside kernel might use it without kernel users
> > bothering.
>
> Right.
>
> > > >
> > > > > > > - simplify cn_notify_request to carry only one range - it simplifies code
> > > > > > > quite a bit - nothing stops clientd from sending several notification
> > > > > > > requests;
> > > > > >
> > > > > > ? Nothing stops clients from sending several notification requests now.
> > > > > > I do not see how it simplifies the things?
> > > > > > Feel free to set idx_num and val_num to 1 and you will have the same.
> > > > >
> > > > > Compare the implementaion - without variable-length notification
> > > > > requests the code is much simplier and flexible.
> > > >
> > > > Hmm, old request had a length, new one - does not.
> > > > But instead of atomically addition of the whole set, one need to send
> > > > couple of events.
> > > > I do not see any profit here.
> > >
> > > Smaller kernel code, simpler notification structure, ability to stop
> > > listenig to a certain events without re-registering the rest of the
> > > block.
> >
> > As I said - set idx and val numbers to one and you will have the same
> > behaviour.
> > Code is neither smaller [ok, you removed a loop] nor simplier
> > - it is functionality decreasing.
>
> Actually, I would do yet another step and just put something like
>
> int cn_request_notify(u32 group, u32 idx, u32 idx_range, u32 val, u32 val_range)
> int cn_cancel_notify(u32 group, u32 idx, u32 idx_range, u32 val, u32 val_range)
>
> into connector core. Then drivers could do something like this:
>
> static int cn_test_want_notify(void)
> {
> int error;
>
> error = cn_request_notify(1, cn_test_id.idx, 10, cn_test_id.val, 10);
> if (error)
> return error;
>
> error = cn_request_notify(1, cn_test_id.idx, 10, cn_test_id.val + 20, 10);
> if (error) {
> /* unregister */
> cn_cancel_notify(1, cn_test_id.idx, 10, cn_test_id.val, 10);
> return error;
> }
>
> return 0;
> }
>
> No messing up with memory allocations and complex message structures.
Yep, it could be done, but it has nothing in common
with request structure changes.
I will place it into TODO, thanks.
> > > > > > Connector with your changes just does not work.
> > > > >
> > > > > As in "I tried to run it and adjusted the application for the new
> > > > > cn_notify_req size and it does not work" or "I am saying it does not
> > > > > work"?
> > > >
> > > > Second, i.e. "I am saing it does not work".
> > > > With your changes userspace just does not see any message being sent by kernelspace.
> > >
> > > Would you care to elaborate on this please. As far as I know
> > > kernel->userspace path was not changed at all.
> >
> > I inserted your module and test module, which sends a message using cn_netlink_send()
> > one time per second.
> > Usersace application bound to -1 group does not receive that messages.
>
> Works for me:
>
> root@core cntest]# ./a.out
> recvfrom successful
> counter = 0
> recvfrom successful
> counter = 1
> recvfrom successful
> counter = 2
> recvfrom successful
> counter = 3
> recvfrom successful
> counter = 4
> recvfrom successful
> counter = 5
> recvfrom successful
> counter = 6
> recvfrom successful
> counter = 7
> recvfrom successful
> counter = 8
>
> (I won't ever show anyone that gutted piece of ulogd that produced these
> messages :) )
>
> Try recompiling your userspace application, messages were changed a bit.
I recompiled everything - still does not work.
Application is just reading from socket after polling, it even does not
know about messages itself.
> > > > > > Dmitry, I do not understand why do you change things in so radical manner,
> > > > > > especially when there are no 1. new interesting features, 2. no performance
> > > > > > improvements, 3. no right design changes.
> > > > >
> > > > > There is a right design change - it keeps the code simple. It peels
> > > > > all the layers you invented for something that does not really need
> > > > > layers - connector is just a simple wrapper over raw netlink.
> > > >
> > > > That is your opinion.
> > > >
> > > > Code is already as simple as possible, since it is layered and each
> > > > piece of code is separated from different logical entities.
> > > >
> > > > > When I look at the code I see the mess of unneeded refcounting, wrong
> > > > > kinds of locking, over-engineering. Of course I want to change it.
> > > >
> > > > There are no unneded reference counting now.
> > > > Locking was not wrong - you only placed semaphore instead of notify BH lock
> > > > just to have ability to sleep under the lock and send callback addition/removing
> > > > notification with GFP_KERNEL mask,
> > > > and it is probably right change from the first view, as I said,
> > > > but it can be implemented in a different way.
> > >
> > > That's what I was referring to as "wring locking". Notifications do
> > > not need to stop bottom handlers, a spinlock or semaphore willl work
> > > just fine. SInce I wanted to allocate memory only after I determined
> > > that it is a new notify request a semaphore was a better choise.
> >
> > I will think of it, but in long-time I do not like such approach -
> > it is some kind of deadlock - we have no memory, so we sleep in allocation,
> > so we can not remove callback since it will sleep in allocation...
>
> There is no deadlock. We are not waiting for memory to be freed, we are
> waiting for paging.
Yep. That is why pool is better.
Actually I even want some kind of pools for skb, but do not think it
will be a good idea.
That will require several of them for different sizes and so on...
> > > > > Keep it simple. Except for your testing environment why do you need to
> > > > > have separate netlink sockets? You have an ample ID space. Why do you
> > > > > need separate cn_dev and cn_queue_dev (and in the end have them at
> > > > > all)? Why do you need "input" handler, do you expect to use different
> > > > > handlers? If so which? And why? Do you expect to use other transports?
> > > > > If so which? And why? You need to have justifiction for every
> > > > > additional layer you putting on.
> > > >
> > > > Sigh.
> > > > It was already answered so many times, as far as I remember...
> > > > Ok, let's do it again.
> > > >
> > > > Several netlink sockets - if you can see, connector uses NETLINK_ULOG
> > > > for it's notification, you can not use the same socket twice, so you
> > > > may have new device with new socket, or several of them, and system
> > > > may work if one initialisation failed.
> > >
> > > Or it may not. Why do you think that having several sockets is safer
> > > than using one? After all, it is just a matter of setting it up and
> > > finally allocating a separate socket number for connector.
> >
> > Yes, there can be such a way.
> > But when connector was not in mainline it used TUN/TAP netlink sockets
> > and tried to get one by one, until successfully allocated.
> > Using dmesg userspace could find needed socket number for itself.
> >
>
> And in the end you have one connector bound to some socket. There is no
> multiple connectors - the goal (as far as I see it) was to have single
> transport web of messages and callbacks for entire kernel. It does not make
> sense to divide it.
Callbacks and messages live in cn_queue_dev - it could be the same for
different transport layers, transport layer - it is cn_dev, they might
be different.
You want to remove even ability to extend the system, since you do not
see and want
to understand that with existing design it could be somehow extended,
with your
patch - it will be completely closed system.
> > > > cn_dev is connector device structure, cn_queue_dev - it's queueing part.
> > > > input handler is needed for ability to receive messages.
> > >
> > > You chose not to understand my question. Why do you need to have is as
> > > a pointer? Do you expect to have different input handler?
> >
> > In runtime - no.
> > cn_dev is a main transport structure, while cn_queue_dev is for
> > callback queue handling.
> >
> > > > Couple of next questions can be answered in one reply:
> > > > original connector's idea was to implement notification based
> > > > not on top of netlink protocol, but using ioctls of char device,
> > > > so there could be different connector device, input method,
> > > > transport and so on. Probably I will not implement it thought.
> > >
> > > IOW this all is remnants of an older design. You have no plans to
> > > utilize the framework and therefore it is not needed anymore and can
> > > be dropped.
> >
> > Different connector devices still there.
> > They use different sockets.
> > Concider netlink2 [with guaranteed delivering - I have some ideas about it] -
> > one just needs to add a new connector device, no new braindumping and
> > recalling how it was cool before.
> > It will require some changes to the connector structure, but it will not
> > be complete rewrite because some time ago someone decided we do not
> > need it and completely removed anything that lives outside simple
> > netlink wrapper.
>
> Think about implications some more - userspace needs to be aware and chose
> what socket to bind to, applications will change... Messages will change.
> In-kernel users will also have to select kind of transport they want to use
> (guaranteed delivery or not) - it looks like you'll be better off having
> these connectors completely separated.
No, I do not think messages will be changed, they are quite
selfcontained,
may be some kind of flags - but existing 32bit length can be closed to
16 bit -
16kb is quite enough, especiall with 1024byte checks in connector
itself.
It is only transport layer that will be changed.
Naive approach is just attaching a timer to selected skb and reinit it
each time
socket has overrun until some timeout, of course with some simple
congestion mechanism.
Logical high-level messages were separated specially to hide transport
layer.
We do not change route table when switching from UDP to TCP routes,
and connector message header is exactly a "route" for included message.
Using flags in connector message header one may select desired connector
from
kernelspace, binding to different socket or using ioctl selects
appropriate
connector from userspace.
And even better - kernelspace might use not flags, but might add/remove
callback
to appropriate transport device and thus cn_netlink_send will select
required device using message idx.val.
Your changes break this idea completely - it will require new queues,
new request lists,
new set of messages, new ids - almost all will be duplicated instead of
new device
strucutre which is 28 bytes in size and has a pointer to the common
part, which
has reference counter for such kind of use.
--
Evgeniy Polyakov
Crash is better than data corruption -- Arthur Grabowski
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 189 bytes --]
^ permalink raw reply
* Re: tg3 support broken on PPC, a workaround
From: Manuel Perez Ayala @ 2005-05-11 6:04 UTC (permalink / raw)
To: Michael Chan; +Cc: netdev
In-Reply-To: <1115743966.8570.26.camel@rh4>
>
> I assume you have a 5700 or 5701 as this code that controls the DMA
> boundaries only affects those devices. Please confirm with the lspci
> output or tg3's probing output.
>
This is the lspci output
0000:00:0b.0 Host bridge: Apple Computer Inc. UniNorth 1.5 AGP
0000:00:10.0 VGA compatible controller: nVidia Corporation NV11
[GeForce2 MX/MX
400] (rev b2)
0001:10:0b.0 Host bridge: Apple Computer Inc. UniNorth 1.5 PCI
0001:10:13.0 Ethernet controller: Broadcom Corporation NetXtreme
BCM5700 Gigabit
Ethernet (rev 12)
0001:10:17.0 ff00: Apple Computer Inc. KeyLargo Mac I/O (rev 03)
0001:10:18.0 USB Controller: Apple Computer Inc. KeyLargo USB
0001:10:19.0 USB Controller: Apple Computer Inc. KeyLargo USB
0002:20:0b.0 Host bridge: Apple Computer Inc. UniNorth 1.5 Internal PCI
0002:20:0e.0 ffff: Lucent Microelectronics FW323 (rev ff)
0002:20:0f.0 Ethernet controller: Apple Computer Inc. UniNorth GMAC (Sun GEM)
(rev 01)
> Can you print out the value of tp->dma_rwctrl in hex just before it is
> written to the register in the line:
>
> tw32(TG3PCI_DMA_RW_CTRL, tp->dma_rwctrl);
>
> Please do this for the working and non-working driver versions.
Value of tp->dma_rwctrl
non-working driver:
On init of driver
tg3.c:v3.10 (September 14, 2004)
PCI: Enabling device 0001:10:13.0 (0014 -> 0016)
tg3: tg3_test_dma #1: 76FF280F
eth1: Tigon3 [partno(BCM95700A6) rev 7102 PHY(5401)] (PCI:33MHz:64-bit)
10/100/1000BaseT Ethernet 00:04:76:3b:51:ae
eth1: RXcsums[1] LinkChgREG[1] MIirq[1] ASF[0] Split[0] WireSpeed[0] TSOcap[0]
I've added the output of the tp->dma_rwctrl value in hex to kern messages:
tg3: tg3_test_dma #1: 76FF280F
On init of network interface:
tg3: tg3_reset_hw: 76FF280F
tg3: eth1: Link is up at 1000 Mbps, full duplex.
tg3: eth1: Flow control is on for TX and on for RX.
I've added too the output of the tp->dma_rwctrl value in hex in the
tg3_reset_hw
function:
tg3: tg3_reset_hw: 76FF280F
Working driver:
On init of driver:
tg3.c:v3.10 (September 14, 2004)
PCI: Enabling device 0001:10:13.0 (0014 -> 0016)
tg3: tg3_test_dma #1: 76FF120F
eth1: Tigon3 [partno(BCM95700A6) rev 7102 PHY(5401)] (PCI:33MHz:64-bit)
10/100/1000BaseT Ethernet 00:04:76:3b:51:ae
eth1: RXcsums[1] LinkChgREG[1] MIirq[1] ASF[0] Split[0] WireSpeed[0] TSOcap[0]
And on init of the network interface:
tg3: tg3_reset_hw: 76FF120F
tg3: eth1: Link is up at 1000 Mbps, full duplex.
tg3: eth1: Flow control is on for TX and on for RX.
Hope this will be useful
Thanks
----------
Manuel Perez Ayala
mperaya@alcazaba.unex.es
Facultad de Biblioteconomía y Documentación
Universidad de Extremadura
^ permalink raw reply
* Re: [1/1] connector/CBUS: new messaging subsystem. Revision number next.
From: Dmitry Torokhov @ 2005-05-11 5:46 UTC (permalink / raw)
To: johnpol
Cc: dmitry.torokhov, netdev, Greg KH, Jamal Hadi Salim, Kay Sievers,
Herbert Xu, James Morris, Guillaume Thouvenin, linux-kernel,
Andrew Morton, Thomas Graf, Jay Lan
In-Reply-To: <20050510222424.5b411c89@zanzibar.2ka.mipt.ru>
On Tuesday 10 May 2005 13:24, Evgeniy Polyakov wrote:
> On Tue, 10 May 2005 12:50:55 -0500
> Dmitry Torokhov <dmitry.torokhov@gmail.com> wrote:
>
> > On 5/10/05, Evgeniy Polyakov <johnpol@2ka.mipt.ru> wrote:
> > > On Tue, 10 May 2005 09:56:45 -0500
> > > Dmitry Torokhov <dmitry.torokhov@gmail.com> wrote:
> > >
> > > > On 5/10/05, Evgeniy Polyakov <johnpol@2ka.mipt.ru> wrote:
> > > > > On Tue, 10 May 2005 01:18:46 -0500
> > > > > Dmitry Torokhov <dtor_core@ameritech.net> wrote:
> > > > >
> > > > > > - drop cn_dev - there is only one connector;
> > > > >
> > > > > There may be several connectors with different socket types.
> > > > > I use it in my environment for tests.
> > > > >
> > > >
> > > > Why do you need that? u32 ID space is not enough?
> > >
> > > They use different netlink sockets.
> > > One of them can fail during initialisation, for example...
> >
> > Are you saying that there more chances successfully allocating 2
> > netlink sockets than one?
>
> Of course - one may be in use by ipt_ULOG for example.
And what does it have to do with multiple connectors and different sockets?
You just need to find one free socket, once you found it you have only one
instance of connector running.
> Although probably it could be better to get new unit from David Miller.
> I want to create some kind of unused-in-kernel socket number -
> each netlink user outside kernel might use it without kernel users
> bothering.
Right.
> > >
> > > > > > - simplify cn_notify_request to carry only one range - it simplifies code
> > > > > > quite a bit - nothing stops clientd from sending several notification
> > > > > > requests;
> > > > >
> > > > > ? Nothing stops clients from sending several notification requests now.
> > > > > I do not see how it simplifies the things?
> > > > > Feel free to set idx_num and val_num to 1 and you will have the same.
> > > >
> > > > Compare the implementaion - without variable-length notification
> > > > requests the code is much simplier and flexible.
> > >
> > > Hmm, old request had a length, new one - does not.
> > > But instead of atomically addition of the whole set, one need to send
> > > couple of events.
> > > I do not see any profit here.
> >
> > Smaller kernel code, simpler notification structure, ability to stop
> > listenig to a certain events without re-registering the rest of the
> > block.
>
> As I said - set idx and val numbers to one and you will have the same
> behaviour.
> Code is neither smaller [ok, you removed a loop] nor simplier
> - it is functionality decreasing.
Actually, I would do yet another step and just put something like
int cn_request_notify(u32 group, u32 idx, u32 idx_range, u32 val, u32 val_range)
int cn_cancel_notify(u32 group, u32 idx, u32 idx_range, u32 val, u32 val_range)
into connector core. Then drivers could do something like this:
static int cn_test_want_notify(void)
{
int error;
error = cn_request_notify(1, cn_test_id.idx, 10, cn_test_id.val, 10);
if (error)
return error;
error = cn_request_notify(1, cn_test_id.idx, 10, cn_test_id.val + 20, 10);
if (error) {
/* unregister */
cn_cancel_notify(1, cn_test_id.idx, 10, cn_test_id.val, 10);
return error;
}
return 0;
}
No messing up with memory allocations and complex message structures.
> > > > > Connector with your changes just does not work.
> > > >
> > > > As in "I tried to run it and adjusted the application for the new
> > > > cn_notify_req size and it does not work" or "I am saying it does not
> > > > work"?
> > >
> > > Second, i.e. "I am saing it does not work".
> > > With your changes userspace just does not see any message being sent by kernelspace.
> >
> > Would you care to elaborate on this please. As far as I know
> > kernel->userspace path was not changed at all.
>
> I inserted your module and test module, which sends a message using cn_netlink_send()
> one time per second.
> Usersace application bound to -1 group does not receive that messages.
Works for me:
root@core cntest]# ./a.out
recvfrom successful
counter = 0
recvfrom successful
counter = 1
recvfrom successful
counter = 2
recvfrom successful
counter = 3
recvfrom successful
counter = 4
recvfrom successful
counter = 5
recvfrom successful
counter = 6
recvfrom successful
counter = 7
recvfrom successful
counter = 8
(I won't ever show anyone that gutted piece of ulogd that produced these
messages :) )
Try recompiling your userspace application, messages were changed a bit.
> > > > > Dmitry, I do not understand why do you change things in so radical manner,
> > > > > especially when there are no 1. new interesting features, 2. no performance
> > > > > improvements, 3. no right design changes.
> > > >
> > > > There is a right design change - it keeps the code simple. It peels
> > > > all the layers you invented for something that does not really need
> > > > layers - connector is just a simple wrapper over raw netlink.
> > >
> > > That is your opinion.
> > >
> > > Code is already as simple as possible, since it is layered and each
> > > piece of code is separated from different logical entities.
> > >
> > > > When I look at the code I see the mess of unneeded refcounting, wrong
> > > > kinds of locking, over-engineering. Of course I want to change it.
> > >
> > > There are no unneded reference counting now.
> > > Locking was not wrong - you only placed semaphore instead of notify BH lock
> > > just to have ability to sleep under the lock and send callback addition/removing
> > > notification with GFP_KERNEL mask,
> > > and it is probably right change from the first view, as I said,
> > > but it can be implemented in a different way.
> >
> > That's what I was referring to as "wring locking". Notifications do
> > not need to stop bottom handlers, a spinlock or semaphore willl work
> > just fine. SInce I wanted to allocate memory only after I determined
> > that it is a new notify request a semaphore was a better choise.
>
> I will think of it, but in long-time I do not like such approach -
> it is some kind of deadlock - we have no memory, so we sleep in allocation,
> so we can not remove callback since it will sleep in allocation...
There is no deadlock. We are not waiting for memory to be freed, we are
waiting for paging.
> > > > Keep it simple. Except for your testing environment why do you need to
> > > > have separate netlink sockets? You have an ample ID space. Why do you
> > > > need separate cn_dev and cn_queue_dev (and in the end have them at
> > > > all)? Why do you need "input" handler, do you expect to use different
> > > > handlers? If so which? And why? Do you expect to use other transports?
> > > > If so which? And why? You need to have justifiction for every
> > > > additional layer you putting on.
> > >
> > > Sigh.
> > > It was already answered so many times, as far as I remember...
> > > Ok, let's do it again.
> > >
> > > Several netlink sockets - if you can see, connector uses NETLINK_ULOG
> > > for it's notification, you can not use the same socket twice, so you
> > > may have new device with new socket, or several of them, and system
> > > may work if one initialisation failed.
> >
> > Or it may not. Why do you think that having several sockets is safer
> > than using one? After all, it is just a matter of setting it up and
> > finally allocating a separate socket number for connector.
>
> Yes, there can be such a way.
> But when connector was not in mainline it used TUN/TAP netlink sockets
> and tried to get one by one, until successfully allocated.
> Using dmesg userspace could find needed socket number for itself.
>
And in the end you have one connector bound to some socket. There is no
multiple connectors - the goal (as far as I see it) was to have single
transport web of messages and callbacks for entire kernel. It does not make
sense to divide it.
> > > cn_dev is connector device structure, cn_queue_dev - it's queueing part.
> > > input handler is needed for ability to receive messages.
> >
> > You chose not to understand my question. Why do you need to have is as
> > a pointer? Do you expect to have different input handler?
>
> In runtime - no.
> cn_dev is a main transport structure, while cn_queue_dev is for
> callback queue handling.
>
> > > Couple of next questions can be answered in one reply:
> > > original connector's idea was to implement notification based
> > > not on top of netlink protocol, but using ioctls of char device,
> > > so there could be different connector device, input method,
> > > transport and so on. Probably I will not implement it thought.
> >
> > IOW this all is remnants of an older design. You have no plans to
> > utilize the framework and therefore it is not needed anymore and can
> > be dropped.
>
> Different connector devices still there.
> They use different sockets.
> Concider netlink2 [with guaranteed delivering - I have some ideas about it] -
> one just needs to add a new connector device, no new braindumping and
> recalling how it was cool before.
> It will require some changes to the connector structure, but it will not
> be complete rewrite because some time ago someone decided we do not
> need it and completely removed anything that lives outside simple
> netlink wrapper.
Think about implications some more - userspace needs to be aware and chose
what socket to bind to, applications will change... Messages will change.
In-kernel users will also have to select kind of transport they want to use
(guaranteed delivery or not) - it looks like you'll be better off having
these connectors completely separated.
--
Dmitry
^ permalink raw reply
* Re: assertion (!atomic_read(&sk->sk_rmem_alloc)) failed at net/netlink/af_netlink.c (122)
From: Herbert Xu @ 2005-05-11 0:58 UTC (permalink / raw)
To: Ken-ichirou MATSUZAWA; +Cc: netdev, David S. Miller
In-Reply-To: <20050510220751.GA459@gondor.apana.org.au>
[-- Attachment #1: Type: text/plain, Size: 1215 bytes --]
On Wed, May 11, 2005 at 08:07:51AM +1000, herbert wrote:
> On Tue, May 10, 2005 at 12:43:32PM +0000, Ken-ichirou MATSUZAWA wrote:
> >
> > Everytime on shutting down, the subject message is logged.
> > It seems that the reason is do_one_broadcast may preempt
> > netlink_recvmsg. Added patch to 2.6.12-rc4 seems resolving this
> > problem.
>
> Thanks for the patch. To help me understand the problem better,
> could you please tell me:
I think I understand your patch now. What's happening is that
1) The skb is sent to socket 1.
2) Someone does a recvmsg on socket 1 and drops the ref on the skb.
Note that the rmalloc is not returned at this point since the
skb is still referenced.
3) The same skb is now sent to socket 2.
I agree with your solution except that we should still do the skb_get
if we can. Here is my version where we only do the skb_get at the
start.
Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
Please let me know whether this works for you or not.
Thanks,
--
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
[-- Attachment #2: p --]
[-- Type: text/plain, Size: 690 bytes --]
--- a/net/netlink/af_netlink.c
+++ b/net/netlink/af_netlink.c
@@ -735,12 +735,7 @@ static inline int do_one_broadcast(struc
sock_hold(sk);
if (p->skb2 == NULL) {
- if (atomic_read(&p->skb->users) != 1) {
- p->skb2 = skb_clone(p->skb, p->allocation);
- } else {
- p->skb2 = p->skb;
- atomic_inc(&p->skb->users);
- }
+ p->skb2 = skb_clone(p->skb, p->allocation);
}
if (p->skb2 == NULL) {
netlink_overrun(sk);
@@ -776,7 +771,8 @@ int netlink_broadcast(struct sock *ssk,
info.delivered = 0;
info.allocation = allocation;
info.skb = skb;
- info.skb2 = NULL;
+ info.skb2 = skb;
+ skb_get(skb);
/* While we sleep in clone, do not allow to change socket list */
^ permalink raw reply
* Re: tg3 support broken on PPC, a workaround
From: David S. Miller @ 2005-05-10 23:36 UTC (permalink / raw)
To: iod00d; +Cc: rick.jones2, netdev
In-Reply-To: <20050510232132.GZ5495@esmail.cup.hp.com>
From: Grant Grundler <iod00d@hp.com>
Subject: Re: tg3 support broken on PPC, a workaround
Date: Tue, 10 May 2005 16:21:32 -0700
> HP PCI bus controllers don't disconnect after a cacheline.
> The latest "LBA" (aka Mercury) will disconnect on 4k page
> boundaries. Alex Williamson confirmed.
Ok, good data point.
> Has anyone confirmed PPC, PPC64 and Alpha PCI/PCI-X bus
> controllers do the same?
>
> ISTR MMRBC (PCI-X only) allows one to specify
> shorter blocks. I'd have to look that up again.
BTW, I just noticed this in the NetBSD tigon3 driver:
#ifdef __brokenalpha__
/*
* Must insure that we do not cross an 8K (bytes) boundary
* for DMA reads. Our highest limit is 1K bytes. This is a
* restriction on some ALPHA platforms with early revision
* 21174 PCI chipsets, such as the AlphaPC 164lx
*/
PCI_SETBIT(sc, BGE_PCI_DMA_RW_CTL, BGE_PCI_READ_BNDRY_1024, 4);
#endif
This whole DMA boundary issue is turning into a very non-trivial one.
And if it is really true that plain PCI boundary crossing cannot be
controlled on non 5700/5701 chips, the tigon3 is certainly not going
to work reliably on systems such as the Alpha mentioned above.
^ permalink raw reply
* Re: tg3 support broken on PPC, a workaround
From: Grant Grundler @ 2005-05-10 23:21 UTC (permalink / raw)
To: Rick Jones; +Cc: netdev, Grant Grundler
In-Reply-To: <42813205.1040709@hp.com>
On Tue, May 10, 2005 at 03:13:25PM -0700, Rick Jones wrote:
> David S. Miller wrote:
> >We really should be disconnecting at single cacheline boundaries
> >on RISC systems. The PCI controllers on RISC machines are
> >going to disconnect the tg3 when it crosses a cache line
> >boundary, so all these setting do is waste PCI bandwidth.
>
> It is my understanding that PA-RISC and IA64 controllers behave
> differently. For confirmation one way or the other, I've cc'd someone who
> could talk about it much more cogently than I.
Yup, thanks rick.
Dave,
HP PCI bus controllers don't disconnect after a cacheline.
The latest "LBA" (aka Mercury) will disconnect on 4k page
boundaries. Alex Williamson confirmed.
Has anyone confirmed PPC, PPC64 and Alpha PCI/PCI-X bus
controllers do the same?
ISTR MMRBC (PCI-X only) allows one to specify
shorter blocks. I'd have to look that up again.
thanks,
grant
^ permalink raw reply
* Re: tg3 support broken on PPC, a workaround
From: Rick Jones @ 2005-05-10 22:13 UTC (permalink / raw)
To: netdev; +Cc: Grant Grundler
In-Reply-To: <20050510.121214.39158393.davem@davemloft.net>
David S. Miller wrote:
> We really should be disconnecting at single cacheline boundaries
> on RISC systems. The PCI controllers on RISC machines are
> going to disconnect the tg3 when it crosses a cache line
> boundary, so all these setting do is waste PCI bandwidth.
It is my understanding that PA-RISC and IA64 controllers behave differently.
For confirmation one way or the other, I've cc'd someone who could talk about it
much more cogently than I.
rick jones
^ permalink raw reply
* Re: assertion (!atomic_read(&sk->sk_rmem_alloc)) failed at net/netlink/af_netlink.c (122)
From: Herbert Xu @ 2005-05-10 22:07 UTC (permalink / raw)
To: Ken-ichirou MATSUZAWA; +Cc: netdev
In-Reply-To: <20050510.214332.-1300551106.chamas@h4.dion.ne.jp>
On Tue, May 10, 2005 at 12:43:32PM +0000, Ken-ichirou MATSUZAWA wrote:
>
> Everytime on shutting down, the subject message is logged.
> It seems that the reason is do_one_broadcast may preempt
> netlink_recvmsg. Added patch to 2.6.12-rc4 seems resolving this
> problem.
Thanks for the patch. To help me understand the problem better,
could you please tell me:
1) Is this reproducible under 2.6.12-rc4 without your patch?
2) What application is holding the netlink socket?
3) Is your machine SMP, PREEMPT, or both?
Thanks,
--
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: tg3 support broken on PPC, a workaround
From: David S. Miller @ 2005-05-10 21:23 UTC (permalink / raw)
To: mchan; +Cc: mperaya, netdev
In-Reply-To: <1115756070.8570.76.camel@rh4>
From: "Michael Chan" <mchan@broadcom.com>
Subject: Re: tg3 support broken on PPC, a workaround
Date: Tue, 10 May 2005 13:14:30 -0700
> I don't think target-initiated disconnects will waste PCI bandwidth
> compared to master-initiated terminations. In both cases, you see the
> same DMA bursts across the bus, only the termination of each burst is
> different.
I think it does Michael. Performance on sparc64 went non-trivially up
when I added the read/write boundary settings initially long ago.
You have the extra phase where the tg3 tries to start the DMA of the
next cacheline, and that is where unnecessary time is lost. I think
it's about 2 clocks you lose if the PCI controller disconnects instead
of tg3.
Tigon3 will drive the data of the next cacheline for 1 cycle and this
is when the PCI controller will disconnect. Tigon3 will drop the data
and respond to the disconnect sometime in the next cycle or so.
All of this activity will not occur if Tigon3 just ends the data phase
itself when the cacheline boundary is hit.
Or are you talking about PCI reads as opposed to writes?
^ permalink raw reply
* Re: tg3 support broken on PPC, a workaround
From: David S. Miller @ 2005-05-10 21:15 UTC (permalink / raw)
To: mchan; +Cc: mperaya, netdev
In-Reply-To: <1115754210.8570.63.camel@rh4>
From: "Michael Chan" <mchan@broadcom.com>
Subject: Re: tg3 support broken on PPC, a workaround
Date: Tue, 10 May 2005 12:43:30 -0700
> There is clear benefit in doing longer bursts when the bridge can handle
> it.
No question.
> It was explained to me that on some risc systems such as ppc, and
> assuming the bridge can handle long DMA bursts, it is still best to
> disconnect at page or cache line boundaries. The reason is that if the
> burst stops at any arbitrary address, the bridge has to refetch the
> cache line and often the mapping information. Disconnecting at multiple
> cache lines is to address this problem while still achieving longer DMA
> bursts.
Ok, I see.
It is aparently causing some kind of trouble for this person
on his PPC system though. I think we should back down to
single-cacheline on non-X86 until we really can get a grasp
on what machines it is both:
1) beneficial
2) does not corrupt data
And that is what my patch aims to do for the time being.
Even once the problem case is resolved, that setting should
be bracketed in some test that specifically only enables
the longer bursting where it actually helps not hinders.
It definitely should not be done unconditonally for all
non-X86 systems.
^ permalink raw reply
* Re: tg3 support broken on PPC, a workaround
From: Michael Chan @ 2005-05-10 20:49 UTC (permalink / raw)
To: David S.Miller; +Cc: mperaya, netdev
In-Reply-To: <20050510.142319.70222971.davem@davemloft.net>
On Tue, 2005-05-10 at 14:23 -0700, David S.Miller wrote:
> From: "Michael Chan" <mchan@broadcom.com>
> Subject: Re: tg3 support broken on PPC, a workaround
> Date: Tue, 10 May 2005 13:14:30 -0700
>
> > I don't think target-initiated disconnects will waste PCI bandwidth
> > compared to master-initiated terminations. In both cases, you see the
> > same DMA bursts across the bus, only the termination of each burst is
> > different.
>
> I think it does Michael. Performance on sparc64 went non-trivially up
> when I added the read/write boundary settings initially long ago.
>
> You have the extra phase where the tg3 tries to start the DMA of the
> next cacheline, and that is where unnecessary time is lost. I think
> it's about 2 clocks you lose if the PCI controller disconnects instead
> of tg3.
>
> Tigon3 will drive the data of the next cacheline for 1 cycle and this
> is when the PCI controller will disconnect. Tigon3 will drop the data
> and respond to the disconnect sometime in the next cycle or so.
>
You're right. This is Disconnect Without Data and it does cost a few
clock cycles compared to Disconnect With Data or master initiated
termination.
^ permalink raw reply
* Re: IPsec performance over UDP
From: Michael Iatrou @ 2005-05-10 20:35 UTC (permalink / raw)
To: Rick Jones; +Cc: netdev
In-Reply-To: <42810057.4020800@hp.com>
When the date was Tuesday 10 May 2005 21:41, Rick Jones wrote:
> > The "unexpected" result is that there is 30% idle time even if the
> > network is not saturated!
>
> Perhaps the sending side got intra-stack flow-controlled?
In that case, plain IP should't also have the same problem (it hasn't)?
--
Michael Iatrou
Electrical and Computer Engineering Dept.
University of Patras, Greece
^ permalink raw reply
* Re: tg3 support broken on PPC, a workaround
From: David S. Miller @ 2005-05-10 20:32 UTC (permalink / raw)
To: mchan; +Cc: mperaya, netdev
In-Reply-To: <20050510.132642.85686818.davem@davemloft.net>
From: "David S. Miller" <davem@davemloft.net>
Subject: Re: tg3 support broken on PPC, a workaround
Date: Tue, 10 May 2005 13:26:42 -0700 (PDT)
> + case 1024:
> + default:
> + tp->dma_rwctrl |=
> + (DMA_RWCTRL_READ_BNDRY_512 |
> + DMA_RWCTRL_WRITE_BNDRY_512);
> + break;
This should use the _1024 bit values, of course.
I've fixed that in my local copy.
^ permalink raw reply
* Re: tg3 support broken on PPC, a workaround
From: David S. Miller @ 2005-05-10 20:26 UTC (permalink / raw)
To: mchan; +Cc: mperaya, netdev
In-Reply-To: <20050510.121214.39158393.davem@davemloft.net>
From: "David S. Miller" <davem@davemloft.net>
Subject: Re: tg3 support broken on PPC, a workaround
Date: Tue, 10 May 2005 12:12:14 -0700 (PDT)
> Anyways, it is clear this code needs to change. :-)
I propose something like the patch below. I unfortunately
discovered that the PCI-X boundary controls are limited, and
even worse PCI-E only allows controlling the write side and
not the read site at all. :-(
I think this should really be considered to be fixed
in future chip revisions, as performance will suffer
unnecessarily without proper boundary controls. Even
just a single bit in the DMA RW control register which
says "do not cross PCI_CACHELINE_SIZE boundary" would
work just fine as that is essentially what the code below
is trying to convince the Tigon3 chip to do. :)
I am running this patch now on my sparc64 SunBlade1500
workstation's onboard 5703.
Comments?
[TG3]: Do not burst across cache line boundary on non-X86
PCI controllers on these systems will disconnect the tg3
when it crosses a cache line boundary anyways, wasting
precious PCI bandwidth.
Signed-off-by: David S. Miller <davem@davemloft.net>
--- drivers/net/tg3.c.~3~ 2005-05-09 15:52:32.000000000 -0700
+++ drivers/net/tg3.c 2005-05-10 13:16:51.000000000 -0700
@@ -8865,33 +8865,93 @@
else
cacheline_size = (int) byte * 4;
- switch (cacheline_size) {
- case 16:
- case 32:
- case 64:
- case 128:
- if ((tp->tg3_flags & TG3_FLAG_PCIX_MODE) &&
- !(tp->tg3_flags2 & TG3_FLG2_PCI_EXPRESS)) {
- tp->dma_rwctrl |=
- DMA_RWCTRL_WRITE_BNDRY_384_PCIX;
- break;
- } else if (tp->tg3_flags2 & TG3_FLG2_PCI_EXPRESS) {
- tp->dma_rwctrl &=
- ~(DMA_RWCTRL_PCI_WRITE_CMD);
+ /* PCI controllers on most RISC systems tend to disconnect
+ * when a device tries to burst across a cache-line boundary.
+ * Therefore, letting tg3 do so just wastes PCI bandwidth.
+ *
+ * Unfortunately, for PCI-E there are only limited
+ * write-side controls for this, and thus for reads
+ * we will still get the disconnects.
+ */
+ if ((tp->tg3_flags & TG3_FLAG_PCIX_MODE) &&
+ !(tp->tg3_flags2 & TG3_FLG2_PCI_EXPRESS)) {
+ switch (cacheline_size) {
+ case 16:
+ case 32:
+ case 64:
+ case 128:
tp->dma_rwctrl |=
- DMA_RWCTRL_WRITE_BNDRY_128_PCIE;
+ (DMA_RWCTRL_READ_BNDRY_128_PCIX |
+ DMA_RWCTRL_WRITE_BNDRY_128_PCIX);
break;
- }
- /* fallthrough */
- case 256:
- if (!(tp->tg3_flags & TG3_FLAG_PCIX_MODE) &&
- !(tp->tg3_flags2 & TG3_FLG2_PCI_EXPRESS))
+
+ case 256:
+ tp->dma_rwctrl |=
+ (DMA_RWCTRL_READ_BNDRY_256_PCIX |
+ DMA_RWCTRL_WRITE_BNDRY_256_PCIX);
+ break;
+
+ default:
+ tp->dma_rwctrl |=
+ (DMA_RWCTRL_READ_BNDRY_384_PCIX |
+ DMA_RWCTRL_WRITE_BNDRY_384_PCIX);
+ break;
+ };
+ } else if (tp->tg3_flags2 & TG3_FLG2_PCI_EXPRESS) {
+ switch (cacheline_size) {
+ case 16:
+ case 32:
+ case 64:
+ tp->dma_rwctrl |=
+ DMA_RWCTRL_WRITE_BNDRY_64_PCIE;
+ break;
+
+ case 128:
+ default:
+ tp->dma_rwctrl |=
+ DMA_RWCTRL_WRITE_BNDRY_128_PCIE;
+ break;
+ };
+ } else {
+ switch (cacheline_size) {
+ case 16:
+ tp->dma_rwctrl |=
+ (DMA_RWCTRL_READ_BNDRY_16 |
+ DMA_RWCTRL_WRITE_BNDRY_16);
+ break;
+ case 32:
+ tp->dma_rwctrl |=
+ (DMA_RWCTRL_READ_BNDRY_32 |
+ DMA_RWCTRL_WRITE_BNDRY_32);
+ break;
+ case 64:
+ tp->dma_rwctrl |=
+ (DMA_RWCTRL_READ_BNDRY_64 |
+ DMA_RWCTRL_WRITE_BNDRY_64);
+ break;
+ case 128:
tp->dma_rwctrl |=
- DMA_RWCTRL_WRITE_BNDRY_256;
- else if (!(tp->tg3_flags2 & TG3_FLG2_PCI_EXPRESS))
+ (DMA_RWCTRL_READ_BNDRY_128 |
+ DMA_RWCTRL_WRITE_BNDRY_128);
+ break;
+ case 256:
+ tp->dma_rwctrl |=
+ (DMA_RWCTRL_READ_BNDRY_256 |
+ DMA_RWCTRL_WRITE_BNDRY_256);
+ break;
+ case 512:
tp->dma_rwctrl |=
- DMA_RWCTRL_WRITE_BNDRY_256_PCIX;
- };
+ (DMA_RWCTRL_READ_BNDRY_512 |
+ DMA_RWCTRL_WRITE_BNDRY_512);
+ break;
+ case 1024:
+ default:
+ tp->dma_rwctrl |=
+ (DMA_RWCTRL_READ_BNDRY_512 |
+ DMA_RWCTRL_WRITE_BNDRY_512);
+ break;
+ };
+ }
}
#endif
^ permalink raw reply
* Re: tg3 support broken on PPC, a workaround
From: Michael Chan @ 2005-05-10 20:14 UTC (permalink / raw)
To: David S.Miller; +Cc: mperaya, netdev
In-Reply-To: <20050510.132642.85686818.davem@davemloft.net>
On Tue, 2005-05-10 at 13:26 -0700, David S.Miller wrote:
> From: "David S. Miller" <davem@davemloft.net>
> Subject: Re: tg3 support broken on PPC, a workaround
> Date: Tue, 10 May 2005 12:12:14 -0700 (PDT)
>
> > Anyways, it is clear this code needs to change. :-)
>
> I propose something like the patch below. I unfortunately
> discovered that the PCI-X boundary controls are limited, and
> even worse PCI-E only allows controlling the write side and
> not the read site at all. :-(
>
> I think this should really be considered to be fixed
> in future chip revisions, as performance will suffer
> unnecessarily without proper boundary controls. Even
> just a single bit in the DMA RW control register which
> says "do not cross PCI_CACHELINE_SIZE boundary" would
> work just fine as that is essentially what the code below
> is trying to convince the Tigon3 chip to do. :)
>
> I am running this patch now on my sparc64 SunBlade1500
> workstation's onboard 5703.
>
> Comments?
DMA boundary control bits are only valid on 5700 and 5701. On all other
PCI/PCIX chips, these bits are no longer defined.
On PCI Express systems, the cache line size register is not used and is
set to zero on most systems.
>
> [TG3]: Do not burst across cache line boundary on non-X86
>
> PCI controllers on these systems will disconnect the tg3
> when it crosses a cache line boundary anyways, wasting
> precious PCI bandwidth.
I don't think target-initiated disconnects will waste PCI bandwidth
compared to master-initiated terminations. In both cases, you see the
same DMA bursts across the bus, only the termination of each burst is
different.
^ permalink raw reply
* Re: tg3 support broken on PPC, a workaround
From: Michael Chan @ 2005-05-10 19:43 UTC (permalink / raw)
To: David S.Miller; +Cc: mperaya, netdev
In-Reply-To: <20050510.121214.39158393.davem@davemloft.net>
On Tue, 2005-05-10 at 12:12 -0700, David S.Miller wrote:
> From: "Michael Chan" <mchan@broadcom.com>
> Subject: Re: tg3 support broken on PPC, a workaround
> Date: Tue, 10 May 2005 09:52:46 -0700
>
> > In the new code, the DMA write bursts will disconnect at multiples of
> > cache lines instead of 1 cache line. And DMA read bursts will not
> > disconnect at cache line boundaries.
>
> We really should be disconnecting at single cacheline boundaries
> on RISC systems. The PCI controllers on RISC machines are
> going to disconnect the tg3 when it crosses a cache line
> boundary, so all these setting do is waste PCI bandwidth.
>
> From the sparc64 PCI controller programmer's manual:
>
> "When a DMA burst transfer attempts to go past a cache line (64B)
> boundary, U2P generates a disconnect. This should cause the
> master device to attempt the transaction again beginning at the
> address of the next untransferred data."
>
This should be fine. If the bridge requires termination at every cache
line, the bridge (target) will initiate disconnect and data will
terminate.
There is clear benefit in doing longer bursts when the bridge can handle
it.
It was explained to me that on some risc systems such as ppc, and
assuming the bridge can handle long DMA bursts, it is still best to
disconnect at page or cache line boundaries. The reason is that if the
burst stops at any arbitrary address, the bridge has to refetch the
cache line and often the mapping information. Disconnecting at multiple
cache lines is to address this problem while still achieving longer DMA
bursts.
^ permalink raw reply
* Re: tg3 support broken on PPC, a workaround
From: David S. Miller @ 2005-05-10 19:12 UTC (permalink / raw)
To: mchan; +Cc: mperaya, netdev
In-Reply-To: <1115743966.8570.26.camel@rh4>
From: "Michael Chan" <mchan@broadcom.com>
Subject: Re: tg3 support broken on PPC, a workaround
Date: Tue, 10 May 2005 09:52:46 -0700
> In the new code, the DMA write bursts will disconnect at multiples of
> cache lines instead of 1 cache line. And DMA read bursts will not
> disconnect at cache line boundaries.
We really should be disconnecting at single cacheline boundaries
on RISC systems. The PCI controllers on RISC machines are
going to disconnect the tg3 when it crosses a cache line
boundary, so all these setting do is waste PCI bandwidth.
>From the sparc64 PCI controller programmer's manual:
"When a DMA burst transfer attempts to go past a cache line (64B)
boundary, U2P generates a disconnect. This should cause the
master device to attempt the transaction again beginning at the
address of the next untransferred data."
Most other RISC systems have PCI controllers which
behave similarly if not identically, although there
are probably some exceptions.
Anyways, it is clear this code needs to change. :-)
^ permalink raw reply
* Re: IPsec performance over UDP
From: Rick Jones @ 2005-05-10 18:41 UTC (permalink / raw)
To: Michael Iatrou; +Cc: netdev
In-Reply-To: <200505101949.12236.m.iatrou@freemail.gr>
Michael Iatrou wrote:
> Hi,
>
> I did some testing for IPsec performance over UDP. I used two identical PCs,
> connected back-to-back, with Intel Xeon 2.8GHz (SMP/SMT disabled), 512MB RAM,
> e1000 (82546EB), running Linux 2.6.11.7.
>
> I tested AES {128,192,256}, DES, 3DES, SHA, MD5 and various combinations of
> them for ESP and AH.
>
> Network performance:
> http://members.hellug.gr/iatrou/udp-throughput.png
>
> CPU utilization:
> http://members.hellug.gr/iatrou/udp-cpu.png
>
> The "unexpected" result is that there is 30% idle time even if the network is
> not saturated!
Perhaps the sending side got intra-stack flow-controlled?
rick jones
^ 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