RE: [Xen-devel][PV-ops][PATCH 1/2] VNIF(netback): Using smart pollinginstead of event notification.

["Xu, Dongxiao" <dongxiao.xu@xxxxxxxxx>]

See below comments.

Thanks!
Dongxiao

________________________________________
From: James Harper [james.harper@xxxxxxxxxxxxxxxx]
Sent: Wednesday, September 30, 2009 2:27 AM
To: Xu, Dongxiao; xen-devel@xxxxxxxxxxxxxxxxxxx
Subject: RE: [Xen-devel][PV-ops][PATCH 1/2] VNIF(netback): Using smart 
pollinginstead of event notification.

>
> +             /* netfront_smartpoll_active indicates whether netfront
timer
> +                 * is active.
> +                 */
> +             if ((netif->smart_poll == 1)) {
> +                     if
(!(netif->rx.sring->netfront_smartpoll_active)) {
> +                             notify_remote_via_irq(irq);
> +
netif->rx.sring->netfront_smartpoll_active = 1;
> +                     }
> +             }

I think that the frontend should use the event field to turn off
notification here. It should set it to the number of outstanding rx
slots less some number, eg if it had filled the ring with rx slots (eg
256), then prod + 128 might be a good number. That way if a lot of small
packets come it, the frontend will still get notified if the ring is
half empty even if the 1Khz timer wasn't elapsed yet. Otherwise the rx
ring could starve.

[Dongxiao]: In our patch, netfront will NOT be notified while polling data. 
Netback only notifies it once when data comes but netfront timer is not active. 
After the notification, netfront will do polling until there is no data coming 
during 
100ms. So it eliminates most of the event channel notification. 

>       netif->tx.rsp_prod_pvt = ++i;
>       RING_PUSH_RESPONSES_AND_CHECK_NOTIFY(&netif->tx, notify);
> -     if (notify)
> +     /* netfront_smartpoll_active indicates whether netfront timer is
> +         * active.
> +         */
> +     if ((netif->smart_poll == 1)) {
> +             if (!(netif->rx.sring->netfront_smartpoll_active)) {
> +                     notify_remote_via_irq(netif->irq);
> +                     netif->rx.sring->netfront_smartpoll_active = 1;
> +             }
> +     } else if (notify)
>               notify_remote_via_irq(netif->irq);
>  }

I'm not so sure about this either. The frontend knows how many
outstanding tx packets it has put on the ring, and it is reasonable to
assume that they will be sent in a timely manner, so I think in this
case the frontend is in the best position to set the event to an
acceptable value and the backend will notify on that basis.

[Dongxiao]: Actually our patch has eliminate most of the notification, so it is 
unnecessary for 
netfront to set the event field. As described before, netfront is only notified 
once when its timer
is not active. Thanks!

James
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel