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 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 04/22] ahci: hardreset classification fix Tejun Heo
2006-05-11 11:59 ` [PATCH 06/22] libata: kill duplicate prototypes Tejun Heo
2006-05-11 11:59 ` [PATCH 05/22] libata: unexport ata_scsi_error() 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 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 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 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 17/22] libata: use new SCR and on/offline functions Tejun Heo
2006-05-11 11:59 ` [PATCH 16/22] libata: implement new SCR handling and port " 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 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 20/22] libata: use dev->ap Tejun Heo
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 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 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 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 18/22] libata: kill old SCR functions and sata_dev_present() 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 19/22] libata: add dev->ap Tejun Heo
2006-05-13 21:47 ` Jeff Garzik
2006-05-11 11:59 ` [PATCH 22/22] libata: use ATA printk helpers 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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.