Re: What should be done with wrong warning "please use bus_type methods." on sd, sr, st and osst?

["Éric Piel" <Eric.Piel@tremplin-utc.net>]

24/03/08 19:16, James Bottomley wrote/a écrit:
> On Mon, 2008-03-24 at 10:59 -0700, Greg KH wrote:
>> On Mon, Mar 24, 2008 at 10:24:07AM -0500, James Bottomley wrote:
>>> A solution would be to duplicate the power management methods in the
>>> scsi_driver structure, but this is a complete waste of space since the
>>> generic driver ones aren't going away (at least according to Kay and
>>> Greg).  I still think the best thing to do is just to turn off this
>>> spurious warning.
>> Do you have a patch that can detect the usage that you currently have so
>> that I can change the warning message to not trigger if things are set
>> up that way instead?
> 
> Well, my suggested fix would be the attached one since you and Kay seem
> to be telling me that converting to bus_type X methods still leaves us
> free to reuse the driver X methods.  If you're planning on deprecating
> the driver X methods, then sure, it makes sense for me to duplicate them
> in the scsi driver.

I guess the problem with removing the warning is that in some other
cases it could really be useful (searching on the web seems to show a
couple of true positives).  I think Greg was more suggesting like adding
a flag ".i_know_what_i_am_doing" somewhere and putting it to 1 to
disable the warning.

Anyway, if the driver X methods are meaning something else, it makes
sense to duplicate them specifically in the scsi driver structure. We are
basically talking about 8 bytes per scsi device, which can be considered
a fair trade-off if it allows to detect bugs in other places of the
kernel. Following is an example of patch.

Eric
PS: Probably I'm an idiot, for the patch I didn't understand how to
move ".remove" to scsi_driver, so I moved it to scsi_device... anyway it's
just an example in order to be sure that everyone is talking about the
same thing.

---
diff --git a/drivers/scsi/scsi_sysfs.c b/drivers/scsi/scsi_sysfs.c
index b9b09a7..7d29099 100644
--- a/drivers/scsi/scsi_sysfs.c
+++ b/drivers/scsi/scsi_sysfs.c
@@ -384,8 +384,8 @@ static int scsi_bus_remove(struct device *dev)
 	 * driver may have altered it and it's being removed */
 	blk_queue_prep_rq(sdev->request_queue, scsi_prep_fn);
 
-	if (drv && drv->remove)
-		err = drv->remove(dev);
+	if (sdev->remove)
+		err = sdev->remove(dev);
 
 	return 0;
 }
diff --git a/drivers/scsi/sr.c b/drivers/scsi/sr.c
index 7ee86d4..da6adfd 100644
--- a/drivers/scsi/sr.c
+++ b/drivers/scsi/sr.c
@@ -83,7 +83,6 @@ static struct scsi_driver sr_template = {
 	.gendrv = {
 		.name   	= "sr",
 		.probe		= sr_probe,
-		.remove		= sr_remove,
 	},
 	.done			= sr_done,
 };
@@ -635,6 +634,7 @@ static int sr_probe(struct device *dev)
 	sprintf(cd->cdi.name, "sr%d", minor);
 
 	sdev->sector_size = 2048;	/* A guess, just in case */
+	sdev->remove = sr_remove;
 
 	/* FIXME: need to handle a get_capabilities failure properly ?? */
 	get_capabilities(cd);
diff --git a/include/scsi/scsi_device.h b/include/scsi/scsi_device.h
index ab7acbe..0809a0b 100644
--- a/include/scsi/scsi_device.h
+++ b/include/scsi/scsi_device.h
@@ -158,6 +158,7 @@ struct scsi_device {
 
 	struct device		sdev_gendev;
 	struct class_device	sdev_classdev;
+	int (*remove)		(struct device *);
 
 	struct execute_work	ew; /* used to get process context on put */
 

--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html