From: Andrew Morton <akpm@osdl.org>
To: Chris Leech <christopher.leech@intel.com>
Cc: linux-kernel@vger.kernel.org, netdev@vger.kernel.org
Subject: Re: [PATCH 8/8] [I/OAT] TCP recv offload to I/OAT
Date: Sat, 4 Mar 2006 23:30:53 -0800 [thread overview]
Message-ID: <20060304233053.46325e65.akpm@osdl.org> (raw)
In-Reply-To: <20060303214236.11908.98881.stgit@gitlost.site>
Chris Leech <christopher.leech@intel.com> wrote:
>
> Locks down user pages and sets up for DMA in tcp_recvmsg, then calls
> dma_async_try_early_copy in tcp_v4_do_rcv
>
+#ifdef CONFIG_NET_DMA
+#ifdef CONFIG_NET_DMA
+#ifdef CONFIG_NET_DMA
+#ifdef CONFIG_NET_DMA
+#ifdef CONFIG_NET_DMA
+#ifdef CONFIG_NET_DMA
+#ifdef CONFIG_NET_DMA
+#ifdef CONFIG_NET_DMA
+#ifdef CONFIG_NET_DMA
+#ifdef CONFIG_NET_DMA
+#ifdef CONFIG_NET_DMA
+#ifdef CONFIG_NET_DMA
+#ifdef CONFIG_NET_DMA
+#ifdef CONFIG_NET_DMA
+#ifdef CONFIG_NET_DMA
+#ifdef CONFIG_NET_DMA
waaay too many ifdefs. There are various tricks we use to minimise them.
> +#ifdef CONFIG_NET_DMA
> + tp->ucopy.dma_chan = NULL;
> + if ((len > sysctl_tcp_dma_copybreak) && !(flags & MSG_PEEK) && !sysctl_tcp_low_latency && __get_cpu_var(softnet_data.net_dma))
> + dma_lock_iovec_pages(msg->msg_iov, len, &tp->ucopy.locked_list);
> +#endif
Please try to fit code into 80 columns.
That's decimal 80 ;)
> @@ -1328,13 +1342,39 @@ do_prequeue:
> }
>
> if (!(flags & MSG_TRUNC)) {
> - err = skb_copy_datagram_iovec(skb, offset,
> - msg->msg_iov, used);
> - if (err) {
> - /* Exception. Bailout! */
> - if (!copied)
> - copied = -EFAULT;
> - break;
> +#ifdef CONFIG_NET_DMA
> + if (!tp->ucopy.dma_chan && tp->ucopy.locked_list)
> + tp->ucopy.dma_chan = get_softnet_dma();
> +
> + if (tp->ucopy.dma_chan) {
> + tp->ucopy.dma_cookie = dma_skb_copy_datagram_iovec(
> + tp->ucopy.dma_chan, skb, offset,
> + msg->msg_iov, used,
> + tp->ucopy.locked_list);
> +
> + if (tp->ucopy.dma_cookie < 0) {
> +
> + printk(KERN_ALERT "dma_cookie < 0\n");
> +
> + /* Exception. Bailout! */
> + if (!copied)
> + copied = -EFAULT;
> + break;
> + }
> + if ((offset + used) == skb->len)
> + copied_early = 1;
> +
Consider trimming some of those blank lines. I don't think they add any
value?
> + } else
> +#endif
> + {
These games with ifdefs and else statements aren't at all pleasant.
Sometimes they're hard to avoid, but you'll probably find that some code
rearrangemnt (in a preceding patch) makes it easier. Like, split this
function into several.
> @@ -1354,15 +1394,33 @@ skip_copy:
>
> if (skb->h.th->fin)
> goto found_fin_ok;
> - if (!(flags & MSG_PEEK))
> - sk_eat_skb(sk, skb);
> + if (!(flags & MSG_PEEK)) {
> + if (!copied_early)
> + sk_eat_skb(sk, skb);
> +#ifdef CONFIG_NET_DMA
> + else {
> + __skb_unlink(skb, &sk->sk_receive_queue);
> + __skb_queue_tail(&sk->sk_async_wait_queue, skb);
> + copied_early = 0;
> + }
> +#endif
> ...
> - sk_eat_skb(sk, skb);
> + if (!(flags & MSG_PEEK)) {
> + if (!copied_early)
> + sk_eat_skb(sk, skb);
> +#ifdef CONFIG_NET_DMA
> + else {
> + __skb_unlink(skb, &sk->sk_receive_queue);
> + __skb_queue_tail(&sk->sk_async_wait_queue, skb);
> + copied_early = 0;
> + }
> +#endif
> + }
etc.
> +#ifdef CONFIG_NET_DMA
> + if (copied_early)
> + __skb_queue_tail(&sk->sk_async_wait_queue, skb);
> + else
> +#endif
> if (eaten)
> __kfree_skb(skb);
> else
etc.
> @@ -4049,6 +4067,52 @@ discard:
> return 0;
> }
>
> +#ifdef CONFIG_NET_DMA
> +int dma_async_try_early_copy(struct sock *sk, struct sk_buff *skb, int hlen)
> +{
> + struct tcp_sock *tp = tcp_sk(sk);
> + int chunk = skb->len - hlen;
> + int dma_cookie;
> + int copied_early = 0;
> +
> + if (tp->ucopy.wakeup)
> + goto out;
In this case a simple
return 0;
would be fine. We haven't done anything yet.
> +#ifdef CONFIG_NET_DMA
> + struct tcp_sock *tp = tcp_sk(sk);
> + if (!tp->ucopy.dma_chan && tp->ucopy.locked_list)
> + tp->ucopy.dma_chan = get_softnet_dma();
> + if (tp->ucopy.dma_chan)
> + ret = tcp_v4_do_rcv(sk, skb);
> + else
> +#endif
> + {
> + if (!tcp_prequeue(sk, skb))
> ret = tcp_v4_do_rcv(sk, skb);
> + }
> } else
etc.
> +#ifdef CONFIG_NET_DMA
> + struct tcp_sock *tp = tcp_sk(sk);
> + if (tp->ucopy.dma_chan)
> + ret = tcp_v6_do_rcv(sk, skb);
> + else
> +#endif
> + {
> + if (!tcp_prequeue(sk, skb))
> + ret = tcp_v6_do_rcv(sk, skb);
> + }
> } else
ow, my eyes!
next prev parent reply other threads:[~2006-03-05 7:32 UTC|newest]
Thread overview: 51+ messages / expand[flat|nested] mbox.gz Atom feed top
2006-03-03 21:40 [PATCH 0/8] Intel I/O Acceleration Technology (I/OAT) Chris Leech
2006-03-03 21:42 ` [PATCH 1/8] [I/OAT] DMA memcpy subsystem Chris Leech
2006-03-04 1:40 ` David S. Miller
2006-03-06 19:39 ` Chris Leech
2006-03-04 19:20 ` Benjamin LaHaise
2006-03-06 19:48 ` Chris Leech
2006-03-03 21:42 ` [PATCH 3/8] [I/OAT] Setup the networking subsystem as a DMA client Chris Leech
2006-03-03 21:42 ` [PATCH 4/8] [I/OAT] Utility functions for offloading sk_buff to iovec copies Chris Leech
2006-03-05 7:15 ` Andrew Morton
2006-03-03 21:42 ` [PATCH 5/8] [I/OAT] Structure changes for TCP recv offload to I/OAT Chris Leech
2006-03-05 7:19 ` Andrew Morton
2006-03-03 21:42 ` [PATCH 6/8] [I/OAT] Rename cleanup_rbuf to tcp_cleanup_rbuf and make non-static Chris Leech
2006-03-03 21:42 ` [PATCH 7/8] [I/OAT] Add a sysctl for tuning the I/OAT offloaded I/O threshold Chris Leech
2006-03-04 11:22 ` Alexey Dobriyan
2006-03-05 7:21 ` Andrew Morton
2006-03-03 21:42 ` [PATCH 8/8] [I/OAT] TCP recv offload to I/OAT Chris Leech
2006-03-04 16:39 ` Pavel Machek
2006-03-04 23:18 ` Greg KH
2006-03-06 19:28 ` Chris Leech
2006-03-05 7:30 ` Andrew Morton [this message]
2006-03-05 8:45 ` Andrew Morton
2006-03-05 10:27 ` David S. Miller
2006-03-06 19:36 ` Chris Leech
2006-03-03 22:27 ` [PATCH 0/8] Intel I/O Acceleration Technology (I/OAT) Jeff Garzik
2006-03-03 22:39 ` Chris Leech
2006-03-03 22:45 ` Jeff Garzik
2006-03-04 11:35 ` Evgeniy Polyakov
2006-03-05 8:09 ` Andrew Morton
2006-03-05 9:02 ` Discourage duplicate symbols in the kernel? [Was: Intel I/O Acc...] Sam Ravnborg
2006-03-05 9:18 ` Andrew Morton
2006-03-06 19:56 ` Chris Leech
2006-03-03 22:58 ` [PATCH 0/8] Intel I/O Acceleration Technology (I/OAT) Kumar Gala
2006-03-03 23:32 ` Chris Leech
2006-03-04 18:46 ` Jan Engelhardt
2006-03-04 21:41 ` David S. Miller
2006-03-04 22:05 ` Gene Heskett
2006-03-04 22:16 ` David S. Miller
2006-03-05 13:45 ` Jan Engelhardt
2006-03-05 13:55 ` Arjan van de Ven
2006-03-05 16:14 ` Matthieu CASTET
2006-03-05 16:30 ` Jeff Garzik
2006-03-06 19:24 ` Chris Leech
2006-03-06 19:15 ` Chris Leech
2006-03-05 1:43 ` Evgeniy Polyakov
2006-03-05 2:08 ` David S. Miller
2006-03-06 17:44 ` Ingo Oeser
2006-03-07 7:44 ` Evgeniy Polyakov
2006-03-07 9:43 ` Ingo Oeser
2006-03-07 10:16 ` Evgeniy Polyakov
2006-03-11 2:27 Chris Leech
2006-03-11 2:29 ` [PATCH 8/8] [I/OAT] TCP recv offload to I/OAT Chris Leech
2006-03-11 9:41 ` Andrew Morton
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20060304233053.46325e65.akpm@osdl.org \
--to=akpm@osdl.org \
--cc=christopher.leech@intel.com \
--cc=linux-kernel@vger.kernel.org \
--cc=netdev@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).