On Thu, 2024-04-18 at 15:36 +0800, Heng Qi wrote:
>
> 在 2024/4/18 下午2:42, Jason Wang 写道:
> > On Wed, Apr 17, 2024 at 3:31 AM Daniel Jurgens <danielj@xxxxxxxxxx> wrote:
> > > The command VQ will no longer be protected by the RTNL lock. Use a
> > > spinlock to protect the control buffer header and the VQ.
> > >
> > > Signed-off-by: Daniel Jurgens <danielj@xxxxxxxxxx>
> > > Reviewed-by: Jiri Pirko <jiri@xxxxxxxxxx>
> > > ---
> > > drivers/net/virtio_net.c | 6 +++++-
> > > 1 file changed, 5 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
> > > index 0ee192b45e1e..d02f83a919a7 100644
> > > --- a/drivers/net/virtio_net.c
> > > +++ b/drivers/net/virtio_net.c
> > > @@ -282,6 +282,7 @@ struct virtnet_info {
> > >
> > > /* Has control virtqueue */
> > > bool has_cvq;
> > > + spinlock_t cvq_lock;
> > Spinlock is instead of mutex which is problematic as there's no
> > guarantee on when the driver will get a reply. And it became even more
> > serious after 0d197a147164 ("virtio-net: add cond_resched() to the
> > command waiting loop").
> >
> > Any reason we can't use mutex?
>
> Hi Jason,
>
> I made a patch set to enable ctrlq's irq on top of this patch set, which
> removes cond_resched().
>
> But I need a little time to test, this is close to fast. So could the
> topic about cond_resched +
> spin lock or mutex lock be wait?
The big problem is that until the cond_resched() is there, replacing
the mutex with a spinlock can/will lead to scheduling while atomic
splats. We can't intentionally introduce such scenario.
Side note: the compiler apparently does not like guard() construct,
leading to new warning, here and in later patches. I'm unsure if the
code simplification is worthy.
Cheers,
Paolo
