Messages in this thread | | | Date | Tue, 19 Oct 1999 16:56:02 -0400 (EDT) | From | Donald Becker <> | Subject | Re: PATCH 2.3.23 pre 2 compile fixes |
| |
On Tue, 19 Oct 1999, Alan Cox wrote:
> > This has done to all of the PCI drivers. See > > http://cesdis.gsfc.nasa.gov/linux/drivers/kern-2.3/index.html > > ftp://cesdis.gsfc.nasa.gov/pub/linux/drivers/kern-2.3/* > > These use the new pci scan code when Linus picked Jeff Garzik's stuff instead
That pci scan code has significant limitations. It doesn't handle the ACPI-D3 problem when warm-booting from Windows. This generates a bunch of bug reports, and they all go to me.
It doesn't handle hot-swap PCI and CardBus. I know other people have "claimed" hot-swap PCI as their project, but it's been working for years with CardBus cards. The new structure allows a single driver binary to support both normal and hot-swap cards, instead compiling a special CardBus-only version of the driver.
And it doesn't help the cross-version compatibility, which no one except users seems to care about.
> > The check_mem_region() functionality doesn't exist in older kernel version. > > The work-around is to use check_region()/register_region() for both memory > > and I/O spaces. > > This is potentially wrong for non x86 and wrong for 2.3.x though even if it > is a sane hack for 2.2.x.
It's "potentially wrong" in theory for x86 as well, but it's always OK in practice. I/O space and memory spaces are disjoint.
> > The problem also points out that the PCI driver probes are incorrectly > > called a bunch of times at boot, and the structure has a foolish flaw that > > limits "eth??" naming to only 16 cards. > > The eth?? thing is a bug. The multiple probe is a symptom of a bug in > the driver. Suppose two drivers are on the machine for the same PCI card > (eg if someone has decided to use 3coms card driver for the 3c90x and > loads yours to get his 3c595 handled too).
There is still a buglet in the Space.c probe structure. It's calling the PCI probe routines multiple times, when it should only try once. (I though that this has been fixed.)
> Should be static or you get a symbol clash with 2 of the drivers compiled > in. > np = (void *)(((long)kmalloc(sizeof(*np), GFP_KERNEL) + 31) & ~31); > memset(np, 0, sizeof(*np)); > > The malloc isnt NULL checked, and the block isnt freed on unload.
This was a common buglet in many of the drivers -- it occured when I converted them to always kmalloc() their dev->priv structure rather than count on the init_etherdev(dev, sizeof(struct net_private)) function.
This results in a tiny one-time memory leak when a driver module is removed. (Note: the much larger data buffers, typically 20-40KB total, are checked for allocation success and released each time the interface is shut down.)
Most drivers, and pci-skeleton.c, have been updates to avoid this. Only the via-rhine.c and starfire.c drives have not been updated.
> So yes you are right about the vmalloc being my error. sorry. The others are > long standing things I've reported before. > > Also notionally init_etherdev() could return NULL and should be checked. THat > is something that needs propogating on since its more of a rule change than > a bug.
OK, I'll make that change to pci-skeleton.c. BTW, the old skeleton.c should be renamed isa-skeleton.c, then deleted.
Donald Becker Scyld Computing Corporation, and USRA-CESDIS, becker@cesdis.gsfc.nasa.gov
- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.rutgers.edu Please read the FAQ at http://www.tux.org/lkml/
| |