multiple interface test case

[Roman Kagan]

On Tue, Apr 12, 2005 at 11:08:27PM +0400, Roman Kagan wrote:
> On Tue, Apr 12, 2005 at 10:08:56PM +0400, Roman Kagan wrote:
> > in drivers/base/bus.c:
> > 
> > void device_release_driver(struct device * dev)
> > {
> >         struct device_driver * drv = dev->driver;
> >         if (drv) {
> >                 sysfs_remove_link(&drv->kobj, kobject_name(&dev->kobj));
> >                 sysfs_remove_link(&dev->kobj, "driver");
> >                 list_del_init(&dev->driver_list);
> >                 device_detach_shutdown(dev);
> >                 if (drv->remove)
> >                         drv->remove(dev);
> >                 dev->driver = NULL;
> >         }
> > }
> > 
> > static void driver_detach(struct device_driver * drv)
> > {
> >         struct list_head * entry, * next;
> >         list_for_each_safe(entry, next, &drv->devices) {
> >                 struct device * dev = container_of(entry, struct device, driver_list);
> >                 device_release_driver(dev);
> >         }
> > }
> > [...]
> > yet.  Can't it be that list_for_each_safe() is not that safe against
> > removal of the entries inside the loop body?
> 
> Hmm, looks fishy to me:
> 
> #define list_for_each_safe(pos, n, head) \
>         for (pos = (head)->next, n = pos->next; pos != (head); \
>                 pos = n, n = pos->next)
> 
> So if entries are deleted inside the loop body (with list_del_init() in
> our case), the loop is screwed up?

To make my point clear: list_for_each_safe() seems to be safe only
w.r.t. the deletion of the current entry.  However, in our
multi-interface scenario, we start with device_release_driver() for the
first interface on the drv->devices list, and then (through
drv->remove(), which calls disconnect(), which calls
usb_driver_release_interface() for other interfaces) we call
device_release_driver() for other interfaces and empty the drv->devices
list, while list_for_each_safe() has already cached the (now invalid)
next entry, and thus goes into an infinite loop.

Does it look reasonable?  I'll try to look how it maps to -mm klist
stuff tomorrow, now I'm falling asleep, sorry.

Roman.