Re: [Xen-devel] [patch] netback: Xennet half die---netback driver didn't detect the jiffies wrapping correctly.

["Yi, Shunli" <syi@xxxxxxxxxxxx>]

> >> >> --- drivers/xen/netback/netback.c.org   2012-11-30 15:48:13.109039998 
> >> >> -0500
> >> >> +++ drivers/xen/netback/netback.c       2012-11-30 15:48:55.212072898 
> >> >> -0500
> >> >> @@ -1272,6 +1272,10 @@ static void net_tx_action(unsigned long
> >> >>                 rmb(); /* Ensure that we see the request before we copy 
> >> >> it. */
> >> >>                 memcpy(&txreq, RING_GET_REQUEST(&netif->tx, i), 
> >> >> sizeof(txreq));
> >> >>  
> >> >> +        /* Check for the jiffies wrapping */
> >> >> +        if (time_after_eq(jiffies, netif->credit_timeout.expires))
> >> >> +            netif->credit_timeout.expires = jiffies;
> >> > 
> >> > Do you not need to remove the similar check from the following block?
> >> 
> >> I don't think so, but I also can't see how that adjustment would 
> >> help in the first place: If it gets executed after a very long 
> >> period of no traffic, it would itself not be able to reliably tell 
> >> whether the clock wrapped.
> > 
> > Hrm, yes, This change would help in the case of a dribble of traffic 
> > which never hits the limit, but not in the case of no timer at all.
> > 
> >> That said, I agree that the code as is appears to have a problem 
> >> (with 32-bit jiffies at least), but I can't see how to easily deal 
> >> with it.
> > 
> > Would it help to always have the pending timer armed, for either the 
> > next tick if credit needs replenishing or for, say MAX_JIFFIES/4 as 
> > a backstop to avoid wrapping issues?
> 
> If that can be made work cleanly, that would probably be the easiest 
> solution. But I don't see MAX_JIFFIES being defined anywhere, and I'm 
> unsure ULONG_MAX/4 would be well received as a timeout on 64-bit 
> systems.


Thanks guys for your input, I'm really a newbie in kernel coding. The patch 
base on following thoughs:
    - If we enabled the credit mechanism, the next_credit should always in 
future of jiffies. 
    - So, the timer should not be in pending(the code will be run, we just 
carry the timer with jiffies) or the timer is set to a time in future ( the 
code will not run )


I'm not sure my understand the credit mechanism right, but we can always 
calculate the next_credit base on jiffies instead of the old timer value:
        unsigned long next_credit =
                        netif->credit_timeout.expires +
                        msecs_to_jiffies(netif->credit_usec / 1000);

> I guess several trillion years *is* a tad extreme ;-) (or my maths is wrong).

>We could always choose a more practical backstop, like a day or even a just 
>few minutes -- we'd expect in general to always be pushing the timeout ahead 
>rather than hitting it.

>Ian



 To report this as spam, please forward to spam@xxxxxxxxxxxxx  Thank you.


 Protected by Websense Hosted Email Security -- www.websense.com 
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel