eliminate pci_dev name

PATCH In review Matthew Wilcox 4 participants 9 messages Nov 14, 2002 – Nov 16, 2002

Subsystems (5)

Network drivers USB PCI Driver core +1 more subsystems

lore

Share & tools

lore

Matthew Wilcox

author

2002-11-14 17:05 UTC #0 lore raw

PATCH 1/1 eliminate pci_dev name

Since struct device doesn't seem to be going away any time soon, let's start trying to use it properly. This patch reclaims 90 bytes from each & every pci_dev in the system. If this patch is accepted, I'll follow up with other patches removing more of the pci_dev components.

drivers/pci/names.c +1 -1

diff -urpNX dontdiff linux-2.5.47/drivers/pci/names.c linux-2.5.47-willy/drivers/pci/names.c
@@ -56,7 +56,7 @@ void __devinit pci_name_device(struct pc
 {
 	const struct pci_vendor_info *vendor_p = pci_vendor_list;
 	int i = VENDORS;
-	char *name = dev->name;
+	char *name = dev->dev.name;
 
 	do {
 		if (vendor_p->vendor == dev->vendor)

drivers/pci/probe.c +4 -5

diff -urpNX dontdiff linux-2.5.47/drivers/pci/probe.c linux-2.5.47-willy/drivers/pci/probe.c
@@ -52,7 +52,7 @@ static void pci_read_bases(struct pci_de
 	for(pos=0; pos<howmany; pos = next) {
 		next = pos+1;
 		res = &dev->resource[pos];
-		res->name = dev->name;
+		res->name = dev->dev.name;
 		reg = PCI_BASE_ADDRESS_0 + (pos << 2);
 		pci_read_config_dword(dev, reg, &l);
 		pci_write_config_dword(dev, reg, ~0);

@@ -112,7 +112,7 @@ static void pci_read_bases(struct pci_de
 			sz = pci_size(sz, PCI_ROM_ADDRESS_MASK);
 			res->end = res->start + (unsigned long) sz;
 		}
-		res->name = dev->name;
+		res->name = dev->dev.name;
 	}
 }

@@ -129,7 +129,7 @@ void __devinit pci_read_bridge_bases(str
 		return;
 
 	if (dev->transparent) {
-		printk("Transparent bridge - %s\n", dev->name);
+		printk("Transparent bridge - %s\n", dev->dev.name);
 		for(i = 0; i < PCI_BUS_NUM_RESOURCES; i++)
 			child->resource[i] = child->parent->resource[i];
 		return;

@@ -355,7 +355,7 @@ int pci_setup_device(struct pci_dev * de
 	u32 class;
 
 	sprintf(dev->slot_name, "%02x:%02x.%d", dev->bus->number, PCI_SLOT(dev->devfn), PCI_FUNC(dev->devfn));
-	sprintf(dev->name, "PCI device %04x:%04x", dev->vendor, dev->device);
+	sprintf(dev->dev.name, "PCI device %04x:%04x", dev->vendor, dev->device);
 	INIT_LIST_HEAD(&dev->pools);
 	
 	pci_read_config_dword(dev, PCI_CLASS_REVISION, &class);

@@ -447,7 +447,6 @@ struct pci_dev * __devinit pci_scan_devi
 	pci_name_device(dev);
 
 	/* now put in global tree */
-	strcpy(dev->dev.name,dev->name);
 	strcpy(dev->dev.bus_id,dev->slot_name);
 
 	device_register(&dev->dev);

drivers/pci/proc.c +1 -1

diff -urpNX dontdiff linux-2.5.47/drivers/pci/proc.c linux-2.5.47-willy/drivers/pci/proc.c
@@ -511,7 +511,7 @@ static int show_dev_config(struct seq_fi
 		seq_printf(m, "    %s", class);
 	else
 		seq_printf(m, "    Class %04x", class_rev >> 16);
-	seq_printf(m, ": %s (rev %d).\n", dev->name, class_rev & 0xff);
+	seq_printf(m, ": %s (rev %d).\n", dev->dev.name, class_rev & 0xff);
 
 	if (dev->irq)
 		seq_printf(m, "      IRQ %d.\n", dev->irq);

drivers/pci/quirks.c +1 -1

diff -urpNX dontdiff linux-2.5.47/drivers/pci/quirks.c linux-2.5.47-willy/drivers/pci/quirks.c
@@ -203,7 +203,7 @@ static void __devinit quirk_io_region(st
 	if (region) {
 		struct resource *res = dev->resource + nr;
 
-		res->name = dev->name;
+		res->name = dev->dev.name;
 		res->start = region;
 		res->end = region + size - 1;
 		res->flags = IORESOURCE_IO;

drivers/pci/setup-res.c +3 -3

diff -urpNX dontdiff linux-2.5.47/drivers/pci/setup-res.c linux-2.5.47-willy/drivers/pci/setup-res.c
@@ -46,11 +46,11 @@ pci_claim_resource(struct pci_dev *dev, 
 		if (err) {
 			printk(KERN_ERR "PCI: Address space collision on "
 			       "region %d of device %s [%lx:%lx]\n",
-			       resource, dev->name, res->start, res->end);
+			       resource, dev->dev.name, res->start, res->end);
 		}
 	} else {
 		printk(KERN_ERR "PCI: No parent found for region %d "
-		       "of device %s\n", resource, dev->name);
+		       "of device %s\n", resource, dev->dev.name);
 	}
 
 	return err;

@@ -155,7 +155,7 @@ pdev_sort_resources(struct pci_dev *dev,
 		if (!r_align) {
 			printk(KERN_WARNING "PCI: Ignore bogus resource %d "
 					    "[%lx:%lx] of %s\n",
-					    i, r->start, r->end, dev->name);
+					    i, r->start, r->end, dev->dev.name);
 			continue;
 		}
 		r_align = (i < PCI_BRIDGE_RESOURCES) ? r_align + 1 : r->start;

drivers/pcmcia/cardbus.c +0 -1

diff -urpNX dontdiff linux-2.5.47/drivers/pcmcia/cardbus.c linux-2.5.47-willy/drivers/pcmcia/cardbus.c
@@ -284,7 +284,6 @@ int cb_alloc(socket_info_t * s)
 
 		pci_setup_device(dev);
 
-		strcpy(dev->dev.name, dev->name);
 		strcpy(dev->dev.bus_id, dev->slot_name);
 
 		/* FIXME: Do we need to enable the expansion ROM? */

drivers/pcmcia/yenta.c +1 -1

diff -urpNX dontdiff linux-2.5.47/drivers/pcmcia/yenta.c linux-2.5.47-willy/drivers/pcmcia/yenta.c
@@ -585,7 +585,7 @@ static void yenta_open_bh(void * data)
 	/* It's OK to overwrite this now */
 	INIT_WORK(&socket->tq_task, yenta_bh, socket);
 
-	if (!socket->cb_irq || request_irq(socket->cb_irq, yenta_interrupt, SA_SHIRQ, socket->dev->name, socket)) {
+	if (!socket->cb_irq || request_irq(socket->cb_irq, yenta_interrupt, SA_SHIRQ, socket->dev->dev.name, socket)) {
 		/* No IRQ or request_irq failed. Poll */
 		socket->cb_irq = 0; /* But zero is a valid IRQ number. */
 		socket->poll_timer.function = yenta_interrupt_wrapper;

drivers/usb/core/hcd-pci.c +2 -2

diff -urpNX dontdiff linux-2.5.47/drivers/usb/core/hcd-pci.c linux-2.5.47-willy/drivers/usb/core/hcd-pci.c
@@ -137,7 +137,7 @@ clean_2:
 	hcd->description = driver->description;
 	hcd->pdev = dev;
 	hcd->self.bus_name = dev->slot_name;
-	hcd->product_desc = dev->name;
+	hcd->product_desc = dev->dev.name;
 
 	if ((retval = hcd_buffer_create (hcd)) != 0) {
 clean_3:

@@ -145,7 +145,7 @@ clean_3:
 		goto clean_2;
 	}
 
-	info ("%s @ %s, %s", hcd->description,  dev->slot_name, dev->name);
+	info ("%s @ %s, %s", hcd->description,  dev->slot_name, dev->dev.name);
 
 #ifndef __sparc__
 	sprintf (buf, "%d", dev->irq);

include/linux/pci.h +0 -1

diff -urpNX dontdiff linux-2.5.47/include/linux/pci.h linux-2.5.47-willy/include/linux/pci.h
@@ -371,7 +371,6 @@ struct pci_dev {
 	struct resource dma_resource[DEVICE_COUNT_DMA];
 	struct resource irq_resource[DEVICE_COUNT_IRQ];
 
-	char		name[90];	/* device name */
 	char		slot_name[8];	/* slot name */
 	int		active;		/* ISAPnP: device is active */
 	int		ro;		/* ISAPnP: read only */

Signature · 1 line

Revolutions do not require corporate support.

Jeff Garzik

2002-11-14 18:21 UTC #1 lore raw

include/linux/pci.h struct pci_dev { · line 376

Quoted 13 lines · Matthew Wilcox

diff -urpNX dontdiff linux-2.5.47/include/linux/pci.h linux-2.5.47-willy/include/linux/pci.h

include/linux/pci.h +0 -0

11:43:40.000000000 -0500

include/linux/pci.h +0 -1

@@ -371,7 +371,6 @@ struct pci_dev {
 	struct resource dma_resource[DEVICE_COUNT_DMA];
 	struct resource irq_resource[DEVICE_COUNT_IRQ];
 
-	char		name[90];	/* device name */
 	char		slot_name[8];	/* slot name */
 	int		active;		/* ISAPnP: device is active */
 	int		ro;		/* ISAPnP: read only */

Patch looks pretty good to me... seems like the obvious (and useful) cleanup.

You should increase DEVICE_NAME_SIZE in include/linux/device.h from 80 to 90, though. I assume you don't want to take the other option, which is to audit every use and all the id strings to make sure they're short enough. In fact, IIRC, device name increased in size due to some really long PCI names, so I think '90' will wind up the preferred value in any case.

Jeff

Matthew Wilcox

author

2002-11-14 18:27 UTC #2 lore raw

Quoted 9 lines · Jeff Garzik

Patch looks pretty good to me... seems like the obvious (and useful) cleanup.
You should increase DEVICE_NAME_SIZE in include/linux/device.h from 80 to 90, though. I assume you don't want to take the other option, which is to audit every use and all the id strings to make sure they're short enough. In fact, IIRC, device name increased in size due to some really long PCI names, so I think '90' will wind up the preferred value in any case.

I didn't break anything that wasn't already broken.

- strcpy(dev->dev.name, dev->name);

Who wants a smack?

Signature · 1 line

Revolutions do not require corporate support.

Linus Torvalds

2002-11-14 18:29 UTC #3 lore raw

In reply to Jeff Garzik

Quoted 7 lines · Jeff Garzik

You should increase DEVICE_NAME_SIZE in include/linux/device.h from 80 to 90, though. I assume you don't want to take the other option, which is to audit every use and all the id strings to make sure they're short enough. In fact, IIRC, device name increased in size due to some really long PCI names, so I think '90' will wind up the preferred value in any case.

Actually, I think we should do the reverse (for testing), and make the name be something small like 8 bytes, and make sure that everybody who writes the name uses strncpy() and snprintf() instead of just blindly writing whatever is in the database.

Otherwise we'll always end up having fragile magic constants.

Anybody willing to do that cleanup?

Linus

Matthew Wilcox

author

2002-11-14 18:37 UTC #4 lore raw

Quoted 8 lines · Linus Torvalds

Actually, I think we should do the reverse (for testing), and make the name be something small like 8 bytes, and make sure that everybody who writes the name uses strncpy() and snprintf() instead of just blindly writing whatever is in the database.
Otherwise we'll always end up having fragile magic constants.
Anybody willing to do that cleanup?

Sure, I can do that. That leads me to think that maybe we should delete name from struct device and just use the one in struct kobject (which is already a mere 16 bytes). But if we're going to go as far down as the kobject... that has a dentry. And dentrys have names. So how about eliminating that too and just creating a dentry with the almost infinitely long name?

Maybe that's too much at this stage of the game.

Signature · 1 line

Revolutions do not require corporate support.

Linus Torvalds

2002-11-14 19:16 UTC #5 lore raw

Quoted 7 lines · Matthew Wilcox

Sure, I can do that. That leads me to think that maybe we should delete name from struct device and just use the one in struct kobject (which is already a mere 16 bytes). But if we're going to go as far down as the kobject... that has a dentry. And dentrys have names. So how about eliminating that too and just creating a dentry with the almost infinitely long name?

I like your thinking, but the "pci->name" is really just a descrition, and what should maybe be in the kobject is the "address", which is "pci->slotname". The two are quite fundamentally different (ie nobody should depend on "name" being unique or constant anything like that, while "slotname" is really a bus-level address (*)).

Linus

(*) "slotname" is also not unique in general, it's only unique _within_ _that_ _bus_. With multiple PCI buses, for example, you can have two devices with the same slotname, if they are on separate segments.

Ingo Oeser

2002-11-15 06:45 UTC #6 lore raw

In reply to Matthew Wilcox

Hi Matthew,

Quoted 8 lines · Matthew Wilcox

Sure, I can do that. That leads me to think that maybe we should delete name from struct device and just use the one in struct kobject (which is already a mere 16 bytes). But if we're going to go as far down as the kobject... that has a dentry. And dentrys have names. So how about eliminating that too and just creating a dentry with the almost infinitely long name?
Maybe that's too much at this stage of the game.

Using the dentry the PERFECT cleanup, because it removes ALL the redundacy and provides infinite length (which is sometimes needed) as well as an optimization for small (DNAME_INLINE_LEN_MIN) names.

So it's the perfect string container, if needed anyway.

Regards

Ingo Oeser

Signature · 1 line

Science is what we can tell a computer. Art is everything else. --- D.E.Knuth

Matthew Wilcox

author

2002-11-16 23:41 UTC #7 lore raw

In reply to Linus Torvalds

Quoted 8 lines · Linus Torvalds

Actually, I think we should do the reverse (for testing), and make the name be something small like 8 bytes, and make sure that everybody who writes the name uses strncpy() and snprintf() instead of just blindly writing whatever is in the database.
Otherwise we'll always end up having fragile magic constants.
Anybody willing to do that cleanup?

I've done something slightly different. I've trimmed it down to 50 bytes and limited the length of the vendor & device descriptions to 20 bytes each. Now we have three cases:

unknown vendor & unknown device -> "PCI device %04x:%04x".
Clearly limited to 21 chars.
known vendor, unknown device -> "PCI device %04x:%04x (%.20s)"

	24 + 20 chars
known vendor, known device -> "%.20s %.20s"
	20 + 1 + 20 + 1, EXCEPT:

multiple devices of the same type add " (#%d)"
so 42 + 4 + 4 = 50. This is the point where an IBMer tells me they intend to sell a machine with > 9999 PCI devices of the same type ;-)

Yeah, I could have done what you asked for, but that looks like needless additional paranoia to me. This limits the PCI IDs to a reasonable amount of space.

For release, I think we do want to go back up to 80. Here's /proc/pci on cl009 at osdl:

root@cl009:~# cat /proc/pci |grep '^    [^ ]'
    Host bridge: ServerWorks CNB20LE Host Bridge (rev 5).
    Host bridge: ServerWorks CNB20LE Host Bridge (#2) (rev 5).
    Ethernet controller: Intel Corp. 82557/8/9 [Ethernet  (rev 8).
    Ethernet controller: Intel Corp. 82557/8/9 [Ethernet  (#2) (rev 8).
    VGA compatible controller: ATI Technologies Inc Rage XL (rev 101).
    ISA bridge: ServerWorks OSB4 South Bridge (rev 79).
    IDE interface: ServerWorks OSB4 IDE Controller (rev 0).
    USB Controller: ServerWorks OSB4/CSB5 USB Contro (rev 4).
    SCSI storage controller: LSI Logic / Symbios  53c896 (rev 7).
    SCSI storage controller: LSI Logic / Symbios  53c896 (#2) (rev 7).

It's usable, but ugly.

Here's the patch:

drivers/net/eepro100.c +1 -1

diff -urpNX dontdiff linux-2.5.47/drivers/net/eepro100.c linux-2.5.47-pci/drivers/net/eepro100.c
@@ -734,7 +734,7 @@ static int __devinit speedo_found1(struc
 	if (eeprom[3] & 0x0100)
 		product = "OEM i82557/i82558 10/100 Ethernet";
 	else
-		product = pdev->name;
+		product = pdev->dev.name;
 
 	printk(KERN_INFO "%s: %s, ", dev->name, product);

drivers/pci/names.c +6 -3

diff -urpNX dontdiff linux-2.5.47/drivers/pci/names.c linux-2.5.47-pci/drivers/pci/names.c
@@ -56,7 +56,7 @@ void __devinit pci_name_device(struct pc
 {
 	const struct pci_vendor_info *vendor_p = pci_vendor_list;
 	int i = VENDORS;
-	char *name = dev->name;
+	char *name = dev->dev.name;
 
 	do {
 		if (vendor_p->vendor == dev->vendor)

@@ -80,12 +80,15 @@ void __devinit pci_name_device(struct pc
 		}
 
 		/* Ok, found the vendor, but unknown device */
-		sprintf(name, "PCI device %04x:%04x (%s)", dev->vendor, dev->device, vendor_p->name);
+		sprintf(name, "PCI device %04x:%04x (%." DEVICE_NAME_HALF "s)",
+				dev->vendor, dev->device, vendor_p->name);
 		return;
 
 		/* Full match */
 		match_device: {
-			char *n = name + sprintf(name, "%s %s", vendor_p->name, device_p->name);
+			char *n = name + sprintf(name, "%." DEVICE_NAME_HALF
+					"s %." DEVICE_NAME_HALF "s",
+					vendor_p->name, device_p->name);
 			int nr = device_p->seen + 1;
 			device_p->seen = nr;
 			if (nr > 1)

drivers/pci/probe.c +4 -5

diff -urpNX dontdiff linux-2.5.47/drivers/pci/probe.c linux-2.5.47-pci/drivers/pci/probe.c
@@ -52,7 +52,7 @@ static void pci_read_bases(struct pci_de
 	for(pos=0; pos<howmany; pos = next) {
 		next = pos+1;
 		res = &dev->resource[pos];
-		res->name = dev->name;
+		res->name = dev->dev.name;
 		reg = PCI_BASE_ADDRESS_0 + (pos << 2);
 		pci_read_config_dword(dev, reg, &l);
 		pci_write_config_dword(dev, reg, ~0);

@@ -112,7 +112,7 @@ static void pci_read_bases(struct pci_de
 			sz = pci_size(sz, PCI_ROM_ADDRESS_MASK);
 			res->end = res->start + (unsigned long) sz;
 		}
-		res->name = dev->name;
+		res->name = dev->dev.name;
 	}
 }

@@ -129,7 +129,7 @@ void __devinit pci_read_bridge_bases(str
 		return;
 
 	if (dev->transparent) {
-		printk("Transparent bridge - %s\n", dev->name);
+		printk("Transparent bridge - %s\n", dev->dev.name);
 		for(i = 0; i < PCI_BUS_NUM_RESOURCES; i++)
 			child->resource[i] = child->parent->resource[i];
 		return;

@@ -355,7 +355,7 @@ int pci_setup_device(struct pci_dev * de
 	u32 class;
 
 	sprintf(dev->slot_name, "%02x:%02x.%d", dev->bus->number, PCI_SLOT(dev->devfn), PCI_FUNC(dev->devfn));
-	sprintf(dev->name, "PCI device %04x:%04x", dev->vendor, dev->device);
+	sprintf(dev->dev.name, "PCI device %04x:%04x", dev->vendor, dev->device);
 	INIT_LIST_HEAD(&dev->pools);
 	
 	pci_read_config_dword(dev, PCI_CLASS_REVISION, &class);

@@ -447,7 +447,6 @@ struct pci_dev * __devinit pci_scan_devi
 	pci_name_device(dev);
 
 	/* now put in global tree */
-	strcpy(dev->dev.name,dev->name);
 	strcpy(dev->dev.bus_id,dev->slot_name);
 
 	device_register(&dev->dev);

drivers/pci/proc.c +1 -1

diff -urpNX dontdiff linux-2.5.47/drivers/pci/proc.c linux-2.5.47-pci/drivers/pci/proc.c
@@ -511,7 +511,7 @@ static int show_dev_config(struct seq_fi
 		seq_printf(m, "    %s", class);
 	else
 		seq_printf(m, "    Class %04x", class_rev >> 16);
-	seq_printf(m, ": %s (rev %d).\n", dev->name, class_rev & 0xff);
+	seq_printf(m, ": %s (rev %d).\n", dev->dev.name, class_rev & 0xff);
 
 	if (dev->irq)
 		seq_printf(m, "      IRQ %d.\n", dev->irq);

drivers/pci/quirks.c +1 -1

diff -urpNX dontdiff linux-2.5.47/drivers/pci/quirks.c linux-2.5.47-pci/drivers/pci/quirks.c
@@ -203,7 +203,7 @@ static void __devinit quirk_io_region(st
 	if (region) {
 		struct resource *res = dev->resource + nr;
 
-		res->name = dev->name;
+		res->name = dev->dev.name;
 		res->start = region;
 		res->end = region + size - 1;
 		res->flags = IORESOURCE_IO;

drivers/pci/setup-irq.c +1 -1

diff -urpNX dontdiff linux-2.5.47/drivers/pci/setup-irq.c linux-2.5.47-pci/drivers/pci/setup-irq.c
@@ -53,7 +53,7 @@ pdev_fixup_irq(struct pci_dev *dev,
 		irq = 0;
 	dev->irq = irq;
 
-	DBGC((KERN_ERR "PCI fixup irq: (%s) got %d\n", dev->name, dev->irq));
+	DBGC((KERN_ERR "PCI fixup irq: (%s) got %d\n", dev->dev.name, dev->irq));
 
 	/* Always tell the device, so the driver knows what is
 	   the real IRQ to use; the device does not use it. */

drivers/pci/setup-res.c +3 -3

diff -urpNX dontdiff linux-2.5.47/drivers/pci/setup-res.c linux-2.5.47-pci/drivers/pci/setup-res.c
@@ -46,11 +46,11 @@ pci_claim_resource(struct pci_dev *dev, 
 		if (err) {
 			printk(KERN_ERR "PCI: Address space collision on "
 			       "region %d of device %s [%lx:%lx]\n",
-			       resource, dev->name, res->start, res->end);
+			       resource, dev->dev.name, res->start, res->end);
 		}
 	} else {
 		printk(KERN_ERR "PCI: No parent found for region %d "
-		       "of device %s\n", resource, dev->name);
+		       "of device %s\n", resource, dev->dev.name);
 	}
 
 	return err;

@@ -155,7 +155,7 @@ pdev_sort_resources(struct pci_dev *dev,
 		if (!r_align) {
 			printk(KERN_WARNING "PCI: Ignore bogus resource %d "
 					    "[%lx:%lx] of %s\n",
-					    i, r->start, r->end, dev->name);
+					    i, r->start, r->end, dev->dev.name);
 			continue;
 		}
 		r_align = (i < PCI_BRIDGE_RESOURCES) ? r_align + 1 : r->start;

drivers/pcmcia/cardbus.c +0 -1

diff -urpNX dontdiff linux-2.5.47/drivers/pcmcia/cardbus.c linux-2.5.47-pci/drivers/pcmcia/cardbus.c
@@ -284,7 +284,6 @@ int cb_alloc(socket_info_t * s)
 
 		pci_setup_device(dev);
 
-		strcpy(dev->dev.name, dev->name);
 		strcpy(dev->dev.bus_id, dev->slot_name);
 
 		/* FIXME: Do we need to enable the expansion ROM? */

drivers/pcmcia/yenta.c +1 -1

diff -urpNX dontdiff linux-2.5.47/drivers/pcmcia/yenta.c linux-2.5.47-pci/drivers/pcmcia/yenta.c
@@ -585,7 +585,7 @@ static void yenta_open_bh(void * data)
 	/* It's OK to overwrite this now */
 	INIT_WORK(&socket->tq_task, yenta_bh, socket);
 
-	if (!socket->cb_irq || request_irq(socket->cb_irq, yenta_interrupt, SA_SHIRQ, socket->dev->name, socket)) {
+	if (!socket->cb_irq || request_irq(socket->cb_irq, yenta_interrupt, SA_SHIRQ, socket->dev->dev.name, socket)) {
 		/* No IRQ or request_irq failed. Poll */
 		socket->cb_irq = 0; /* But zero is a valid IRQ number. */
 		socket->poll_timer.function = yenta_interrupt_wrapper;

drivers/usb/core/hcd-pci.c +2 -2

diff -urpNX dontdiff linux-2.5.47/drivers/usb/core/hcd-pci.c linux-2.5.47-pci/drivers/usb/core/hcd-pci.c
@@ -137,7 +137,7 @@ clean_2:
 	hcd->description = driver->description;
 	hcd->pdev = dev;
 	hcd->self.bus_name = dev->slot_name;
-	hcd->product_desc = dev->name;
+	hcd->product_desc = dev->dev.name;
 
 	if ((retval = hcd_buffer_create (hcd)) != 0) {
 clean_3:

@@ -145,7 +145,7 @@ clean_3:
 		goto clean_2;
 	}
 
-	info ("%s @ %s, %s", hcd->description,  dev->slot_name, dev->name);
+	info ("%s @ %s, %s", hcd->description,  dev->slot_name, dev->dev.name);
 
 #ifndef __sparc__
 	sprintf (buf, "%d", dev->irq);

include/linux/device.h +6 -3

diff -urpNX dontdiff linux-2.5.47/include/linux/device.h linux-2.5.47-pci/include/linux/device.h
@@ -23,14 +23,17 @@
 #ifndef _DEVICE_H_
 #define _DEVICE_H_
 
-#include <linux/types.h>
 #include <linux/config.h>
 #include <linux/ioport.h>
+#include <linux/kobject.h>
 #include <linux/list.h>
 #include <linux/sched.h>
-#include <linux/kobject.h>
+#include <linux/spinlock.h>
+#include <linux/types.h>
+#include <asm/atomic.h>
 
-#define DEVICE_NAME_SIZE	80
+#define DEVICE_NAME_SIZE	50
+#define DEVICE_NAME_HALF	__stringify(20)	/* Less than half to accommodate slop */
 #define DEVICE_ID_SIZE		32
 #define BUS_ID_SIZE		16

include/linux/pci.h +0 -1

diff -urpNX dontdiff linux-2.5.47/include/linux/pci.h linux-2.5.47-pci/include/linux/pci.h
@@ -371,7 +371,6 @@ struct pci_dev {
 	struct resource dma_resource[DEVICE_COUNT_DMA];
 	struct resource irq_resource[DEVICE_COUNT_IRQ];
 
-	char		name[90];	/* device name */
 	char		slot_name[8];	/* slot name */
 	int		active;		/* ISAPnP: device is active */
 	int		ro;		/* ISAPnP: read only */

Signature · 1 line

Revolutions do not require corporate support.

Jeff Garzik

2002-11-14 19:03 UTC #8 lore raw

In reply to Jeff Garzik

Quoted 12 lines · Matthew Wilcox

Quoted 8 lines · Linus Torvalds
Actually, I think we should do the reverse (for testing), and make the name be something small like 8 bytes, and make sure that everybody who writes the name uses strncpy() and snprintf() instead of just blindly writing whatever is in the database.
Otherwise we'll always end up having fragile magic constants.
Anybody willing to do that cleanup?

Quoted 6 lines

Sure, I can do that. That leads me to think that maybe we should delete name from struct device and just use the one in struct kobject (which is already a mere 16 bytes). But if we're going to go as far down as the kobject... that has a dentry. And dentrys have names. So how about eliminating that too and just creating a dentry with the almost infinitely long name?

Remember that the names we are talking about here is the English descriptive name of the PCI device... there is only _one_ need for these names: /proc/pci

If there is going to be this much thought put into it (what a waste of brain cycles <g>), then let's just remove CONFIG_PCI_NAMES and the name field completely. lspci prints them out from its own database anyway, and the only _real_ use is to provide descriptive names for devices in /proc/pci. Why bother? For existing code, just use pdev->slot_name instead, which is what happens anyway for hotplugged devices that appear after we drop the PCI name database during the __init phase.

Comments?

Jeff