Dan Smith wrote:
> The previous change to the buffer save/restore logic caused us to
> incorrectly attempt to restore buffers on a socket in a few instances
> where flags may not be restored yet or we are in a temporarily altered
> state.
> 
> So, keep the same logic, but save a flag to indicate whether buffers were
> checkpointed to avoid the ambiguity at restore time.
> 
> Also, move the UNIX buffer restore point to the main function to avoid
> potentially ignoring the flag on a socket we assume wouldn't have saved
> buffers.
> 
> Signed-off-by: Dan Smith <[email protected]>

Looks good.

So now that ckpt_sock_need_buffers() is used only inside
net/checkpoint.c and only in one place, let's make it static
(inline) there, or even code in-place ?

Otherwise - Ack.

Oren.

> ---
>  include/linux/checkpoint_hdr.h |    2 ++
>  net/checkpoint.c               |    4 +++-
>  net/ipv4/checkpoint.c          |    2 +-
>  net/unix/checkpoint.c          |    9 ++++++---
>  4 files changed, 12 insertions(+), 5 deletions(-)
> 
> diff --git a/include/linux/checkpoint_hdr.h b/include/linux/checkpoint_hdr.h
> index 9e8d518..f4f9577 100644
> --- a/include/linux/checkpoint_hdr.h
> +++ b/include/linux/checkpoint_hdr.h
> @@ -596,6 +596,8 @@ struct ckpt_hdr_file_eventfd {
>  struct ckpt_hdr_socket {
>       struct ckpt_hdr h;
>  
> +     __u8 has_buffers;
> +
>       struct { /* struct socket */
>               __u64 flags;
>               __u8 state;
> diff --git a/net/checkpoint.c b/net/checkpoint.c
> index 0d90af2..de8e454 100644
> --- a/net/checkpoint.c
> +++ b/net/checkpoint.c
> @@ -761,6 +761,8 @@ static int __do_sock_checkpoint(struct ckpt_ctx *ctx, 
> struct sock *sk)
>       if (!h)
>               return -ENOMEM;
>  
> +     h->has_buffers = ckpt_sock_need_buffers(sk);
> +
>       /* part I: common to all sockets */
>       ret = sock_cptrst(ctx, sk, h, CKPT_CPT);
>       if (ret < 0)
> @@ -776,7 +778,7 @@ static int __do_sock_checkpoint(struct ckpt_ctx *ctx, 
> struct sock *sk)
>               goto out;
>  
>       /* part III: socket buffers */
> -     if (ckpt_sock_need_buffers(sk))
> +     if (h->has_buffers)
>               ret = sock_defer_write_buffers(ctx, sk);
>   out:
>       ckpt_hdr_put(ctx, h);
> diff --git a/net/ipv4/checkpoint.c b/net/ipv4/checkpoint.c
> index eb02175..ec42e57 100644
> --- a/net/ipv4/checkpoint.c
> +++ b/net/ipv4/checkpoint.c
> @@ -539,7 +539,7 @@ int inet_restore(struct ckpt_ctx *ctx,
>               }
>       }
>  
> -     if (ckpt_sock_need_buffers(sock->sk))
> +     if (h->has_buffers)
>               ret = inet_defer_restore_buffers(ctx, sock->sk);
>   out:
>       ckpt_hdr_put(ctx, in);
> diff --git a/net/unix/checkpoint.c b/net/unix/checkpoint.c
> index 1fa3d7e..47d38e2 100644
> --- a/net/unix/checkpoint.c
> +++ b/net/unix/checkpoint.c
> @@ -443,9 +443,6 @@ static int unix_restore_connected(struct ckpt_ctx *ctx,
>               ckpt_debug("unix_defer_join: %i\n", ret);
>       }
>  
> -     if (ckpt_sock_need_buffers(sock->sk) && !ret)
> -             ret = unix_defer_restore_buffers(ctx, un->this);
> -
>       return ret;
>  }
>  
> @@ -639,6 +636,12 @@ int unix_restore(struct ckpt_ctx *ctx, struct socket 
> *sock,
>       else
>               ckpt_err(ctx, ret, "bad af_unix state %i\n", h->sock.state);
>  
> +     if (ret < 0)
> +             goto out;
> +
> +     if (h->has_buffers)
> +             ret = unix_defer_restore_buffers(ctx, un->this);
> +
>   out:
>       ckpt_hdr_put(ctx, un);
>       kfree(cwd);
_______________________________________________
Containers mailing list
[email protected]
https://lists.linux-foundation.org/mailman/listinfo/containers

_______________________________________________
Devel mailing list
[email protected]
https://openvz.org/mailman/listinfo/devel

Reply via email to