Subject: Re: PR 31468 -- distributed LKMs are not useful because of incompatible config
To: Nathan J. Williams <nathanw@wasabisystems.com>
From: Chuck Silvers <chuq@chuq.com>
List: tech-kern
Date: 10/30/2005 10:01:19
On Sun, Oct 30, 2005 at 12:39:11PM -0500, Nathan J. Williams wrote:
> Chuck Silvers <chuq@chuq.com> writes:
> 
> > does anyone object to turning off DIAGNOSTIC in all the GENERIC configs?
> > this seems like reasonable anyway, and it fixes this issue in nicely.
> 
> Why is this preferable to turning *on* DIAGNOSTIC in all the GENERIC
> configs?

to use i386 as an example, DIAGNOSTIC was originally on but later turned off
with this explanation:

----------------------------
revision 1.403
date: 2001/04/21 20:49:14;  author: fvdl;  state: Exp;  lines: +3 -3
Move the DIAGNOSTIC option to a seperate config file, it's too expensive
to be in GENERIC (which should be a kernel ready for production use).
----------------------------

so unless someone can show that DIAGNOSTIC no longer causes any noticable
performance impact, it should stay off here.  it seems likely that if there
was a performance impact on this platform then there would be on all the
others too, so it should be off in their GENERICs for the same reason.
unless you disagree with the original rationale for turning it off here.  :-)


> >  - these structures have an extra field if DIAGNOSTIC is on:
> > 
> > 	struct irframe_softc
> > 	struct usbd_xfer
> > 	struct wsemul_vt100_emuldata
> > 	struct cpu_data
> 
> The one DIAGNOSTIC field in usbd_xfer is only accessed by the host
> controller driver (including allocation and deallocation), so its
> presence or absence just affects offsets. Would it be sufficent to
> move it to the end of the structure? Alternately, keeping 4 extra
> bytes on a 112-byte structure (on ILP32) isn't much to worry about.

having different functions have different notions of the definition
of a structure just seems like a bad practice that we should avoid.

if memory is a concern then we could move this field into the space
currently used by the implicit padding between the "flags" and "timeout"
fields.  16 bits should be enough for this purpose.


> For the rest it seems pretty clear that the structures aren't numerous
> enough for size to be a big concern. It's probably worth marking that
> those members aren't filled with valid data if DIAGNOSTIC is off.

ok, I'll be sure to add comments to that effect.

-Chuck