Re: [PATCH 07/12] use a dynamic pool of sk_buffs to keep up with fast targets
Hi. I will address the style issues and other things that Andrew Morton pointed out---Thanks again for the feedback. As far as the skb pool goes, I'm afraid my comment is misleading. What this Patch Does Even before this recent series of 12 patches to 2.6.22-rc4, the aoe driver was reusing a small set of skbs that were allocated once and were only used for outbound AoE commands. The network layer cannot be allowed to put_page on the data that is still associated with a bio we haven't returned to the block layer, so the aoe driver (even before the patch under discussion) is still the owner of skbs that have been handed to the network layer for transmission. We need to keep track of these skbs so that we can free them, but by tracking them, we can also easily re-use them. The new patch was a response to the behavior of certain network drivers. We cannot reuse an skb that the network driver still has in its transmit ring. Network drivers can defer transmit ring cleanup and then use the state in the skb to determine how many data segments to clean up in its transmit ring. The tg3 driver is one driver that behaves in this way. When the network driver defers cleanup of its transmit ring, the aoe driver can find itself in a situation where it would like to send an AoE command, and the AoE target is ready for more work, but the network driver still has all of the pre-allocated skbs. In that case, the new patch just calls alloc_skb, as you'd expect. We don't want to get carried away, though. We try not to do excessive allocation in the write path, so we cap the number of skbs we dynamically allocate. Probably calling it a "dynamic pool" is misleading. We were already trying to use a small fixed-size set of pre-allocated skbs before this patch, and this patch just provides a little headroom (with a ceiling, though) to accomodate network drivers that hang onto skbs, by allocating when needed. The d->skbpool_hd list of allocated skbs is necessary so that we can free them later. We didn't notice the need for this headroom until AoE targets got fast enough, but the comment summarizing this patch still wasn't very good. So, when I resubmit this patch, I will use a different comment: dynamically allocate a capped number of skbs when necessary Alternatives If the network layer never did a put_page on the pages in the bio's we get from the block layer, then it would be possible for us to hand skbs to the network layer and forget about them, allowing the network layer to free skbs itself (and thereby calling our own skb->destructor callback function if we needed that). In that case we could get rid of the pre-allocated skbs and also the d->skbpool_hd, instead just calling alloc_skb every time we wanted to transmit a packet. The slab allocator would effectively maintain the list of skbs. Besides a loss of CPU cache locality, the main concern with that approach the danger that it would increase the likelihood of deadlock when VM is trying to free pages by writing dirty data from the page cache through the aoe driver out to persistent storage on an AoE device. Right now we have a situation where we have pre-allocation that corresponds to how much we use, which seems ideal. Of course, there's still the separate issue of receiving the packets that tell us that a write has successfully completed on the AoE target. When memory is low and VM is using AoE to flush dirty data to free up pages, it would be perfect if there were a way for us to register a fast callback that could recognize write command completion responses. But I don't think the current problems with the receive side of the situation are a justification for exacerbating the problem on the transmit side. -- Support - http://www.coraid.com/support/howto.html Ed L Cashin <[EMAIL PROTECTED]> - To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 1/1] make network DMA usable for non-tcp drivers
Here is a patch against the netdev-2.6 git tree that makes the net DMA feature usable for drivers like the ATA over Ethernet block driver, which can use dma_skb_copy_datagram_iovec when receiving data from the network. The change was suggested on kernelnewbies. http://article.gmane.org/gmane.linux.kernel.kernelnewbies/21663 Signed-off-by: Ed L. Cashin <[EMAIL PROTECTED]> --- drivers/dma/Kconfig |2 +- net/core/user_dma.c |2 ++ 2 files changed, 3 insertions(+), 1 deletions(-) diff --git a/drivers/dma/Kconfig b/drivers/dma/Kconfig index 72be6c6..270d23e 100644 --- a/drivers/dma/Kconfig +++ b/drivers/dma/Kconfig @@ -14,7 +14,7 @@ config DMA_ENGINE comment "DMA Clients" config NET_DMA - bool "Network: TCP receive copy offload" + bool "Network: receive copy offload" depends on DMA_ENGINE && NET default y ---help--- diff --git a/net/core/user_dma.c b/net/core/user_dma.c index 0ad1cd5..69d0b15 100644 --- a/net/core/user_dma.c +++ b/net/core/user_dma.c @@ -130,3 +130,5 @@ end: fault: return -EFAULT; } + +EXPORT_SYMBOL(dma_skb_copy_datagram_iovec); -- 1.5.2.1 - To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2.6.18-rc4] aoe [04/13]: zero copy write 1 of 2
[EMAIL PROTECTED]> <[EMAIL PROTECTED]> <20060822212150.
[EMAIL PROTECTED]> <[EMAIL PROTECTED]>
In-Reply-To: <[EMAIL PROTECTED]>
User-Agent: Mutt/1.5.11+cvs20060126
Hi. Back in August I sent out some patches for the aoe driver, and
Alan objected to the direct setting of skb->len in one of them. I
asked whether he was suggesting that we use something like this
instead of setting skb->len:
skb->data_len = 0;
skb_trim(skb, 0);
skb_put(skb, sizeof *h + sizeof *ah + DEFAULTBCNT);
... and Alan said that was acceptible because it takes care of
skb->tail, checks for overflow, and is more future proof.
So I took some time to implement the necessary changes, but then it
became apparent that there was a problem.
The skb_trim and skb_put macros are only for non-linear skbuffs, but
we are only using the area inside the skbuff itself for packet
headers, not data.
When we do something like this:
if (bio_data_dir(buf->bio) == WRITE) {
skb_fill_page_desc(skb, 0, bv->bv_page, buf->bv_off, bcnt);
ah.aflags |= AOEAFL_WRITE;
skb->len += bcnt;
skb->data_len = bcnt;
t->wpkts++;
... skb->tail isn't really relevant, because we are only using the
pre-allocated part of the skbuff for headers, and the headers aren't
expanding here. Other parts of the kernel that aren't putting data in
the skbuff itself set the skb->len directly.
e1000_main.c
ip_output.c
tcp.c
ip6_output.c
So is it correct for the callers of skb_fill_page_desc to set skb->len
or is another interface needed besides skb_put/skb_trim? Such a new
interface would be able to maintain the consistency of the skbuff
fields without assuming that the data is in the skbuff itself.
If a new interface is needed, then it seems like we should set
skb->len in this patch until the new interface is ready.
--
Ed L Cashin <[EMAIL PROTECTED]>
-
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: ATA over Ethernet potential reference after free of net-device.
Ben Greear <[EMAIL PROTECTED]> writes: > Hello! > > I believe that the aoecmd_cfg method in aoecmd.c can reference > the net-devices after free. The reason is that it grabs and releases > the interface with dev_hold, dev_put, but after putting everything, it > then does the transmit of the skbs... Thanks, I'll make a note to look into that. (I'll be out of town soon for a few days.) Based on your understanding of the issue, can you trigger any problematic behavior? -- Ed L Cashin <[EMAIL PROTECTED]> - To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2.6.15-git9a] aoe [1/1]: do not stop retransmit timer when device goes down
On Thu, Jan 26, 2006 at 01:04:37AM +0300, Al Boldi wrote: > Ed L. Cashin wrote: > > This patch is a bugfix that follows and depends on the > > eight aoe driver patches sent January 19th. > > Will they also fix this? > Or is this an md bug? No, this patch fixes a bug that would cause an AoE device to be totally unusable, so I think mdadm or mkraid would get an error that the device was not available before it tried to make a new md device. > It only happens with aoe. It looks like in setting up the raid, sysfs_create_link probably has this going off: BUG_ON(!kobj || !kobj->dentry || !name); > Also, why is aoe slower than nbd? It wasn't when I tried it. The userland vblade is slow. Maybe that's affecting your results? > md: bind > [ cut here ] > kernel BUG at fs/sysfs/symlink.c:87! > invalid operand: [#1] > CPU:0 > EIP:0060:[]Not tainted VLI > EFLAGS: 00210246 (2.6.15) > EIP is at sysfs_create_link+0x56/0x60 > eax: c66de390 ebx: ecx: c03db91f edx: c7ee0040 > esi: c211bdf8 edi: c7ca0400 ebp: c66de360 esp: c211bdb4 > ds: 007b es: 007b ss: 0068 > Process mkraid (pid: 701, threadinfo=c211b000 task=c2300600) > Stack: c7ca0424 c66de390 c211bdf8 c66de390 c02e5997 c66de390 c6b1b5ec > c03db91f >00200296 c0207d56 c66de3a8 c66de360 c02e650f c66de390 0980 > 5c4725a7 >98831dc4 65687465 652f6472 00302e30 3feed8a3 891a1652 7f3dc64e > ab9a9a72 > Call Trace: > [] bind_rdev_to_array+0x157/0x1a0 > [] kobject_init+0x16/0x50 > [] md_import_device+0xbf/0x1c0 > [] add_new_disk+0x22d/0x390 > [] get_random_bytes+0x2f/0x40 > [] copy_from_user+0x4e/0x90 > [] md_ioctl+0x2e8/0x710 > [] blkdev_driver_ioctl+0x56/0x70 > [] blkdev_ioctl+0x93/0x1a0 > [] block_ioctl+0x2b/0x30 > [] do_ioctl+0x6e/0x80 > [] vfs_ioctl+0x6a/0x1e0 > [] sys_ioctl+0x45/0x70 > [] syscall_call+0x7/0xb > Code: 4c 24 04 8b 44 24 18 89 1c 24 89 44 24 08 e8 f2 fe ff ff 8b 53 08 89 c1 > ff 42 70 0f 8e 0b 02 00 00 8b 5c 24 0c 89 c8 83 c4 10 c3 <0f> 0b 57 00 5e a6 > 3d c0 eb be 8b 44 24 04 8b 40 30 89 44 24 04 > -- Ed L Cashin <[EMAIL PROTECTED]> - To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
2.6.16.11 BUG at tg3.c:2917
<0>Kernel panic - not syncing: Aiee, killing interrupt handler! -- Ed L Cashin <[EMAIL PROTECTED]> - To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: 2.6.16.11 BUG at tg3.c:2917
On Thu, Apr 27, 2006 at 08:45:24AM -0700, Michael Chan wrote: > On Thu, 2006-04-27 at 12:52 -0400, Ed L. Cashin wrote: > > -- [please bite here ] - > > Kernel BUG at drivers/net/tg3.c:2917 > > invalid opcode: [1] SMP > > CPU 0 > > Most likely caused by IO re-ordering. Try the test patch in this > discussion: > > http://marc.theaimsgroup.com/?l=linux-netdev&m=113890239404768&w=2 I'm afraid I might be generating noise here. After my initial post I found that I cannot trigger a panic without the latest changes to the aoe driver in place. I haven't been able to trigger a panic using the aoe driver inside 2.6.16.11. I think we've identified the problem in the aoe driver, but if I'm wrong, I will certainly try the TG3_FLAG_MBOX_WRITE_REORDER patch you mention. -- Ed L Cashin <[EMAIL PROTECTED]> - To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
