2021-01-15 05:55:20

by Arseny Krasnov

Subject: [RFC PATCH v2 01/13] af_vsock: implement 'vsock_wait_data()'.

This adds 'vsock_wait_data()' function which is called from user's read
syscall and waits until new socket data is arrived. It was based on code
from stream dequeue logic and moved to separate function because it will
be called both from SOCK_STREAM and SOCK_SEQPACKET receive loops.

Signed-off-by: Arseny Krasnov <[email protected]>
---
net/vmw_vsock/af_vsock.c | 47 ++++++++++++++++++++++++++++++++++++++++
1 file changed, 47 insertions(+)

diff --git a/net/vmw_vsock/af_vsock.c b/net/vmw_vsock/af_vsock.c
index b12d3a322242..af716f5a93a4 100644
--- a/net/vmw_vsock/af_vsock.c
+++ b/net/vmw_vsock/af_vsock.c
@@ -1822,6 +1822,53 @@ static int vsock_stream_sendmsg(struct socket *sock, struct msghdr *msg,
return err;
}

+static int vsock_wait_data(struct sock *sk, struct wait_queue_entry *wait,
+ long timeout,
+ struct vsock_transport_recv_notify_data *recv_data,
+ size_t target)
+{
+ int err = 0;
+ struct vsock_sock *vsk;
+ const struct vsock_transport *transport;
+
+ vsk = vsock_sk(sk);
+ transport = vsk->transport;
+
+ if (sk->sk_err != 0 ||
+ (sk->sk_shutdown & RCV_SHUTDOWN) ||
+ (vsk->peer_shutdown & SEND_SHUTDOWN)) {
+ finish_wait(sk_sleep(sk), wait);
+ return -1;
+ }
+ /* Don't wait for non-blocking sockets. */
+ if (timeout == 0) {
+ err = -EAGAIN;
+ finish_wait(sk_sleep(sk), wait);
+ return err;
+ }
+
+ if (recv_data) {
+ err = transport->notify_recv_pre_block(vsk, target, recv_data);
+ if (err < 0) {
+ finish_wait(sk_sleep(sk), wait);
+ return err;
+ }
+ }
+
+ release_sock(sk);
+ timeout = schedule_timeout(timeout);
+ lock_sock(sk);
+
+ if (signal_pending(current)) {
+ err = sock_intr_errno(timeout);
+ finish_wait(sk_sleep(sk), wait);
+ } else if (timeout == 0) {
+ err = -EAGAIN;
+ finish_wait(sk_sleep(sk), wait);
+ }
+
+ return err;
+}

static int
vsock_stream_recvmsg(struct socket *sock, struct msghdr *msg, size_t len,
--
2.25.1

2021-01-18 15:05:43

by Stefano Garzarella

Subject: Re: [RFC PATCH v2 01/13] af_vsock: implement 'vsock_wait_data()'.

On Fri, Jan 15, 2021 at 08:40:25AM +0300, Arseny Krasnov wrote:
>This adds 'vsock_wait_data()' function which is called from user's read
>syscall and waits until new socket data is arrived. It was based on code
>from stream dequeue logic and moved to separate function because it will
>be called both from SOCK_STREAM and SOCK_SEQPACKET receive loops.
>
>Signed-off-by: Arseny Krasnov <[email protected]>
>---
> net/vmw_vsock/af_vsock.c | 47 ++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 47 insertions(+)
>
>diff --git a/net/vmw_vsock/af_vsock.c b/net/vmw_vsock/af_vsock.c
>index b12d3a322242..af716f5a93a4 100644
>--- a/net/vmw_vsock/af_vsock.c
>+++ b/net/vmw_vsock/af_vsock.c
>@@ -1822,6 +1822,53 @@ static int vsock_stream_sendmsg(struct socket *sock, struct msghdr *msg,
> return err;
> }
>
>+static int vsock_wait_data(struct sock *sk, struct wait_queue_entry *wait,
>+ long timeout,
>+ struct vsock_transport_recv_notify_data *recv_data,
>+ size_t target)
>+{
>+ int err = 0;
>+ struct vsock_sock *vsk;
>+ const struct vsock_transport *transport;

Please be sure that here and in all of the next patches, you follow the
"Reverse Christmas tree" rule followed in net/ for the local variable
declarations (order variable declaration lines longest to shortest).

>+
>+ vsk = vsock_sk(sk);
>+ transport = vsk->transport;
>+
>+ if (sk->sk_err != 0 ||
>+ (sk->sk_shutdown & RCV_SHUTDOWN) ||
>+ (vsk->peer_shutdown & SEND_SHUTDOWN)) {
>+ finish_wait(sk_sleep(sk), wait);
>+ return -1;
>+ }
>+ /* Don't wait for non-blocking sockets. */
>+ if (timeout == 0) {
>+ err = -EAGAIN;
>+ finish_wait(sk_sleep(sk), wait);
>+ return err;
>+ }
>+
>+ if (recv_data) {
>+ err = transport->notify_recv_pre_block(vsk, target, recv_data);
>+ if (err < 0) {
>+ finish_wait(sk_sleep(sk), wait);
>+ return err;
>+ }
>+ }
>+
>+ release_sock(sk);
>+ timeout = schedule_timeout(timeout);
>+ lock_sock(sk);
>+
>+ if (signal_pending(current)) {
>+ err = sock_intr_errno(timeout);
>+ finish_wait(sk_sleep(sk), wait);
>+ } else if (timeout == 0) {
>+ err = -EAGAIN;
>+ finish_wait(sk_sleep(sk), wait);
>+ }
>+

Since we are calling finish_wait() before return in all path, why not
doing somethig like this:

out:
finish_wait(sk_sleep(sk), wait);
>+ return err;
>+}

Then in the error paths you can do:

err = XXX;
goto out;

Thanks,
Stefano

>
> static int
> vsock_stream_recvmsg(struct socket *sock, struct msghdr *msg, size_t len,
>--
>2.25.1
>