Skip site navigation (1)Skip section navigation (2)
Date:      Mon, 24 Sep 2007 19:25:21 +0200
From:      "Attilio Rao" <attilio@freebsd.org>
To:        "John Baldwin" <jhb@freebsd.org>
Cc:        cvs-src@freebsd.org, src-committers@freebsd.org, cvs-all@freebsd.org
Subject:   Re: cvs commit: src/sys/amd64/amd64 local_apic.c src/sys/i386/acpica madt.c src/sys/i386/i386 local_apic.c src/sys/kern subr_smp.c src/sys/sun4v/mdesc mdesc_init.c
Message-ID:  <3bbf2fe10709241025r4e0d25c6x328ac21dc6bc99c@mail.gmail.com>
In-Reply-To: <200709241147.49288.jhb@freebsd.org>
References:  <200709112254.l8BMsB7P074637@repoman.freebsd.org> <200709211436.15444.jhb@freebsd.org> <3bbf2fe10709211338j6dbab59am1ad67c86c1a05baa@mail.gmail.com> <200709241147.49288.jhb@freebsd.org>

next in thread | previous in thread | raw e-mail | index | archive | help
2007/9/24, John Baldwin <jhb@freebsd.org>:
>
> It's a gratuitous change if you aren't actually _fixing_ anything.   I think
> there is a fundamental thing you are missing here though:  it is ok to have
> multiple SYSINIT's with the same subsystem and order if they are independent.
> All device drivers register themselves with new-bus at the same subsystem and
> order for example.  There is no need to "dream up" a false ordering if it is
> not needed.  Now you have made B always come after A.  What if a future code
> change makes A depend on B?  It is best to solve these problems when they
> appear.

I don't, personally, think this approach is fine and last efforts in
order to fix topology basically showed what I'm saying. However this
is just an opinion of mine, so this is nothing of highly definitive.

> > Plain wrong?
> > You mean maybe that it doesn't matter that (SI_ORDER_CPU -1) was
> > shared with mptable_register()? And what about if someone adds
> > dependency between the two? madt changes are perfectly working as they
> > don't explicitly require to run before of mptable_register().
> > And to be precise, there is no madt_register() in amd64, so I have no
> > idea what are you speaking about. :(.
>
> I guess you don't understand what these register routines do.

Whee, please, I don't think one needs a big fantasy in order to
understand them, and I think currently there are a lot of more
difficult bits in the kernel than these functions :)

My question is: what makes you so unconfortable with this patch?
The ordering is fine, some things can be switched as they don't
directly depends by each other but definitively they are in a good
ordering. In the future you won't have problem of messy startup and if
you really need to change the order you can do explicitly. So the
problem is ...?

> See above.  Are you going to go throw and assign a static order to ever device
> driver now just in case the one's registration routine might someday depend
> on another?

Yes, and it doesn't sound so unreasonable.

> It hardcodes an ordering that isn't there and that might have to be reworked
> in the future if a real dependency does show up that is not the order you
> currently just randomly chose.

randomly choose?
Please be pragmatic. This ordering is not failing at all. I cannot
think, for example, cpu_startup() to run after lapic bits...

Attilio


-- 
Peace can only be achieved by understanding - A. Einstein



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?3bbf2fe10709241025r4e0d25c6x328ac21dc6bc99c>