DM-Devel Archive on lore.kernel.org
 help / color / Atom feed
From: mwilck@suse.com
To: Christophe Varoqui <christophe.varoqui@opensvc.com>,
	Benjamin Marzinski <bmarzins@redhat.com>
Cc: lixiaokeng@huawei.com, dm-devel@redhat.com,
	Martin Wilck <mwilck@suse.com>
Subject: [dm-devel] [PATCH v2 28/29] multipathd: sanitize uxsock_listen()
Date: Fri, 16 Oct 2020 12:45:00 +0200
Message-ID: <20201016104501.8700-29-mwilck@suse.com> (raw)
In-Reply-To: <20201016104501.8700-1-mwilck@suse.com>

From: Martin Wilck <mwilck@suse.com>

We were allocating 1025 poll fds, which is not optimal. Fix it, and make this
more easily customizable in general. Use POLLFDS_BASE rather than the
hard-coded "2" for the number of fds we poll besides client connections.
Introduce a maximum number of clients that can connect. When this number is
reached, we simply stop polling the accept socket, so that new connections
aren't accepted any more.  Don't attempt to realloc() the pollfd array if the
number of clients decreases. It's unlikely to ever be more than one or two
pages. Finally, there's no need to wake up every 5s. Our signal handling is
robust. Just sleep forever in ppoll() if nothing happens.

Signed-off-by: Martin Wilck <mwilck@suse.com>
---
 multipathd/uxlsnr.c | 70 ++++++++++++++++++++++++++++-----------------
 1 file changed, 43 insertions(+), 27 deletions(-)

diff --git a/multipathd/uxlsnr.c b/multipathd/uxlsnr.c
index ce2b680..cd462b6 100644
--- a/multipathd/uxlsnr.c
+++ b/multipathd/uxlsnr.c
@@ -41,14 +41,25 @@
 #include "cli.h"
 #include "uxlsnr.h"
 
-static struct timespec sleep_time = {5, 0};
-
 struct client {
 	struct list_head node;
 	int fd;
 };
 
-#define MIN_POLLS 1023
+/* The number of fds we poll on, other than individual client connections */
+#define POLLFDS_BASE 2
+#define POLLFD_CHUNK (4096 / sizeof(struct pollfd))
+/* Minimum mumber of pollfds to reserve for clients */
+#define MIN_POLLS (POLLFD_CHUNK - POLLFDS_BASE)
+/*
+ * Max number of client connections allowed
+ * During coldplug, there may be a large number of "multipath -u"
+ * processes connecting.
+ */
+#define MAX_CLIENTS (16384 - POLLFDS_BASE)
+
+/* Compile-time error if POLLFD_CHUNK is too small */
+static __attribute__((unused)) char ___a[-(MIN_POLLS <= 0)];
 
 static LIST_HEAD(clients);
 static pthread_mutex_t client_lock = PTHREAD_MUTEX_INITIALIZER;
@@ -282,13 +293,13 @@ void * uxsock_listen(uxsock_trigger_fn uxsock_trigger, long ux_sock,
 	char *inbuf;
 	char *reply;
 	sigset_t mask;
-	int old_clients = MIN_POLLS;
+	int max_pfds = MIN_POLLS + POLLFDS_BASE;
 	/* conf->sequence_nr will be 1 when uxsock_listen is first called */
 	unsigned int sequence_nr = 0;
 	struct watch_descriptors wds = { .conf_wd = -1, .dir_wd = -1 };
 
 	condlog(3, "uxsock: startup listener");
-	polls = (struct pollfd *)MALLOC((MIN_POLLS + 2) * sizeof(struct pollfd));
+	polls = MALLOC(max_pfds * sizeof(*polls));
 	if (!polls) {
 		condlog(0, "uxsock: failed to allocate poll fds");
 		exit_daemon();
@@ -312,28 +323,33 @@ void * uxsock_listen(uxsock_trigger_fn uxsock_trigger, long ux_sock,
 		list_for_each_entry(c, &clients, node) {
 			num_clients++;
 		}
-		if (num_clients != old_clients) {
+		if (num_clients + POLLFDS_BASE > max_pfds) {
 			struct pollfd *new;
-			if (num_clients <= MIN_POLLS && old_clients > MIN_POLLS) {
-				new = REALLOC(polls, (2 + MIN_POLLS) *
-						sizeof(struct pollfd));
-			} else if (num_clients <= MIN_POLLS && old_clients <= MIN_POLLS) {
-				new = polls;
-			} else {
-				new = REALLOC(polls, (2 + num_clients) *
-						sizeof(struct pollfd));
-			}
-			if (!new) {
-				condlog(0, "%s: failed to realloc %d poll fds",
-					"uxsock", 2 + num_clients);
-				num_clients = old_clients;
-			} else {
-				old_clients = num_clients;
+			int n_new = max_pfds + POLLFD_CHUNK;
+
+			new = REALLOC(polls, n_new * sizeof(*polls));
+			if (new) {
+				max_pfds = n_new;
 				polls = new;
+			} else {
+				condlog(1, "%s: realloc failure, %d clients not served",
+					__func__,
+					num_clients + POLLFDS_BASE - max_pfds);
+				num_clients = max_pfds - POLLFDS_BASE;
 			}
 		}
-		polls[0].fd = ux_sock;
-		polls[0].events = POLLIN;
+		if (num_clients < MAX_CLIENTS) {
+			polls[0].fd = ux_sock;
+			polls[0].events = POLLIN;
+		} else {
+			/*
+			 * New clients can't connect, num_clients won't grow
+			 * to MAX_CLIENTS or higher
+			 */
+			condlog(1, "%s: max client connections reached, pausing polling",
+				__func__);
+			polls[0].fd = -1;
+		}
 
 		reset_watch(notify_fd, &wds, &sequence_nr);
 		if (notify_fd == -1 || (wds.conf_wd == -1 && wds.dir_wd == -1))
@@ -343,19 +359,19 @@ void * uxsock_listen(uxsock_trigger_fn uxsock_trigger, long ux_sock,
 		polls[1].events = POLLIN;
 
 		/* setup the clients */
-		i = 2;
+		i = POLLFDS_BASE;
 		list_for_each_entry(c, &clients, node) {
 			polls[i].fd = c->fd;
 			polls[i].events = POLLIN;
 			i++;
-			if (i >= 2 + num_clients)
+			if (i >= max_pfds)
 				break;
 		}
 		n_pfds = i;
 		pthread_cleanup_pop(1);
 
 		/* most of our life is spent in this call */
-		poll_count = ppoll(polls, n_pfds, &sleep_time, &mask);
+		poll_count = ppoll(polls, n_pfds, NULL, &mask);
 
 		handle_signals(false);
 		if (poll_count == -1) {
@@ -388,7 +404,7 @@ void * uxsock_listen(uxsock_trigger_fn uxsock_trigger, long ux_sock,
 		}
 
 		/* see if a client wants to speak to us */
-		for (i = 2; i < n_pfds; i++) {
+		for (i = POLLFDS_BASE; i < n_pfds; i++) {
 			if (polls[i].revents & POLLIN) {
 				struct timespec start_time;
 
-- 
2.28.0


--
dm-devel mailing list
dm-devel@redhat.com
https://www.redhat.com/mailman/listinfo/dm-devel


  parent reply index

Thread overview: 49+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-10-16 10:44 [dm-devel] [PATCH v2 00/29] libmultipath: improve cleanup on exit mwilck
2020-10-16 10:44 ` [dm-devel] [PATCH v2 01/29] multipathd: uxlsnr: avoid deadlock " mwilck
2020-10-20 19:04   ` Benjamin Marzinski
2020-10-16 10:44 ` [dm-devel] [PATCH v2 02/29] multipathd: Fix liburcu memory leak mwilck
2020-10-16 10:44 ` [dm-devel] [PATCH v2 03/29] multipathd: move handling of io_err_stat_attr into libmultipath mwilck
2020-10-16 10:44 ` [dm-devel] [PATCH v2 04/29] multipathd: move vecs desctruction into cleanup function mwilck
2020-10-16 10:44 ` [dm-devel] [PATCH v2 05/29] multipathd: make some globals static mwilck
2020-10-16 10:44 ` [dm-devel] [PATCH v2 06/29] multipathd: move threads destruction into separate function mwilck
2020-10-16 10:44 ` [dm-devel] [PATCH v2 07/29] multipathd: move conf " mwilck
2020-10-19 18:56   ` Benjamin Marzinski
2020-10-16 10:44 ` [dm-devel] [PATCH v2 08/29] multipathd: move pid " mwilck
2020-10-16 10:44 ` [dm-devel] [PATCH v2 09/29] multipathd: close pidfile on exit mwilck
2020-10-16 10:44 ` [dm-devel] [PATCH v2 10/29] multipathd: add helper for systemd notification at exit mwilck
2020-10-16 10:44 ` [dm-devel] [PATCH v2 11/29] multipathd: child(): call cleanups in failure case, too mwilck
2020-10-16 10:44 ` [dm-devel] [PATCH v2 12/29] multipathd: unwatch_all_dmevents: check if waiter is initialized mwilck
2020-10-16 10:44 ` [dm-devel] [PATCH v2 13/29] multipathd: print error message if config can't be loaded mwilck
2020-10-16 10:44 ` [dm-devel] [PATCH v2 14/29] libmultipath: add libmp_dm_exit() mwilck
2020-10-19 19:07   ` Benjamin Marzinski
2020-10-16 10:44 ` [dm-devel] [PATCH v2 15/29] multipathd: fixup libdm deinitialization mwilck
2020-10-16 10:44 ` [dm-devel] [PATCH v2 16/29] libmultipath: log_thread_stop(): check if logarea is initialized mwilck
2020-10-16 10:44 ` [dm-devel] [PATCH v2 17/29] multipathd: add cleanup_child() exit handler mwilck
2020-10-16 10:44 ` [dm-devel] [PATCH v2 18/29] libmultipath: fix log_thread startup and teardown mwilck
2020-10-19 20:00   ` Benjamin Marzinski
2020-10-26 13:58     ` Martin Wilck
2020-10-16 10:44 ` [dm-devel] [PATCH v2 19/29] multipathd: move cleanup_{prio, checkers, foreign} to libmultipath_exit mwilck
2020-10-16 10:44 ` [dm-devel] [PATCH v2 20/29] multipath: use atexit() for cleanup handlers mwilck
2020-10-16 10:44 ` [dm-devel] [PATCH v2 21/29] mpathpersist: " mwilck
2020-10-16 10:44 ` [dm-devel] [PATCH v2 22/29] multipath: fix leaks in check_path_valid() mwilck
2020-10-16 10:44 ` [dm-devel] [PATCH v2 23/29] multipath-tools: mpath-tools.supp: file with valgrind suppressions mwilck
2020-10-19 20:01   ` Benjamin Marzinski
2020-10-16 10:44 ` [dm-devel] [PATCH v2 24/29] libmultipath: use libmp_verbosity to track verbosity mwilck
2020-10-19 20:38   ` Benjamin Marzinski
2020-10-26 14:47     ` Martin Wilck
2020-10-16 10:44 ` [dm-devel] [PATCH v2 25/29] libmultipath: introduce symbolic values for logsink mwilck
2020-10-16 20:13   ` Benjamin Marzinski
2020-10-16 10:44 ` [dm-devel] [PATCH v2 26/29] libmultipath: simplify dlog() mwilck
2020-10-19 21:07   ` Benjamin Marzinski
2020-10-16 10:44 ` [dm-devel] [PATCH v2 27/29] multipathd: common code for "-k" and command args mwilck
2020-10-19 21:51   ` Benjamin Marzinski
2020-10-16 10:45 ` mwilck [this message]
2020-10-19 23:33   ` [dm-devel] [PATCH v2 28/29] multipathd: sanitize uxsock_listen() Benjamin Marzinski
2020-10-26 13:54     ` Martin Wilck
2020-10-16 10:45 ` [dm-devel] [PATCH v2 29/29] libmultipath: fix race between log_safe and log_thread_stop() mwilck
2020-10-20  2:20   ` Benjamin Marzinski
2020-10-26 16:22     ` Martin Wilck
2020-10-26 17:24       ` Martin Wilck
2020-11-03  0:11         ` Benjamin Marzinski
2020-11-04 12:36           ` Martin Wilck
2020-11-04 15:46             ` Benjamin Marzinski

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20201016104501.8700-29-mwilck@suse.com \
    --to=mwilck@suse.com \
    --cc=bmarzins@redhat.com \
    --cc=christophe.varoqui@opensvc.com \
    --cc=dm-devel@redhat.com \
    --cc=lixiaokeng@huawei.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

DM-Devel Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/dm-devel/0 dm-devel/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 dm-devel dm-devel/ https://lore.kernel.org/dm-devel \
		dm-devel@redhat.com
	public-inbox-index dm-devel

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/com.redhat.dm-devel


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git