fdstore: Unlimit fdstore queue on start

Submitted by Cyrill Gorcunov on June 27, 2018, 11:55 a.m.

Details

Message ID 20180627115510.10918-1-gorcunov@gmail.com
State Accepted
Series "fdstore: Unlimit fdstore queue on start"
Commit cfdee6e57c2759493b33169e6712c71f080e1077
Headers show

Commit Message

Cyrill Gorcunov June 27, 2018, 11:55 a.m.
We use fdstore intensively for example when handling
bindmounted sockets and ghost dgram sockets. The system
limit for per-socket queue may not be enough if someone
generate lots of ghost sockets (150 and more as been
detected on default fedora 27).

To make it operatable lets unlimit fdstore queue size
on startup.

Signed-off-by: Cyrill Gorcunov <gorcunov@gmail.com>
---
 criu/fdstore.c | 15 +++++++++++++--
 1 file changed, 13 insertions(+), 2 deletions(-)

Patch hide | download patch | download mbox

diff --git a/criu/fdstore.c b/criu/fdstore.c
index 8d3a6c89b1dc..88a5a9a153cf 100644
--- a/criu/fdstore.c
+++ b/criu/fdstore.c
@@ -20,6 +20,8 @@  static struct fdstore_desc {
 
 int fdstore_init(void)
 {
+	/* In kernel a bufsize has type int and a value is doubled. */
+	uint32_t buf[2] = { INT_MAX / 2, INT_MAX / 2 };
 	struct sockaddr_un addr;
 	unsigned int addrlen;
 	struct stat st;
@@ -44,6 +46,13 @@  int fdstore_init(void)
 		return -1;
 	}
 
+	if (setsockopt(sk, SOL_SOCKET, SO_SNDBUFFORCE, &buf[0], sizeof(buf[0])) < 0 ||
+	    setsockopt(sk, SOL_SOCKET, SO_RCVBUFFORCE, &buf[1], sizeof(buf[1])) < 0) {
+		pr_perror("Unable to set SO_SNDBUFFORCE/SO_RCVBUFFORCE");
+		close(sk);
+		return -1;
+	}
+
 	addr.sun_family = AF_UNIX;
 	addrlen = snprintf(addr.sun_path, sizeof(addr.sun_path), "X/criu-fdstore-%"PRIx64, st.st_ino);
 	addrlen += sizeof(addr.sun_family);
@@ -79,11 +88,13 @@  int fdstore_init(void)
 int fdstore_add(int fd)
 {
 	int sk = get_service_fd(FDSTORE_SK_OFF);
-	int id;
+	int id, ret;
 
 	mutex_lock(&desc->lock);
 
-	if (send_fd(sk, NULL, 0, fd)) {
+	ret = send_fd(sk, NULL, 0, fd);
+	if (ret) {
+		pr_perror("Can't send fd %d into store\n", fd);
 		mutex_unlock(&desc->lock);
 		return -1;
 	}

Comments

Andrey Vagin June 28, 2018, 10:52 p.m.
On Wed, Jun 27, 2018 at 02:55:10PM +0300, Cyrill Gorcunov wrote:
> We use fdstore intensively for example when handling
> bindmounted sockets and ghost dgram sockets. The system
> limit for per-socket queue may not be enough if someone
> generate lots of ghost sockets (150 and more as been
> detected on default fedora 27).
> 
> To make it operatable lets unlimit fdstore queue size
> on startup.

Where is a test?

> 
> Signed-off-by: Cyrill Gorcunov <gorcunov@gmail.com>
> ---
>  criu/fdstore.c | 15 +++++++++++++--
>  1 file changed, 13 insertions(+), 2 deletions(-)
> 
> diff --git a/criu/fdstore.c b/criu/fdstore.c
> index 8d3a6c89b1dc..88a5a9a153cf 100644
> --- a/criu/fdstore.c
> +++ b/criu/fdstore.c
> @@ -20,6 +20,8 @@ static struct fdstore_desc {
>  
>  int fdstore_init(void)
>  {
> +	/* In kernel a bufsize has type int and a value is doubled. */
> +	uint32_t buf[2] = { INT_MAX / 2, INT_MAX / 2 };
>  	struct sockaddr_un addr;
>  	unsigned int addrlen;
>  	struct stat st;
> @@ -44,6 +46,13 @@ int fdstore_init(void)
>  		return -1;
>  	}
>  
> +	if (setsockopt(sk, SOL_SOCKET, SO_SNDBUFFORCE, &buf[0], sizeof(buf[0])) < 0 ||
> +	    setsockopt(sk, SOL_SOCKET, SO_RCVBUFFORCE, &buf[1], sizeof(buf[1])) < 0) {
> +		pr_perror("Unable to set SO_SNDBUFFORCE/SO_RCVBUFFORCE");
> +		close(sk);
> +		return -1;
> +	}
> +
>  	addr.sun_family = AF_UNIX;
>  	addrlen = snprintf(addr.sun_path, sizeof(addr.sun_path), "X/criu-fdstore-%"PRIx64, st.st_ino);
>  	addrlen += sizeof(addr.sun_family);
> @@ -79,11 +88,13 @@ int fdstore_init(void)
>  int fdstore_add(int fd)
>  {
>  	int sk = get_service_fd(FDSTORE_SK_OFF);
> -	int id;
> +	int id, ret;
>  
>  	mutex_lock(&desc->lock);
>  
> -	if (send_fd(sk, NULL, 0, fd)) {
> +	ret = send_fd(sk, NULL, 0, fd);
> +	if (ret) {
> +		pr_perror("Can't send fd %d into store\n", fd);
>  		mutex_unlock(&desc->lock);
>  		return -1;
>  	}
> -- 
> 2.14.4
>
Cyrill Gorcunov June 28, 2018, 10:57 p.m.
On Thu, Jun 28, 2018 at 03:52:13PM -0700, Andrey Vagin wrote:
> On Wed, Jun 27, 2018 at 02:55:10PM +0300, Cyrill Gorcunov wrote:
> > We use fdstore intensively for example when handling
> > bindmounted sockets and ghost dgram sockets. The system
> > limit for per-socket queue may not be enough if someone
> > generate lots of ghost sockets (150 and more as been
> > detected on default fedora 27).
> > 
> > To make it operatable lets unlimit fdstore queue size
> > on startup.
> 
> Where is a test?

How to test it?
Andrey Vagin June 28, 2018, 11:07 p.m.
On Fri, Jun 29, 2018 at 01:57:55AM +0300, Cyrill Gorcunov wrote:
> On Thu, Jun 28, 2018 at 03:52:13PM -0700, Andrey Vagin wrote:
> > On Wed, Jun 27, 2018 at 02:55:10PM +0300, Cyrill Gorcunov wrote:
> > > We use fdstore intensively for example when handling
> > > bindmounted sockets and ghost dgram sockets. The system
> > > limit for per-socket queue may not be enough if someone
> > > generate lots of ghost sockets (150 and more as been
> > > detected on default fedora 27).
> > > 
> > > To make it operatable lets unlimit fdstore queue size
> > > on startup.
> > 
> > Where is a test?
> 
> How to test it?

For example, you have my patch which reproduces the problem. Probably,
you need to rework it a bit.
Andrey Vagin July 11, 2018, 12:03 a.m.
Applied

On Wed, Jun 27, 2018 at 02:55:10PM +0300, Cyrill Gorcunov wrote:
> We use fdstore intensively for example when handling
> bindmounted sockets and ghost dgram sockets. The system
> limit for per-socket queue may not be enough if someone
> generate lots of ghost sockets (150 and more as been
> detected on default fedora 27).
> 
> To make it operatable lets unlimit fdstore queue size
> on startup.
> 
> Signed-off-by: Cyrill Gorcunov <gorcunov@gmail.com>
> ---
>  criu/fdstore.c | 15 +++++++++++++--
>  1 file changed, 13 insertions(+), 2 deletions(-)
> 
> diff --git a/criu/fdstore.c b/criu/fdstore.c
> index 8d3a6c89b1dc..88a5a9a153cf 100644
> --- a/criu/fdstore.c
> +++ b/criu/fdstore.c
> @@ -20,6 +20,8 @@ static struct fdstore_desc {
>  
>  int fdstore_init(void)
>  {
> +	/* In kernel a bufsize has type int and a value is doubled. */
> +	uint32_t buf[2] = { INT_MAX / 2, INT_MAX / 2 };
>  	struct sockaddr_un addr;
>  	unsigned int addrlen;
>  	struct stat st;
> @@ -44,6 +46,13 @@ int fdstore_init(void)
>  		return -1;
>  	}
>  
> +	if (setsockopt(sk, SOL_SOCKET, SO_SNDBUFFORCE, &buf[0], sizeof(buf[0])) < 0 ||
> +	    setsockopt(sk, SOL_SOCKET, SO_RCVBUFFORCE, &buf[1], sizeof(buf[1])) < 0) {
> +		pr_perror("Unable to set SO_SNDBUFFORCE/SO_RCVBUFFORCE");
> +		close(sk);
> +		return -1;
> +	}
> +
>  	addr.sun_family = AF_UNIX;
>  	addrlen = snprintf(addr.sun_path, sizeof(addr.sun_path), "X/criu-fdstore-%"PRIx64, st.st_ino);
>  	addrlen += sizeof(addr.sun_family);
> @@ -79,11 +88,13 @@ int fdstore_init(void)
>  int fdstore_add(int fd)
>  {
>  	int sk = get_service_fd(FDSTORE_SK_OFF);
> -	int id;
> +	int id, ret;
>  
>  	mutex_lock(&desc->lock);
>  
> -	if (send_fd(sk, NULL, 0, fd)) {
> +	ret = send_fd(sk, NULL, 0, fd);
> +	if (ret) {
> +		pr_perror("Can't send fd %d into store\n", fd);
>  		mutex_unlock(&desc->lock);
>  		return -1;
>  	}
> -- 
> 2.14.4
>