diff mbox series

[v3] hackbench: Add af_inet mode besides af_unix and pipe

Message ID 20220523155515.3649-1-wuyihao@linux.alibaba.com
State Superseded
Headers show
Series [v3] hackbench: Add af_inet mode besides af_unix and pipe | expand

Commit Message

Yihao Wu May 23, 2022, 3:55 p.m. UTC
We observerved strange performance regression with hackbench, but only with
its default mode instead of pipe mode. It turned out it was unix domain
socket to blame rather than the scheduler. Because on older kernels, there
was a big lock. And modifications to the scheduler made the contension
worse.

However pipe mode is much different from the default af_unix mode. So it's
hard to prove it's really the case. So we think it might be a good idea to
have af_inet mode for hackbench.

After implementing it, we compare these three modes. And the result shows
as what we expect, that only af_unix mode has regression, the others don't.
This proves adding af_inet mode is valuable.

Signed-off-by: Yihao Wu <wuyihao@linux.alibaba.com>
---
v1->v2: Drop the first patch
v2->v3: Fix EOLs in the commit log

 src/hackbench/hackbench.c | 57 ++++++++++++++++++++++++++++++++++++++-
 1 file changed, 56 insertions(+), 1 deletion(-)

Comments

John Kacur June 24, 2022, 4:28 p.m. UTC | #1
On Mon, 23 May 2022, Yihao Wu wrote:

> We observerved strange performance regression with hackbench, but only with
> its default mode instead of pipe mode. It turned out it was unix domain
> socket to blame rather than the scheduler. Because on older kernels, there
> was a big lock. And modifications to the scheduler made the contension
> worse.
> 
> However pipe mode is much different from the default af_unix mode. So it's
> hard to prove it's really the case. So we think it might be a good idea to
> have af_inet mode for hackbench.
> 
> After implementing it, we compare these three modes. And the result shows
> as what we expect, that only af_unix mode has regression, the others don't.
> This proves adding af_inet mode is valuable.
> 
> Signed-off-by: Yihao Wu <wuyihao@linux.alibaba.com>
> ---
> v1->v2: Drop the first patch
> v2->v3: Fix EOLs in the commit log
> 
>  src/hackbench/hackbench.c | 57 ++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 56 insertions(+), 1 deletion(-)
> 
> diff --git a/src/hackbench/hackbench.c b/src/hackbench/hackbench.c
> index 268c232..18c9284 100644
> --- a/src/hackbench/hackbench.c
> +++ b/src/hackbench/hackbench.c
> @@ -20,11 +20,14 @@
>  #include <string.h>
>  #include <errno.h>
>  #include <unistd.h>
> +#include <sys/ioctl.h>
>  #include <sys/types.h>
>  #include <sys/socket.h>
>  #include <sys/wait.h>
>  #include <sys/time.h>
>  #include <sys/poll.h>
> +#include <netinet/in.h>
> +#include <arpa/inet.h>
>  #include <limits.h>
>  #include <getopt.h>
>  #include <signal.h>
> @@ -46,6 +49,7 @@ static unsigned int fifo = 0;
>  static unsigned int process_mode = PROCESS_MODE;
>  
>  static int use_pipes = 0;
> +static int use_inet = 0;
>  
>  struct sender_context {
>  	unsigned int num_fds;
> @@ -99,6 +103,7 @@ static void print_usage_exit(int error)
>  	       "-h       --help            print this message\n"
>  	       "-l       --loops=LOOPS     how many message should be send\n"
>  	       "-p       --pipe            send data via a pipe\n"
> +	       "-i       --inet            send data via a inet tcp connection\n"
>  	       "-s       --datasize=SIZE   message size\n"
>  	       "-T       --threads         use POSIX threads\n"
>  	       "-P       --process         use fork (default)\n"
> @@ -106,11 +111,52 @@ static void print_usage_exit(int error)
>  	exit(error);
>  }
>  
> +static int inet_socketpair(int fds[2])
> +{
> +	int s1, s2;
> +	struct sockaddr_in sin;
> +	unsigned long ul = 1;
> +
> +	if ((s1 = socket(AF_INET, SOCK_STREAM, 0)) < 0)
> +		barf("socket");
> +	if ((s2 = socket(AF_INET, SOCK_STREAM, 0)) < 0)
> +		barf("socket");
> +
> +	socklen_t len = sizeof(sin);
> +	bzero(&sin, len);
> +	sin.sin_family = AF_INET;
> +	sin.sin_port = 0;
> +	sin.sin_addr.s_addr = inet_addr("127.0.0.1");
> +
> +	if (bind(s1, &sin, len) < 0)
> +		barf("bind");
> +	getsockname(s1, &sin, &len);
> +	if (listen(s1, 10) < 0)
> +		barf("listen");
> +	if (ioctl(s2, FIONBIO, &ul) < 0)
> +		barf("ioctl");
> +	if (ioctl(s1, FIONBIO, &ul) < 0)
> +		barf("ioctl");
> +	connect(s2, &sin, len);
> +	if ((fds[0] = accept(s1, &sin, &len)) < 0)
> +		barf("accept");
> +	ul = 0;
> +	if (ioctl(s2, FIONBIO, &ul) < 0)
> +		barf("ioctl");
> +	fds[1] = s2;
> +	close(s1);
> +
> +	return 0;
> +}
> +
>  static void fdpair(int fds[2])
>  {
>  	if (use_pipes) {
>  		if (pipe(fds) == 0)
>  			return;
> +	} else if (use_inet) {
> +		if (inet_socketpair(fds) == 0)
> +			return;
>  	} else {
>  		if (socketpair(AF_UNIX, SOCK_STREAM, 0, fds) == 0)
>  			return;
> @@ -364,13 +410,14 @@ static void process_options(int argc, char *argv[])
>  			{"help",	no_argument,		NULL, 'h'},
>  			{"loops",	required_argument,	NULL, 'l'},
>  			{"pipe",	no_argument,		NULL, 'p'},
> +			{"inet",	no_argument,		NULL, 'i'},
>  			{"datasize",	required_argument,	NULL, 's'},
>  			{"threads",	no_argument,		NULL, 'T'},
>  			{"processes",	no_argument,		NULL, 'P'},
>  			{NULL, 0, NULL, 0}
>  		};
>  
> -		int c = getopt_long(argc, argv, "f:Fg:hl:ps:TP",
> +		int c = getopt_long(argc, argv, "f:Fg:hl:pis:TP",
>  				    longopts, &optind);
>  		if (c == -1) {
>  			break;
> @@ -402,6 +449,9 @@ static void process_options(int argc, char *argv[])
>  		case 'p':
>  			use_pipes = 1;
>  			break;
> +		case 'i':
> +			use_inet = 1;
> +			break;
>  		case 's':
>  			if (!(argv[optind] && (datasize = atoi(optarg)) > 0)) {
>  				fprintf(stderr, "%s: --datasize|-s requires an integer > 0\n", argv[0]);
> @@ -418,6 +468,11 @@ static void process_options(int argc, char *argv[])
>  			print_usage_exit(1);
>  		}
>  	}
> +
> +	if (use_pipes && use_inet) {
> +		fprintf(stderr, "%s: --pipe|-p and --inet|-i cannot be used together\n", argv[0]);
> +		print_usage_exit(1);
> +	}
>  }
>  
>  void sigcatcher(int sig) {
> -- 
> 2.32.0.604.gb1f3e1269
> 
> 
Signed-off-by: John Kacur <jkacur@redhat.com>
diff mbox series

Patch

diff --git a/src/hackbench/hackbench.c b/src/hackbench/hackbench.c
index 268c232..18c9284 100644
--- a/src/hackbench/hackbench.c
+++ b/src/hackbench/hackbench.c
@@ -20,11 +20,14 @@ 
 #include <string.h>
 #include <errno.h>
 #include <unistd.h>
+#include <sys/ioctl.h>
 #include <sys/types.h>
 #include <sys/socket.h>
 #include <sys/wait.h>
 #include <sys/time.h>
 #include <sys/poll.h>
+#include <netinet/in.h>
+#include <arpa/inet.h>
 #include <limits.h>
 #include <getopt.h>
 #include <signal.h>
@@ -46,6 +49,7 @@  static unsigned int fifo = 0;
 static unsigned int process_mode = PROCESS_MODE;
 
 static int use_pipes = 0;
+static int use_inet = 0;
 
 struct sender_context {
 	unsigned int num_fds;
@@ -99,6 +103,7 @@  static void print_usage_exit(int error)
 	       "-h       --help            print this message\n"
 	       "-l       --loops=LOOPS     how many message should be send\n"
 	       "-p       --pipe            send data via a pipe\n"
+	       "-i       --inet            send data via a inet tcp connection\n"
 	       "-s       --datasize=SIZE   message size\n"
 	       "-T       --threads         use POSIX threads\n"
 	       "-P       --process         use fork (default)\n"
@@ -106,11 +111,52 @@  static void print_usage_exit(int error)
 	exit(error);
 }
 
+static int inet_socketpair(int fds[2])
+{
+	int s1, s2;
+	struct sockaddr_in sin;
+	unsigned long ul = 1;
+
+	if ((s1 = socket(AF_INET, SOCK_STREAM, 0)) < 0)
+		barf("socket");
+	if ((s2 = socket(AF_INET, SOCK_STREAM, 0)) < 0)
+		barf("socket");
+
+	socklen_t len = sizeof(sin);
+	bzero(&sin, len);
+	sin.sin_family = AF_INET;
+	sin.sin_port = 0;
+	sin.sin_addr.s_addr = inet_addr("127.0.0.1");
+
+	if (bind(s1, &sin, len) < 0)
+		barf("bind");
+	getsockname(s1, &sin, &len);
+	if (listen(s1, 10) < 0)
+		barf("listen");
+	if (ioctl(s2, FIONBIO, &ul) < 0)
+		barf("ioctl");
+	if (ioctl(s1, FIONBIO, &ul) < 0)
+		barf("ioctl");
+	connect(s2, &sin, len);
+	if ((fds[0] = accept(s1, &sin, &len)) < 0)
+		barf("accept");
+	ul = 0;
+	if (ioctl(s2, FIONBIO, &ul) < 0)
+		barf("ioctl");
+	fds[1] = s2;
+	close(s1);
+
+	return 0;
+}
+
 static void fdpair(int fds[2])
 {
 	if (use_pipes) {
 		if (pipe(fds) == 0)
 			return;
+	} else if (use_inet) {
+		if (inet_socketpair(fds) == 0)
+			return;
 	} else {
 		if (socketpair(AF_UNIX, SOCK_STREAM, 0, fds) == 0)
 			return;
@@ -364,13 +410,14 @@  static void process_options(int argc, char *argv[])
 			{"help",	no_argument,		NULL, 'h'},
 			{"loops",	required_argument,	NULL, 'l'},
 			{"pipe",	no_argument,		NULL, 'p'},
+			{"inet",	no_argument,		NULL, 'i'},
 			{"datasize",	required_argument,	NULL, 's'},
 			{"threads",	no_argument,		NULL, 'T'},
 			{"processes",	no_argument,		NULL, 'P'},
 			{NULL, 0, NULL, 0}
 		};
 
-		int c = getopt_long(argc, argv, "f:Fg:hl:ps:TP",
+		int c = getopt_long(argc, argv, "f:Fg:hl:pis:TP",
 				    longopts, &optind);
 		if (c == -1) {
 			break;
@@ -402,6 +449,9 @@  static void process_options(int argc, char *argv[])
 		case 'p':
 			use_pipes = 1;
 			break;
+		case 'i':
+			use_inet = 1;
+			break;
 		case 's':
 			if (!(argv[optind] && (datasize = atoi(optarg)) > 0)) {
 				fprintf(stderr, "%s: --datasize|-s requires an integer > 0\n", argv[0]);
@@ -418,6 +468,11 @@  static void process_options(int argc, char *argv[])
 			print_usage_exit(1);
 		}
 	}
+
+	if (use_pipes && use_inet) {
+		fprintf(stderr, "%s: --pipe|-p and --inet|-i cannot be used together\n", argv[0]);
+		print_usage_exit(1);
+	}
 }
 
 void sigcatcher(int sig) {