search for: __vsock_release

On Mon, Sep 30, 2019 at 06:43:50PM +0000, Dexuan Cui wrote: > Lockdep is unhappy if two locks from the same class are held. > > Fix the below warning for hyperv and virtio sockets (vmci socket code > doesn't have the issue) by using lock_sock_nested() when __vsock_release() > is called recursively: > > ============================================ > WARNING: possible recursive locking detected > 5.3.0+ #1 Not tainted > -------------------------------------------- > server/1795 is trying to acquire lock: > ffff8880c5158990 (sk_lock-AF_VSOCK){+...

Hi Dexuan, On Thu, Sep 26, 2019 at 01:11:27AM +0000, Dexuan Cui wrote: > Lockdep is unhappy if two locks from the same class are held. > > Fix the below warning for hyperv and virtio sockets (vmci socket code > doesn't have the issue) by using lock_sock_nested() when __vsock_release() > is called recursively: > > ============================================ > WARNING: possible recursive locking detected > 5.3.0+ #1 Not tainted > -------------------------------------------- > server/1795 is trying to acquire lock: > ffff8880c5158990 (sk_lock-AF_VSOCK){+...

.... :-) Sure! Just to be clear, I'm proposing this (plus the changes in the transports and yours useful comments): --- a/net/vmw_vsock/af_vsock.c +++ b/net/vmw_vsock/af_vsock.c @@ -638,7 +638,7 @@ struct sock *__vsock_create(struct net *net, } EXPORT_SYMBOL_GPL(__vsock_create); -static void __vsock_release(struct sock *sk) +static void __vsock_release(struct sock *sk, int level) { if (sk) { struct sk_buff *skb; @@ -650,7 +650,7 @@ static void __vsock_release(struct sock *sk) transport->release(vsk); - lock_sock(sk); + lock_sock...

...------------------------------------------- python3/872 is trying to acquire lock: ffff88802b650110 (sk_lock-AF_VSOCK){+.+.}, at: virtio_transport_release+0x34/0x330 [vmw_vsock_virtio_transport_common] but task is already holding lock: ffff88803597ce10 (sk_lock-AF_VSOCK){+.+.}, at: __vsock_release+0x3f/0x130 [vsock] other info that might help us debug this: Possible unsafe locking scenario: CPU0 ---- lock(sk_lock-AF_VSOCK); lock(sk_lock-AF_VSOCK); *** DEADLOCK *** May be due to missing lock nesting notation 2 locks held by python3...

...rom: Dexuan Cui <decui at microsoft.com> Date: Mon, 30 Sep 2019 18:43:50 +0000 > Lockdep is unhappy if two locks from the same class are held. > > Fix the below warning for hyperv and virtio sockets (vmci socket code > doesn't have the issue) by using lock_sock_nested() when __vsock_release() > is called recursively: ... > Tested-by: Stefano Garzarella <sgarzare at redhat.com> > Signed-off-by: Dexuan Cui <decui at microsoft.com> Applied, thanks.

...the > server on the guest, there will be a panic on a ThunderX2 (armv8a server): ... > The race condition is as follows: > Task1 Task2 > ===== ===== > __sock_release virtio_transport_recv_pkt > __vsock_release vsock_find_bound_socket (found sk) > lock_sock_nested > vsock_remove_sock > sock_orphan > sk_set_socket(sk, NULL) > sk->sk_shutdown = SHUTDOWN_MASK > ... > release_sock > lock_sock >...

...ork+0x10/0x18 ^ ^ Maybe we can remove these two columns from the commit message. > > The race condition as follows: > Task1 Task2 > ===== ===== > __sock_release virtio_transport_recv_pkt > __vsock_release vsock_find_bound_socket (found) > lock_sock_nested > vsock_remove_sock > sock_orphan > sk_set_socket(sk, NULL) Here we can add: sk->sk_shutdown = SHUTDOWN_MASK; > ... > release_sock > lock_soc...

...; [ 463.719048][ T5040] kthread+0x128/0x130 > [ 463.719052][ T5040] ret_from_fork+0x10/0x18 > > The race condition as follows: > Task1 Task2 > ===== ===== > __sock_release virtio_transport_recv_pkt > __vsock_release vsock_find_bound_socket (found) > lock_sock_nested > vsock_remove_sock > sock_orphan > sk_set_socket(sk, NULL) > ... > release_sock > lock_sock > virtio_transport_re...

...uct socket *sock, + struct sock *parent, + gfp_t priority, + unsigned short type, + int kern) { struct sock *sk; struct vsock_sock *psk; @@ -639,7 +639,6 @@ struct sock *__vsock_create(struct net *net, return sk; } -EXPORT_SYMBOL_GPL(__vsock_create); static void __vsock_release(struct sock *sk, int level) { @@ -705,6 +704,13 @@ static int vsock_queue_rcv_skb(struct sock *sk, struct sk_buff *skb) return err; } +struct sock *vsock_create_connected(struct sock *parent) +{ + return __vsock_create(sock_net(parent), NULL, parent, GFP_KERNEL, + parent->sk_type,...

...@@ -616,7 +619,7 @@ struct sock *__vsock_create(struct net *net, vsk->connect_timeout = VSOCK_DEFAULT_CONNECT_TIMEOUT; } - if (transport->init(vsk, psk) < 0) { + if (vsk->transport->init(vsk, psk) < 0) { sk_free(sk); return NULL; } @@ -638,7 +641,7 @@ static void __vsock_release(struct sock *sk) vsk = vsock_sk(sk); pending = NULL; /* Compiler warning. */ - transport->release(vsk); + vsk->transport->release(vsk); lock_sock(sk); sock_orphan(sk); @@ -662,7 +665,7 @@ static void vsock_sk_destruct(struct sock *sk) { struct vsock_sock *vsk = vsock_s...

...@@ -616,7 +619,7 @@ struct sock *__vsock_create(struct net *net, vsk->connect_timeout = VSOCK_DEFAULT_CONNECT_TIMEOUT; } - if (transport->init(vsk, psk) < 0) { + if (vsk->transport->init(vsk, psk) < 0) { sk_free(sk); return NULL; } @@ -641,7 +644,7 @@ static void __vsock_release(struct sock *sk, int level) /* The release call is supposed to use lock_sock_nested() * rather than lock_sock(), if a sock lock should be acquired. */ - transport->release(vsk); + vsk->transport->release(vsk); /* When "level" is SINGLE_DEPTH_NESTING, use the nes...

...MADDR_PORT_ANY); @@ -629,11 +689,6 @@ static struct sock *__vsock_create(struct net *net, vsk->buffer_max_size = VSOCK_DEFAULT_BUFFER_MAX_SIZE; } - if (vsk->transport->init(vsk, psk) < 0) { - sk_free(sk); - return NULL; - } - return sk; } @@ -650,7 +705,10 @@ static void __vsock_release(struct sock *sk, int level) /* The release call is supposed to use lock_sock_nested() * rather than lock_sock(), if a sock lock should be acquired. */ - vsk->transport->release(vsk); + if (vsk->transport) + vsk->transport->release(vsk); + else if (sk->sk_type == SO...

...11 +682,6 @@ struct sock *__vsock_create(struct net *net, vsk->buffer_max_size = VSOCK_DEFAULT_BUFFER_MAX_SIZE; } - if (vsk->transport->init(vsk, psk) < 0) { - sk_free(sk); - return NULL; - } - return sk; } EXPORT_SYMBOL_GPL(__vsock_create); @@ -648,7 +696,10 @@ static void __vsock_release(struct sock *sk) vsk = vsock_sk(sk); pending = NULL; /* Compiler warning. */ - vsk->transport->release(vsk); + if (vsk->transport) + vsk->transport->release(vsk); + else if (sk->sk_type == SOCK_STREAM) + vsock_remove_sock(vsk); lock_sock(sk); sock_orphan(sk)...

Most of the patches are reviewed by Dexuan, Stefan, and Jorgen. The following patches need reviews: - [11/15] vsock: add multi-transports support - [12/15] vsock/vmci: register vmci_transport only when VMCI guest/host are active - [15/15] vhost/vsock: refuse CID assigned to the guest->host transport RFC: https://patchwork.ozlabs.org/cover/1168442/ v1:

This series adds the multi-transports support to vsock, following this proposal: https://www.spinics.net/lists/netdev/msg575792.html With the multi-transports support, we can use VSOCK with nested VMs (using also different hypervisors) loading both guest->host and host->guest transports at the same time. Before this series, vmci-transport supported this behavior but only using VMware

This series adds the multi-transports support to vsock, following this proposal: https://www.spinics.net/lists/netdev/msg575792.html With the multi-transports support, we can use VSOCK with nested VMs (using also different hypervisors) loading both guest->host and host->guest transports at the same time. Before this series, vmci-transport supported this behavior but only using VMware

Hi all, this series adds the multi-transports support to vsock, following this proposal: https://www.spinics.net/lists/netdev/msg575792.html With the multi-transports support, we can use vsock with nested VMs (using also different hypervisors) loading both guest->host and host->guest transports at the same time. Before this series, vmci-transport supported this behavior but only using

Hi all, this series adds the multi-transports support to vsock, following this proposal: https://www.spinics.net/lists/netdev/msg575792.html With the multi-transports support, we can use vsock with nested VMs (using also different hypervisors) loading both guest->host and host->guest transports at the same time. Before this series, vmci-transport supported this behavior but only using

From: Andy King <acking at vmware.com> ** Introduce VM Sockets *** In an effort to improve the out-of-the-box experience with Linux kernels for VMware users, VMware is working on readying the VM Sockets (VSOCK, formerly VMCI Sockets) (vmw_vsock) kernel module for inclusion in the Linux kernel. The purpose of this post is to acquire feedback on the vmw_vsock kernel module. Unlike previous

From: Andy King <acking at vmware.com> ** Introduce VM Sockets *** In an effort to improve the out-of-the-box experience with Linux kernels for VMware users, VMware is working on readying the VM Sockets (VSOCK, formerly VMCI Sockets) (vmw_vsock) kernel module for inclusion in the Linux kernel. The purpose of this post is to acquire feedback on the vmw_vsock kernel module. Unlike previous