Re: [patch ide-dev 8/9] make ide_task_ioctl() use REQ_DRIVE_TASKFILE
On Wed, 02 Mar 2005 01:08:56 -0500, Jeff Garzik [EMAIL PROTECTED] wrote: Bartlomiej Zolnierkiewicz wrote: Yes but it seems that you've assumed that ioctl == flagged taskfile and fs/internal == normal taskfile which is _not_ what I aim for. I want fully-flagged taskfile handling like flagged_taskfile() and hot path simpler taskfile handling like do_rw_taskfile() (at least for now - we can remove hot path later) where both can be used for fs/internal/ioctl requests (depending on the flags). There is no effective difference in performance between writeb() writeb() writeb() writeb() and if (bit 1) writeb() if (bit 2) writeb() if (bit 3) writeb() if (bit 4) writeb() The cost of a repeated bit test on the same unsigned long is _zero_. It's already in L1 cache. The I/Os are slow, and adding bit tests will certainly it is not _zero_ ;-) I agree that it is negligible compared to the cost of I/O not measurably decrease performance. (this is the reason why I do not object to using ioread32() and iowrite32()... it just adds a simple test) Plus, it is better to have a single path for all taskfiles, to ensure that the path is well-tested. libata's -tf_load() and -tf_read() hooks should be updated to use the more fine-grained flags that Tejun is proposing. Note that on SATA, this is largely irrelevant. The functions ata_tf_read() and ata_tf_load() should be updated for flagged taskfiles, because these will be used with PATA drivers. The hooks implemented in individual SATA drivers will not be updated. The reason is that SATA transmits an entire copy of the taskfile to/from the device all at once, in the form of a Frame Information Structure (FIS) -- essentially a SATA packet. agreed Tejun, one-path approach for IDE driver is fine with me - To unsubscribe from this list: send the line unsubscribe linux-ide in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [patch ide-dev 8/9] make ide_task_ioctl() use REQ_DRIVE_TASKFILE
Bartlomiej Zolnierkiewicz wrote: On Wed, 02 Mar 2005 01:08:56 -0500, Jeff Garzik [EMAIL PROTECTED] wrote: Bartlomiej Zolnierkiewicz wrote: Yes but it seems that you've assumed that ioctl == flagged taskfile and fs/internal == normal taskfile which is _not_ what I aim for. I want fully-flagged taskfile handling like flagged_taskfile() and hot path simpler taskfile handling like do_rw_taskfile() (at least for now - we can remove hot path later) where both can be used for fs/internal/ioctl requests (depending on the flags). There is no effective difference in performance between writeb() writeb() writeb() writeb() and if (bit 1) writeb() if (bit 2) writeb() if (bit 3) writeb() if (bit 4) writeb() The cost of a repeated bit test on the same unsigned long is _zero_. It's already in L1 cache. The I/Os are slow, and adding bit tests will certainly it is not _zero_ ;-) I agree that it is negligible compared to the cost of I/O True :) Jeff - To unsubscribe from this list: send the line unsubscribe linux-ide in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [patch ide-dev 8/9] make ide_task_ioctl() use REQ_DRIVE_TASKFILE
On Sunday 27 February 2005 08:36, Tejun Heo wrote:
Hello, Bartlomiej.
This patch should be modified to use flagged taskfile if the
task_end_request_fix patch isn't applied. As non-flagged taskfile
won't return valid result registers, TASK ioctl users won't get the
correct register output.
Nope, it works just fine because REQ_DRIVE_TASK used only
no-data protocol, please check task_no_data_intr().
IMHO, this flag-to-get-result-registers thing is way too subtle. How
about keeping old behavior by just not copying out register outputs in
ide_taskfile_ioctl() in applicable cases instead of not reading
registers when ending commands? That is, unless there's some
noticeable performance impacts I'm not aware of.
This would miss whole point of not _reading_ these registers (IO is slow).
IMHO new flags denoting {in,out} registers should be added (to linux/ata.h
to share them with libata) so new code can be sane and old flags would map
on new flags when needed.
-
To unsubscribe from this list: send the line unsubscribe linux-ide in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [patch ide-dev 8/9] make ide_task_ioctl() use REQ_DRIVE_TASKFILE
Hi,
Bartlomiej Zolnierkiewicz wrote:
On Sunday 27 February 2005 08:36, Tejun Heo wrote:
Hello, Bartlomiej.
This patch should be modified to use flagged taskfile if the
task_end_request_fix patch isn't applied. As non-flagged taskfile
won't return valid result registers, TASK ioctl users won't get the
correct register output.
Nope, it works just fine because REQ_DRIVE_TASK used only
no-data protocol, please check task_no_data_intr().
Sorry, I missed that. IDE really has a lot of ways to finish a
command, doesn't it? hdio.txt is gonna look ugly. :-)
IMHO, this flag-to-get-result-registers thing is way too subtle. How
about keeping old behavior by just not copying out register outputs in
ide_taskfile_ioctl() in applicable cases instead of not reading
registers when ending commands? That is, unless there's some
noticeable performance impacts I'm not aware of.
This would miss whole point of not _reading_ these registers (IO is slow).
IMHO new flags denoting {in,out} registers should be added (to linux/ata.h
to share them with libata) so new code can be sane and old flags would map
on new flags when needed.
Please do it.
Or, let me know what you have in mind (added fields, flag names,
etc...); then, I'll do it. I think we also need to hear Jeff's opinion
as things need to be added to ata.h.
Thanks.
--
tejun
-
To unsubscribe from this list: send the line unsubscribe linux-ide in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [patch ide-dev 8/9] make ide_task_ioctl() use REQ_DRIVE_TASKFILE
Hi,
On Monday 28 February 2005 16:24, Tejun Heo wrote:
Hi,
Bartlomiej Zolnierkiewicz wrote:
On Sunday 27 February 2005 08:36, Tejun Heo wrote:
Hello, Bartlomiej.
This patch should be modified to use flagged taskfile if the
task_end_request_fix patch isn't applied. As non-flagged taskfile
won't return valid result registers, TASK ioctl users won't get the
correct register output.
Nope, it works just fine because REQ_DRIVE_TASK used only
no-data protocol, please check task_no_data_intr().
Sorry, I missed that. IDE really has a lot of ways to finish a
command, doesn't it? hdio.txt is gonna look ugly. :-)
Yep but it was a lot more fun when there were three PIO codepaths. ;-)
hdio.txt doesn't need to know about driver internals so no problem here.
IMHO, this flag-to-get-result-registers thing is way too subtle. How
about keeping old behavior by just not copying out register outputs in
ide_taskfile_ioctl() in applicable cases instead of not reading
registers when ending commands? That is, unless there's some
noticeable performance impacts I'm not aware of.
This would miss whole point of not _reading_ these registers (IO is slow).
IMHO new flags denoting {in,out} registers should be added (to linux/ata.h
to share them with libata) so new code can be sane and old flags would map
on new flags when needed.
Please do it.
Or, let me know what you have in mind (added fields, flag names,
etc...); then, I'll do it. I think we also need to hear Jeff's opinion
as things need to be added to ata.h.
I was thinking about:
* adding ATA_TFLAG_{IN,OUT}_xxx flags (there is enough free
place for all flags in -flags field of struct ata_taskfile)
* teaching flagged_taskfile() about these flags and mapping -tf_out_flags
onto ATA_TFLAG_OUT_xxx (simple mapping no need to move -tf_out_flags
to ide_taskfile_ioctl() etc. - no risk of breaking something)
* moving flagged taskfile writing to ide_tf_load_discrete() helper
* adding ide_tf_read_discrete() helper for use by ide_end_drive_cmd()
and mapping -tf_in_flags onto ATA_TFLAG_IN_xxx (ditto)
If you like this plan feel free to implement it.
I'm also open for better ideas, comments etc.
Bartlomiej
-
To unsubscribe from this list: send the line unsubscribe linux-ide in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
