On Tue, 15 Jan 2008, Oliver Neukum wrote:
> > > --- 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?
>
> I found no way to deal with them without pushing the autosuspend code
> into the generic code.
I thought up something. It's a bit hackish, so people probably won't
like it... You could define a new SCSI_IOCTL_AUTORESUME code, for
internal use only, and pass it down to the driver from within the block
layer.
> > > + /* 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.
>
> How is this racy? We hold BKL in ioctl().
We do? Even for block devices? (In any case, I just don't like the
BKL.)
> > > --- 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?
>
> To be added once I figure out how to handle cd drives used to play audio.
> Adding a timer for CDs' maximum play time seems a bit gross.
It may not matter. sr.c doesn't have suspend and resume methods, so
all an autosuspend could accomplish would be to suspend the transport.
Unless the CD player is bus-powered, which seems unlikely, no harm
would be done -- the drive should keep on playing even when the USB
interface (for example) is suspended. But I admit I haven't actually
tried it...
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
