linux-kernel - Re: [PATCH 1/7] PCI: Make sriov work with hotplug remove

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Mon, 23 Jan 2012 10:45:35 -0800
From:	Linus Torvalds <torvalds@...ux-foundation.org>
To:	Yinghai Lu <yinghai@...nel.org>
Cc:	Jesse Barnes <jbarnes@...tuousgeek.org>,
	Kenji Kaneshige <kaneshige.kenji@...fujitsu.com>,
	Matthew Wilcox <matthew@....cx>, linux-pci@...r.kernel.org,
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH 1/7] PCI: Make sriov work with hotplug remove

On Mon, Jan 23, 2012 at 10:30 AM, Yinghai Lu <yinghai@...nel.org> wrote:
> in list_for_each_safe()
>
> #define list_for_each_safe(pos, n, head) \
>        for (pos = (head)->next, n = pos->next; pos != (head); \
>                pos = n, n = pos->next)
>
> n is saved before, and safe only mean pos could be freed from the
> list, but n still can be used for next loop.
>
> in our case, the list have PF and several VFs, when
> pci_stop_bus_device() is called for PF, pos are still valid, but
> VFs are removed from the list. so n will not be valid.

That still doesn't explain anything.

The whole point of the safe version is that if the entry that is being
worked on is removed, then we can still use the next one.

Why isn't this magically true in this case? If some *other* random
entry than the one that is being iterated over can magically be
removed, then the whole thing is just pure and utter crap, and no
amount of list maintenance can ever fix it.

So explain.

> https://lkml.org/lkml/2011/10/15/141
> or from attached one.

This still doesn't make sense. Why do that stupid allocation? Why not
just move the entry? Why doesn't just the sane approach work?

Exactly why does not the obvious

    /* stop bus device for phys_fn at first */
    list_for_each_safe(l, n, &bus->devices) {
        struct pci_dev *dev = pci_dev_b(l);
        if (!dev->is_virtfn)
            pci_stop_bus_device(dev);
    }

work? What the f*^& does that pci_stop_bus_device() function do to
that bus->devices list that isn't just a "list_del()" of that single
entry? And if it does anything else, it should damn well stop doing
that.

The *exact* same loop is then apparently working for the virtual
device case, so what the hell is going on that it wouldn't work for
the physical one?

What's the confusion? Why all the crazy idiotic code that makes no sense?

                      Linus
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/