Re: [PATCH] scsi: fix lockdep warning

From: James Bottomley
Date: Sat Mar 20 2010 - 11:42:05 EST


On Sat, 2010-03-20 at 00:17 -0700, Yinghai Lu wrote:
> Index: linux-2.6/drivers/scsi/scsi_sysfs.c
> ===================================================================
> --- linux-2.6.orig/drivers/scsi/scsi_sysfs.c
> +++ linux-2.6/drivers/scsi/scsi_sysfs.c
> @@ -907,20 +907,27 @@ int scsi_sysfs_add_sdev(struct scsi_devi
>
> /* create queue files, which may be writable, depending on the
> host */
> if (sdev->host->hostt->change_queue_depth) {
> + sysfs_bin_attr_init(&sdev_attr_queue_depth_rw);
> error = device_create_file(&sdev->sdev_gendev,
> &sdev_attr_queue_depth_rw);
> + sysfs_bin_attr_init(&sdev_attr_queue_ramp_up_period);
> error = device_create_file(&sdev->sdev_gendev,
>
> &sdev_attr_queue_ramp_up_period);
> }
> - else
> + else {
> + sysfs_bin_attr_init(&dev_attr_queue_depth);
> error = device_create_file(&sdev->sdev_gendev,
> &dev_attr_queue_depth);
> + }
> if (error)
> return error;
>
> - if (sdev->host->hostt->change_queue_type)
> + if (sdev->host->hostt->change_queue_type) {
> + sysfs_bin_attr_init(&sdev_attr_queue_type_rw);
> error = device_create_file(&sdev->sdev_gendev,
> &sdev_attr_queue_type_rw);
> - else
> + } else {
> + sysfs_bin_attr_init(&dev_attr_queue_type);
> error = device_create_file(&sdev->sdev_gendev,
> &dev_attr_queue_type);
> + }
> if (error)
> return error;
>
> @@ -935,6 +942,7 @@ int scsi_sysfs_add_sdev(struct scsi_devi
> /* add additional host specific attributes */
> if (sdev->host->hostt->sdev_attrs) {
> for (i = 0; sdev->host->hostt->sdev_attrs[i]; i++) {
> + sysfs_bin_attr_init(sdev->host->hostt->sdev_attrs[i]);
> error = device_create_file(&sdev->sdev_gendev,
> sdev->host->hostt->sdev_attrs[i]);
> if (error)
> @@ -1060,6 +1068,7 @@ int scsi_sysfs_add_host(struct Scsi_Host
> /* add host specific attributes */
> if (shost->hostt->shost_attrs) {
> for (i = 0; shost->hostt->shost_attrs[i]; i++) {
> + sysfs_bin_attr_init(shost->hostt->shost_attrs[i]);
> error = device_create_file(&shost->shost_dev,
> shost->hostt->shost_attrs[i]);
> if (error)


These are all clearly wrong. The attribute in question is a device
attribute not a binary attribute. It only actually compiles because
whoever did sysfs_bin_attr_init() has no type checking on the define and
it just so happens that struct device_attr has a member named attr as
well.

Also, none of these are dynamic attributes, so they shouldn't actually
need initialisation even for lockdep, so what went wrong?

James


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