Re: [PATCH 8/8] [I/OAT] TCP recv offload to I/OAT
From: Andrew Morton <hidden>
Date: 2006-03-05 07:32:29
Also in:
lkml
Chris Leech [off-list ref] 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 ;)
quoted hunk ↗ jump to hunk
@@ -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.
quoted hunk ↗ jump to hunk
@@ -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.
quoted hunk ↗ jump to hunk
@@ -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);
+ }
} elseetc.
+#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);
+ }
} elseow, my eyes!