Am Mittwoch, 14. November 2007 schrieb Alex Chiang: > Hi Eike, > > * Rolf Eike Beer <[email protected]>: > > Alex Chiang wrote: > > > --- a/drivers/pci/hotplug/fakephp.c > > > +++ b/drivers/pci/hotplug/fakephp.c > > > @@ -93,6 +93,7 @@ static int add_slot(struct pci_dev *dev) > > > struct dummy_slot *dslot; > > > struct hotplug_slot *slot; > > > int retval = -ENOMEM; > > > + static int count = 1; > > > > > > slot = kzalloc(sizeof(struct hotplug_slot), GFP_KERNEL); > > > if (!slot) > > > @@ -106,7 +107,8 @@ static int add_slot(struct pci_dev *dev) > > > slot->info->max_bus_speed = PCI_SPEED_UNKNOWN; > > > slot->info->cur_bus_speed = PCI_SPEED_UNKNOWN; > > > > > > - slot->name = &dev->dev.bus_id[0]; > > > + slot->name = kmalloc(8, GFP_KERNEL); > > > + sprintf(slot->name, "fake%d", count++); > > > dbg("slot->name = %s\n", slot->name); > > > > > > dslot = kmalloc(sizeof(struct dummy_slot), GFP_KERNEL); > > > > This is ugly. Please do it the way we already do e.g. for > > acpiphp: add a char[8] to "struct dummy_slot" and just > > reference that here. > > I took at look at the code in acpiphp you're talking about, and > I'm not sure why you think the above is "ugly". We're talking > about a runtime vs compile time storage for the name, and doing a > kmalloc/sprintf is a pretty standard idiom. > > BTW, I did incorporate both Linas' and Willy's comments, by > changing the kmalloc size to an explicit 32, and using snprintf > instead. > > In any case, for your particular comment, I think I'm going to > leave it alone for now, and let Greg weigh in with the final > recommendation. Because we have another allocation of very small size for every slot here. struct dummy_slot has a size of 4 pointers, that's 16 byte for 32 bit architectures. Putting 8 byte of additional storage here would save a complete allocation on 32 bit platforms. For 64 bit platforms the memory usage is the same but we do less allocations (i.e. less points to fail) and less memory fragmentation. Btw: your code lacks a check if kmalloc() returns NULL. Eike
Attachment:
signature.asc
Description: This is a digitally signed message part.
- Follow-Ups:
- References:
- [PATCH 0/5][RFC] Physical PCI slot objects
- From: Alex Chiang <[email protected]>
- Re: [Pcihpd-discuss] [PATCH 2/5] Construct one fakephp slot per pci slot
- From: Rolf Eike Beer <[email protected]>
- Re: [Pcihpd-discuss] [PATCH 2/5] Construct one fakephp slot per pci slot
- From: Alex Chiang <[email protected]>
- [PATCH 0/5][RFC] Physical PCI slot objects
- Prev by Date: Re: [BUG] New Kernel Bugs
- Next by Date: [PATCH] [ISDN] sc: Fix sndpkt to have the correct number of arguments
- Previous by thread: Re: [Pcihpd-discuss] [PATCH 2/5] Construct one fakephp slot per pci slot
- Next by thread: Re: [Pcihpd-discuss] [PATCH 2/5] Construct one fakephp slot per pci slot
- Index(es):
 |