Re: [PATCH net] vsock/bpf: release sock lock while waiting for data in recvmsg

[jrmmhm . kernel]

On 10/5/26 09:59, Michal Luczaj wrote:
> I took a look at v2 and I don't really have any functional comments. Well,
> besides the use of READ_ONCE, but that's probably something we need to
> tackle separately?

Thanks for looking. Yes, I'd keep it separate. As far as I can see, the
virtio_transport_common.c writers of vsk->peer_shutdown run under
lock_sock() from virtio_transport_recv_pkt(); its WRITE_ONCE() is there
for the lockless vsock_poll(). The writes outside lock_sock() are in the
reset paths: virtio_vsock_reset_sock() (sk->sk_state, sk->sk_err),
vhost_vsock_reset_orphans() (those plus peer_shutdown and SOCK_DONE) and
vmci_transport_handle_detach(). Both wait loops read these fields; the
state check is new in v3. The native vsock_connectible_wait_data() reads
sk_err the same plain way, so I'd rather annotate both paths together in
one patch than only the sockmap copy. Did you have other accesses in
mind?

> A single-patch v2 addresses not only the original bug, but also sashiko's
> complains about the pre-existing issues (making the subject stale?), but I
> didn't know if you'd rather have all of that stuffed together, so I
> refrained from commenting to avoid confusing the author.

Good point, thanks. v3 is a two-patch series: 1/2 keeps the subject and is
the lock release with the wait loop it needs, 2/2 handles the len == 0,
MSG_ERRQUEUE and MSG_OOB reads. It also addresses the Medium and Low
netdev Sashiko comments on v2; the pre-existing ingress_skb case will be a
separate patch.

Bobby, since the vsock sockmap support is yours, a look at v3 would be very
welcome whenever you find the time.

Thanks,
Jerome