[PATCH v4] tools/xenconsoled: Increase file descriptor limit

[PATCH v4] tools/xenconsoled: Increase file descriptor limit

[Andrew Cooper]

XenServer's VM density testing uncovered a regression when moving from
sysvinit to systemd where the file descriptor limit dropped from 4096 to
1024. (XenServer had previously inserted a ulimit statement into its
initscripts.)

One solution is to use LimitNOFILE=4096 in xenconsoled.service to match the
lost ulimit, but that is only a stopgap solution.

As Xenconsoled genuinely needs a large number of file descriptors if a large
number of domains are running, attempt to increase the limit.

Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
CC: Ian Campbell <Ian.Campbell@citrix.com>
CC: Ian Jackson <Ian.Jackson@eu.citrix.com>
CC: Wei Liu <wei.liu2@citrix.com>

---
v4:
 * Calculate fd limit based on domid ABI - result is 132008 fds
 * Warn if sufficient fds are not available.
v3:
 * Hide Linux specific bits in #ifdef __linux__
v2:
 * Always increase soft limit to hard limit
 * Correct commment regarding number of file descriptors
 * long -> unsigned long as that appears to be the underlying type of an rlim_t
---
 tools/console/daemon/main.c |   78 +++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 78 insertions(+)

diff --git a/tools/console/daemon/main.c b/tools/console/daemon/main.c
index 92d2fc4..7e3b9a5 100644
--- a/tools/console/daemon/main.c
+++ b/tools/console/daemon/main.c
@@ -26,6 +26,7 @@
 #include <string.h>
 #include <signal.h>
 #include <sys/types.h>
+#include <sys/resource.h>
 
 #include "xenctrl.h"
 
@@ -55,6 +56,81 @@ static void version(char *name)
 	printf("Xen Console Daemon 3.0\n");
 }
 
+static void increase_fd_limit(void)
+{
+	/*
+	 * We require many file descriptors:
+	 * - per domain: pty master, pty slave, logfile and evtchn
+	 * - misc extra: hypervisor log, privcmd, gntdev, std...
+	 *
+	 * Allow a generous 1000 for misc, and calculate the maximum possible
+	 * number of fds which could be used.
+	 */
+	unsigned min_fds = (DOMID_FIRST_RESERVED * 4) + 1000;
+	struct rlimit lim;
+
+	if (getrlimit(RLIMIT_NOFILE, &lim) < 0) {
+		fprintf(stderr, "Failed to obtain fd limit: %s\n",
+			strerror(errno));
+		exit(1);
+	}
+
+	/* Do we already have sufficient? Great! */
+	if (lim.rlim_cur >= min_fds)
+		return;
+
+	/* Attempt to locate the system maximum. */
+#ifdef __linux__
+	{
+		FILE *fs_nr_open = fopen("/proc/sys/fs/nr_open", "r");
+		if (fs_nr_open) {
+			unsigned long nr;
+
+			if ((fscanf(fs_nr_open, "%lu", &nr) == 1) &&
+			    (nr < min_fds)) {
+				syslog(LOG_WARNING,
+				       "Max system fds (%lu) less than minimum "
+				       "requried (%u) - May run out with lots of domains",
+				       nr, min_fds);
+				min_fds = nr;
+			}
+			fclose(fs_nr_open);
+		}
+	}
+#endif
+
+	/*
+	 * Will min_fds fit within our current hard limit?
+	 * (likely on *BSD, unlikely on Linux)
+	 * If so, raise our soft limit.
+	 */
+	if (min_fds <= lim.rlim_max) {
+		struct rlimit new = {
+			.rlim_cur = min_fds,
+			.rlim_max = lim.rlim_max,
+		};
+
+		if (setrlimit(RLIMIT_NOFILE, &new) < 0)
+			syslog(LOG_WARNING,
+			       "Unable to increase fd soft limit: %lu -> %u, "
+			       "hard %lu (%s) - May run out with lots of domains",
+			       lim.rlim_cur, min_fds, lim.rlim_max,
+			       strerror(errno));
+	} else {
+		/*
+		 * Lets hope that, as a root process, we have sufficient
+		 * privilege to up the hard limit.
+		 */
+		struct rlimit new = { .rlim_cur = min_fds, .rlim_max = min_fds };
+
+		if (setrlimit(RLIMIT_NOFILE, &new) < 0)
+			syslog(LOG_WARNING,
+			       "Unable to increase fd hard limit: %lu -> %u (%s)"
+			       " - May run out with lots of domains",
+			       lim.rlim_max, min_fds, strerror(errno));
+	}
+}
+
 int main(int argc, char **argv)
 {
 	const char *sopts = "hVvit:o:";
@@ -154,6 +230,8 @@ int main(int argc, char **argv)
 	openlog("xenconsoled", syslog_option, LOG_DAEMON);
 	setlogmask(syslog_mask);
 
+	increase_fd_limit();
+
 	if (!is_interactive) {
 		daemonize(pidfile ? pidfile : "/var/run/xenconsoled.pid");
 	}
-- 
1.7.10.4

Re: [PATCH v4] tools/xenconsoled: Increase file descriptor limit

[Ian Jackson]

Andrew Cooper writes ("[PATCH v4] tools/xenconsoled: Increase file descriptor limit"):
> XenServer's VM density testing uncovered a regression when moving from
> sysvinit to systemd where the file descriptor limit dropped from 4096 to
> 1024. (XenServer had previously inserted a ulimit statement into its
> initscripts.)
> 
> One solution is to use LimitNOFILE=4096 in xenconsoled.service to match the
> lost ulimit, but that is only a stopgap solution.
> 
> As Xenconsoled genuinely needs a large number of file descriptors if a large
> number of domains are running, attempt to increase the limit.
...

There's still a lot of code here I think we can do without.

Why do we care about the system maximum ?

> +	/*
> +	 * Will min_fds fit within our current hard limit?
> +	 * (likely on *BSD, unlikely on Linux)
> +	 * If so, raise our soft limit.
> +	 */
> +	if (min_fds <= lim.rlim_max) {
> +		struct rlimit new = {
> +			.rlim_cur = min_fds,
> +			.rlim_max = lim.rlim_max,
> +		};
> +
> +		if (setrlimit(RLIMIT_NOFILE, &new) < 0)
> +			syslog(LOG_WARNING,
> +			       "Unable to increase fd soft limit: %lu -> %u, "
> +			       "hard %lu (%s) - May run out with lots of domains",
> +			       lim.rlim_cur, min_fds, lim.rlim_max,
> +			       strerror(errno));
> +	} else {
> +		/*
> +		 * Lets hope that, as a root process, we have sufficient
> +		 * privilege to up the hard limit.
> +		 */
> +		struct rlimit new = { .rlim_cur = min_fds, .rlim_max = min_fds };
> +
> +		if (setrlimit(RLIMIT_NOFILE, &new) < 0)
> +			syslog(LOG_WARNING,
> +			       "Unable to increase fd hard limit: %lu -> %u (%s)"
> +			       " - May run out with lots of domains",
> +			       lim.rlim_max, min_fds, strerror(errno));
> +	}

This is very repetitive.  The only difference between the two branches
is (a) the value of .rlim_max and (b) the log message.  (b) can be
dealt with by making the log message depend only on the contents of
new.

Ian.

Re: [PATCH v4] tools/xenconsoled: Increase file descriptor limit

[Andrew Cooper]

On 27/02/15 16:13, Ian Jackson wrote:
> Andrew Cooper writes ("[PATCH v4] tools/xenconsoled: Increase file descriptor limit"):
>> XenServer's VM density testing uncovered a regression when moving from
>> sysvinit to systemd where the file descriptor limit dropped from 4096 to
>> 1024. (XenServer had previously inserted a ulimit statement into its
>> initscripts.)
>>
>> One solution is to use LimitNOFILE=4096 in xenconsoled.service to match the
>> lost ulimit, but that is only a stopgap solution.
>>
>> As Xenconsoled genuinely needs a large number of file descriptors if a large
>> number of domains are running, attempt to increase the limit.
> ...
>
> There's still a lot of code here I think we can do without.
>
> Why do we care about the system maximum ?

In the case that the system maximum is less than 132008 but greater than
the current hard limit, we want to grab as many FDs as we can.

I can drop it completely if you are happy that noone is really going to
xenconsoled on a Linux system with nr_open less than 1M.

>
>> +	/*
>> +	 * Will min_fds fit within our current hard limit?
>> +	 * (likely on *BSD, unlikely on Linux)
>> +	 * If so, raise our soft limit.
>> +	 */
>> +	if (min_fds <= lim.rlim_max) {
>> +		struct rlimit new = {
>> +			.rlim_cur = min_fds,
>> +			.rlim_max = lim.rlim_max,
>> +		};
>> +
>> +		if (setrlimit(RLIMIT_NOFILE, &new) < 0)
>> +			syslog(LOG_WARNING,
>> +			       "Unable to increase fd soft limit: %lu -> %u, "
>> +			       "hard %lu (%s) - May run out with lots of domains",
>> +			       lim.rlim_cur, min_fds, lim.rlim_max,
>> +			       strerror(errno));
>> +	} else {
>> +		/*
>> +		 * Lets hope that, as a root process, we have sufficient
>> +		 * privilege to up the hard limit.
>> +		 */
>> +		struct rlimit new = { .rlim_cur = min_fds, .rlim_max = min_fds };
>> +
>> +		if (setrlimit(RLIMIT_NOFILE, &new) < 0)
>> +			syslog(LOG_WARNING,
>> +			       "Unable to increase fd hard limit: %lu -> %u (%s)"
>> +			       " - May run out with lots of domains",
>> +			       lim.rlim_max, min_fds, strerror(errno));
>> +	}
> This is very repetitive.  The only difference between the two branches
> is (a) the value of .rlim_max and (b) the log message.  (b) can be
> dealt with by making the log message depend only on the contents of
> new.

I will see what I can do.

~Andrew