Am Dienstag, 15. Januar 2008 17:19:02 schrieb Alan Stern:
> 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.
Argh. There's a limit to depth I'll sink. If you can't do better, please put
autoresume/autosuspend into the driver core.
> > > > + /* 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.)
No, you are right. I'll add locking.
> > > > --- 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...
I'll check whether I have a drive enclosure that has an audio exit.
Regards
Oliver
-
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
