lkml.org 
[lkml]   [1998]   [Mar]   [5]   [last100]   RSS Feed
Views: [more markup]  [less markup]  [headers]  [forward] 
 
Messages in this thread
/
From"A.N.Kuznetsov" <>
SubjectRe: minor patch for 2.1.88 net/core/sock.c
DateThu, 5 Mar 1998 23:38:35 +0300 (MSK)
Hello!

>  		atomic_add(size, &sk->wmem_alloc);
>  		mem = kmalloc(size, priority);
> -		if (mem) {
> -			/* Recheck because kmalloc might sleep */
> -			if (atomic_read(&sk->wmem_alloc)+size < sk->sndbuf)
> -				return mem; 
> -			kfree_s(mem, size);
> -		} 
> +		if (mem)
> +			return mem; 
>  		atomic_sub(size, &sk->wmem_alloc);
+>		sk->write_space(sk);
>  	}
>  	return mem;

The code was wrong, but the fix is wrong too.
We cannot leave atomic_read(&sk->wmem_alloc) >= sk->sndbuf here
(it can occur, if sk->wmem_alloc was increased by another process while sleep),
because it would block subsequent sock_alloc_send_skb forever.

Besides, sk->write_space(sk) after subtraction is missing.

Seems, the code should look like:
(Andy, please, check it. I believe you meaned exactly this thing)

/* Main idea:
	Total wmem_alloc consumed by sock_kmalloc cannot exceed
	sk->sndbuf minus 1 byte.

	In this case, we always can transmit at least one skb
	of data, and hence we avoid dead lock in sock_alloc_send_skb.
 */

void *sock_kmalloc(struct sock *sk, int size, int priority)
{
	void *mem = NULL;
	if (atomic_read(&sk->wmem_alloc)+size < sk->sndbuf) {
		/* First do the add, to avoid the race if kmalloc
 		 * might sleep.
		 */
		atomic_add(size, &sk->wmem_alloc);
		mem = kmalloc(size, priority);
		if (mem) {
			/* Recheck because kmalloc might sleep */
			if (atomic_read(&sk->wmem_alloc) < sk->sndbuf)
				return mem; 
			sock_kfree_s(sk, mem, size);
			return NULL;
		}
	}
	return mem;
}

Alexey Kuznetsov

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.rutgers.edu

\
 
 \ /
  Last update: 2005-03-22 12:41    [from the cache]
©2003-2008