On Mon, Dec 19 2005, Linus Torvalds wrote:
>
>
> On Mon, 19 Dec 2005, Ben Collins wrote:
> >
> > > > case CDROMEJECT:
> > > > - rq = blk_get_request(q, WRITE, __GFP_WAIT);
> > > > - rq->flags |= REQ_BLOCK_PC;
> > > > - rq->data = NULL;
> > > > - rq->data_len = 0;
> > > > - rq->timeout = BLK_DEFAULT_TIMEOUT;
> > > > - memset(rq->cmd, 0, sizeof(rq->cmd));
> > > > - rq->cmd[0] = GPCMD_START_STOP_UNIT;
> > > > - rq->cmd[4] = 0x02 + (close != 0);
> > > > - rq->cmd_len = 6;
> > > > - err = blk_execute_rq(q, bd_disk, rq, 0);
> > > > - blk_put_request(rq);
> > > > + err = 0;
> > > > +
> > > > + err |= blk_send_allow_medium_removal(q, bd_disk);
> > > > + err |= blk_send_start_stop(q, bd_disk, 0x01);
> > > > + err |= blk_send_start_stop(q, bd_disk, 0x02);
> > >
> > > Do this in the eject tool, if it's required for some devices.
> >
> > It already is in eject tool, but as described, that requires root
> > access. Not something I want to force a user to do in order to eject
> > their CDROM/iPod/USBStick in gnome. What exactly is wrong with the
> > commands? If they are harmless for devices that don't need it, and fix a
> > huge number of problems (did you see the Cc list on the bug report?) for
> > users with affected devices, then what's the harm?
>
> I do agree that the suggested patch seems to be a real cleanup, regardless
> of whether the original code bug has now been fixed or not.
Apparently two seperate issues.
>
> Are there devices that really want the old sequence?
>
> Also, do we really need to send fist a start_stop 1 and then a 2?
The 0x01 looks really suspicious to me, it should just cause extra wait
and activity on most devices.
> Wouldn't the _logical_ thing be to replace the old code with just a
> cleaned-up-version of what the old code did, ie just do
>
> err = blk_send_start_stop(q, bd_disk, 0x02);
>
> for the eject case? That way we could do the patch as a pure cleanup, and
> then a separate patch might change the singe "start_stop 2" with the more
> complex sequence.
That would work.
--
Jens Axboe
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[Index of Archives]
[Kernel Newbies]
[Netfilter]
[Bugtraq]
[Photo]
[Stuff]
[Gimp]
[Yosemite News]
[MIPS Linux]
[ARM Linux]
[Linux Security]
[Linux RAID]
[Video 4 Linux]
[Linux for the blind]
[Linux Resources]
