From: Albert Lee <albertcc@tw.ibm.com>
To: Tejun Heo <htejun@gmail.com>
Cc: jgarzik@pobox.com, alan@lxorguk.ukuu.org.uk, axboe@suse.de,
forrest.zhao@intel.com, efalk@google.com,
linux-ide@vger.kernel.org
Subject: Re: [PATCH 13/22] libata: implement qc->result_tf
Date: Thu, 18 May 2006 15:22:12 +0800 [thread overview]
Message-ID: <446C20A4.7020204@tw.ibm.com> (raw)
In-Reply-To: <11473487911193-git-send-email-htejun@gmail.com>
Tejun Heo wrote:
> Add qc->result_tf and ATA_QCFLAG_RESULT_TF. This moves the
> responsibility of loading result TF from post-compltion path to qc
> execution path. qc->result_tf is loaded if explicitly requested or
> the qc failsa. This allows more efficient completion implementation
> and correct handling of result TF for controllers which don't have
> global TF representation such as sil3124/32.
>
> Signed-off-by: Tejun Heo <htejun@gmail.com>
>
> ---
>
> drivers/scsi/libata-core.c | 4 ++--
> drivers/scsi/libata-scsi.c | 18 +++++++-----------
> include/linux/libata.h | 16 ++++++++++++++--
> 3 files changed, 23 insertions(+), 15 deletions(-)
>
> 46a93ec2cb1124f507e0cad7dcb65794a15af844
> diff --git a/drivers/scsi/libata-core.c b/drivers/scsi/libata-core.c
> index 464fd46..4cb7828 100644
> --- a/drivers/scsi/libata-core.c
> +++ b/drivers/scsi/libata-core.c
> @@ -955,7 +955,6 @@ void ata_qc_complete_internal(struct ata
> {
> struct completion *waiting = qc->private_data;
>
> - qc->ap->ops->tf_read(qc->ap, &qc->tf);
> complete(waiting);
> }
>
> @@ -997,6 +996,7 @@ unsigned ata_exec_internal(struct ata_po
> qc->tf = *tf;
> if (cdb)
> memcpy(qc->cdb, cdb, ATAPI_CDB_LEN);
> + qc->flags |= ATA_QCFLAG_RESULT_TF;
> qc->dma_dir = dma_dir;
> if (dma_dir != DMA_NONE) {
> ata_sg_init_one(qc, buf, buflen);
> @@ -1034,7 +1034,7 @@ unsigned ata_exec_internal(struct ata_po
> /* finish up */
> spin_lock_irqsave(&ap->host_set->lock, flags);
>
> - *tf = qc->tf;
> + *tf = qc->result_tf;
> err_mask = qc->err_mask;
>
> ata_qc_free(qc);
> diff --git a/drivers/scsi/libata-scsi.c b/drivers/scsi/libata-scsi.c
> index b0c83c2..ce90b63 100644
> --- a/drivers/scsi/libata-scsi.c
> +++ b/drivers/scsi/libata-scsi.c
> @@ -537,7 +537,7 @@ void ata_to_sense_error(unsigned id, u8
> void ata_gen_ata_desc_sense(struct ata_queued_cmd *qc)
> {
> struct scsi_cmnd *cmd = qc->scsicmd;
> - struct ata_taskfile *tf = &qc->tf;
> + struct ata_taskfile *tf = &qc->result_tf;
> unsigned char *sb = cmd->sense_buffer;
> unsigned char *desc = sb + 8;
>
> @@ -608,7 +608,7 @@ void ata_gen_ata_desc_sense(struct ata_q
> void ata_gen_fixed_sense(struct ata_queued_cmd *qc)
> {
> struct scsi_cmnd *cmd = qc->scsicmd;
> - struct ata_taskfile *tf = &qc->tf;
> + struct ata_taskfile *tf = &qc->result_tf;
> unsigned char *sb = cmd->sense_buffer;
>
> memset(sb, 0, SCSI_SENSE_BUFFERSIZE);
> @@ -1199,14 +1199,11 @@ static void ata_scsi_qc_complete(struct
> */
> if (((cdb[0] == ATA_16) || (cdb[0] == ATA_12)) &&
> ((cdb[2] & 0x20) || need_sense)) {
> - qc->ap->ops->tf_read(qc->ap, &qc->tf);
> ata_gen_ata_desc_sense(qc);
> } else {
> if (!need_sense) {
> cmd->result = SAM_STAT_GOOD;
> } else {
> - qc->ap->ops->tf_read(qc->ap, &qc->tf);
> -
> /* TODO: decide which descriptor format to use
> * for 48b LBA devices and call that here
> * instead of the fixed desc, which is only
> @@ -1217,10 +1214,8 @@ static void ata_scsi_qc_complete(struct
> }
> }
>
> - if (need_sense) {
> - /* The ata_gen_..._sense routines fill in tf */
> - ata_dump_status(qc->ap->id, &qc->tf);
> - }
> + if (need_sense)
> + ata_dump_status(qc->ap->id, &qc->result_tf);
>
> qc->scsidone(cmd);
>
> @@ -2004,7 +1999,6 @@ static void atapi_sense_complete(struct
> * a sense descriptors, since that's only
> * correct for ATA, not ATAPI
> */
> - qc->ap->ops->tf_read(qc->ap, &qc->tf);
> ata_gen_ata_desc_sense(qc);
> }
>
> @@ -2080,7 +2074,6 @@ static void atapi_qc_complete(struct ata
> * a sense descriptors, since that's only
> * correct for ATA, not ATAPI
> */
> - qc->ap->ops->tf_read(qc->ap, &qc->tf);
> ata_gen_ata_desc_sense(qc);
> } else {
> u8 *scsicmd = cmd->cmnd;
> @@ -2361,6 +2354,9 @@ ata_scsi_pass_thru(struct ata_queued_cmd
> */
> qc->nsect = cmd->bufflen / ATA_SECT_SIZE;
>
> + /* request result TF */
> + qc->flags |= ATA_QCFLAG_RESULT_TF;
> +
> return 0;
>
> invalid_fld:
> diff --git a/include/linux/libata.h b/include/linux/libata.h
> index a117ca2..3d15c00 100644
> --- a/include/linux/libata.h
> +++ b/include/linux/libata.h
> @@ -162,7 +162,9 @@ enum {
> ATA_QCFLAG_SINGLE = (1 << 2), /* no s/g, just a single buffer */
> ATA_QCFLAG_DMAMAP = ATA_QCFLAG_SG | ATA_QCFLAG_SINGLE,
> ATA_QCFLAG_IO = (1 << 3), /* standard IO command */
> - ATA_QCFLAG_EH_SCHEDULED = (1 << 4), /* EH scheduled */
> + ATA_QCFLAG_RESULT_TF = (1 << 4), /* result TF requested */
> +
> + ATA_QCFLAG_EH_SCHEDULED = (1 << 16), /* EH scheduled */
>
> /* host set flags */
> ATA_HOST_SIMPLEX = (1 << 0), /* Host is simplex, one DMA channel per host_set only */
> @@ -343,7 +345,7 @@ struct ata_queued_cmd {
> struct scatterlist *__sg;
>
> unsigned int err_mask;
> -
> + struct ata_taskfile result_tf;
> ata_qc_cb_t complete_fn;
>
> void *private_data;
> @@ -824,6 +826,10 @@ static inline void ata_qc_reinit(struct
> qc->err_mask = 0;
>
> ata_tf_init(qc->ap, &qc->tf, qc->dev->devno);
> +
> + /* init result_tf such that it indicates normal completion */
> + qc->result_tf.command = ATA_DRDY;
> + qc->result_tf.feature = 0;
> }
>
> /**
> @@ -839,9 +845,15 @@ static inline void ata_qc_reinit(struct
> */
> static inline void ata_qc_complete(struct ata_queued_cmd *qc)
> {
> + struct ata_port *ap = qc->ap;
> +
> if (unlikely(qc->flags & ATA_QCFLAG_EH_SCHEDULED))
> return;
>
> + /* read result TF if failed or requested */
> + if (qc->err_mask || qc->flags & ATA_QCFLAG_RESULT_TF)
> + ap->ops->tf_read(ap, &qc->result_tf);
> +
> __ata_qc_complete(qc);
> }
>
This patch makes qc->tf well preserved, except we still have
qc->tf got overwritten by ata_pio_bytes().
Maybe we also need the attached follow-up patch?
--
albert
--- 02_atapi_pio_bytes/drivers/scsi/libata-core.c 2006-05-18 14:20:32.000000000 +0800
+++ 03_atapi_pio_bytes/drivers/scsi/libata-core.c 2006-05-18 14:25:41.000000000 +0800
@@ -3861,10 +3861,10 @@ static void atapi_pio_bytes(struct ata_q
unsigned int ireason, bc_lo, bc_hi, bytes;
int i_write, do_write = (qc->tf.flags & ATA_TFLAG_WRITE) ? 1 : 0;
- ap->ops->tf_read(ap, &qc->tf);
- ireason = qc->tf.nsect;
- bc_lo = qc->tf.lbam;
- bc_hi = qc->tf.lbah;
+ ap->ops->tf_read(ap, &qc->result_tf);
+ ireason = qc->result_tf.nsect;
+ bc_lo = qc->result_tf.lbam;
+ bc_hi = qc->result_tf.lbah;
bytes = (bc_hi << 8) | bc_lo;
/* shall be cleared to zero, indicating xfer of data */
next prev parent reply other threads:[~2006-05-18 7:22 UTC|newest]
Thread overview: 53+ messages / expand[flat|nested] mbox.gz Atom feed top
2006-05-11 11:59 [PATCHSET 01/11] prep for new EH Tejun Heo
2006-05-11 11:59 ` [PATCH 08/22] libata: clear ap->active_tag atomically w.r.t. command completion Tejun Heo
2006-05-11 11:59 ` [PATCH 03/22] libata: silly fix in ata_scsi_start_stop_xlat() Tejun Heo
2006-05-11 11:59 ` [PATCH 05/22] libata: unexport ata_scsi_error() Tejun Heo
2006-05-11 11:59 ` [PATCH 06/22] libata: kill duplicate prototypes Tejun Heo
2006-05-11 11:59 ` [PATCH 04/22] ahci: hardreset classification fix Tejun Heo
2006-05-11 11:59 ` [PATCH 02/22] SCSI: implement host_eh_scheduled hack for libata Tejun Heo
2006-05-13 21:34 ` Jeff Garzik
2006-05-11 11:59 ` [PATCH 09/22] libata: hold host_set lock while finishing internal qc Tejun Heo
2006-05-11 11:59 ` [PATCH 07/22] libata: fix ->phy_reset class code handling in ata_bus_probe() Tejun Heo
2006-05-11 11:59 ` [PATCH 01/22] SCSI: Introduce scsi_req_abort_cmd (REPOST) Tejun Heo
2006-05-13 21:21 ` Jeff Garzik
2006-05-14 2:00 ` Luben Tuikov
2006-05-14 2:01 ` Luben Tuikov
2006-05-14 2:04 ` Jeff Garzik
2006-05-14 2:08 ` Luben Tuikov
2006-05-14 2:12 ` Jeff Garzik
2006-05-11 11:59 ` [PATCH 10/22] libata: use preallocated buffers Tejun Heo
2006-05-17 5:34 ` Albert Lee
2006-05-17 12:47 ` Jeff Garzik
2006-05-18 2:52 ` Albert Lee
2006-05-11 11:59 ` [PATCH 19/22] libata: add dev->ap Tejun Heo
2006-05-13 21:47 ` Jeff Garzik
2006-05-11 11:59 ` [PATCH 11/22] libata: move ->set_mode() handling into ata_set_mode() Tejun Heo
2006-05-11 11:59 ` [PATCH 16/22] libata: implement new SCR handling and port on/offline functions Tejun Heo
2006-05-13 21:43 ` Jeff Garzik
2006-05-13 23:18 ` Tejun Heo
2006-05-14 1:10 ` Jeff Garzik
2006-05-14 1:29 ` Tejun Heo
2006-05-14 1:35 ` Jeff Garzik
2006-05-11 11:59 ` [PATCH 15/22] libata: init ap->cbl to ATA_CBL_SATA early Tejun Heo
2006-05-13 21:42 ` Jeff Garzik
2006-05-11 11:59 ` [PATCH 17/22] libata: use new SCR and on/offline functions Tejun Heo
2006-05-11 11:59 ` [PATCH 20/22] libata: use dev->ap Tejun Heo
2006-05-11 11:59 ` [PATCH 12/22] libata: remove postreset handling from ata_do_reset() Tejun Heo
2006-05-11 11:59 ` [PATCH 13/22] libata: implement qc->result_tf Tejun Heo
2006-05-18 7:10 ` Albert Lee
2006-05-18 7:22 ` Tejun Heo
2006-05-18 7:22 ` Albert Lee [this message]
2006-05-18 7:27 ` Tejun Heo
2006-05-18 7:53 ` Albert Lee
2006-05-18 8:10 ` Tejun Heo
2006-05-18 9:51 ` [PATCH 1/1] libata: use qc->result_tf for temp taskfile storage Albert Lee
2006-05-11 11:59 ` [PATCH 18/22] libata: kill old SCR functions and sata_dev_present() Tejun Heo
2006-05-11 11:59 ` [PATCH 14/22] sata_sil24: update TF image only when necessary Tejun Heo
2006-05-13 21:42 ` Jeff Garzik
2006-05-11 11:59 ` [PATCH 21/22] libata: implement ATA printk helpers Tejun Heo
2006-05-14 2:00 ` Jeff Garzik
2006-05-16 10:23 ` Albert Lee
2006-05-16 10:29 ` Tejun Heo
2006-05-11 11:59 ` [PATCH 22/22] libata: use " Tejun Heo
2006-05-13 21:49 ` Jeff Garzik
2006-05-13 21:52 ` [PATCHSET 01/11] prep for new EH Jeff Garzik
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=446C20A4.7020204@tw.ibm.com \
--to=albertcc@tw.ibm.com \
--cc=alan@lxorguk.ukuu.org.uk \
--cc=albertl@mail.com \
--cc=axboe@suse.de \
--cc=efalk@google.com \
--cc=forrest.zhao@intel.com \
--cc=htejun@gmail.com \
--cc=jgarzik@pobox.com \
--cc=linux-ide@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).