On Fri, 16 Mar 2012, Ming Lei wrote:
> This patch fixes one lockdep warning[1] inside usb_stor_pre_reset.
>
> If the current configuration includes multiple mass storage interfaces,
> the 'AA' lockdep warning will be triggered since the lock class of
> 'us->dev_mutex' is acquired two times in .reset path. It isn't
> a real deadlock, so just take the lockdep_set_class annotation to
> remove the warning.
> --- a/drivers/usb/storage/usb.c
> +++ b/drivers/usb/storage/usb.c
> @@ -132,6 +132,35 @@ static struct us_unusual_dev for_dynamic_ids =
> #undef COMPLIANT_DEV
> #undef USUAL_DEV
>
> +#ifdef CONFIG_LOCKDEP
> +
> +static struct lock_class_key us_interface_key[USB_MAXINTERFACES];
> +
> +static void us_set_lock_class(struct mutex *mutex,
> + struct usb_interface *intf)
> +{
> + struct usb_device *udev = interface_to_usbdev(intf);
> + struct usb_host_config *config = udev->actconfig;
> + int i;
> +
> + if (config)
This test isn't needed. The code will never run if udev->actconfig is
NULL.
> + for (i = 0; i < config->desc.bNumInterfaces; i++)
> + if (config->interface[i] == intf) {
> + lockdep_set_class(mutex,
> + &us_interface_key[i]);
> + return;
> + }
> + dev_err(&intf->dev, "something weird!");
Ugh. "something weird!" is not a useful message; it doesn't tell the
reader anything about what happened. You can simply ignore this
case; it should never occur.
You can move the lockdep_set_class call after the loop and replace
the "return" with "break". Then if you want to be really safe, define
the length of us_interface_key[] to be USB_MAXINTERFACES + 1. But I
don't think that's necessary.
And also, please fix the braces as Peter requested.
Alan Stern
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
