On Sun, 27 Feb 2005 15:49:22 +0900, Tejun Heo <[EMAIL PROTECTED]> wrote:
> On Thu, Feb 24, 2005 at 04:58:03PM +0100, Bartlomiej Zolnierkiewicz wrote:
> > On Thu, 10 Feb 2005 17:38:14 +0900 (KST), Tejun Heo <[EMAIL PROTECTED]>
> > wrote:
> > >
> > > 01_ide_task_end_request_fix.patch
> > >
> > > task_end_request() modified to always call ide_end_drive_cmd()
> > > for taskfile requests. Previously, ide_end_drive_cmd() was
> > > called only when task->tf_out_flags.all was set. Also,
> > > ide_dma_intr() is modified to use task_end_request().
> > >
> > > * fixes taskfile ioctl oops bug which was caused by referencing
> > > NULL rq->rq_disk of taskfile requests.
> >
> > I fixed it in slightly different way in ide-dev-2.6 - by calling
> > ide_end_request() instead of ->end_request().
>
> Taskfile DMA path is still broken. Also calling ide_end_request()
> will work there, but IMHO it's just cleaner to finish special commands
> inside ide_end_drive_cmd(). Currently,
I agree but please note that your patch makes *all* taskfile registers to
be exposed through HDIO_DRIVE_TASKFILE regardless of ->rf_in_flags
(and obviously later you can't revert this change).
> * Successful flagged taskfile -> ide_end_drive_cmd()
> * All other successful non-DMA special cmds -> ide_end_request()
> * Successful DMA taskfile -> segfault
Have you tested it? Why would it segfault?
> * All failed special cmds -> ide_end_drive_cmd()
>
> It just shouldn't be like this. :-(
Yep.
> >
> > > * enables TASKFILE ioctls to get valid register outputs on
> > > successful completion.
> >
> > This change makes *all* taskfile registers to be read on completion
> > of *any* command. Currently this is done only for flagged taskfiles
> > and commands using no-data protocol.
> >
> > With all your changes it will be also done for:
> > * HDIO_DRIVE_[TASKFILE,CMD] ioctls
> > * /proc/ide/hd?/{identify,smart_thresholds,smart_values}
> > but reading back all registers is not always needed.
>
> None is on a hot path or even near to one, but maybe I don't have
> enough experience with old hardware. Are there some old hardware
> which make the additional reads stand out?
I dunno, better be safe then sorry, after all we are working
on the *stable* kernel tree.
Converting HDIO_DRIVE_CMD and in-kernel users to
soon-to-be-implemented ;-) sane discrete interface shouldn't
require much effort.
> > It is already bad enough (and we can't fix it cause it is exported
> > to user-space through HDIO_DRIVE_TASKFILE), we shouldn't
> > make it worse.
>
> Yeah, the whole IDE ioctl interface seems disturbingly messy. :-(
>
> * Register output is available only if
> 1. The command fails.
> 2. The command is a flagged taskfile.
3. the command uses no-data protocol
> * taskfile->device_head is used regardless of outflags setting.
> * In flagged taskfile, taskfile->device_head can turn on the device
> bit, so we can issue commands to hdb with permissions to hda.
> * In TASK and TASKFILE, LBA commands can be issued to drives in CHS
> mode but the reverse isn't true. However, in TASKFILE, if the
> command isn't flagged, the lower nibble of device register is
> zeroed depending on addressing setting.
> * taskfile->data endianess is reversed on big endian machines.
> * ide_reg_valid_t endianess issue.
> * And, none of above is documented.
>
> So, I don't know. Do you think we should keep all of the above
> behaviors? Please let me know; then, I'll update ioctl/hdio.txt so
Now read again the list above and think about amount of time required
to *safely* fix all the issues mentioned vs introducing sane interface.
Please also note that once we expose something to user-space
applications may depend on the broken behavior so we can't fix all
issues anyway.
If somebody implements SG_IO ioctl and SCSI command pass-through
from libata for IDE driver (and add possibility for discrete taskfiles), we can
just deprecate HDIO_DRIVE_TASKFILE, forget about it and some time later
remove this FPOS.
> that people can at least know these gotchas.
Please do so, thanks.
-
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/
