unix_dgram_recvmsg wakeups [test results & programs]

[to be followed by a patch]

Currently, unix_dgram_recvmsg does a wake up on the peer_wait queue of a socket after every received datagram. This seems excessive, especially considering that only SOCK_DGRAM client sockets in an n:1 asssociation with a server socket ever wait for the associated condition.

Values are based on 500 runs of either test program.

SOCK_SEQPACKET throughput (B/s)

stock patched
---------------------------------------------------------------
avg 14714579.91 17898665.25 +26.64%
median 14409539.95 16429584.38 +14.02%
dev 1582631.52 3475160.01

min 13588814.36 14864694.12
max 25960203 27264811.18

SOCK_DGRAM 100 clients/ 1 server (B/s)

stock patched
---------------------------------------------------------------
avg 57394.06 58222.93 +1.44%
median 55878.12 56993.55 +2%
dev 3540.02 3488.49

min 54167.33 53165.51
max 67971.15 66466.99

SOCK_SEQPACKET test program
---------------------------
#include <inttypes.h>
#include <stdlib.h>
#include <stdio.h>
#include <sys/socket.h>
#include <sys/time.h>
#include <sys/wait.h>
#include <unistd.h>

enum {
    MSG_SZ =	16,
    MSGS =	1000000
};

static char msg[MSG_SZ];

static uint64_t tv2u(struct timeval *tv)

{
    uint64_t u;

    u = tv->tv_sec;
    u *= 1000000;
    return u + tv->tv_usec;
}

int main(void)

{
    struct timeval start, stop;
    uint64_t t_diff;
    double rate;
    int sks[2];
    unsigned remain;
    char buf[MSG_SZ];

    socketpair(AF_UNIX, SOCK_SEQPACKET, 0, sks);

    if (fork() == 0) {
	close(*sks);
	
	gettimeofday(&start, 0);
	while (read(sks[1], buf, sizeof(buf)) > 0);
	gettimeofday(&stop, 0);

	t_diff = tv2u(&stop);
	t_diff -= tv2u(&start);
	rate = MSG_SZ * MSGS;
	rate /= t_diff;
	rate *= 1000000;
	printf("%f\n", rate);

	fflush(stdout);
	_exit(0);
    }

    close(sks[1]);
    
    remain = MSGS;
    do write(*sks, msg, sizeof(msg)); while (--remain);
    close(*sks);

    wait(NULL);
    return 0;
}

--------------------------

SOCK_DGRAM test program
-----------------------
#include <inttypes.h>
#include <stdlib.h>
#include <stdio.h>
#include <sys/poll.h>
#include <sys/socket.h>
#include <sys/time.h>
#include <sys/un.h>
#include <sys/wait.h>
#include <unistd.h>

#define SERVER_ADDR "\0dgt-2"

enum {
    MSG_SZ =	16,
    MSGS =	1000
};

static char msg[MSG_SZ];
static struct sockaddr_un server_sun;

static uint64_t tv2u(struct timeval *tv)

{
    uint64_t u;

    u = tv->tv_sec;
    u *= 1000000;
    return u + tv->tv_usec;
}

static void server(int sk, unsigned want)

{
    struct timeval start, stop;
    char buf[MSG_SZ];
    uint64_t t_diff;
    double rate;
    
    gettimeofday(&start, 0);
    do read(sk, buf, sizeof(buf)); while (--want);
    gettimeofday(&stop, 0);

    t_diff = tv2u(&stop);
    t_diff -= tv2u(&start);
    rate = MSG_SZ * MSGS;
    rate /= t_diff;
    rate *= 1000000;
    printf("%f\n", rate);
}

static void client(void)

{
    unsigned remain;
    int sk;

    sk = socket(AF_UNIX, SOCK_DGRAM, 0);
    connect(sk, (struct sockaddr *)&server_sun, sizeof(server_sun));
    
    remain = MSGS;
    do write(sk, msg, sizeof(msg)); while (--remain);

    _exit(0);
}

int main(int argc, char **argv)

{
    unsigned n_clients, want;
    int sk;

    n_clients = atoi(argv[1]);

    server_sun.sun_family = AF_UNIX;
    memcpy(server_sun.sun_path, SERVER_ADDR, sizeof(SERVER_ADDR));
    sk = socket(AF_UNIX, SOCK_DGRAM, 0);
    bind(sk, (struct sockaddr *)&server_sun, sizeof(server_sun));

    want = n_clients * MSGS;
    do if (fork() == 0) client(); while (--n_clients);
    
    server(sk, want);
    return 0;
}

The current unix_dgram_recvmsg does a wake up for every received datagram. This seems wasteful as only SOCK_DGRAM client sockets in an n:1 association with a server socket will ever wait because of the associated condition. The patch below changes the function such that the wake up only happens if wq_has_sleeper indicates that someone actually wants to be notified. Testing with SOCK_SEQPACKET and SOCK_DGRAM socket seems to confirm that this is an improvment.

---

index 4e95bdf..7aba73e 100644
@@ -2057,8 +2057,10 @@ static int unix_dgram_recvmsg(struct socket *sock, struct msghdr *msg,
 		goto out_unlock;
 	}
 
-	wake_up_interruptible_sync_poll(&u->peer_wait,
-					POLLOUT | POLLWRNORM | POLLWRBAND);
+	if (wq_has_sleeper(&u->peer_wq))
+		wake_up_interruptible_sync_poll(&u->peer_wait,
+						POLLOUT | POLLWRNORM |
+						POLLWRBAND);
 
 	if (msg->msg_name)
 		unix_copy_addr(msg, skb->sk);

From: Rainer Weikusat <[email protected]>
Date: Thu, 26 Nov 2015 19:23:15 +0000

Applied, thanks.