On Mon, Jan 23, 2012 at 10:30 AM, Yinghai Lu <yinghai@xxxxxxxxxx> 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-pci" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html