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

[PATCH v2] 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, and is well behaved with its descriptors,
attempt to up the limit to the system maximum.

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>

---
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 |   45 +++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 45 insertions(+)

diff --git a/tools/console/daemon/main.c b/tools/console/daemon/main.c
index 92d2fc4..0bb9d8a 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,48 @@ static void version(char *name)
 	printf("Xen Console Daemon 3.0\n");
 }
 
+/*
+ * Xenconsoled requires two file descriptors for each PV console (pty and log
+ * file), which can easily exceed the default of 1024 if many guests are
+ * running.  Try to set the fd limit to the system maximum, falling back to a
+ * default of 4096.
+ */
+static void increase_fd_limit(void)
+{
+	FILE *fs_nr_open;
+	struct rlimit lim;
+	unsigned long nr_open = 4096;
+
+	if (getrlimit(RLIMIT_NOFILE, &lim) < 0)
+		return;
+
+	/* Increase the soft limit to the current hard limit. */
+	if (lim.rlim_cur < lim.rlim_max) {
+		lim.rlim_cur = lim.rlim_max;
+		if (setrlimit(RLIMIT_NOFILE, &lim) < 0)
+			return;
+	}
+
+	/* Attempt to locate the system maximum. */
+	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_open = nr;
+		fclose(fs_nr_open);
+	}
+
+	/*
+	 * In the likely case that we are a root process with
+	 * CAP_SYS_RESOURCE, attempt to up our hard limit.
+	 */
+	if (nr_open > lim.rlim_max) {
+		lim.rlim_cur = lim.rlim_max = nr_open;
+		setrlimit(RLIMIT_NOFILE, &lim);
+	}
+}
+
 int main(int argc, char **argv)
 {
 	const char *sopts = "hVvit:o:";
@@ -154,6 +197,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 v2] tools/xenconsoled: Increase file descriptor limit

[Wei Liu]

On Tue, Feb 17, 2015 at 04:21:24PM +0000, Andrew Cooper wrote:
> 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, and is well behaved with its descriptors,
> attempt to up the limit to the system maximum.
> 
> 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>
> 
> ---
> 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 |   45 +++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 45 insertions(+)
> 
> diff --git a/tools/console/daemon/main.c b/tools/console/daemon/main.c
> index 92d2fc4..0bb9d8a 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,48 @@ static void version(char *name)
>  	printf("Xen Console Daemon 3.0\n");
>  }
>  
> +/*
> + * Xenconsoled requires two file descriptors for each PV console (pty and log
> + * file), which can easily exceed the default of 1024 if many guests are
> + * running.  Try to set the fd limit to the system maximum, falling back to a
> + * default of 4096.
> + */
> +static void increase_fd_limit(void)
> +{
> +	FILE *fs_nr_open;
> +	struct rlimit lim;
> +	unsigned long nr_open = 4096;
> +
> +	if (getrlimit(RLIMIT_NOFILE, &lim) < 0)
> +		return;
> +
> +	/* Increase the soft limit to the current hard limit. */
> +	if (lim.rlim_cur < lim.rlim_max) {
> +		lim.rlim_cur = lim.rlim_max;
> +		if (setrlimit(RLIMIT_NOFILE, &lim) < 0)
> +			return;
> +	}
> +
> +	/* Attempt to locate the system maximum. */
> +	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_open = nr;
> +		fclose(fs_nr_open);
> +	}
> +
> +	/*
> +	 * In the likely case that we are a root process with
> +	 * CAP_SYS_RESOURCE, attempt to up our hard limit.
> +	 */
> +	if (nr_open > lim.rlim_max) {
> +		lim.rlim_cur = lim.rlim_max = nr_open;
> +		setrlimit(RLIMIT_NOFILE, &lim);
> +	}
> +}
> +

This function looks Linux centric (/proc and CAP_SYSRESOURCE). Please be
considerate to other Unices. :-)

Also you might want to log failures along the line.

Wei.

>  int main(int argc, char **argv)
>  {
>  	const char *sopts = "hVvit:o:";
> @@ -154,6 +197,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 v2] tools/xenconsoled: Increase file descriptor limit

[Andrew Cooper]

On 17/02/15 16:28, Wei Liu wrote:
> On Tue, Feb 17, 2015 at 04:21:24PM +0000, Andrew Cooper wrote:
>> 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, and is well behaved with its descriptors,
>> attempt to up the limit to the system maximum.
>>
>> 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>
>>
>> ---
>> 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 |   45 +++++++++++++++++++++++++++++++++++++++++++
>>  1 file changed, 45 insertions(+)
>>
>> diff --git a/tools/console/daemon/main.c b/tools/console/daemon/main.c
>> index 92d2fc4..0bb9d8a 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,48 @@ static void version(char *name)
>>  	printf("Xen Console Daemon 3.0\n");
>>  }
>>  
>> +/*
>> + * Xenconsoled requires two file descriptors for each PV console (pty and log
>> + * file), which can easily exceed the default of 1024 if many guests are
>> + * running.  Try to set the fd limit to the system maximum, falling back to a
>> + * default of 4096.
>> + */
>> +static void increase_fd_limit(void)
>> +{
>> +	FILE *fs_nr_open;
>> +	struct rlimit lim;
>> +	unsigned long nr_open = 4096;
>> +
>> +	if (getrlimit(RLIMIT_NOFILE, &lim) < 0)
>> +		return;
>> +
>> +	/* Increase the soft limit to the current hard limit. */
>> +	if (lim.rlim_cur < lim.rlim_max) {
>> +		lim.rlim_cur = lim.rlim_max;
>> +		if (setrlimit(RLIMIT_NOFILE, &lim) < 0)
>> +			return;
>> +	}
>> +
>> +	/* Attempt to locate the system maximum. */
>> +	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_open = nr;
>> +		fclose(fs_nr_open);
>> +	}
>> +
>> +	/*
>> +	 * In the likely case that we are a root process with
>> +	 * CAP_SYS_RESOURCE, attempt to up our hard limit.
>> +	 */
>> +	if (nr_open > lim.rlim_max) {
>> +		lim.rlim_cur = lim.rlim_max = nr_open;
>> +		setrlimit(RLIMIT_NOFILE, &lim);
>> +	}
>> +}
>> +
> This function looks Linux centric (/proc and CAP_SYSRESOURCE). Please be
> considerate to other Unices. :-)
>
> Also you might want to log failures along the line.
>
> Wei.

setrlimit() is posix.  From the manpage:

CONFORMING TO
       getrlimit(), setrlimit(): SVr4, 4.3BSD, POSIX.1-2001.

"/proc/sys/fs/nr_open" is likely Linux only, but not fatal, and doesn't
prevent an arbitrary grab for 4096 descriptors.


I am not sure what an admin could usefully do with a logged failure
message here.  Xenconsoled will most likely not function in an
environment where it is not sufficiently privileged to make this limit
adjustment (use of privcmd and /dev/ptmx).

~Andrew

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

[Wei Liu]

On Tue, Feb 17, 2015 at 04:37:08PM +0000, Andrew Cooper wrote:
[...]
> >> +
> > This function looks Linux centric (/proc and CAP_SYSRESOURCE). Please be
> > considerate to other Unices. :-)
> >
> > Also you might want to log failures along the line.
> >
> > Wei.
> 
> setrlimit() is posix.  From the manpage:
> 
> CONFORMING TO
>        getrlimit(), setrlimit(): SVr4, 4.3BSD, POSIX.1-2001.
> 

True.

> "/proc/sys/fs/nr_open" is likely Linux only, but not fatal, and doesn't
> prevent an arbitrary grab for 4096 descriptors.
> 

Please use

  #if defined(__linux__)

for Linux specific code.

> 
> I am not sure what an admin could usefully do with a logged failure
> message here.  Xenconsoled will most likely not function in an
> environment where it is not sufficiently privileged to make this limit
> adjustment (use of privcmd and /dev/ptmx).
> 

Does xenconsoled requires CAP_SYS_RESOURCE to make use of privcmd and
/dev/ptmx? If not, there is a case that your setrlimit fails
while other parts can still be functional, right?

Wei.

> ~Andrew

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

[Andrew Cooper]

On 17/02/15 16:48, Wei Liu wrote:
> On Tue, Feb 17, 2015 at 04:37:08PM +0000, Andrew Cooper wrote:
>
>> I am not sure what an admin could usefully do with a logged failure
>> message here.  Xenconsoled will most likely not function in an
>> environment where it is not sufficiently privileged to make this limit
>> adjustment (use of privcmd and /dev/ptmx).
>>
> Does xenconsoled requires CAP_SYS_RESOURCE to make use of privcmd and
> /dev/ptmx? If not, there is a case that your setrlimit fails
> while other parts can still be functional, right?

A slim chance, yes if someone has specifically been playing with
capabilities.

I still don't see how an error message would be useful.  Even if the
call fails, the process still has the default number of file descriptors
and can function fine up until that limit.  If errors were meaningful in
this case, the function wouldn't be void.

~Andrew

[PATCH v3] 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, and is well behaved with its descriptors,
attempt to up the limit to the system maximum.

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>

---
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 |   47 +++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 47 insertions(+)

diff --git a/tools/console/daemon/main.c b/tools/console/daemon/main.c
index 92d2fc4..20ffe6b 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,50 @@ static void version(char *name)
 	printf("Xen Console Daemon 3.0\n");
 }
 
+/*
+ * Xenconsoled requires two file descriptors for each PV console (pty and log
+ * file), which can easily exceed the default of 1024 if many guests are
+ * running.  Try to set the fd limit to the system maximum, falling back to a
+ * default of 4096.
+ */
+static void increase_fd_limit(void)
+{
+	FILE *fs_nr_open;
+	struct rlimit lim;
+	unsigned long nr_open = 4096;
+
+	if (getrlimit(RLIMIT_NOFILE, &lim) < 0)
+		return;
+
+	/* Increase the soft limit to the current hard limit. */
+	if (lim.rlim_cur < lim.rlim_max) {
+		lim.rlim_cur = lim.rlim_max;
+		if (setrlimit(RLIMIT_NOFILE, &lim) < 0)
+			return;
+	}
+
+#ifdef __linux__
+	/* Attempt to locate the system maximum. */
+	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_open = nr;
+		fclose(fs_nr_open);
+	}
+#endif
+
+	/*
+	 * In the likely case that we are a root process with
+	 * CAP_SYS_RESOURCE, attempt to up our hard limit.
+	 */
+	if (nr_open > lim.rlim_max) {
+		lim.rlim_cur = lim.rlim_max = nr_open;
+		setrlimit(RLIMIT_NOFILE, &lim);
+	}
+}
+
 int main(int argc, char **argv)
 {
 	const char *sopts = "hVvit:o:";
@@ -154,6 +199,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 v2] tools/xenconsoled: Increase file descriptor limit

[Wei Liu]

On Tue, Feb 17, 2015 at 05:31:19PM +0000, Andrew Cooper wrote:
> On 17/02/15 16:48, Wei Liu wrote:
> > On Tue, Feb 17, 2015 at 04:37:08PM +0000, Andrew Cooper wrote:
> >
> >> I am not sure what an admin could usefully do with a logged failure
> >> message here.  Xenconsoled will most likely not function in an
> >> environment where it is not sufficiently privileged to make this limit
> >> adjustment (use of privcmd and /dev/ptmx).
> >>
> > Does xenconsoled requires CAP_SYS_RESOURCE to make use of privcmd and
> > /dev/ptmx? If not, there is a case that your setrlimit fails
> > while other parts can still be functional, right?
> 
> A slim chance, yes if someone has specifically been playing with
> capabilities.
> 
> I still don't see how an error message would be useful.  Even if the
> call fails, the process still has the default number of file descriptors
> and can function fine up until that limit.  If errors were meaningful in
> this case, the function wouldn't be void.
> 

Fair enough. I don't have very strong opinion. Was just thinking some
error message might help debugging if the call fails and the limit is
hit.

Wei.

> ~Andrew

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

[Don Slutz]

On 02/17/15 12:55, Andrew Cooper wrote:
> 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, and is well behaved with its descriptors,
> attempt to up the limit to the system maximum.
> 
> 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>
> 

For what it is worth:

Reviewed-by: Don Slutz <dslutz@verizon.com>

   -Don Slutz

> ---
> 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 |   47 +++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 47 insertions(+)
> 
> diff --git a/tools/console/daemon/main.c b/tools/console/daemon/main.c
> index 92d2fc4..20ffe6b 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,50 @@ static void version(char *name)
>  	printf("Xen Console Daemon 3.0\n");
>  }
>  
> +/*
> + * Xenconsoled requires two file descriptors for each PV console (pty and log
> + * file), which can easily exceed the default of 1024 if many guests are
> + * running.  Try to set the fd limit to the system maximum, falling back to a
> + * default of 4096.
> + */
> +static void increase_fd_limit(void)
> +{
> +	FILE *fs_nr_open;
> +	struct rlimit lim;
> +	unsigned long nr_open = 4096;
> +
> +	if (getrlimit(RLIMIT_NOFILE, &lim) < 0)
> +		return;
> +
> +	/* Increase the soft limit to the current hard limit. */
> +	if (lim.rlim_cur < lim.rlim_max) {
> +		lim.rlim_cur = lim.rlim_max;
> +		if (setrlimit(RLIMIT_NOFILE, &lim) < 0)
> +			return;
> +	}
> +
> +#ifdef __linux__
> +	/* Attempt to locate the system maximum. */
> +	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_open = nr;
> +		fclose(fs_nr_open);
> +	}
> +#endif
> +
> +	/*
> +	 * In the likely case that we are a root process with
> +	 * CAP_SYS_RESOURCE, attempt to up our hard limit.
> +	 */
> +	if (nr_open > lim.rlim_max) {
> +		lim.rlim_cur = lim.rlim_max = nr_open;
> +		setrlimit(RLIMIT_NOFILE, &lim);
> +	}
> +}
> +
>  int main(int argc, char **argv)
>  {
>  	const char *sopts = "hVvit:o:";
> @@ -154,6 +199,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");
>  	}
> 

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

[Wei Liu]

On Tue, Feb 17, 2015 at 05:55:52PM +0000, Andrew Cooper wrote:
> 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, and is well behaved with its descriptors,
> attempt to up the limit to the system maximum.
> 
> 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>

Acked-by: Wei Liu <wei.liu2@citrix.com>

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

[Ian Campbell]

On Tue, 2015-02-17 at 17:55 +0000, Andrew Cooper wrote:
> +	/*
> +	 * In the likely case that we are a root process with
> +	 * CAP_SYS_RESOURCE, attempt to up our hard limit.
> +	 */

(Nit: also the soft limit...)

Acked-by: Ian Campbell <ian.campbell@citrix.com>

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

[Ian Jackson]

Wei Liu writes ("Re: [PATCH v3] tools/xenconsoled: Increase file descriptor limit"):
> On Tue, Feb 17, 2015 at 05:55:52PM +0000, Andrew Cooper wrote:
> > 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.)

I think that putting something like this in the initscripts is a good
idea.

I don't like the idea of this kind of resource limit mangling (quite
vigorous, too!) being buried in the C code.

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

Why is this only a stopgap solution ?

> > As Xenconsoled genuinely needs a large number of file descriptors
> > if a large number of domains are running, and is well behaved with
> > its descriptors, attempt to up the limit to the system maximum.

Perhaps you mean it should be "unlimited" but that doesn't seem right
either.  The value should be some multiple of the maximum number of
domains.

Ian.

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

[Andrew Cooper]

On 19/02/15 16:30, Ian Jackson wrote:
> Wei Liu writes ("Re: [PATCH v3] tools/xenconsoled: Increase file descriptor limit"):
>> On Tue, Feb 17, 2015 at 05:55:52PM +0000, Andrew Cooper wrote:
>>> 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.)
> I think that putting something like this in the initscripts is a good
> idea.
>
> I don't like the idea of this kind of resource limit mangling (quite
> vigorous, too!) being buried in the C code.
>
>>> One solution is to use LimitNOFILE=4096 in xenconsoled.service to match the
>>> lost ulimit, but that is only a stopgap solution.
> Why is this only a stopgap solution ?

It is yet another place with an arbitrary limit, which is one more
moving part to go wrong.

>
>>> As Xenconsoled genuinely needs a large number of file descriptors
>>> if a large number of domains are running, and is well behaved with
>>> its descriptors, attempt to up the limit to the system maximum.
> Perhaps you mean it should be "unlimited" but that doesn't seem right
> either.

Where do I perhaps mean "unlimited"?

Attempting to set NOFILE with RLIM_INFINITY is an unconditional failure,
and there is unfortunately no RLIM_SYSTEM_MAX.

> The value should be some multiple of the maximum number of
> domains.

The maximum number of domains (limited by the domid abi) is 32751

However, a brief grep through the code shows that the fd situation is
far worse than I first thought.  There are 4 FDs held open per
domain[1], and some overhead for hypervisor logging, gntdev and privcmd.

That would put the number of fds needed at 131004 + overhead (20
perhaps?).  This is substantially larger than the default on Linux, but
within the default system max of 2^20.

~Andrew

[1] along with a todo note suggesting that opening evtchn for each
domain is inefficient.

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

[David Vrabel]

On 19/02/2015 17:56, Andrew Cooper wrote:
>
> [1] along with a todo note suggesting that opening evtchn for each
> domain is inefficient.

The evtchn device has per-fd locks so using only a single fd for all 
events may scale badly.

David

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

[Andrew Cooper]

On 19/02/15 17:56, Andrew Cooper wrote:
> On 19/02/15 16:30, Ian Jackson wrote:
>> Wei Liu writes ("Re: [PATCH v3] tools/xenconsoled: Increase file descriptor limit"):
>>> On Tue, Feb 17, 2015 at 05:55:52PM +0000, Andrew Cooper wrote:
>>>> 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.)
>> I think that putting something like this in the initscripts is a good
>> idea.
>>
>> I don't like the idea of this kind of resource limit mangling (quite
>> vigorous, too!) being buried in the C code.
>>
>>>> One solution is to use LimitNOFILE=4096 in xenconsoled.service to match the
>>>> lost ulimit, but that is only a stopgap solution.
>> Why is this only a stopgap solution ?
> It is yet another place with an arbitrary limit, which is one more
> moving part to go wrong.
>
>>>> As Xenconsoled genuinely needs a large number of file descriptors
>>>> if a large number of domains are running, and is well behaved with
>>>> its descriptors, attempt to up the limit to the system maximum.
>> Perhaps you mean it should be "unlimited" but that doesn't seem right
>> either.
> Where do I perhaps mean "unlimited"?
>
> Attempting to set NOFILE with RLIM_INFINITY is an unconditional failure,
> and there is unfortunately no RLIM_SYSTEM_MAX.
>
>> The value should be some multiple of the maximum number of
>> domains.
> The maximum number of domains (limited by the domid abi) is 32751
>
> However, a brief grep through the code shows that the fd situation is
> far worse than I first thought.  There are 4 FDs held open per
> domain[1], and some overhead for hypervisor logging, gntdev and privcmd.
>
> That would put the number of fds needed at 131004 + overhead (20
> perhaps?).  This is substantially larger than the default on Linux, but
> within the default system max of 2^20.
>
> ~Andrew

Ping?

>
> [1] along with a todo note suggesting that opening evtchn for each
> domain is inefficient.
>
>
>
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel

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

[Ian Jackson]

Andrew Cooper writes ("Re: [PATCH v3] tools/xenconsoled: Increase file descriptor limit"):
> On 19/02/15 16:30, Ian Jackson wrote:
> > Why is this only a stopgap solution ?
> 
> It is yet another place with an arbitrary limit, which is one more
> moving part to go wrong.

OIC.  I meant `unlimited', but...

> >>> As Xenconsoled genuinely needs a large number of file descriptors
> >>> if a large number of domains are running, and is well behaved with
> >>> its descriptors, attempt to up the limit to the system maximum.
> > Perhaps you mean it should be "unlimited" but that doesn't seem right
> > either.
> 
> Where do I perhaps mean "unlimited"?
> 
> Attempting to set NOFILE with RLIM_INFINITY is an unconditional failure,
> and there is unfortunately no RLIM_SYSTEM_MAX.

Urgh.

I think this is a bug in Linux.

  http://pubs.opengroup.org/onlinepubs/9699919799/functions/getrlimit.html

  [EPERM]
      The limit specified to setrlimit() would have raised the maximum
      limit value, and the calling process does not have appropriate
      privileges.

But that's not what's happening, or at least, it's a contorted
interpretation.

Best in shell is using `sysctl':
   ulimit -H -n "$(sysctl -n fs.nr_open)"
But not very portable.

So I am now convinced this should be done in C :-/.

> > The value should be some multiple of the maximum number of
> > domains.
> 
> The maximum number of domains (limited by the domid abi) is 32751
...
> That would put the number of fds needed at 131004 + overhead (20
> perhaps?).  This is substantially larger than the default on Linux, but
> within the default system max of 2^20.

Considering this, and looking at your actual code I think you should
simply set the value to 131004[1] (plus say 1000 spare for libc use
and so on) and not bother with trying to find the system maximum.

It would be nice to warn if the attempt fails.

[1] Computed from appropriate #defines.

Ian.