Re: RFC: device attachment/detachment

[Christoph Egger <Christoph_Egger%gmx.de@localhost>]

Andrew Doran wrote:
> On Fri, May 01, 2009 at 07:21:18PM -0500, David Young wrote:
> 
>> On Sat, May 02, 2009 at 12:56:43AM +0200, Christoph Egger wrote:
>>> Attached patch modifies the driver's attachment function to
>>> return an error code, zero for success.
>>> I modified all drivers necessary to compile an amd64 GENERIC
>>> kernel plus bwi(4). The intended functional change is in
>>> subr_autoconf.c, there's no intended functional change in the
>>> drivers.
>> I agree that attach routines should return with a result code.
> 
> Yes, thanks for looking at this!
> 
>> Gradually modernize both the attach & detach routine of each driver:
>> make the attach routines reliably track the resources that were
>> acquired.  Let the detach routines try to release only those resources
>> that were acquired.  Use CFATTACH_DECL4{,_NEW)(, DVF_DETACH_SHUTDOWN) to
>> register only those devices whose attach & detach routine DTRT.
> 
> I dislike this somewhat this because it's open-ended and does not improve
> the existing standard ("panicing or leaking resources is OK"). That is,
> anyone can go cut-n-paste and get the old mechanism. By mandating the return
> we would be giving a stronger signal about what's required.

I agree on that. My patch changes the semantic in one go.
Note, that my patch breaks the build of all
kernels but amd64 GENERIC. I can't change all drivers alone and need
help for this. Given the fact of build breakage, I am sure I get help
from a lot of people, *if* I can apply my patch as Andrew suggests
below. And while people are at the drivers to make them build,
they can fix the driver's error handling, too.

> Here is a suggestion of another way:
> 
> Apply Christoph's patch, and generate a list of all the attach functions in
> tree along with corresponding file name. Put the list into a csv somewhere
> (src/sys/doc? unimportant really) and say "these are the drivers that we
> have audited". It could be imported into OpenOffice or an SQL database and
> then one can go look for stuff to work on. Or a dumb web-app could look
> after it.
> 
> File,Function,Attach OK,Detach OK,Is MPSAFE,Uses device_t
> dev/pci/iop_pci.c,iop_pci_attach,N,N,N,N
> 
> What do you think?

I agree on that. I like this much better than dyoung's proposal.

Christoph