On Mon, Mar 29, 2021 at 2:26 AM Stefan Hajnoczi <stefanha@xxxxxxxxxx> wrote: > > On Fri, Mar 26, 2021 at 04:40:09PM -0700, Jiang Wang . wrote: > > I thought about this and discussed it with my colleague Cong Wang. > > One idea is to make current asynchronous send_pkt flow to be synchronous, > > then if the virtqueue is full, the function can return ENOMEM all the way back > > to the caller and the caller can check the return value of sendmsg > > and slow down when necessary. > > > > In the spec, we can put something like, if the virtqueue is full, the caller > > should be notified with an error etc. > > > > In terms of implementation, that means we will remove the current > > send_pkt_work for both stream and dgram sockets. Currently, the > > code path uses RCU and a work queue, then grab a mutex in the > > work queue function. Since we cannot grab mutex when in rcu > > critical section, we have to change RCU to a normal reference > > counting mechanism. I think this is doable. The drawback is > > that the reference counting in general spends a little more > > cycles than the RCU, so there is a small price to pay. Another > > option is to use Sleepable RCU and remove the work queue. > > > > What do you guys think? > > I think the tx code path is like this because of reliable delivery. > Maybe a separate datagram rx/tx code path would be simpler? I thought about this too. dgram can have a separate rx/tx path from stream types. In this case, the the_virtio_vsock will still be shared because the virtqueues have to be in one structure. Then the_virtio_vsock will be protected by a rcu and a reference counting ( or a sleepable RCU ). In vhost_vsock_dev_release, it will wait for both rcu and another one to be finished and then free the memory. I think this is doable. Let me know if there is a better way to do it. Btw, I think dgram tx code path will be quite different from stream, but dgram rx path will be similar to stream type. > Take the datagram tx virtqueue lock, try to add the packet into the > virtqueue, and return -ENOBUFS if the virtqueue is full. Then use the > datagram socket's sndbuf accounting to prevent queuing up too many > packets. When a datagram tx virtqueue buffer is used by the device, > select queued packets for transmission. I am not sure about the last sentence. In the new design, there will be no other queued packets for dgram (except those in the virtqueue). When dgram tx virtqueue is freed by the device, the driver side just needs to free some space. No need to trigger more transmission. Unlike the stream tx/rx code > path there is no dependency between tx and rx because we don't have the > credit mechanism. > > btw, I will also add some SENDBUF restrictions for the dgram > > sockets, but I don't think it needs to be in the spec. > > Yes, the spec doesn't need to explain implementation-specific issues. > > If there are common implementation issues then the spec can explain them > in general terms (not referring to Linux internals) to help > implementors. Got it. Thanks. I will send a v2 spec soon. > Stefan _______________________________________________ Virtualization mailing list Virtualization@xxxxxxxxxxxxxxxxxxxxxxxxxx https://lists.linuxfoundation.org/mailman/listinfo/virtualization
