Re: virtio_net: another race with virtio_net and enable_cb

[Rusty Russell <rusty@rustcorp.com.au>]

On Monday 26 May 2008 19:29:27 Christian Borntraeger wrote:
> Hello Rusty,

Hi Christian,

    As always, you post an interesting bug!

>         /* Out of packets? */
>         if (received < budget) {
>                 netif_rx_complete(vi->dev, napi);
>                 if (unlikely(!vi->rvq->vq_ops->enable_cb(vi->rvq))
>                     && napi_schedule_prep(napi)) {
>                         vi->rvq->vq_ops->disable_cb(vi->rvq);
>                         __netif_rx_schedule(vi->dev, napi);
>                         goto again;
>                 }
>         }

Yes, clearly if napi_schedule_prep() fails (ie. another poll is scheduled), we 
don't disable callbacks, and another poll will be run with callbacks enabled, 
triggering this bug.  It can only happen if the enable_cb succeeds (no new 
work) then a new packet arrives before napi_schedule_prep().

You should be able to verify that you are hitting this window by putting a 
printk in the !enable_cb && !napi_schedule_prep case.

Now, we can't do an unconditional disable_cb(), as it won't prevent this race, 
and would make it worse if it ran after the other poll's enable_cb() and left 
us without a callback to wake us up.  We could do a disable_cb() at the top 
of virtnet_poll, but I certainly don't want to write the comment explaining 
why.

So your fix is correct and sensible.  Applied.

Thanks!
Rusty.