Re: pci_alloc_resource is broken

--- On Tue, 3/31/09, John Baldwin <jhb_at_freebsd.org> wrote:

> From: John Baldwin <jhb_at_freebsd.org>
> Subject: Re: pci_alloc_resource is broken
> To: barney_cordoba_at_yahoo.com
> Cc: freebsd-current_at_freebsd.org
> Date: Tuesday, March 31, 2009, 10:55 AM
> On Monday 30 March 2009 6:18:24 pm Barney Cordoba wrote:
> > I dunno, but its pretty lame that you can't read a
> register on a card
> > without hacking the kernel. The entire point of having
> a generic bus
> > management system is to create a relatively
> transparent mechanism for
> > accessing such resources, but then if you try to share
> a resource
> > you find out that the system doesn't support
> shared resources and
> > requires proprietary hacking of the drivers. 
> 
> Most OS's tend to assume that a single driver is all
> that is needed for a 
> single PCI function.  Windows does allow filter drivers
> which you could use 
> to have multiple drivers for a given PCI function.  In
> FreeBSD we only allow 
> you to have one.  However, you are free to have your
> "one" driver actually be 
> a virtual bus that allows for multiple leaf drivers.  You
> can look at 
> sys/dev/pci/vga_pci.c for an example of this.  It allows
> drm, acpi_video, and 
> agp to all attach to a VGA adapter.  It even allows sharing
> of the BARs so 
> the different sub-drivers can each allocate the BAR and use
> it.  If you have 
> designed your hardware such that it places multiple logical
> functions into a 
> single PCI function (rather than having a separate PCI
> function for each 
> logical function) then you can take the bus approach.  You
> can even have both 
> the virtual bus driver and the NIC driver "open"
> and the magic frobnitz 
> driver in a binary foo.ko if you want.
> 
> > We don't want to have to make the 
> > ethernet functionality "proprietary" or
> required to be obtained from
> > a static source. So if some dude in Pakistan wants to
> grab a bug fix
> > he can do it without whining to the vendor that the
> code is proprietary,
> > thus burdening the vendor to re-hack every iteration
> of code that some
> > customer in some corner of the world wants to run for
> one reason or 
> > another. The alternative is telling the customer that
> he can't have the 
> > bug fix until the vendor gets around to releasing a
> version that has the 
> > latest patch. It doesn't have to run on other
> OSes. Its a logistic issue.
> > Its a matter of streamlining the work flow. Having
> separate drivers 
> > makes upgrading easier. You don't have to hack 37
> source files every time
> > you decide to grab a -stable because some dude at
> freebsd.org decided to
> > change a macro. Are you getting this?
> 
> You need more "commercial" experience because
> some "commercial" OS's are worse 
> about changing macros than FreeBSD is. :)  A word of
> advice: your 
> condescending tone is not going to win you any friends or
> get your questions 
> answered any sooner.  Quite the contrary in fact.

Its really you who is being condescending. You're trying to 
justify bad coding by telling someone who has been
in business for 20 years with 3000 active customers that
he needs more commercial experience. If you were Bill Gates
I'd accept that. But you're a programmer. Perhaps your idea
of "commercial" is the Apple API. Commercial means not a
toy. Not in a lab. People using  your OS to earn a living. To
feed their families. When an ISP's FreeBSD router panics it
might cost him business; perhaps his livelihood. Try to be
a bit less flippant about things.

 > As a side issue, the number of panics I found along
> the way is a bit 
> > alarming. Its pretty lazy coding. Panic should really
> only be called
> > when a condition is unrecoverable; not every time
> someone doesn't feel
> > like writing the couple of lines of code to handle the
> condition.
> 
> To be honest, in the case of device drivers it can be
> helpful to make the 
> system panic more easily since many of them are lazy about
> checking return 
> values from functions (I've had to deal with several
> out-of-tree drivers).  
> Also, FWIW, other bus devices do panic in this particular
> case.  The fact 
> that PCI doesn't panic in 5.x - 7.x is actually due to
> a bug (resulting in 
> the aforementioned resource leak) that I fixed in 8.

It can be helpful if you live in a lab. 

Here's what the man page for bus_alloc_resource says:

"A pointer to struct resource is returned on success, a null pointer otherwise."

Here is what the man page for panic says:

"panic -- bring down system on fatal error"

Now is a resource in use a "fatal" system error? 

Is falling into resource_list_alloc() when you know that the resource
list exists quality coding?

Face it please. You replaced one bug with another. If you spent half the
effort doing it correctly as you are spending defending what you've done
we'd all be a lot happier.


Barney