Re: [PATCH v3] xmit_compl_seq: information to reclaim vmsplice buffers

[Eric Dumazet <eric.dumazet@gmail.com>]

Le jeudi 23 septembre 2010 à 14:35 -0700, Tom Herbert a écrit :

> diff --git a/net/ipv4/tcp.c b/net/ipv4/tcp.c
> index 3e8a4db..3d8d33f 100644
> --- a/net/ipv4/tcp.c
> +++ b/net/ipv4/tcp.c
> @@ -1768,6 +1768,12 @@ skip_copy:
>  
>  	TCP_CHECK_TIMER(sk);
>  	release_sock(sk);
> +
> +	/* Copy the first unacked seq into the receive msg control part. */
> +	if (sock_flag(sk, SOCK_XMIT_COMPL_SEQ))
> +		put_cmsg(msg, SOL_SOCKET, SCM_XMIT_COMPL_SEQ,
> +		    sizeof(tp->snd_una), &tp->snd_una);
> +
>  	return copied;
>  
>  out:

Tom,

I took the time to suggest : copy tp->snd_una before release_sock().

Why do you try to avoid this copy and introduce a bug ?

Hint : put_cmsg() (and copy_to_user()) makes no guarantee the copy is
atomic. 

It can be implemented by a 4 bytes copy, faut during this copy or being
interrupted, and application might get a garbled value, if tcp stack
modifies tp->snd_una during the copy.

If you want to optimize this (because release_sock() can process the
socket backlog, and update snd_una), please use :


	/* Copy the first unacked seq into the receive msg control part.
	 * As put_cmsg() might sleep, we must copy snd_una in a private var.
	 * Integer reads are atomic on all arches, so this copy can be
	 * performed while socket is unlocked.
	 */
	if (sock_flag(sk, SOCK_XMIT_COMPL_SEQ)) {
		u32 snd_una = tp->snd_una;

		put_cmsg(msg, SOL_SOCKET, SCM_XMIT_COMPL_SEQ,
			 sizeof(snd_una), &snd_una);
	}

Thanks !