| Message ID | 12358322831019-git-send-email-ilpo.jarvinen@helsinki.fi |
|---|---|
| State | Rejected, archived |
| Delegated to: | David Miller |
| Headers | show |
From: "Ilpo Järvinen" <ilpo.jarvinen@helsinki.fi> Date: Sat, 28 Feb 2009 16:44:39 +0200 > From: Ilpo Järvinen <ilpo.jarvinen@helsinki.fi> > > In the pure assignment case, the earlier zeroing is > still in effect. > > Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@helsinki.fi> I skipped this one. These tests could be there to avoid dirtying a cacheline when unnecessary. And so unless we can prove the condition always hits and we always do the write, we should keep the tests there. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, 2 Mar 2009, David Miller wrote: > From: "Ilpo Järvinen" <ilpo.jarvinen@helsinki.fi> > Date: Sat, 28 Feb 2009 16:44:39 +0200 > > > From: Ilpo Järvinen <ilpo.jarvinen@helsinki.fi> > > > > In the pure assignment case, the earlier zeroing is > > still in effect. > > > > Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@helsinki.fi> > > I skipped this one. > > These tests could be there to avoid dirtying a cacheline > when unnecessary. And so unless we can prove the condition > always hits and we always do the write, we should keep > the tests there. We'll be dirty it anyway (not that I check), the first "real" statement in tcp_rcv_established is: tp->rx_opt.saw_tstamp = 0; ...that'll land on the same dword. :-/ I suppose the blocks are there just because they had more complexity inside when they had to calculate the eff_sacks too (maybe it would have been better to just remove them in that drop-patch so you would have had less head-ache :-)). Besides, it isn't very nice to have tx/rx or rx'es on different cpus anyway, no?
From: "Ilpo Järvinen" <ilpo.jarvinen@helsinki.fi> Date: Mon, 2 Mar 2009 13:54:17 +0200 (EET) > We'll be dirty it anyway (not that I check), the first "real" statement > in tcp_rcv_established is: > > tp->rx_opt.saw_tstamp = 0; > > ...that'll land on the same dword. :-/ > > I suppose the blocks are there just because they had more complexity > inside when they had to calculate the eff_sacks too (maybe it would > have been better to just remove them in that drop-patch so you would > have had less head-ache :-)). That sounds like a good commit log entry for when you resubmit this patch :-) -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/net/ipv4/tcp_input.c b/net/ipv4/tcp_input.c index e4442a2..464c8a4 100644 --- a/net/ipv4/tcp_input.c +++ b/net/ipv4/tcp_input.c @@ -4325,8 +4325,7 @@ static void tcp_data_queue(struct sock *sk, struct sk_buff *skb) TCP_ECN_accept_cwr(tp, skb); - if (tp->rx_opt.dsack) - tp->rx_opt.dsack = 0; + tp->rx_opt.dsack = 0; /* Queue data for delivery to the user. * Packets in sequence go to the receive queue. @@ -4445,7 +4444,6 @@ drop: /* Initial out of order segment, build 1 SACK. */ if (tcp_is_sack(tp)) { tp->rx_opt.num_sacks = 1; - tp->rx_opt.dsack = 0; tp->selective_acks[0].start_seq = TCP_SKB_CB(skb)->seq; tp->selective_acks[0].end_seq = TCP_SKB_CB(skb)->end_seq; diff --git a/net/ipv4/tcp_output.c b/net/ipv4/tcp_output.c index 1555bb7..d5c7245 100644 --- a/net/ipv4/tcp_output.c +++ b/net/ipv4/tcp_output.c @@ -441,8 +441,7 @@ static void tcp_options_write(__be32 *ptr, struct tcp_sock *tp, *ptr++ = htonl(sp[this_sack].end_seq); } - if (tp->rx_opt.dsack) - tp->rx_opt.dsack = 0; + tp->rx_opt.dsack = 0; } }