Re: [PATCH] ip_ct_refresh_optimization.patch

[Martin Josefsson <gandalf@wlug.westbo.se>]

[-- Attachment #1: Type: text/plain, Size: 1621 bytes --]

On Sat, 2002-09-14 at 14:52, Marcus Sundberg wrote:
> Martin Josefsson <gandalf@wlug.westbo.se> writes:
> 
> > Yes I know it's broken, I do have a newer version but I havn't committed
> > it to cvs. I think Patrick Shaaf's solution is a better one and we
> > should probably go with something like that.
> 
> Probably yes, but that's no reason to have a completely broken patch
> which can be fixed by a one-liner in CVS. It should IMHO either be
> fixed or removed asap.

I agree, honestly I had almost forgotten that these patches were in cvs
until I read your mail :)
I've committed your patch now. Thanks.

> That patch is also incorrect, but it works because it updates to
> often, not to seldom. If the new timeout is one jiffie earlier than
> the previous timeout your expression will evaluate to 0xffffffff
> and thus update the timer even though there's no need. The cast
> is useless btw, because all operands, and thus the result also, are
> already unsigned long.

I know it will update too often. And about the "bogus" cast, I usually
like explicit casts like that when I think it helps codereadabilty.

As a fun thing you can add a printk() in ip_ct_refesh() and compare the
results without this timer-optimization and with it. I did that when I
first looked at this, it was quite horrible to see a timer get deleted
and a new added with the exact same timeout over and over again.

Have you performed any benchmarks/profiles with and without this
optimization?

-- 
/Martin

Never argue with an idiot. They drag you down to their level, then beat
you with experience.

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 189 bytes --]