lkml.org 
[lkml]   [2009]   [Mar]   [9]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [PATCH v10 1/7] PCI: initialize and release SR-IOV capability
    On Sat, Mar 07, 2009 at 10:38:45AM +0800, Greg KH wrote:
    > On Fri, Mar 06, 2009 at 01:08:10PM -0700, Matthew Wilcox wrote:
    > > On Fri, Feb 20, 2009 at 02:54:42PM +0800, Yu Zhao wrote:
    > > > + list_for_each_entry(pdev, &dev->bus->devices, bus_list)
    > > > + if (pdev->sriov)
    > > > + break;
    > > > + if (list_empty(&dev->bus->devices) || !pdev->sriov)
    > > > + pdev = NULL;
    > > > + ctrl = 0;
    > > > + if (!pdev && pci_ari_enabled(dev->bus))
    > > > + ctrl |= PCI_SRIOV_CTRL_ARI;
    > > > +
    > >
    > > I don't like this loop. At the end of a list_for_each_entry() loop,
    > > pdev will not be pointing at a pci_device, it'll be pointing to some
    > > offset from &dev->bus->devices. So checking pdev->sriov at this point
    > > is really, really bad. I would prefer to see something like this:
    > >
    > > ctrl = 0;
    > > list_for_each_entry(pdev, &dev->bus->devices, bus_list) {
    > > if (pdev->sriov)
    > > goto ari_enabled;
    > > }
    > >
    > > if (pci_ari_enabled(dev->bus))
    > > ctrl = PCI_SRIOV_CTRL_ARI;
    > > ari_enabled:
    > > pci_write_config_word(dev, pos + PCI_SRIOV_CTRL, ctrl);
    >
    > No, please use bus_for_each_dev() instead, or bus_find_device(), don't
    > walk the bus list by hand. I'm kind of surprised that even builds. Hm,
    > in looking at the 2.6.29-rc kernels, I notice it will not even build at
    > all, you are now forced to use those functions, which is good.

    The devices haven't been added at this time, so we can't use
    bus_for_each_dev(). I guess that's why the `bus->devices' exists, and
    actually pci_bus_add_devices() walks the bus list same way to retrieve
    the devices and add them.

    Thanks,
    Yu


    \
     
     \ /
      Last update: 2009-03-10 02:21    [W:0.020 / U:1.540 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site