On Mon, Oct 31, 2005 at 02:46:50PM -0800, Bill Studenmund wrote:
> On Sun, Oct 30, 2005 at 09:04:56AM -0800, Chuck Silvers wrote:
> > hi,
> > 
> > does anyone object to turning off DIAGNOSTIC in all the GENERIC configs?
> > this seems like reasonable anyway, and it fixes this issue in nicely.
> 
> As a direct answer to your question, I think it may be best to leave it on 
> for -current kernels and to turn it off for release kernels.
> 
> I think though we are muddling three issues in this thread:
> 
> 1) Should DIAGNOSTIC be on in GENERIC kernels (my answer's above, but I
> don't feel very strongly about this)?

I think that anyone using -current will be capable of deciding for
themselves whether they want these options or not.  rather than add
a requirement that people making release branches also go and change
a zillion config files, I'd prefer to just turn this off in GENERIC
in -current too.


> 2) Should our default shipping LKMs have options that differ from our
> default shipping kernels?

obviously the LKM binaries in a release should be compatible with
the default kernel in a release ("default" in this case being "GENERIC").

some others kernels that are also in a release (eg. GENERIC.MP) are
not LKM-compatible with GENERIC, so one set of LKM binaries cannot
currently be compatible with all kernels in a release.  having multiple
sets of LKMs is not a good solution.

I do not think that we should try to solve this for 3.0.
having the 3.0 LKMs be compatible with GENERIC is good enough for now.


> 3) Should DIAGNOSTIC make LKMs incompatible?
> 
> I agree with you that I think the answer to #3 should be no, and support 
> your efforts to change this. :-)
> 
> However I think we really need to fix #2; while getting rid of DIAGNOSTIC 
> gets rid of the pain, the issue is still there. Also, I don't think the 
> fix is that hard. :-)
> 
> All I think we need to do is add a make variable, say KMOD_KERNCONFOPTS, 
> conditionally set it in bsd.kmod.mk, and always add the variable to 
> CPPFLAGS and CFLAGS (and anything else that needs it).
> 
> We then just make sure that the conditional define matches GENERIC and 
> we're set!

if all GENERICs have all the options that change the LKM ABI turned off
(which is what I'd prefer), then the existing build process for LKMs
(which doesn't define any of these flags) is fine.  so I'll leave this
enhancement to the LKM build structure for later.


> > in the longer term, I think that DIAGNOSTIC should not make LKMs incompatible.
> > I checked the source tree for reasons why turning on this option might cause
> > incompatibilities, and I found just a few things:
> > 
> >  - these structures have an extra field if DIAGNOSTIC is on:
> > 
> > 	struct irframe_softc
> > 	struct usbd_xfer
> > 	struct wsemul_vt100_emuldata
> > 	struct cpu_data
> > 
> > 	struct ehci_xfer
> > 	struct ohci_soft_itd
> > 	struct uhci_intr_info
> > 	struct union_node
> > 
> > 
> >    I'm guessing that structures in the first group are significant for LKMs
> >    whereas those the second group are not.
> > 
> >    I think that for structures in the first group we should either have
> >    these fields always present, or just remove them and the code that
> >    references them.  It would be nice to do that for the second group
> >    as well, just so that it's easier to make sure we don't add more things
> >    to the first group in the future.
> 
> I'd say make the data always present.

fine with me, I'll do that.


> >  - these headers define functions as inline iff !DIAGNOSTIC
> > 	sys/vnode.h
> > 		holdrelel
> > 		vholdl
> > 		vref
> > 
> >    I think we should just make these not be inline.
> 
> Maybe make them a separate inline control, like we have for VOPs? I 
> thought inlining was faster for some cases.. ??

I really doubt it would make any measurable difference.
these functions just aren't called so often that it matters.

-Chuck


> > any objections to these longer-term changes?
> 
> No. Comments & thoughs as above, but no objections.
> 
> Take care,
> 
> Bill