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.ga11...@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);
>       }
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Reply via email to