Messages in this thread Patch in this message | | | Date | Thu, 10 Sep 2015 12:23:22 +0200 | From | Peter Zijlstra <> | Subject | Re: [PATCH v3] perf, x86: Fix multi-segment problem of perf_event_intel_uncore |
| |
On Fri, Sep 04, 2015 at 02:28:36AM +0900, Taku Izumi wrote:
> +struct pci2phy_map *__find_pci2phy_map(int segment) > +{ > + struct pci2phy_map *map; > + > + lockdep_assert_held(&pci2phy_map_lock); > + > + list_for_each_entry(map, &pci2phy_map_head, list) { > + if (map->segment == segment) > + return map; > + } > + > + map = kmalloc(sizeof(struct pci2phy_map), GFP_ATOMIC);
So Ingo would really rather you didn't add a GFP_ATOMIC here; they are rather fragile.
http://lkml.kernel.org/r/20150901113147.GA11161@gmail.com
I figured you'd come up with something... :/
> + if (map) { > + map->segment = segment; > + list_add_tail(&map->list, &pci2phy_map_head); > + } > + > + return map; > +}
Something like this might work; but please consider it carefully, because I certainly didn't.
struct pci2phy_map *__find_pci2phy_map(int segment) { struct pci2phy_map *map, *alloc = NULL;
lockdep_assert_held(&pci2phy_map_lock);
retry: list_for_each_entry(map, &pci2phy_map_head, list) { if (map->segment == segment) goto done; }
if (!alloc) { raw_spin_unlock(&pci2phy_map_lock); alloc = kmalloc(sizeof(struct pci2phy_map), GFP_KERNEL); raw_spin_lock(&pci2phy_map_lock);
if (alloc) goto retry;
return NULL; } else { map = alloc; alloc = NULL; }
map->segment = segment; list_add_tail(&map->list, &pci2phy_map_head);
done: if (alloc) kfree(alloc);
return map; }
> @@ -1106,16 +1107,26 @@ static int snbep_pci2phy_map_init(int devid) > err = pci_read_config_dword(ubox_dev, 0x54, &config); > if (err) > break; > + > + segment = pci_domain_nr(ubox_dev->bus); > + raw_spin_lock(&pci2phy_map_lock); > + map = __find_pci2phy_map(segment); > + if (!map) { > + err = -ENOMEM; > + break;
And here you leak the lock..
> + } > + > /* > * every three bits in the Node ID mapping register maps > * to a particular node. > */ > for (i = 0; i < 8; i++) { > if (nodeid == ((config >> (3 * i)) & 0x7)) { > - uncore_pcibus_to_physid[bus] = i; > + map->pbus_to_physid[bus] = i; > break; > } > } > + raw_spin_unlock(&pci2phy_map_lock); > }
| |