search for: lock_sock_nested

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: > ffff8880c51589...

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: > ffff8880c51589...

...> > > + /* When "level" is 2, use the nested version to avoid the > > > + * warning "possible recursive locking detected". > > > + */ > > > + if (level == 1) > > > + lock_sock(sk); > > > > Since lock_sock() calls lock_sock_nested(sk, 0), could we use directly > > lock_sock_nested(sk, level) with level = 0 in vsock_release() and > > level = SINGLE_DEPTH_NESTING here in the while loop? > > > > Thanks, > > Stefano > > IMHO it's better to make the lock usage more explicit, as the patch...

...r): ... > 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 > virtio_transport_recv_connecting >...

From: 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.

...ted 5.3.0-vsock #17 Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.12.0-2.fc30 04/01/2014 Call Trace: dump_stack+0x85/0xc0 __lock_acquire.cold+0xad/0x22b lock_acquire+0xc4/0x1a0 ? virtio_transport_release+0x34/0x330 [vmw_vsock_virtio_transport_common] lock_sock_nested+0x5d/0x80 ? virtio_transport_release+0x34/0x330 [vmw_vsock_virtio_transport_common] virtio_transport_release+0x34/0x330 [vmw_vsock_virtio_transport_common] ? mark_held_locks+0x49/0x70 ? _raw_spin_unlock_irqrestore+0x44/0x60 __vsock_release+0x2d/0x130 [vsock] __vsock_re...

...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_recv_connecting > sk->sk_socket-&g...

...rom 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_sock > virtio_transport_recv_connecting...

..._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 nested * version to avoid the warning "possible recursive locking @@ -670,7 +673,...

...*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 == SOCK_STREAM) + vsock_remove_sock(vsk); /* When "level" is SINGLE_DEPTH_N...

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

This series is based on v4.7. This RFC is the implementation for the new VIRTIO Socket device. It is developed in parallel with the VIRTIO device specification and proves the design. Once the specification has been accepted I will send a non-RFC version of this patch series. v6: * Add VHOST_VSOCK_SET_RUNNING ioctl to start/stop vhost cleanly * Add graceful shutdown to avoid port reuse while

This series is based on v4.7. This RFC is the implementation for the new VIRTIO Socket device. It is developed in parallel with the VIRTIO device specification and proves the design. Once the specification has been accepted I will send a non-RFC version of this patch series. v6: * Add VHOST_VSOCK_SET_RUNNING ioctl to start/stop vhost cleanly * Add graceful shutdown to avoid port reuse while