On Fri, 21 Dec 2007, David Miller wrote:
> From: Herbert Xu <[EMAIL PROTECTED]>
> Date: Fri, 21 Dec 2007 17:29:27 +0800
>
> > On Fri, Dec 21, 2007 at 01:27:20AM -0800, David Miller wrote:
> > >
> > > It's two shifts, and this gets scheduled along with the other
> > > instructions on many cpus so it's effectively free.
> > >
> > > I don't see why this is even worth mentioning and discussing.
> >
> > I totally agree. Two shifts are way better than a branch.
>
> We take probably a thousand+ 100+ cycle cache misses in the TCP stack
> on big window openning ACKs.
>
> Instead of discussing ways to solve that huge performance killer we're
> wanking about two friggin' integer shifts.
>
> It's hilarious isn't it? :-)
I don't think obfuscated code is hilarious. Instead of the convoluted
and dense code:
/* Defer for less than two clock ticks. */
if (tp->tso_deferred &&
((jiffies << 1) >> 1) - (tp->tso_deferred >> 1) > 1)
You can have the much simpler and more easily understandable:
/* Defer for less than two clock ticks. */
if (tp->tso_deferred && (jiffies - tp->tso_deferred) > 1)
And instead of:
/* Ok, it looks like it is advisable to defer. */
tp->tso_deferred = 1 | (jiffies<<1);
return 1;
You could do as Ilpo suggested:
/* Ok, it looks like it is advisable to defer. */
tp->tso_deferred = max_t(u32, jiffies, 1);
return 1;
Or perhaps more efficiently:
/* Ok, it looks like it is advisable to defer. */
tp->tso_deferred = jiffies;
if (unlikely(jiffies == 0))
tp->tso_deferred = 1;
return 1;
Or perhaps even:
/* Ok, it looks like it is advisable to defer. */
tp->tso_deferred = jiffies;
/* need to return a non-zero value to defer, which means won't
* defer if jiffies == 0 but it's only a 1 in 4 billion event
* (and avoids a compare/branch by not checking jiffies)
/
return jiffies;
Since it really only needs a non-zero return value to defer.
See, no branches needed and much clearer code. That seems worthwhile
to me from a code maintenance standpoint, even if it isn't any speed
improvement.
And what about the 64-bit jiffies versus 32-bit tp->tso_deferred issue?
Should tso_deferred be made unsigned long to match jiffies?
-Bill
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
