On Mon, Jan 22, 2024 at 10:12 AM Zhu Yanjun <yanjun.zhu@xxxxxxxxx> wrote: > > > 在 2024/1/20 1:29, Andrew Lunn 写道: > >>>>> while (!virtqueue_get_buf(vi->cvq, &tmp) && > >>>>> - !virtqueue_is_broken(vi->cvq)) > >>>>> + !virtqueue_is_broken(vi->cvq)) { > >>>>> + if (timeout) > >>>>> + timeout--; > >>>> This is not really a timeout, just a loop counter. 200 iterations could > >>>> be a very short time on reasonable H/W. I guess this avoid the soft > >>>> lockup, but possibly (likely?) breaks the functionality when we need to > >>>> loop for some non negligible time. > >>>> > >>>> I fear we need a more complex solution, as mentioned by Micheal in the > >>>> thread you quoted. > >>> Got it. I also look forward to the more complex solution to this problem. > >> Can we add a device capability (new feature bit) such as ctrq_wait_timeout > >> to get a reasonable timeout? > > The usual solution to this is include/linux/iopoll.h. If you can sleep > > read_poll_timeout() otherwise read_poll_timeout_atomic(). > > I read carefully the functions read_poll_timeout() and > read_poll_timeout_atomic(). The timeout is set by the caller of the 2 > functions. FYI, in order to avoid a swtich of atomic or not, we need convert rx mode setting to workqueue first: https://www.mail-archive.com/virtualization@xxxxxxxxxxxxxxxxxxxxxxxxxx/msg60298.html > > As such, can we add a module parameter to customize this timeout value > by the user? Who is the "user" here, or how can the "user" know the value? > > Or this timeout value is stored in device register, virtio_net driver > will read this timeout value at initialization? See another thread. The design needs to be general, or you can post a RFC. In another thought, we've already had a tx watchdog, maybe we can have something similar to cvq and use timeout + reset in that case. Thans > > Zhu Yanjun > > > > > Andrew >