c/r: some cleanup for sockets

c/r: some cleanup for sockets

[Oren Laadan]

Dan,

While trying to fix a compilation warning from the socket c/r code, I
ended up doing some cleanup, that isn't particuarly structured.

It compiles flawlessly for me, but I didn't give it much testing. Let
me know what you think.

Thanks,

Oren.

[PATCH 1/3] c/r: cleanup sock_file_restore()/sock_file_checkpoint()

[Oren Laadan]

This patch was intended to fix a compilation warning regarding the
sock_file_restore():

checkpoint/files.c: At top level:
checkpoint/files.c:573: warning: initialization from incompatible pointer type

and evolved to some cleanup of that code, making the following changes:

* Make ckpt_hdr_socket be part of ckpt_hdr_file_socket; There is no
  good reason to use a separate header for it. This saves write/read
  of an extra structure.

* Fold do_sock_file_checkpoint() into sock_file_checkpoint(), and also
  do_sock_file_restore() into sock_file_restore(). This makes the code
  a bit simpler and more streamlined.

* Move sock_file_{checkpoint,restore} to net/checkpoint.c, which is a
  more proper place than net/socket.c.

* Properly define sock_file_{checkpoint,restore} in header file

* Have sock_file_restore() call restore_file_common(), which was
  omitted previously.


Signed-off-by: Oren Laadan <orenl-eQaUEPhvms7ENvBUuze7eA@public.gmane.org>
---
 include/linux/checkpoint_hdr.h |  116 +++++++++++++++++++--------------------
 include/linux/net.h            |    2 +
 include/net/af_unix.h          |    6 +-
 include/net/sock.h             |   10 ++--
 net/checkpoint.c               |   94 ++++++++++++++++++++++++--------
 net/socket.c                   |   84 +----------------------------
 net/unix/checkpoint.c          |    8 +---
 7 files changed, 138 insertions(+), 182 deletions(-)

diff --git a/include/linux/checkpoint_hdr.h b/include/linux/checkpoint_hdr.h
index 53d3f37..4d5c22a 100644
--- a/include/linux/checkpoint_hdr.h
+++ b/include/linux/checkpoint_hdr.h
@@ -94,7 +94,6 @@ enum {
 	CKPT_HDR_SIGPENDING,
 
 	CKPT_HDR_FD_SOCKET = 701,
-	CKPT_HDR_SOCKET,
 	CKPT_HDR_SOCKET_QUEUE,
 	CKPT_HDR_SOCKET_BUFFER,
 	CKPT_HDR_SOCKET_UNIX,
@@ -366,8 +365,65 @@ struct ckpt_hdr_file_pipe {
 	__s32 pipe_objref;
 } __attribute__((aligned(8)));
 
+/* socket */
+struct ckpt_hdr_socket {
+	struct { /* struct socket */
+		__u64 flags;
+		__u8 state;
+	} socket __attribute__ ((aligned(8)));
+
+	struct { /* struct sock_common */
+		__u32 bound_dev_if;
+		__u32 reuse;
+		__u16 family;
+		__u8 state;
+	} sock_common __attribute__ ((aligned(8)));
+
+	struct { /* struct sock */
+		__s64 rcvlowat;
+		__u64 flags;
+
+		__u32 err;
+		__u32 err_soft;
+		__u32 priority;
+		__s32 rcvbuf;
+		__s32 sndbuf;
+		__u16 type;
+		__s16 backlog;
+
+		__u8 protocol;
+		__u8 state;
+		__u8 shutdown;
+		__u8 userlocks;
+		__u8 no_check;
+
+		struct linger linger;
+		struct timeval rcvtimeo;
+		struct timeval sndtimeo;
+	} sock __attribute__ ((aligned(8)));
+} __attribute__ ((aligned(8)));
+
+struct ckpt_hdr_socket_queue {
+	struct ckpt_hdr h;
+	__u32 skb_count;
+	__u32 total_bytes;
+} __attribute__ ((aligned(8)));
+
+#define CKPT_UNIX_LINKED 1
+struct ckpt_hdr_socket_unix {
+	struct ckpt_hdr h;
+	__s32 this;
+	__s32 peer;
+	__u32 flags;
+	__u32 laddr_len;
+	__u32 raddr_len;
+	struct sockaddr_un laddr;
+	struct sockaddr_un raddr;
+} __attribute__ ((aligned(8)));
+
 struct ckpt_hdr_file_socket {
 	struct ckpt_hdr_file common;
+	struct ckpt_hdr_socket socket;
 } __attribute__((aligned(8)));
 
 struct ckpt_hdr_utsns {
@@ -576,64 +632,6 @@ struct ckpt_hdr_ipc_sem {
 	__u32 sem_nsems;
 } __attribute__((aligned(8)));
 
-#define CKPT_UNIX_LINKED 1
-struct ckpt_hdr_socket_unix {
-	struct ckpt_hdr h;
-	__s32 this;
-	__s32 peer;
-	__u32 flags;
-	__u32 laddr_len;
-	__u32 raddr_len;
-	struct sockaddr_un laddr;
-	struct sockaddr_un raddr;
-} __attribute__ ((aligned(8)));
-
-struct ckpt_hdr_socket {
-	struct ckpt_hdr h;
-
-	struct { /* struct socket */
-		__u64 flags;
-		__u8 state;
-	} socket __attribute__ ((aligned(8)));
-
-	struct { /* struct sock_common */
-		__u32 bound_dev_if;
-		__u32 reuse;
-		__u16 family;
-		__u8 state;
-	} sock_common __attribute__ ((aligned(8)));
-
-	struct { /* struct sock */
-		__s64 rcvlowat;
-		__u64 flags;
-
-		__u32 err;
-		__u32 err_soft;
-		__u32 priority;
-		__s32 rcvbuf;
-		__s32 sndbuf;
-		__u16 type;
-		__s16 backlog;
-
-		__u8 protocol;
-		__u8 state;
-		__u8 shutdown;
-		__u8 userlocks;
-		__u8 no_check;
-
-		struct linger linger;
-		struct timeval rcvtimeo;
-		struct timeval sndtimeo;
-
-	} sock __attribute__ ((aligned(8)));
-
-} __attribute__ ((aligned(8)));
-
-struct ckpt_hdr_socket_queue {
-	struct ckpt_hdr h;
-	__u32 skb_count;
-	__u32 total_bytes;
-} __attribute__ ((aligned(8)));
 
 #define CKPT_TST_OVERFLOW_16(a, b) \
 	((sizeof(a) > sizeof(b)) && ((a) > SHORT_MAX))
diff --git a/include/linux/net.h b/include/linux/net.h
index 4fc2ffd..2c4a75d 100644
--- a/include/linux/net.h
+++ b/include/linux/net.h
@@ -223,6 +223,8 @@ extern int   	     sock_sendmsg(struct socket *sock, struct msghdr *msg,
 				  size_t len);
 extern int	     sock_recvmsg(struct socket *sock, struct msghdr *msg,
 				  size_t size, int flags);
+extern int	     sock_attach_fd(struct socket *sock, struct file *file,
+				    int flags);
 extern int 	     sock_map_fd(struct socket *sock, int flags);
 extern struct socket *sockfd_lookup(int fd, int *err);
 #define		     sockfd_put(sock) fput(sock->file)
diff --git a/include/net/af_unix.h b/include/net/af_unix.h
index 7aef51b..35b5b9c 100644
--- a/include/net/af_unix.h
+++ b/include/net/af_unix.h
@@ -70,6 +70,8 @@ static inline void unix_sysctl_unregister(struct net *net) {}
 #endif
 
 #ifdef CONFIG_CHECKPOINT
+struct ckpt_ctx;
+struct ckpt_hdr_socket;
 
 #ifdef CONFIG_UNIX_MODULE
 /* FIXME: Our current scheme won't work with CONFIG_UNIX=m */
@@ -77,9 +79,7 @@ static inline void unix_sysctl_unregister(struct net *net) {}
 #endif
 
 #ifdef CONFIG_UNIX
-extern int sock_unix_checkpoint(struct ckpt_ctx *ctx,
-				struct socket *socket,
-				struct ckpt_hdr_socket *h);
+extern int sock_unix_checkpoint(struct ckpt_ctx *ctx, struct socket *socket);
 extern int sock_unix_restore(struct ckpt_ctx *ctx,
 			     struct ckpt_hdr_socket *h,
 			     struct socket *socket);
diff --git a/include/net/sock.h b/include/net/sock.h
index da75f2f..8e3b050 100644
--- a/include/net/sock.h
+++ b/include/net/sock.h
@@ -1643,12 +1643,10 @@ extern __u32 sysctl_rmem_default;
 #ifdef CONFIG_CHECKPOINT
 /* Checkpoint/Restart Functions */
 struct ckpt_ctx;
-struct ckpt_hdr_socket;
-extern int sock_file_checkpoint(struct ckpt_ctx *, void *);
-extern void *sock_file_restore(struct ckpt_ctx *);
-extern struct socket *do_sock_file_restore(struct ckpt_ctx *,
-					   struct ckpt_hdr_socket *);
-extern int do_sock_file_checkpoint(struct ckpt_ctx *ctx, struct file *file);
+struct ckpt_hdr_file;
+extern int sock_file_checkpoint(struct ckpt_ctx *ctx, struct file *file);
+extern struct file *sock_file_restore(struct ckpt_ctx *ctx,
+				      struct ckpt_hdr_file *h);
 #endif
 
 #endif	/* _SOCK_H */
diff --git a/net/checkpoint.c b/net/checkpoint.c
index ebbd68a..b97cb89 100644
--- a/net/checkpoint.c
+++ b/net/checkpoint.c
@@ -323,44 +323,50 @@ static int sock_cptrst(struct ckpt_ctx *ctx,
 		return 0;
 }
 
-int do_sock_file_checkpoint(struct ckpt_ctx *ctx, struct file *file)
+int sock_file_checkpoint(struct ckpt_ctx *ctx, struct file *file)
 {
+	struct ckpt_hdr_file_socket *h;
 	struct socket *socket = file->private_data;
 	struct sock *sock = socket->sk;
-	struct ckpt_hdr_socket *h;
-	int ret = 0;
+	int ret;
 
-	h = ckpt_hdr_get_type(ctx, sizeof(*h), CKPT_HDR_SOCKET);
+	h = ckpt_hdr_get_type(ctx, sizeof(*h), CKPT_HDR_FILE);
 	if (!h)
 		return -ENOMEM;
 
-	ret = sock_cptrst(ctx, sock, h, CKPT_CPT);
-	if (ret)
+	h->common.f_type = CKPT_FILE_SOCKET;
+
+	/* part I: common to all sockets */
+	ret = sock_cptrst(ctx, sock, &h->socket, CKPT_CPT);
+	if (ret < 0)
+		goto out;
+	ret = checkpoint_file_common(ctx, file, &h->common);
+	if (ret < 0)
+		goto out;
+	ret = ckpt_write_obj(ctx, (struct ckpt_hdr *) h);
+	if (ret < 0)
 		goto out;
 
+	/* part II: per socket type state */
 	if (sock->sk_family == AF_UNIX) {
-		ret = sock_unix_checkpoint(ctx, socket, h);
-		if (ret)
-			goto out;
+		ret = sock_unix_checkpoint(ctx, socket);
 	} else {
 		ckpt_write_err(ctx, "unsupported socket family %i",
 			       sock->sk_family);
 		ret = -ENOSYS;
-		goto out;
 	}
+	if (ret < 0)
+		goto out;
 
+	/* part III: socket buffers */
 	if (sock->sk_state != TCP_LISTEN) {
 		ret = sock_write_buffers(ctx, &sock->sk_receive_queue);
 		if (ret)
 			goto out;
-
 		ret = sock_write_buffers(ctx, &sock->sk_write_queue);
-		if (ret)
-			goto out;
 	}
  out:
 	ckpt_hdr_put(ctx, h);
-
 	return ret;
 }
 
@@ -400,12 +406,36 @@ struct ckpt_hdr_socket_queue *ckpt_sock_read_buffer_hdr(struct ckpt_ctx *ctx,
 		return h;
 }
 
-struct socket *do_sock_file_restore(struct ckpt_ctx *ctx,
-				    struct ckpt_hdr_socket *h)
+static struct file *sock_alloc_attach_fd(struct socket *socket)
 {
+	struct file *file;
+	int err;
+
+	file = get_empty_filp();
+	if (!file)
+		return ERR_PTR(ENOMEM);
+
+	err = sock_attach_fd(socket, file, 0);
+	if (err < 0) {
+		put_filp(file);
+		file = ERR_PTR(err);
+	}
+
+	return file;
+}
+
+struct file *sock_file_restore(struct ckpt_ctx *ctx, struct ckpt_hdr_file *ptr)
+{
+	struct ckpt_hdr_file_socket *hh = (struct ckpt_hdr_file_socket *) ptr;
+	struct ckpt_hdr_socket *h = &hh->socket;
 	struct socket *socket;
+	struct file *file;
 	int ret;
 
+	if (ptr->h.type != CKPT_HDR_FILE  ||
+	    ptr->h.len != sizeof(*hh) || ptr->f_type != CKPT_FILE_SOCKET)
+		return ERR_PTR(-EINVAL);
+
 	if ((h->sock.type != SOCK_DGRAM) && (h->sock.type != SOCK_STREAM)) {
 		ckpt_debug("Socket type %i not supported", h->sock.type);
 		return ERR_PTR(-EINVAL);
@@ -415,6 +445,10 @@ struct socket *do_sock_file_restore(struct ckpt_ctx *ctx,
 	if (ret < 0)
 		return ERR_PTR(ret);
 
+	/*
+	 * part II: per socket type state
+	 * (also takes care of part III: socket buffer)
+	 */
 	if (h->sock_common.family == AF_UNIX) {
 		ret = sock_unix_restore(ctx, h, socket);
 		ckpt_debug("sock_unix_restore: %i\n", ret);
@@ -422,17 +456,29 @@ struct socket *do_sock_file_restore(struct ckpt_ctx *ctx,
 		ckpt_debug("unsupported family %i\n", h->sock_common.family);
 		ret = -ENOSYS;
 	}
+	if (ret < 0)
+		goto err;
 
-	if (ret)
-		goto out;
-
+	/* part I: common to all sockets */
 	ret = sock_cptrst(ctx, socket->sk, h, CKPT_RST);
- out:
-	if (ret) {
-		sock_release(socket);
-		socket = ERR_PTR(ret);
+	if (ret < 0)
+		goto err;
+
+	file = sock_alloc_attach_fd(socket);
+	if (IS_ERR(file)) {
+		ret = PTR_ERR(file);
+		goto err;
 	}
 
-	return socket;
+	ret = restore_file_common(ctx, file, ptr);
+	if (ret < 0) {
+		fput(file);
+		file = ERR_PTR(ret);
+	}
+	return file;
+
+ err:
+	sock_release(socket);
+	return ERR_PTR(ret);
 }
 
diff --git a/net/socket.c b/net/socket.c
index 8732fe2..f3b501d 100644
--- a/net/socket.c
+++ b/net/socket.c
@@ -96,8 +96,6 @@
 #include <net/sock.h>
 #include <linux/netfilter.h>
 
-#include <linux/checkpoint.h>
-
 static int sock_no_open(struct inode *irrelevant, struct file *dontcare);
 static ssize_t sock_aio_read(struct kiocb *iocb, const struct iovec *iov,
 			 unsigned long nr_segs, loff_t pos);
@@ -373,7 +371,7 @@ static int sock_alloc_fd(struct file **filep, int flags)
 	return fd;
 }
 
-static int sock_attach_fd(struct socket *sock, struct file *file, int flags)
+int sock_attach_fd(struct socket *sock, struct file *file, int flags)
 {
 	struct dentry *dentry;
 	struct qstr name = { .name = "" };
@@ -420,86 +418,6 @@ int sock_map_fd(struct socket *sock, int flags)
 	return fd;
 }
 
-#ifdef CONFIG_CHECKPOINT
-int sock_file_checkpoint(struct ckpt_ctx *ctx, void *ptr)
-{
-	struct ckpt_hdr_file_socket *h;
-	int ret;
-	struct file *file = ptr;
-
-	h = ckpt_hdr_get_type(ctx, sizeof(*h), CKPT_HDR_FILE);
-	if (!h)
-		return -ENOMEM;
-
-	h->common.f_type = CKPT_FILE_SOCKET;
-
-	ret = checkpoint_file_common(ctx, file, &h->common);
-	if (ret < 0)
-		goto out;
-	ret = ckpt_write_obj(ctx, (struct ckpt_hdr *) h);
-	if (ret < 0)
-		goto out;
-
-	ret = do_sock_file_checkpoint(ctx, file);
- out:
-	ckpt_hdr_put(ctx, h);
-	return ret;
-}
-
-static struct file *sock_alloc_attach_fd(struct socket *socket)
-{
-	struct file *file;
-	int err;
-
-	file = get_empty_filp();
-	if (!file)
-		return ERR_PTR(ENOMEM);
-
-	err = sock_attach_fd(socket, file, 0);
-	if (err < 0) {
-		put_filp(file);
-		file = ERR_PTR(err);
-	}
-
-	return file;
-}
-
-void *sock_file_restore(struct ckpt_ctx *ctx)
-{
-	struct ckpt_hdr_socket *h = NULL;
-	struct socket *socket = NULL;
-	struct file *file = NULL;
-	int err;
-
-	h = ckpt_read_obj_type(ctx, sizeof(*h), CKPT_HDR_SOCKET);
-	if (IS_ERR(h))
-		return h;
-
-	socket = do_sock_file_restore(ctx, h);
-	if (IS_ERR(socket)) {
-		err = PTR_ERR(socket);
-		goto err_put;
-	}
-
-	file = sock_alloc_attach_fd(socket);
-	if (IS_ERR(file)) {
-		err = PTR_ERR(file);
-		goto err_release;
-	}
-
-	ckpt_hdr_put(ctx, h);
-
-	return file;
-
- err_release:
-	sock_release(socket);
- err_put:
-	ckpt_hdr_put(ctx, h);
-
-	return ERR_PTR(err);
-}
-#endif /* CONFIG_CHECKPOINT */
-
 static struct socket *sock_from_file(struct file *file, int *err)
 {
 	if (file->f_op == &socket_file_ops)
diff --git a/net/unix/checkpoint.c b/net/unix/checkpoint.c
index 209e556..69fdcf1 100644
--- a/net/unix/checkpoint.c
+++ b/net/unix/checkpoint.c
@@ -55,9 +55,7 @@ static int sock_unix_write_cwd(struct ckpt_ctx *ctx,
 	return ret;
 }
 
-int sock_unix_checkpoint(struct ckpt_ctx *ctx,
-			 struct socket *socket,
-			 struct ckpt_hdr_socket *h)
+int sock_unix_checkpoint(struct ckpt_ctx *ctx, struct socket *socket)
 {
 	struct unix_sock *sk = unix_sk(socket->sk);
 	struct unix_sock *pr = unix_sk(sk->peer);
@@ -98,10 +96,6 @@ int sock_unix_checkpoint(struct ckpt_ctx *ctx,
 		goto out;
 	}
 
-	ret = ckpt_write_obj(ctx, (struct ckpt_hdr *) h);
-	if (ret < 0)
-		goto out;
-
 	ret = ckpt_write_obj(ctx, (struct ckpt_hdr *) un);
 	if (ret < 0)
 		goto out;
-- 
1.6.0.4

Re: [PATCH 1/3] c/r: cleanup sock_file_restore()/sock_file_checkpoint()

[Dan Smith]

OL> checkpoint/files.c: At top level:
OL> checkpoint/files.c:573: warning: initialization from incompatible pointer type

Yeah, I must've missed that among all the other c/r-induced
warnings... :)

OL> and evolved to some cleanup of that code, making the following
OL> changes:

Seems (and tests) okay to me.

Acked-by: Dan Smith <danms-r/Jw6+rmf7HQT0dZR+AlfA@public.gmane.org>

-- 
Dan Smith
IBM Linux Technology Center
email: danms-r/Jw6+rmf7HQT0dZR+AlfA@public.gmane.org

[PATCH 2/3] c/r: add checkpoint/restore methods to struct proto_ops

[Oren Laadan]

This patch adds checkpoint and restore methods to struct proto_ops
which should allow compilation of af-unix sockets as a module. It also
simplifies the code.

Signed-off-by: Oren Laadan <orenl-eQaUEPhvms7ENvBUuze7eA@public.gmane.org>
---
 include/linux/net.h   |    7 ++++++
 include/net/af_unix.h |   29 ++------------------------
 net/checkpoint.c      |   53 +++++++++++++++++++++++-------------------------
 net/unix/af_unix.c    |    6 +++++
 net/unix/checkpoint.c |    6 ++--
 5 files changed, 44 insertions(+), 57 deletions(-)

diff --git a/include/linux/net.h b/include/linux/net.h
index 2c4a75d..96c7e22 100644
--- a/include/linux/net.h
+++ b/include/linux/net.h
@@ -147,6 +147,9 @@ struct sockaddr;
 struct msghdr;
 struct module;
 
+struct ckpt_ctx;
+struct ckpt_hdr_socket;
+
 struct proto_ops {
 	int		family;
 	struct module	*owner;
@@ -191,6 +194,10 @@ struct proto_ops {
 				      int offset, size_t size, int flags);
 	ssize_t 	(*splice_read)(struct socket *sock,  loff_t *ppos,
 				       struct pipe_inode_info *pipe, size_t len, unsigned int flags);
+	int		(*checkpoint)(struct ckpt_ctx *ctx,
+				      struct socket *sock);
+	int		(*restore)(struct ckpt_ctx *ctx, struct socket *sock,
+				   struct ckpt_hdr_socket *h);
 };
 
 struct net_proto_family {
diff --git a/include/net/af_unix.h b/include/net/af_unix.h
index 35b5b9c..e265e9e 100644
--- a/include/net/af_unix.h
+++ b/include/net/af_unix.h
@@ -72,32 +72,9 @@ static inline void unix_sysctl_unregister(struct net *net) {}
 #ifdef CONFIG_CHECKPOINT
 struct ckpt_ctx;
 struct ckpt_hdr_socket;
-
-#ifdef CONFIG_UNIX_MODULE
-/* FIXME: Our current scheme won't work with CONFIG_UNIX=m */
-#error "CONFIG_UNIX=m not currently supported by CONFIG_CHECKPOINT"
-#endif
-
-#ifdef CONFIG_UNIX
-extern int sock_unix_checkpoint(struct ckpt_ctx *ctx, struct socket *socket);
-extern int sock_unix_restore(struct ckpt_ctx *ctx,
-			     struct ckpt_hdr_socket *h,
-			     struct socket *socket);
-#else
-static inline int sock_unix_checkpoint(struct ckpt_ctx *ctx,
-				       struct socket *socket,
-				       struct ckpt_hdr_socket *h)
-{
-	return -ENOSYS;
-}
-
-static inline int sock_unix_restore(struct ckpt_ctx *ctx,
-				    struct ckpt_hdr_socket *h,
-				    struct socket *socket)
-{
-	return -ENOSYS;
-}
-#endif /* CONFIG_UNIX */
+extern int sock_unix_checkpoint(struct ckpt_ctx *ctx, struct socket *sock);
+extern int sock_unix_restore(struct ckpt_ctx *ctx, struct socket *sock,
+			     struct ckpt_hdr_socket *h);
 #endif /* CONFIG_CHECKPOINT */
 
 #endif
diff --git a/net/checkpoint.c b/net/checkpoint.c
index b97cb89..9f92d7a 100644
--- a/net/checkpoint.c
+++ b/net/checkpoint.c
@@ -326,10 +326,15 @@ static int sock_cptrst(struct ckpt_ctx *ctx,
 int sock_file_checkpoint(struct ckpt_ctx *ctx, struct file *file)
 {
 	struct ckpt_hdr_file_socket *h;
-	struct socket *socket = file->private_data;
-	struct sock *sock = socket->sk;
+	struct socket *sock = file->private_data;
+	struct sock *sk = sock->sk;
 	int ret;
 
+	if (!sock->ops->checkpoint) {
+		ckpt_write_err(ctx, "socket (proto_ops: %pS)", sock->ops);
+		return -ENOSYS;
+	}
+
 	h = ckpt_hdr_get_type(ctx, sizeof(*h), CKPT_HDR_FILE);
 	if (!h)
 		return -ENOMEM;
@@ -337,7 +342,7 @@ int sock_file_checkpoint(struct ckpt_ctx *ctx, struct file *file)
 	h->common.f_type = CKPT_FILE_SOCKET;
 
 	/* part I: common to all sockets */
-	ret = sock_cptrst(ctx, sock, &h->socket, CKPT_CPT);
+	ret = sock_cptrst(ctx, sk, &h->socket, CKPT_CPT);
 	if (ret < 0)
 		goto out;
 	ret = checkpoint_file_common(ctx, file, &h->common);
@@ -348,22 +353,16 @@ int sock_file_checkpoint(struct ckpt_ctx *ctx, struct file *file)
 		goto out;
 
 	/* part II: per socket type state */
-	if (sock->sk_family == AF_UNIX) {
-		ret = sock_unix_checkpoint(ctx, socket);
-	} else {
-		ckpt_write_err(ctx, "unsupported socket family %i",
-			       sock->sk_family);
-		ret = -ENOSYS;
-	}
+	ret = sock->ops->checkpoint(ctx, sock);
 	if (ret < 0)
 		goto out;
 
 	/* part III: socket buffers */
-	if (sock->sk_state != TCP_LISTEN) {
-		ret = sock_write_buffers(ctx, &sock->sk_receive_queue);
+	if (sk->sk_state != TCP_LISTEN) {
+		ret = sock_write_buffers(ctx, &sk->sk_receive_queue);
 		if (ret)
 			goto out;
-		ret = sock_write_buffers(ctx, &sock->sk_write_queue);
+		ret = sock_write_buffers(ctx, &sk->sk_write_queue);
 	}
  out:
 	ckpt_hdr_put(ctx, h);
@@ -428,7 +427,7 @@ struct file *sock_file_restore(struct ckpt_ctx *ctx, struct ckpt_hdr_file *ptr)
 {
 	struct ckpt_hdr_file_socket *hh = (struct ckpt_hdr_file_socket *) ptr;
 	struct ckpt_hdr_socket *h = &hh->socket;
-	struct socket *socket;
+	struct socket *sock;
 	struct file *file;
 	int ret;
 
@@ -436,35 +435,33 @@ struct file *sock_file_restore(struct ckpt_ctx *ctx, struct ckpt_hdr_file *ptr)
 	    ptr->h.len != sizeof(*hh) || ptr->f_type != CKPT_FILE_SOCKET)
 		return ERR_PTR(-EINVAL);
 
-	if ((h->sock.type != SOCK_DGRAM) && (h->sock.type != SOCK_STREAM)) {
-		ckpt_debug("Socket type %i not supported", h->sock.type);
+	if (h->sock.type & ~SOCK_TYPE_MASK)
 		return ERR_PTR(-EINVAL);
-	}
 
-	ret = sock_create(h->sock_common.family, h->sock.type, 0, &socket);
+	ret = sock_create(h->sock_common.family, h->sock.type, 0, &sock);
 	if (ret < 0)
 		return ERR_PTR(ret);
 
+	if (!sock->ops->restore) {
+		ckpt_debug("proto_ops lacks checkpoint: %pS\n", sock->ops);
+		ret = -EINVAL;
+		goto err;
+	}
+
 	/*
 	 * part II: per socket type state
 	 * (also takes care of part III: socket buffer)
 	 */
-	if (h->sock_common.family == AF_UNIX) {
-		ret = sock_unix_restore(ctx, h, socket);
-		ckpt_debug("sock_unix_restore: %i\n", ret);
-	} else {
-		ckpt_debug("unsupported family %i\n", h->sock_common.family);
-		ret = -ENOSYS;
-	}
+	ret = sock->ops->restore(ctx, sock, h);
 	if (ret < 0)
 		goto err;
 
 	/* part I: common to all sockets */
-	ret = sock_cptrst(ctx, socket->sk, h, CKPT_RST);
+	ret = sock_cptrst(ctx, sock->sk, h, CKPT_RST);
 	if (ret < 0)
 		goto err;
 
-	file = sock_alloc_attach_fd(socket);
+	file = sock_alloc_attach_fd(sock);
 	if (IS_ERR(file)) {
 		ret = PTR_ERR(file);
 		goto err;
@@ -478,7 +475,7 @@ struct file *sock_file_restore(struct ckpt_ctx *ctx, struct ckpt_hdr_file *ptr)
 	return file;
 
  err:
-	sock_release(socket);
+	sock_release(sock);
 	return ERR_PTR(ret);
 }
 
diff --git a/net/unix/af_unix.c b/net/unix/af_unix.c
index fc3ebb9..667397d 100644
--- a/net/unix/af_unix.c
+++ b/net/unix/af_unix.c
@@ -523,6 +523,8 @@ static const struct proto_ops unix_stream_ops = {
 	.recvmsg =	unix_stream_recvmsg,
 	.mmap =		sock_no_mmap,
 	.sendpage =	sock_no_sendpage,
+	.checkpoint =	sock_unix_checkpoint,
+	.restore =	sock_unix_restore,
 };
 
 static const struct proto_ops unix_dgram_ops = {
@@ -544,6 +546,8 @@ static const struct proto_ops unix_dgram_ops = {
 	.recvmsg =	unix_dgram_recvmsg,
 	.mmap =		sock_no_mmap,
 	.sendpage =	sock_no_sendpage,
+	.checkpoint =	sock_unix_checkpoint,
+	.restore =	sock_unix_restore,
 };
 
 static const struct proto_ops unix_seqpacket_ops = {
@@ -565,6 +569,8 @@ static const struct proto_ops unix_seqpacket_ops = {
 	.recvmsg =	unix_dgram_recvmsg,
 	.mmap =		sock_no_mmap,
 	.sendpage =	sock_no_sendpage,
+	.checkpoint =	sock_unix_checkpoint,
+	.restore =	sock_unix_restore,
 };
 
 static struct proto unix_proto = {
diff --git a/net/unix/checkpoint.c b/net/unix/checkpoint.c
index 69fdcf1..d2431a4 100644
--- a/net/unix/checkpoint.c
+++ b/net/unix/checkpoint.c
@@ -472,9 +472,9 @@ static int sock_unix_precheck(struct socket *socket,
 	return 0;
 }
 
-int sock_unix_restore(struct ckpt_ctx *ctx,
-		      struct ckpt_hdr_socket *h,
-		      struct socket *socket)
+int sock_unix_restore(struct ckpt_ctx *ctx, struct socket *socket,
+		      struct ckpt_hdr_socket *h)
+
 {
 	struct ckpt_hdr_socket_unix *un;
 	int ret = -EINVAL;
-- 
1.6.0.4

Re: [PATCH 2/3] c/r: add checkpoint/restore methods to struct proto_ops

[Dan Smith]

OL> -	if ((h->sock.type != SOCK_DGRAM) && (h->sock.type != SOCK_STREAM)) {
OL> -		ckpt_debug("Socket type %i not supported", h->sock.type);
OL> +	if (h->sock.type & ~SOCK_TYPE_MASK)
OL>  		return ERR_PTR(-EINVAL);
OL> -	}

The idea here was to prevent a rewritten checkpoint stream from
causing us to call sock_create() with a type that we don't support.
Checking the mask may be functionally equivalent right now, but may
open the door for an interesting failure later, no?

OL>  static const struct proto_ops unix_seqpacket_ops = {
OL> @@ -565,6 +569,8 @@ static const struct proto_ops unix_seqpacket_ops = {
OL>  	.recvmsg =	unix_dgram_recvmsg,
OL>  	.mmap =		sock_no_mmap,
OL>  	.sendpage =	sock_no_sendpage,
OL> +	.checkpoint =	sock_unix_checkpoint,
OL> +	.restore =	sock_unix_restore,
OL>  };

Have you tested SEQPACKET?  I haven't.

Other than that, it looks fine to me and fails the same set of tests
that the current tree does.

-- 
Dan Smith
IBM Linux Technology Center
email: danms-r/Jw6+rmf7HQT0dZR+AlfA@public.gmane.org

Re: [PATCH 2/3] c/r: add checkpoint/restore methods to struct proto_ops

[Oren Laadan]

Dan Smith wrote:
> OL> -	if ((h->sock.type != SOCK_DGRAM) && (h->sock.type != SOCK_STREAM)) {
> OL> -		ckpt_debug("Socket type %i not supported", h->sock.type);
> OL> +	if (h->sock.type & ~SOCK_TYPE_MASK)
> OL>  		return ERR_PTR(-EINVAL);
> OL> -	}
> 
> The idea here was to prevent a rewritten checkpoint stream from
> causing us to call sock_create() with a type that we don't support.
> Checking the mask may be functionally equivalent right now, but may
> open the door for an interesting failure later, no?

I figured that if we called a sock_create() for a type that isn't
supported by checkpoint/restart, we will catch it down the code by
testing sock->ops->restore. That way, I don't need to "teach"
sock_file_restore() about socket types and protocols.

> 
> OL>  static const struct proto_ops unix_seqpacket_ops = {
> OL> @@ -565,6 +569,8 @@ static const struct proto_ops unix_seqpacket_ops = {
> OL>  	.recvmsg =	unix_dgram_recvmsg,
> OL>  	.mmap =		sock_no_mmap,
> OL>  	.sendpage =	sock_no_sendpage,
> OL> +	.checkpoint =	sock_unix_checkpoint,
> OL> +	.restore =	sock_unix_restore,
> OL>  };
> 
> Have you tested SEQPACKET?  I haven't.

Hehe ... I guess I'm too eager :)

Do you see any reason why it should work ?  The only difference (for
afunix) between dgram and seqpacket is the requirement that the socket
be connected - see unix_seqpacket_sendmsg().

> 
> Other than that, it looks fine to me and fails the same set of tests
> that the current tree does.
> 

Thanks,

Oren.

Re: [PATCH 2/3] c/r: add checkpoint/restore methods to struct proto_ops

[Dan Smith]

OL> I figured that if we called a sock_create() for a type that isn't
OL> supported by checkpoint/restart, we will catch it down the code by
OL> testing sock->ops->restore. That way, I don't need to "teach"
OL> sock_file_restore() about socket types and protocols.

Okay, yeah I suppose so.  In that case, why even have the check?
certainly sock_create() checks that somewhere down the line, right?

OL> Do you see any reason why it should work ?  The only difference
OL> (for afunix) between dgram and seqpacket is the requirement that
OL> the socket be connected - see unix_seqpacket_sendmsg().

Not for AF_UNIX, no.  I just said I hadn't tested it :)

-- 
Dan Smith
IBM Linux Technology Center
email: danms-r/Jw6+rmf7HQT0dZR+AlfA@public.gmane.org

Re: [PATCH 2/3] c/r: add checkpoint/restore methods to struct proto_ops

[Oren Laadan]

Dan Smith wrote:
> OL> I figured that if we called a sock_create() for a type that isn't
> OL> supported by checkpoint/restart, we will catch it down the code by
> OL> testing sock->ops->restore. That way, I don't need to "teach"
> OL> sock_file_restore() about socket types and protocols.
> 
> Okay, yeah I suppose so.  In that case, why even have the check?
> certainly sock_create() checks that somewhere down the line, right?

Hmm... true.  I was distracted by the test in socket() syscalls.
Will remove the test.

Oren.


> 
> OL> Do you see any reason why it should work ?  The only difference
> OL> (for afunix) between dgram and seqpacket is the requirement that
> OL> the socket be connected - see unix_seqpacket_sendmsg().
> 
> Not for AF_UNIX, no.  I just said I hadn't tested it :)
> 

[PATCH 3/3] c/r: cleanup - follow naming conventions

[Oren Laadan]

More cleanup of the sockets c/r code. Now that we use the methods from
struct proto_ops (checkpoint,restore), we no longer need the prefix
"sock_unix_..." for function names in net/unix/checkpoint.c; Instead
we follow the convention (e.g. in net/unix/afunix.c) "unix_...".

Speaking about code conventions, it is also customary to use 'sock'
for a struct socket, and a 'sk' for struct sock. This patch also makes
changes accordingly where necessary.

Signed-off-by: Oren Laadan <orenl-eQaUEPhvms7ENvBUuze7eA@public.gmane.org>
---
 include/net/af_unix.h |    6 +-
 net/checkpoint.c      |   91 +++++++++++++++-----------------
 net/unix/af_unix.c    |   12 ++--
 net/unix/checkpoint.c |  139 +++++++++++++++++++++++--------------------------
 4 files changed, 119 insertions(+), 129 deletions(-)

diff --git a/include/net/af_unix.h b/include/net/af_unix.h
index e265e9e..eba9142 100644
--- a/include/net/af_unix.h
+++ b/include/net/af_unix.h
@@ -72,9 +72,9 @@ static inline void unix_sysctl_unregister(struct net *net) {}
 #ifdef CONFIG_CHECKPOINT
 struct ckpt_ctx;
 struct ckpt_hdr_socket;
-extern int sock_unix_checkpoint(struct ckpt_ctx *ctx, struct socket *sock);
-extern int sock_unix_restore(struct ckpt_ctx *ctx, struct socket *sock,
-			     struct ckpt_hdr_socket *h);
+extern int unix_checkpoint(struct ckpt_ctx *ctx, struct socket *sock);
+extern int unix_restore(struct ckpt_ctx *ctx, struct socket *sock,
+			struct ckpt_hdr_socket *h);
 #endif /* CONFIG_CHECKPOINT */
 
 #endif
diff --git a/net/checkpoint.c b/net/checkpoint.c
index 9f92d7a..12ca1e6 100644
--- a/net/checkpoint.c
+++ b/net/checkpoint.c
@@ -1,7 +1,8 @@
 /*
  *  Copyright 2009 IBM Corporation
  *
- *  Author: Dan Smith <danms-r/Jw6+rmf7HQT0dZR+AlfA@public.gmane.org>
+ *  Authors: Dan Smith <danms-r/Jw6+rmf7HQT0dZR+AlfA@public.gmane.org>
+ *  	     Oren Laadan <orenl-eQaUEPhvms7ENvBUuze7eA@public.gmane.org>
  *
  *  This program is free software; you can redistribute it and/or
  *  modify it under the terms of the GNU General Public License as
@@ -140,25 +141,23 @@ static int sock_write_buffers(struct ckpt_ctx *ctx, struct sk_buff_head *queue)
 	return ret;
 }
 
-int ckpt_sock_getnames(struct ckpt_ctx *ctx,
-		       struct socket *socket,
+int ckpt_sock_getnames(struct ckpt_ctx *ctx, struct socket *sock,
 		       struct sockaddr *loc, unsigned *loc_len,
 		       struct sockaddr *rem, unsigned *rem_len)
 {
 	int ret;
 
-	ret = sock_getname(socket, loc, loc_len);
+	ret = sock_getname(sock, loc, loc_len);
 	if (ret) {
 		ckpt_write_err(ctx, "Unable to getname of local: %i", ret);
 		return -EINVAL;
 	}
 
-	ret = sock_getpeer(socket, rem, rem_len);
+	ret = sock_getpeer(sock, rem, rem_len);
 	if (ret) {
-		if ((socket->sk->sk_type != SOCK_DGRAM) &&
-		    (socket->sk->sk_state == TCP_ESTABLISHED)) {
-			ckpt_write_err(ctx, "Unable to getname of remote: %i",
-				       ret);
+		if ((sock->sk->sk_type != SOCK_DGRAM) &&
+		    (sock->sk->sk_state == TCP_ESTABLISHED)) {
+			ckpt_write_err(ctx, "socket get peer name: %i", ret);
 			return -EINVAL;
 		}
 		*rem_len = 0;
@@ -186,7 +185,7 @@ static int sock_cptrst_verify(struct ckpt_hdr_socket *h)
 	return 0;
 }
 
-static int sock_cptrst_opt(int op, struct socket *socket,
+static int sock_cptrst_opt(int op, struct socket *sock,
 			   int optname, char *opt, int len)
 {
 	mm_segment_t fs;
@@ -196,32 +195,32 @@ static int sock_cptrst_opt(int op, struct socket *socket,
 	set_fs(KERNEL_DS);
 
 	if (op == CKPT_CPT)
-		ret = sock_getsockopt(socket, SOL_SOCKET, optname, opt, &len);
+		ret = sock_getsockopt(sock, SOL_SOCKET, optname, opt, &len);
 	else
-		ret = sock_setsockopt(socket, SOL_SOCKET, optname, opt, len);
+		ret = sock_setsockopt(sock, SOL_SOCKET, optname, opt, len);
 
 	set_fs(fs);
 
 	return ret;
 }
 
-#define CKPT_COPY_SOPT(op, sock, name, opt) \
-	sock_cptrst_opt(op, sock->sk_socket, name, (char *)opt, sizeof(*opt))
+#define CKPT_COPY_SOPT(op, sk, name, opt) \
+	sock_cptrst_opt(op, sk->sk_socket, name, (char *)opt, sizeof(*opt))
 
-static int sock_cptrst_bufopts(int op, struct sock *sock,
+static int sock_cptrst_bufopts(int op, struct sock *sk,
 			       struct ckpt_hdr_socket *h)
 
 {
-	if (CKPT_COPY_SOPT(op, sock, SO_RCVBUF, &h->sock.rcvbuf))
+	if (CKPT_COPY_SOPT(op, sk, SO_RCVBUF, &h->sock.rcvbuf))
 		if ((op == CKPT_RST) &&
-		    CKPT_COPY_SOPT(op, sock, SO_RCVBUFFORCE, &h->sock.rcvbuf)) {
+		    CKPT_COPY_SOPT(op, sk, SO_RCVBUFFORCE, &h->sock.rcvbuf)) {
 			ckpt_debug("Failed to set SO_RCVBUF");
 			return -EINVAL;
 		}
 
-	if (CKPT_COPY_SOPT(op, sock, SO_SNDBUF, &h->sock.sndbuf))
+	if (CKPT_COPY_SOPT(op, sk, SO_SNDBUF, &h->sock.sndbuf))
 		if ((op == CKPT_RST) &&
-		    CKPT_COPY_SOPT(op, sock, SO_SNDBUFFORCE, &h->sock.sndbuf)) {
+		    CKPT_COPY_SOPT(op, sk, SO_SNDBUFFORCE, &h->sock.sndbuf)) {
 			ckpt_debug("Failed to set SO_SNDBUF");
 			return -EINVAL;
 		}
@@ -239,64 +238,62 @@ static int sock_cptrst_bufopts(int op, struct sock *sock,
 	return 0;
 }
 
-static int sock_cptrst(struct ckpt_ctx *ctx,
-		       struct sock *sock,
-		       struct ckpt_hdr_socket *h,
-		       int op)
+static int sock_cptrst(struct ckpt_ctx *ctx, struct sock *sk,
+		       struct ckpt_hdr_socket *h, int op)
 {
-	if (sock->sk_socket) {
-		CKPT_COPY(op, h->socket.flags, sock->sk_socket->flags);
-		CKPT_COPY(op, h->socket.state, sock->sk_socket->state);
+	if (sk->sk_socket) {
+		CKPT_COPY(op, h->socket.flags, sk->sk_socket->flags);
+		CKPT_COPY(op, h->socket.state, sk->sk_socket->state);
 	}
 
-	CKPT_COPY(op, h->sock_common.bound_dev_if, sock->sk_bound_dev_if);
-	CKPT_COPY(op, h->sock_common.family, sock->sk_family);
+	CKPT_COPY(op, h->sock_common.bound_dev_if, sk->sk_bound_dev_if);
+	CKPT_COPY(op, h->sock_common.family, sk->sk_family);
 
-	CKPT_COPY(op, h->sock.shutdown, sock->sk_shutdown);
-	CKPT_COPY(op, h->sock.userlocks, sock->sk_userlocks);
-	CKPT_COPY(op, h->sock.no_check, sock->sk_no_check);
-	CKPT_COPY(op, h->sock.protocol, sock->sk_protocol);
-	CKPT_COPY(op, h->sock.err, sock->sk_err);
-	CKPT_COPY(op, h->sock.err_soft, sock->sk_err_soft);
-	CKPT_COPY(op, h->sock.type, sock->sk_type);
-	CKPT_COPY(op, h->sock.state, sock->sk_state);
-	CKPT_COPY(op, h->sock.backlog, sock->sk_max_ack_backlog);
+	CKPT_COPY(op, h->sock.shutdown, sk->sk_shutdown);
+	CKPT_COPY(op, h->sock.userlocks, sk->sk_userlocks);
+	CKPT_COPY(op, h->sock.no_check, sk->sk_no_check);
+	CKPT_COPY(op, h->sock.protocol, sk->sk_protocol);
+	CKPT_COPY(op, h->sock.err, sk->sk_err);
+	CKPT_COPY(op, h->sock.err_soft, sk->sk_err_soft);
+	CKPT_COPY(op, h->sock.type, sk->sk_type);
+	CKPT_COPY(op, h->sock.state, sk->sk_state);
+	CKPT_COPY(op, h->sock.backlog, sk->sk_max_ack_backlog);
 
 	/* TODO:
 	 * Break out setting each of the flags to use setsockopt() or
 	 * perform proper security check
 	 */
-	CKPT_COPY(op, h->sock.flags, sock->sk_flags);
+	CKPT_COPY(op, h->sock.flags, sk->sk_flags);
 
-	if (sock_cptrst_bufopts(op, sock, h))
+	if (sock_cptrst_bufopts(op, sk, h))
 		return -EINVAL;
 
-	if (CKPT_COPY_SOPT(op, sock, SO_REUSEADDR, &h->sock_common.reuse)) {
+	if (CKPT_COPY_SOPT(op, sk, SO_REUSEADDR, &h->sock_common.reuse)) {
 		ckpt_debug("Failed to set SO_REUSEADDR");
 		return -EINVAL;
 	}
 
-	if (CKPT_COPY_SOPT(op, sock, SO_PRIORITY, &h->sock.priority)) {
+	if (CKPT_COPY_SOPT(op, sk, SO_PRIORITY, &h->sock.priority)) {
 		ckpt_debug("Failed to set SO_PRIORITY");
 		return -EINVAL;
 	}
 
-	if (CKPT_COPY_SOPT(op, sock, SO_RCVLOWAT, &h->sock.rcvlowat)) {
+	if (CKPT_COPY_SOPT(op, sk, SO_RCVLOWAT, &h->sock.rcvlowat)) {
 		ckpt_debug("Failed to set SO_RCVLOWAT");
 		return -EINVAL;
 	}
 
-	if (CKPT_COPY_SOPT(op, sock, SO_LINGER, &h->sock.linger)) {
+	if (CKPT_COPY_SOPT(op, sk, SO_LINGER, &h->sock.linger)) {
 		ckpt_debug("Failed to set SO_LINGER");
 		return -EINVAL;
 	}
 
-	if (CKPT_COPY_SOPT(op, sock, SO_SNDTIMEO, &h->sock.sndtimeo)) {
+	if (CKPT_COPY_SOPT(op, sk, SO_SNDTIMEO, &h->sock.sndtimeo)) {
 		ckpt_debug("Failed to set SO_SNDTIMEO");
 		return -EINVAL;
 	}
 
-	if (CKPT_COPY_SOPT(op, sock, SO_RCVTIMEO, &h->sock.rcvtimeo)) {
+	if (CKPT_COPY_SOPT(op, sk, SO_RCVTIMEO, &h->sock.rcvtimeo)) {
 		ckpt_debug("Failed to set SO_RCVTIMEO");
 		return -EINVAL;
 	}
@@ -405,7 +402,7 @@ struct ckpt_hdr_socket_queue *ckpt_sock_read_buffer_hdr(struct ckpt_ctx *ctx,
 		return h;
 }
 
-static struct file *sock_alloc_attach_fd(struct socket *socket)
+static struct file *sock_alloc_attach_fd(struct socket *sock)
 {
 	struct file *file;
 	int err;
@@ -414,7 +411,7 @@ static struct file *sock_alloc_attach_fd(struct socket *socket)
 	if (!file)
 		return ERR_PTR(ENOMEM);
 
-	err = sock_attach_fd(socket, file, 0);
+	err = sock_attach_fd(sock, file, 0);
 	if (err < 0) {
 		put_filp(file);
 		file = ERR_PTR(err);
diff --git a/net/unix/af_unix.c b/net/unix/af_unix.c
index 667397d..da6405d 100644
--- a/net/unix/af_unix.c
+++ b/net/unix/af_unix.c
@@ -523,8 +523,8 @@ static const struct proto_ops unix_stream_ops = {
 	.recvmsg =	unix_stream_recvmsg,
 	.mmap =		sock_no_mmap,
 	.sendpage =	sock_no_sendpage,
-	.checkpoint =	sock_unix_checkpoint,
-	.restore =	sock_unix_restore,
+	.checkpoint =	unix_checkpoint,
+	.restore =	unix_restore,
 };
 
 static const struct proto_ops unix_dgram_ops = {
@@ -546,8 +546,8 @@ static const struct proto_ops unix_dgram_ops = {
 	.recvmsg =	unix_dgram_recvmsg,
 	.mmap =		sock_no_mmap,
 	.sendpage =	sock_no_sendpage,
-	.checkpoint =	sock_unix_checkpoint,
-	.restore =	sock_unix_restore,
+	.checkpoint =	unix_checkpoint,
+	.restore =	unix_restore,
 };
 
 static const struct proto_ops unix_seqpacket_ops = {
@@ -569,8 +569,8 @@ static const struct proto_ops unix_seqpacket_ops = {
 	.recvmsg =	unix_dgram_recvmsg,
 	.mmap =		sock_no_mmap,
 	.sendpage =	sock_no_sendpage,
-	.checkpoint =	sock_unix_checkpoint,
-	.restore =	sock_unix_restore,
+	.checkpoint =	unix_checkpoint,
+	.restore =	unix_restore,
 };
 
 static struct proto unix_proto = {
diff --git a/net/unix/checkpoint.c b/net/unix/checkpoint.c
index d2431a4..772ab5f 100644
--- a/net/unix/checkpoint.c
+++ b/net/unix/checkpoint.c
@@ -8,17 +8,15 @@
 
 #define UNIX_ADDR_EMPTY(a) (a <= sizeof(short))
 
-static inline int sock_unix_need_cwd(struct sockaddr_un *addr,
-				     unsigned long len)
+static inline int unix_need_cwd(struct sockaddr_un *addr, unsigned long len)
 {
 	return (!UNIX_ADDR_EMPTY(len)) &&
 		addr->sun_path[0] &&
 		(addr->sun_path[0] != '/');
 }
 
-static int sock_unix_write_cwd(struct ckpt_ctx *ctx,
-			       struct sock *sock,
-			       const char *sockpath)
+static int unix_write_cwd(struct ckpt_ctx *ctx,
+			  struct sock *sk, const char *sockpath)
 {
 	struct path path;
 	char *buf;
@@ -31,8 +29,8 @@ static int sock_unix_write_cwd(struct ckpt_ctx *ctx,
 	if (!buf)
 		return -ENOMEM;
 
-	path.dentry = unix_sk(sock)->dentry;
-	path.mnt = unix_sk(sock)->mnt;
+	path.dentry = unix_sk(sk)->dentry;
+	path.mnt = unix_sk(sk)->mnt;
 
 	fqpath = ckpt_fill_fname(&path, &ctx->fs_mnt, buf, &len);
 	if (IS_ERR(fqpath)) {
@@ -55,16 +53,16 @@ static int sock_unix_write_cwd(struct ckpt_ctx *ctx,
 	return ret;
 }
 
-int sock_unix_checkpoint(struct ckpt_ctx *ctx, struct socket *socket)
+int unix_checkpoint(struct ckpt_ctx *ctx, struct socket *sock)
 {
-	struct unix_sock *sk = unix_sk(socket->sk);
+	struct unix_sock *sk = unix_sk(sock->sk);
 	struct unix_sock *pr = unix_sk(sk->peer);
 	struct ckpt_hdr_socket_unix *un;
 	int new;
 	int ret = -ENOMEM;
 
-	if ((socket->sk->sk_state == TCP_LISTEN) &&
-	    !skb_queue_empty(&socket->sk->sk_receive_queue)) {
+	if ((sock->sk->sk_state == TCP_LISTEN) &&
+	    !skb_queue_empty(&sock->sk->sk_receive_queue)) {
 		ckpt_write_err(ctx, "listening socket has unaccepted peers");
 		return -EBUSY;
 	}
@@ -73,7 +71,7 @@ int sock_unix_checkpoint(struct ckpt_ctx *ctx, struct socket *socket)
 	if (!un)
 		return -EINVAL;
 
-	ret = ckpt_sock_getnames(ctx, socket,
+	ret = ckpt_sock_getnames(ctx, sock,
 				 (struct sockaddr *)&un->laddr, &un->laddr_len,
 				 (struct sockaddr *)&un->raddr, &un->raddr_len);
 	if (ret)
@@ -100,15 +98,15 @@ int sock_unix_checkpoint(struct ckpt_ctx *ctx, struct socket *socket)
 	if (ret < 0)
 		goto out;
 
-	if (sock_unix_need_cwd(&un->laddr, un->laddr_len))
-		ret = sock_unix_write_cwd(ctx, socket->sk, un->laddr.sun_path);
+	if (unix_need_cwd(&un->laddr, un->laddr_len))
+		ret = unix_write_cwd(ctx, sock->sk, un->laddr.sun_path);
  out:
 	ckpt_hdr_put(ctx, un);
 
 	return ret;
 }
 
-static int sock_read_buffer_sendmsg(struct ckpt_ctx *ctx, struct sock *sock)
+static int sock_read_buffer_sendmsg(struct ckpt_ctx *ctx, struct sock *sk)
 {
 	struct msghdr msg;
 	struct kvec kvec;
@@ -138,7 +136,7 @@ static int sock_read_buffer_sendmsg(struct ckpt_ctx *ctx, struct sock *sock)
 	if (ret < 0)
 		goto out;
 
-	ret = kernel_sendmsg(sock->sk_socket, &msg, &kvec, 1, len);
+	ret = kernel_sendmsg(sk->sk_socket, &msg, &kvec, 1, len);
 	ckpt_debug("kernel_sendmsg(%i): %i\n", len, ret);
 	if ((ret > 0) && (ret != len))
 		ret = -ENOMEM;
@@ -148,9 +146,8 @@ static int sock_read_buffer_sendmsg(struct ckpt_ctx *ctx, struct sock *sock)
 	return ret;
 }
 
-static int sock_unix_read_buffers(struct ckpt_ctx *ctx,
-				  struct sock *sock,
-				  uint32_t *bufsize)
+static int unix_read_buffers(struct ckpt_ctx *ctx,
+			     struct sock *sk, uint32_t *bufsize)
 {
 	uint8_t sock_shutdown;
 	struct ckpt_hdr_socket_queue *h;
@@ -162,11 +159,11 @@ static int sock_unix_read_buffers(struct ckpt_ctx *ctx,
 		return PTR_ERR(h);
 
 	/* If peer is shutdown, unshutdown it for this process */
-	sock_shutdown = sock->sk_shutdown;
-	sock->sk_shutdown &= ~SHUTDOWN_MASK;
+	sock_shutdown = sk->sk_shutdown;
+	sk->sk_shutdown &= ~SHUTDOWN_MASK;
 
 	for (i = 0; i < h->skb_count; i++) {
-		ret = sock_read_buffer_sendmsg(ctx, sock);
+		ret = sock_read_buffer_sendmsg(ctx, sk);
 		ckpt_debug("read_buffer_sendmsg(%i): %i\n", i, ret);
 		if (ret < 0)
 			break;
@@ -181,14 +178,14 @@ static int sock_unix_read_buffers(struct ckpt_ctx *ctx,
 		ret = 0;
 	}
 
-	sock->sk_shutdown = sock_shutdown;
+	sk->sk_shutdown = sock_shutdown;
 	ckpt_hdr_put(ctx, h);
 
 	return ret;
 }
 
-static struct unix_address *sock_unix_makeaddr(struct sockaddr_un *sun_addr,
-					       unsigned len)
+static struct unix_address *unix_makeaddr(struct sockaddr_un *sun_addr,
+					  unsigned len)
 {
 	struct unix_address *addr;
 
@@ -206,10 +203,9 @@ static struct unix_address *sock_unix_makeaddr(struct sockaddr_un *sun_addr,
 	return addr;
 }
 
-static int sock_unix_join(struct ckpt_ctx *ctx,
-			  struct sock *a,
-			  struct sock *b,
-			  struct ckpt_hdr_socket_unix *un)
+static int unix_join(struct ckpt_ctx *ctx,
+		     struct sock *a, struct sock *b,
+		     struct ckpt_hdr_socket_unix *un)
 {
 	struct unix_address *addr = NULL;
 
@@ -235,9 +231,9 @@ static int sock_unix_join(struct ckpt_ctx *ctx,
 	b->sk_peercred.gid = a->sk_peercred.gid;
 
 	if (!UNIX_ADDR_EMPTY(un->raddr_len))
-		addr = sock_unix_makeaddr(&un->raddr, un->raddr_len);
+		addr = unix_makeaddr(&un->raddr, un->raddr_len);
 	else if (!UNIX_ADDR_EMPTY(un->laddr_len))
-		addr = sock_unix_makeaddr(&un->laddr, un->laddr_len);
+		addr = unix_makeaddr(&un->laddr, un->laddr_len);
 
 	if (IS_ERR(addr))
 		return PTR_ERR(addr);
@@ -249,10 +245,10 @@ static int sock_unix_join(struct ckpt_ctx *ctx,
 	return 0;
 }
 
-static int sock_unix_restore_connected(struct ckpt_ctx *ctx,
-				       struct ckpt_hdr_socket *h,
-				       struct ckpt_hdr_socket_unix *un,
-				       struct socket *socket)
+static int unix_restore_connected(struct ckpt_ctx *ctx,
+				  struct ckpt_hdr_socket *h,
+				  struct ckpt_hdr_socket_unix *un,
+				  struct socket *sock)
 {
 	struct sock *this = ckpt_obj_fetch(ctx, un->this, CKPT_OBJ_SOCK);
 	struct sock *peer = ckpt_obj_fetch(ctx, un->peer, CKPT_OBJ_SOCK);
@@ -265,19 +261,19 @@ static int sock_unix_restore_connected(struct ckpt_ctx *ctx,
 
 		old->sk = NULL;
 		sock_release(old);
-		sock_graft(this, socket);
+		sock_graft(this, sock);
 
 	} else if ((PTR_ERR(this) == -EINVAL) && (PTR_ERR(peer) == -EINVAL)) {
 		/* We're first */
-		int family = socket->sk->sk_family;
-		int type = socket->sk->sk_type;
+		int family = sock->sk->sk_family;
+		int type = sock->sk->sk_type;
 
 		ret = sock_create(family, type, 0, &tmp);
 		ckpt_debug("sock_create: %i\n", ret);
 		if (ret)
 			goto out;
 
-		this = socket->sk;
+		this = sock->sk;
 		peer = tmp->sk;
 
 		ret = ckpt_obj_insert(ctx, this, un->this, CKPT_OBJ_SOCK);
@@ -288,8 +284,8 @@ static int sock_unix_restore_connected(struct ckpt_ctx *ctx,
 		if (ret < 0)
 			goto out;
 
-		ret = sock_unix_join(ctx, this, peer, un);
-		ckpt_debug("sock_unix_join: %i\n", ret);
+		ret = unix_join(ctx, this, peer, un);
+		ckpt_debug("unix_join: %i\n", ret);
 		if (ret)
 			goto out;
 
@@ -312,13 +308,13 @@ static int sock_unix_restore_connected(struct ckpt_ctx *ctx,
 	 *       where sendto() has been used on some of the buffers
 	 */
 
-	ret = sock_unix_read_buffers(ctx, peer, &peer->sk_sndbuf);
-	ckpt_debug("sock_unix_read_buffers: %i\n", ret);
+	ret = unix_read_buffers(ctx, peer, &peer->sk_sndbuf);
+	ckpt_debug("unix_read_buffers: %i\n", ret);
 	if (ret)
 		goto out;
 
 	/* Read peer's buffers and expect 0 */
-	ret = sock_unix_read_buffers(ctx, peer, NULL);
+	ret = unix_read_buffers(ctx, peer, NULL);
  out:
 	if (tmp && ret)
 		sock_release(tmp);
@@ -326,7 +322,7 @@ static int sock_unix_restore_connected(struct ckpt_ctx *ctx,
 	return ret;
 }
 
-static int sock_unix_unlink(const char *name)
+static int unix_unlink(const char *name)
 {
 	struct path spath;
 	struct path ppath;
@@ -364,10 +360,10 @@ static int sock_unix_unlink(const char *name)
 /* Call bind() for socket, optionally changing (temporarily) to @path first
  * if non-NULL
  */
-static int sock_unix_chdir_and_bind(struct socket *socket,
-				    const char *path,
-				    struct sockaddr *addr,
-				    unsigned long addrlen)
+static int unix_chdir_and_bind(struct socket *sock,
+			       const char *path,
+			       struct sockaddr *addr,
+			       unsigned long addrlen)
 {
 	struct sockaddr_un *un = (struct sockaddr_un *)addr;
 	int ret;
@@ -392,10 +388,10 @@ static int sock_unix_chdir_and_bind(struct socket *socket,
 		write_unlock(&current->fs->lock);
 	}
 
-	ret = sock_unix_unlink(un->sun_path);
+	ret = unix_unlink(un->sun_path);
 	ckpt_debug("unlink(%s): %i\n", un->sun_path, ret);
 	if ((ret == 0) || (ret == -ENOENT))
-		ret = sock_bind(socket, addr, addrlen);
+		ret = sock_bind(sock, addr, addrlen);
 
 	if (path) {
 		write_lock(&current->fs->lock);
@@ -409,42 +405,40 @@ static int sock_unix_chdir_and_bind(struct socket *socket,
 	return ret;
 }
 
-static int sock_unix_fakebind(struct socket *socket,
-			      struct sockaddr_un *addr,
-			      unsigned long len)
+static int unix_fakebind(struct socket *sock,
+			 struct sockaddr_un *addr, unsigned long len)
 {
 	struct unix_address *uaddr;
 
-	uaddr = sock_unix_makeaddr(addr, len);
+	uaddr = unix_makeaddr(addr, len);
 	if (IS_ERR(uaddr))
 		return PTR_ERR(uaddr);
 
-	unix_sk(socket->sk)->addr = uaddr;
+	unix_sk(sock->sk)->addr = uaddr;
 
 	return 0;
 }
 
-static int sock_unix_bind(struct ckpt_hdr_socket *h,
-			  struct ckpt_hdr_socket_unix *un,
-			  struct socket *socket,
-			  const char *path)
+static int unix_restore_bind(struct ckpt_hdr_socket *h,
+			     struct ckpt_hdr_socket_unix *un,
+			     struct socket *sock,
+			     const char *path)
 {
 	struct sockaddr *addr = (struct sockaddr *)&un->laddr;
 	unsigned long len = un->laddr_len;
 
 	if (!un->laddr.sun_path[0])
-		return sock_bind(socket, addr, len);
+		return sock_bind(sock, addr, len);
 	else if (!(un->flags & CKPT_UNIX_LINKED))
-		return sock_unix_fakebind(socket, &un->laddr, len);
+		return unix_fakebind(sock, &un->laddr, len);
 	else
-		return sock_unix_chdir_and_bind(socket, path, addr, len);
+		return unix_chdir_and_bind(sock, path, addr, len);
 }
 
 /* Some easy pre-flight checks before we get underway */
-static int sock_unix_precheck(struct socket *socket,
-			      struct ckpt_hdr_socket *h)
+static int unix_precheck(struct socket *sock, struct ckpt_hdr_socket *h)
 {
-	struct net *net = sock_net(socket->sk);
+	struct net *net = sock_net(sock->sk);
 
 	if ((h->socket.state == SS_CONNECTING) ||
 	    (h->socket.state == SS_DISCONNECTING) ||
@@ -472,7 +466,7 @@ static int sock_unix_precheck(struct socket *socket,
 	return 0;
 }
 
-int sock_unix_restore(struct ckpt_ctx *ctx, struct socket *socket,
+int unix_restore(struct ckpt_ctx *ctx, struct socket *sock,
 		      struct ckpt_hdr_socket *h)
 
 {
@@ -480,7 +474,7 @@ int sock_unix_restore(struct ckpt_ctx *ctx, struct socket *socket,
 	int ret = -EINVAL;
 	char *cwd = NULL;
 
-	ret = sock_unix_precheck(socket, h);
+	ret = unix_precheck(sock, h);
 	if (ret)
 		return ret;
 
@@ -491,7 +485,7 @@ int sock_unix_restore(struct ckpt_ctx *ctx, struct socket *socket,
 	if (un->peer < 0)
 		goto out;
 
-	if (sock_unix_need_cwd(&un->laddr, un->laddr_len)) {
+	if (unix_need_cwd(&un->laddr, un->laddr_len)) {
 		cwd = ckpt_read_string(ctx, PATH_MAX);
 		if (IS_ERR(cwd)) {
 			ret = PTR_ERR(cwd);
@@ -501,15 +495,15 @@ int sock_unix_restore(struct ckpt_ctx *ctx, struct socket *socket,
 
 	if ((h->sock.state != TCP_ESTABLISHED) &&
 	    !UNIX_ADDR_EMPTY(un->laddr_len)) {
-		ret = sock_unix_bind(h, un, socket, cwd);
+		ret = unix_restore_bind(h, un, sock, cwd);
 		if (ret)
 			goto out;
 	}
 
 	if ((h->sock.state == TCP_ESTABLISHED) || (h->sock.state == TCP_CLOSE))
-		ret = sock_unix_restore_connected(ctx, h, un, socket);
+		ret = unix_restore_connected(ctx, h, un, sock);
 	else if (h->sock.state == TCP_LISTEN)
-		ret = socket->ops->listen(socket, h->sock.backlog);
+		ret = sock->ops->listen(sock, h->sock.backlog);
 	else
 		ckpt_debug("unsupported UNIX socket state %i\n", h->sock.state);
  out:
@@ -517,4 +511,3 @@ int sock_unix_restore(struct ckpt_ctx *ctx, struct socket *socket,
 	kfree(cwd);
 	return ret;
 }
-
-- 
1.6.0.4

Re: [PATCH 3/3] c/r: cleanup - follow naming conventions

[Dan Smith]

OL> More cleanup of the sockets c/r code. Now that we use the methods
OL> from struct proto_ops (checkpoint,restore), we no longer need the
OL> prefix "sock_unix_..." for function names in
OL> net/unix/checkpoint.c; Instead we follow the convention (e.g. in
OL> net/unix/afunix.c) "unix_...".

No complaints.

Acked-by: Dan Smith <danms-r/Jw6+rmf7HQT0dZR+AlfA@public.gmane.org>

-- 
Dan Smith
IBM Linux Technology Center
email: danms-r/Jw6+rmf7HQT0dZR+AlfA@public.gmane.org