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
