r2267 - trunk/varnish-cache/bin/varnishd

[Poul-Henning Kamp]

In message <ujr8x4t9kgo.fsf at false.linpro.no>, =?iso-8859-1?Q?Dag-Erling_Sm=F8rg
rav?= writes:

>> Modified:
>>    trunk/varnish-cache/bin/varnishd/cache_acceptor_kqueue.c
>> Log:
>> Use a bitmap to check if we own a given file handle, before we
>> do anything about the associated session.
>>
>> May be relevant relative to: #162
>
>You know very well that I objected to this patch.  There is absolutely
>zero evidence that it helps #162, and non-zero evidence that it has a
>significant negative impact on performance.  Please revert it.

There is zero evidence that this has any performance impact.  The
CPU usage problem is also in 1.2 without my patch.

Checking for the suspected condition behind #162s problem can only
be correctly implemented by only looking at the file handle, we are
not, despite what you may think or desire, allowed to access the
session structure after we have handed the session back.

A bitmap is the simplest, lightest and safest way to do implement
that check.

(And the binheap vs TAILQ discussion doesn't even remotely come
close to this, as both of them would require dereferencing of
the session structure before the check could be carried out).

I already wrote to you, when you commited the temporary workaround,
that it did not look 100% water-tight to me, but that it was a
good bandgap, even at a increased keevent syscall load.

This commit have no implemented the check in the correct way
and reduced the syscall loading to its former level.

Case closed, can we focus on more important things now ?


-- 
Poul-Henning Kamp       | UNIX since Zilog Zeus 3.20
phk at FreeBSD.ORG         | TCP/IP since RFC 956
FreeBSD committer       | BSD since 4.3-tahoe    
Never attribute to malice what can adequately be explained by incompetence.