Request to back out Luigis polled-net patch in -stable.

[Poul-Henning Kamp]

------- Forwarded Message

To: core at freebsd.org
Subject: Request to back out Luigis polled-net patch in -stable.
From: Poul-Henning Kamp <phk at freebsd.org>
Date: Fri, 07 Dec 2001 17:13:24 +0100
Message-ID: <22749.1007741604 at critter.freebsd.dk>
Sender: phk at critter.freebsd.dk


I have not read the entire patch in details, but have focused on
the part of the code which I have special insight into.

My complaint is therefore mainly based on one single file:
src/sys/kern/kern_clock.c which has been spammed with a lot
of stuff which has absolutely no business being there.

Just for starters: Adding this to sys/kern/kern_clock.c:

+#ifdef DEVICE_POLLING
+#include <i386/include/md_var.h> /* for vm_page_zero_idle() */
+#include <sys/socket.h> /* needed by sys/if.h */
+#include <net/if.h> /* for IFF_* flags */
+#include <net/netisr.h> /* for NETISR_POLL */
+[...]
+#endif /* DEVICE_POLLING */

None of this as any business here, and it certainly doesn't
even look remotely possible for this to work on !i386 platforms.

Most of the rest of the addition is code which should not be in
sys/kern in the first place but sys/net.

Just because it is #ifdef DEVICE_POLLING doesn't mean it can
violate our architecture.

Furthermore, according to the commit message, this was approved
by jkh in his role of release-engineer, but I find it disturbing
that the approval seems to have happened without any technical
or architectural review of the proposed patch.

Finally, I thought a significant change like this would have to
go into -current at least at the same time, but preferably before
it goes into -stable.  Considering the repeated claims of +10%
performance for this suite of changes, there is a very good chance
that people will pick it up and use it, leaving us with a big
problem when 5.0-R time comes around and we have to explain A)
why the feature isn't there, and B) why performance is worse.

Summary:

I request that luigis patch gets backed out for the following reasons:

	1. The patch is architecturally a mess.

	2. The approval seems to have happened purely on
	   "glossy sales-brochure" backing without any
	   technical or architectural input.

	3. It didn't go into -current first and has received
	   no shakeout before it went into -stable.

I am _NOT_ saying that the idea is bad, it sounds pretty intelligent
to me, but in that case it is even more important to not mess it up.

I would also like to point to the parallel piece of code: Jun-Itohs
ALTQ for which he reliably has maintained a patch relative to the
4.X branch and which despite various peoples requests have not
haphazardly been committed into -stable.  And in that context one
should not forget that ALTQ has a lot longer and better trackrecord
of high quality than Luigis poll-code, or any of Luigis code for
that matter.

- -- 
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.

------- End of Forwarded Message


To Unsubscribe: send mail to majordomo at FreeBSD.org
with "unsubscribe freebsd-arch" in the body of the message