On Wed, May 29, 2019 at 09:28:52PM -0700, David Miller wrote: > From: Stefano Garzarella <[email protected]> > Date: Tue, 28 May 2019 12:56:20 +0200 > > > @@ -68,7 +68,13 @@ struct virtio_vsock { > > > > static struct virtio_vsock *virtio_vsock_get(void) > > { > > - return the_virtio_vsock; > > + struct virtio_vsock *vsock; > > + > > + mutex_lock(&the_virtio_vsock_mutex); > > + vsock = the_virtio_vsock; > > + mutex_unlock(&the_virtio_vsock_mutex); > > + > > + return vsock; > > This doesn't do anything as far as I can tell. > > No matter what, you will either get the value before it's changed or > after it's changed. > > Since you should never publish the pointer by assigning it until the > object is fully initialized, this can never be a problem even without > the mutex being there. > > Even if you sampled the the_virtio_sock value right before it's being > set to NULL by the remove function, that still can happen with the > mutex held too.
Yes, I found out when I was answering Jason's question [1]. :( I proposed to use RCU to solve this issue, do you agree? Let me know if there is a better way. > > This function is also terribly named btw, it implies that a reference > count is being taken. But that's not what this function does, it > just returns the pointer value as-is. What do you think if I remove the function, using directly the_virtio_vsock? (should be easier to use with RCU API) Thanks, Stefano [1] https://lore.kernel.org/netdev/20190529105832.oz3sagbne5teq3nt@steredhat _______________________________________________ Virtualization mailing list [email protected] https://lists.linuxfoundation.org/mailman/listinfo/virtualization
