On Tue, 15 Jan 2008, Oliver Neukum wrote:
> Hi Alan,
>
> here's a simple implementation to handle ioctl() by blocking
> autosuspend until the device is closed again.
>
> It is relative to your patch set.
A few comments are below.
> --- linux-as/drivers/scsi/sd.c 2008-01-15 14:17:05.000000000 +0100
> +++ linux-2.6.24-scsi-pm/drivers/scsi/sd.c 2008-01-15 14:20:13.000000000
> +0100
> @@ -711,6 +718,19 @@ static int sd_ioctl(struct inode * inode
> case SCSI_IOCTL_GET_BUS_NUMBER:
> return scsi_ioctl(sdp, cmd, p);
> default:
Do all ioctls filter through this routine? It looks like requests
coming through block/scsi_ioctl.c will bypass this code. Have you
decided to ignore those requests for now?
> + /* closer filtering should go here */
> +#ifdef CONFIG_SCSI_DYNAMIC_PM
> + if (!sdp->autosuspend_ioctl_blocked) {
> + error = scsi_autoresume_device(sdp);
> + if (error < 0)
> + return error;
> + /* check for lost race due to drop of BKL */
> + if (sdp->autosuspend_ioctl_blocked)
> + scsi_autosuspend_device(sdp);
> + else
> + sdp->autosuspend_ioctl_blocked = 1;
This is still racy; you need a real synchronization mechanism. For
instance, you could use sdp->pm_mutex.
> + }
> +#endif
> error = scsi_cmd_ioctl(filp, disk->queue, disk, cmd, p);
> if (error != -ENOTTY)
> return error;
> --- linux-as/include/scsi/scsi_device.h 2008-01-15 14:17:05.000000000
> +0100
> +++ linux-2.6.24-scsi-pm/include/scsi/scsi_device.h 2008-01-14
> 12:45:12.000000000 +0100
> @@ -177,6 +177,7 @@ struct scsi_device {
> unsigned auto_pm:1; /* doing autosuspend or autoresume */
> unsigned autosuspend_disabled:1; /* autosuspend & autoresume */
> unsigned autoresume_disabled:1; /* disabled by the user */
> + unsigned autosuspend_ioctl_blocked:1; /* disabled due to ioctl use */
> unsigned skip_sys_resume:1; /* skip the next system resume */
> unsigned use_ULD_pm:1; /* call the Upper-Level Driver's
> * suspend/resume methods */
The new flag is present for all SCSI devices, but you added code to use
it only in the sd driver. What about the other upper-level SCSI
drivers?
Alan Stern
-
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
