libata: improve media error handling
PATCH 1/1 libata: improve media error handling
Hello, Jeff.
This patchset improves media error handling. The problem is that ATA devices transfer "indeterminate" amount of data on device error making handling media errors difficult for highlevel driver to handle. The current high level driver, sd, assumes that data upto the first failed block has been transferred successfully, so data corruption is possible.
This patchset makes libata-eh perform IO upto the first failed block and report the first failed block to highlevel driver iff the partial transfer was successful. Note that after this patchset LLDs using old EH don't report the first failed block to high level. It wasn't safe to begin with and no libata driver properly reported the first failed sector before improve-sense-data-generation patchset anyway.
With this patchset applied, libata media error handling is well integrated with sd.
This patchset is against
upstream(cfd15b0011498986ef14b6c53f5eaba89d2171f3)
+ improve-sense-data-generation patchset[1]
Thanks.
--
tejun
[1] http://thread.gmane.org/gmane.linux.ide/14035/focus=14035
PATCH 1/4 libata: implement ata_exec_internal_sg()
Sg'ify ata_exec_internal() and call it ata_exec_internal_sg(). Wrapper function around ata_exec_internal_sg() is implemented to provide ata_exec_internal() interface.
--- drivers/ata/libata-core.c | 49 ++++++++++++++++++++++++++++++++++++++------ drivers/ata/libata.h | 4 +++ 2 files changed, 46 insertions(+), 7 deletions(-)
drivers/ata/libata-core.c +42 -7
@@ -1042,13 +1042,13 @@ void ata_qc_complete_internal(struct ata }/** -* ata_exec_internal - execute libata internal command +* ata_exec_internal_sg - execute libata internal command * @dev: Device to which the command is sent * @tf: Taskfile registers for the command and the result * @cdb: CDB for packet command * @dma_dir: Data tranfer direction of the command -* @buf: Data buffer of the command -* @buflen: Length of data buffer +* @sg: sg list for the data buffer of the command +* @n_elem: Number of sg entries * * Executes libata internal command with timeout. @tf contains * command on entry and result on return. Timeout and error
@@ -1062,9 +1062,10 @@ void ata_qc_complete_internal(struct ata* RETURNS: * Zero on success, AC_ERR_* mask on failure */ -unsigned ata_exec_internal (struct ata_device *dev, -struct ata_taskfile *tf,const u8 *cdb, -int dma_dir,void *buf,unsigned int buflen) +unsigned ata_exec_internal_sg (struct ata_device *dev, +struct ata_taskfile *tf,const u8 *cdb, +int dma_dir,struct scatterlist *sg, +unsigned int n_elem) {struct ata_port *ap = dev->ap;u8 command = tf->command;
@@ -1120,7 +1121,12 @@ unsigned ata_exec_internal(struct ata_de qc->flags |=ATA_QCFLAG_RESULT_TF ; qc->dma_dir = dma_dir;if (dma_dir !=DMA_NONE ) { -ata_sg_init_one (qc, buf, buflen); +unsigned int i, buflen =0 ; + +for (i =0 ; i < n_elem; i++) + buflen += sg[i].length; + +ata_sg_init (qc, sg, n_elem); qc->nsect = buflen /ATA_SECT_SIZE ; }
@@ -1204,6 +1210,35 @@ unsigned ata_exec_internal(struct ata_de }/** +* ata_exec_internal_sg - execute libata internal command +* @dev: Device to which the command is sent +* @tf: Taskfile registers for the command and the result +* @cdb: CDB for packet command +* @dma_dir: Data tranfer direction of the command +* @buf: Data buffer of the command +* @buflen: Length of data buffer +* +* Wrapper around ata_exec_internal_sg() which takes simple +* buffer instead of sg list. +* +* LOCKING: +* None. Should be called with kernel context, might sleep. +* +* RETURNS: +* Zero on success, AC_ERR_* mask on failure +*/ +unsigned ata_exec_internal (struct ata_device *dev, +struct ata_taskfile *tf,const u8 *cdb, +int dma_dir,void *buf,unsigned int buflen) +{ +struct scatterlist sg; + +sg_init_one (&sg, buf, buflen); + +return ata_exec_internal_sg (dev, tf, cdb, dma_dir, &sg,1 ); +} + +/** * ata_do_simple_cmd - execute simple internal command * @dev: Device to which the command is sent * @cmd: Opcode to execute
drivers/ata/libata.h +4 -0
@@ -51,6 +51,10 @@ extern void ata_port_flush_task(struct aextern unsigned ata_exec_internal (struct ata_device *dev,struct ata_taskfile *tf,const u8 *cdb,int dma_dir,void *buf,unsigned int buflen); +extern unsigned ata_exec_internal_sg (struct ata_device *dev, +struct ata_taskfile *tf,const u8 *cdb, +int dma_dir,struct scatterlist *sg, +unsigned int n_elem);extern unsigned int ata_do_simple_cmd (struct ata_device *dev,u8 cmd);extern int ata_dev_read_id (struct ata_device *dev,unsigned int *p_class,int post_reset,u16 *id);
Signature · 1 line
1.4.3.3
Re: [PATCH 1/4] libata: implement ata_exec_internal_sg()
Quoted 5 lines · Tejun Heo
Sg'ify
ata_exec_internal()and call itata_exec_internal_sg(). Wrapper function aroundata_exec_internal_sg()is implemented to provideata_exec_internal()interface.Signed-off-by Tejun Heo <[email protected]>
ACK
PATCH 2/4 libata: separate out rw ATA taskfile building into ata_build_tf()
In reply to Tejun HeoSeparate out rw ATA taskfile building from ata_scsi_rw_xlat() into ata_build_tf(). This will be used to improve media error handling.
--- drivers/ata/libata-core.c | 133 +++++++++++++++++++++++++++++++++++++++++++-- drivers/ata/libata-scsi.c | 119 +++++------------------------------------ drivers/ata/libata.h | 4 +- 3 files changed, 144 insertions(+), 112 deletions(-)
drivers/ata/libata-core.c +128 -5
@@ -199,7 +199,8 @@ static const u8 ata_rw_cmds[] = {/** * ata_rwcmd_protocol - set taskfile r/w commands and protocol -* @qc: command to examine and configure +* @tf: command to examine and configure +* @dev: device tf belongs to * * Examine the device configuration and tf->flags to calculate * the proper read/write commands and protocol to use.
@@ -207,10 +208,8 @@ static const u8 ata_rw_cmds[] = {* LOCKING: * caller. */ -int ata_rwcmd_protocol (struct ata_queued_cmd *qc) +static int ata_rwcmd_protocol (struct ata_taskfile *tf,struct ata_device *dev) { -struct ata_taskfile *tf = &qc->tf; -struct ata_device *dev = qc->dev;u8 cmd;int index, fua, lba48, write;
@@ -222,7 +221,7 @@ int ata_rwcmd_protocol(struct ata_queuedif (dev->flags &ATA_DFLAG_PIO ) { tf->protocol =ATA_PROT_PIO ; index = dev->multi_count ?0 :8 ; - }else if (lba48 && (qc->ap->flags &ATA_FLAG_PIO_LBA48 )) { + }else if (lba48 && (dev->ap->flags &ATA_FLAG_PIO_LBA48 )) {/* Unable to use DMA due to host limitation */ tf->protocol =ATA_PROT_PIO ; index = dev->multi_count ?0 :8 ;
@@ -283,6 +282,130 @@ u64 ata_tf_read_block(struct ata_taskfil }/** +* ata_build_tf - Build ATA taskfile for given request +* @tf: Target ATA taskfile +* @dev: ATA device @tf belongs to +* @block: Block address +* @n_block: Number of blocks +* @tf_flags: RW/FUA etc... +* @tag: tag +* +* LOCKING: +* None. +* +* Build ATA taskfile @tf for request described by @block, +* @n_block, @tf_flags and @tag on @dev. +* +* RETURNS: +* +* 0 on success, -ERANGE if the request is too large for @dev, +* -EINVAL if the request is invalid. +*/ +int ata_build_tf (struct ata_taskfile *tf,struct ata_device *dev, +u64 block,u32 n_block,unsigned int tf_flags, +unsigned int tag) +{ + tf->flags |=ATA_TFLAG_ISADDR |ATA_TFLAG_DEVICE ; + tf->flags |= tf_flags; + +if ((dev->flags & (ATA_DFLAG_PIO |ATA_DFLAG_NCQ_OFF | +ATA_DFLAG_NCQ )) ==ATA_DFLAG_NCQ ) { +/* yay, NCQ */ +if (!lba_48_ok (block, n_block)) +return -ERANGE ; + + tf->protocol =ATA_PROT_NCQ ; + tf->flags |=ATA_TFLAG_LBA |ATA_TFLAG_LBA48 ; + +if (tf->flags &ATA_TFLAG_WRITE ) + tf->command =ATA_CMD_FPDMA_WRITE ; +else + tf->command =ATA_CMD_FPDMA_READ ; + + tf->nsect = tag <<3 ; + tf->hob_feature = (n_block >>8 ) &0xff ; + tf->feature = n_block &0xff ; + + tf->hob_lbah = (block >>40 ) &0xff ; + tf->hob_lbam = (block >>32 ) &0xff ; + tf->hob_lbal = (block >>24 ) &0xff ; + tf->lbah = (block >>16 ) &0xff ; + tf->lbam = (block >>8 ) &0xff ; + tf->lbal = block &0xff ; + + tf->device =1 <<6 ; +if (tf->flags &ATA_TFLAG_FUA ) + tf->device |=1 <<7 ; + }else if (dev->flags &ATA_DFLAG_LBA ) { + tf->flags |=ATA_TFLAG_LBA ; + +if (lba_28_ok (block, n_block)) { +/* use LBA28 */ + tf->device |= (block >>24 ) &0xf ; + }else if (lba_48_ok (block, n_block)) { +if (!(dev->flags &ATA_DFLAG_LBA48 )) +return -ERANGE ; + +/* use LBA48 */ + tf->flags |=ATA_TFLAG_LBA48 ; + + tf->hob_nsect = (n_block >>8 ) &0xff ; + + tf->hob_lbah = (block >>40 ) &0xff ; + tf->hob_lbam = (block >>32 ) &0xff ; + tf->hob_lbal = (block >>24 ) &0xff ; + }else +/* request too large even for LBA48 */ +return -ERANGE ; + +if (unlikely (ata_rwcmd_protocol (tf, dev) <0 )) +return -EINVAL ; + + tf->nsect = n_block &0xff ; + + tf->lbah = (block >>16 ) &0xff ; + tf->lbam = (block >>8 ) &0xff ; + tf->lbal = block &0xff ; + + tf->device |=ATA_LBA ; + }else { +/* CHS */ +u32 sect, head, cyl, track; + +/* The request -may- be too large for CHS addressing. */ +if (!lba_28_ok (block, n_block)) +return -ERANGE ; + +if (unlikely (ata_rwcmd_protocol (tf, dev) <0 )) +return -EINVAL ; + +/* Convert LBA to CHS */ + track = (u32 )block / dev->sectors; + cyl = track / dev->heads; + head = track % dev->heads; + sect = (u32 )block % dev->sectors +1 ; + +DPRINTK ("block %u track %u cyl %u head %u sect %u\n" , + (u32 )block, track, cyl, head, sect); + +/* Check whether the converted CHS can fit. +Cylinder: 0-65535 +Head: 0-15 +Sector: 1-255*/ +if ((cyl >>16 ) || (head >>4 ) || (sect >>8 ) || (!sect)) +return -ERANGE ; + + tf->nsect = n_block &0xff ;/* Sector count 0 means 256 sectors */ + tf->lbal = sect; + tf->lbam = cyl; + tf->lbah = cyl >>8 ; + tf->device |= head; + } + +return 0 ; +} + +/** * ata_pack_xfermask - Pack pio, mwdma and udma masks into xfer_mask * @pio_mask: pio_mask * @mwdma_mask: mwdma_mask
drivers/ata/libata-scsi.c +13 -106
150 lines not on this page · raw message
drivers/ata/libata.h +3 -1
@@ -44,7 +44,9 @@ extern int atapi_enabled;extern int atapi_dmadir;extern int libata_fua;extern struct ata_queued_cmd *ata_qc_new_init (struct ata_device *dev); -extern int ata_rwcmd_protocol (struct ata_queued_cmd *qc); +extern int ata_build_tf (struct ata_taskfile *tf,struct ata_device *dev, +u64 block,u32 n_block,unsigned int tf_flags, +unsigned int tag);extern u64 ata_tf_read_block (struct ata_taskfile *tf,struct ata_device *dev);extern void ata_dev_disable (struct ata_device *dev);extern void ata_port_flush_task (struct ata_port *ap);
Signature · 1 line
1.4.3.3
Re: [PATCH 2/4] libata: separate out rw ATA taskfile building into ata_build_tf()
This logic sequence bothers me slightly:
Quoted 2 lines · Tejun Heo
Separate out rw ATA taskfile building from
ata_scsi_rw_xlat()intoata_build_tf(). This will be used to improve media error handling.
...
Quoted 5 lines
drivers/ata/libata-core.c +5
-0+if ((dev->flags & (ATA_DFLAG_PIO |ATA_DFLAG_NCQ_OFF | +ATA_DFLAG_NCQ )) ==ATA_DFLAG_NCQ ) { +/* yay, NCQ */ +if (!lba_48_ok (block, n_block)) +return -ERANGE ;
...
drivers/ata/libata-core.c u64 ata_tf_read_block(struct ata_taskfil · line 347
Quoted 9 lines
drivers/ata/libata-core.c +9
-0+ }else if (dev->flags &ATA_DFLAG_LBA ) { + tf->flags |=ATA_TFLAG_LBA ; + +if (lba_28_ok (block, n_block)) { +/* use LBA28 */ + tf->device |= (block >>24 ) &0xf ; + }else if (lba_48_ok (block, n_block)) { +if (!(dev->flags &ATA_DFLAG_LBA48 )) +return -ERANGE ;
...
In the first case, if !lba_48_ok(), then we should at least attempt to see if the request can be issued via non-NCQ.
Something like this:
if (lba_48_ok() && dev->flags & (ATA_DFLAG_PIO|ATA_DFLAG_NCQ_OFF|ATA_DFLAG_NCQ)) == ATA_DFLAG_NCQ) {
/* yay, NCQ */
...
} else if (dev->flags & ATA_DFLAG_LBA) {
...
..except prettier! ;)
In theory, it looks like an impossible sequence anyway..
if we have the NCQ flag set, then we probably already *know* that LBA48 is okay. But since we're not making that assumption, let's give it the benefit of a doubt and try non-NCQ if LBA48 is a no-go.
Cheers
Re: [PATCH 2/4] libata: separate out rw ATA taskfile building into ata_build_tf()
Quoted 34 lines · Mark Lord
This logic sequence bothers me slightly:
Quoted 2 lines · Tejun Heo
Separate out rw ATA taskfile building from
ata_scsi_rw_xlat()intoata_build_tf(). This will be used to improve media error handling....
Quoted 5 lines
drivers/ata/libata-core.c +5
-0+if ((dev->flags & (ATA_DFLAG_PIO |ATA_DFLAG_NCQ_OFF | +ATA_DFLAG_NCQ )) ==ATA_DFLAG_NCQ ) { +/* yay, NCQ */ +if (!lba_48_ok (block, n_block)) +return -ERANGE ;...
drivers/ata/libata-core.c u64 ata_tf_read_block(struct ata_taskfil · line 347
Quoted 9 lines
drivers/ata/libata-core.c +9
-0+ }else if (dev->flags &ATA_DFLAG_LBA ) { + tf->flags |=ATA_TFLAG_LBA ; + +if (lba_28_ok (block, n_block)) { +/* use LBA28 */ + tf->device |= (block >>24 ) &0xf ; + }else if (lba_48_ok (block, n_block)) { +if (!(dev->flags &ATA_DFLAG_LBA48 )) +return -ERANGE ;...
In the first case, if !lba_48_ok(), then we should at least attempt to see if the request can be issued via non-NCQ.
Something like this:
if (
lba_48_ok()&&dev->flags& (ATA_DFLAG_PIO|ATA_DFLAG_NCQ_OFF|ATA_DFLAG_NCQ)) ==ATA_DFLAG_NCQ) {
/* yay, NCQ */
...
} else if (dev->flags&ATA_DFLAG_LBA) {
...
Quoted 6 lines
..except prettier! ;)
In theory, it looks like an impossible sequence anyway..
if we have the NCQ flag set, then we probably already *know* that LBA48 is okay. But since we're not making that assumption, let's give it the benefit of a doubt and try non-NCQ if LBA48 is a no-go.
First of all, that function is moved verbatim from ata_scsi_rw_xlat(), and I don't really understand your point. lba_48_ok() checks whether the given block and n_block are inside LBA48 limit. NCQ commands use LBA48 (well, nsect is in the different registers but the limits are the same) which is the largest addressing mode currently available. So, if a command cannot be issued using NCQ, it cannot be issued w/ any command mode. ie. if !lba_48_ok(), that command just cannot be issued.
Am I misunderstanding you?
Signature · 1 line
tejun
Re: [PATCH 2/4] libata: separate out rw ATA taskfile building into ata_build_tf()
Quoted 2 lines · Tejun Heo
First of all, that function is moved verbatim from
ata_scsi_rw_xlat(),
Yeah, I knew that.
Don't mind me, I'm just have a bit of a scatterbrain day, switching back and forth between reviewing patches, and deep thinking about another task.
For some odd reason I thought "lba_48_ok" checked whether or not a drive could do LBA48.. probably because I have a routine by that exact name in several drivers of my own coding.
But of course it's different here: it's just checking the requested
block against the LBA48 range. But is this even necessary here?
Presumably we already checked the block number against the device max before getting to this point?
Or did we? I don't actually see that code nearby, but perhaps the SCSI mid-layer did it for us? If not, *where* do we have that check?
Anyway, it will all work as is.
I'm just wondering if we can nuke a few lines or not.
Cheers
Re: [PATCH 2/4] libata: separate out rw ATA taskfile building into ata_build_tf()
In reply to Tejun HeoQuoted 4 lines · Tejun Heo
Separate out rw ATA taskfile building from
ata_scsi_rw_xlat()intoata_build_tf(). This will be used to improve media error handling.Signed-off-by Tejun Heo <[email protected]>
ACK, provided you name it ata_build_rw_tf() or ata_build_addr_tf() or somesuch
PATCH 3/4 libata: prepare ata_sg_clean() for invocation from EH
In reply to Tejun HeoMake ata_sg_clean() global and don't allow NCQ for internal commands.
--- drivers/ata/libata-core.c | 6 +++--- drivers/ata/libata.h | 1 + 2 files changed, 4 insertions(+), 3 deletions(-)
drivers/ata/libata-core.c +3 -3
@@ -309,7 +309,8 @@ int ata_build_tf(struct ata_taskfile *tf tf->flags |= tf_flags;if ((dev->flags & (ATA_DFLAG_PIO |ATA_DFLAG_NCQ_OFF | -ATA_DFLAG_NCQ )) ==ATA_DFLAG_NCQ ) { +ATA_DFLAG_NCQ )) ==ATA_DFLAG_NCQ && +likely (tag !=ATA_TAG_INTERNAL )) {/* yay, NCQ */ if (!lba_48_ok (block, n_block))return -ERANGE ;
@@ -3517,8 +3518,7 @@ static unsigned int ata_dev_init_params(* LOCKING: * spin_lock_irqsave(host lock) */ - -static void ata_sg_clean (struct ata_queued_cmd *qc) +void ata_sg_clean (struct ata_queued_cmd *qc) {struct ata_port *ap = qc->ap;struct scatterlist *sg = qc->__sg;
drivers/ata/libata.h +1 -0
@@ -66,6 +66,7 @@ extern int sata_down_spd_limit(struct atextern int sata_set_spd_needed (struct ata_port *ap);extern int ata_down_xfermask_limit (struct ata_device *dev,int force_pio0);extern int ata_set_mode (struct ata_port *ap,struct ata_device **r_failed_dev); +extern void ata_sg_clean (struct ata_queued_cmd *qc);extern void ata_qc_free (struct ata_queued_cmd *qc);extern void ata_qc_issue (struct ata_queued_cmd *qc);extern void __ata_qc_complete (struct ata_queued_cmd *qc);
Signature · 1 line
1.4.3.3
Re: [PATCH 3/4] libata: prepare ata_sg_clean() for invocation from EH
Quoted 3 lines · Tejun Heo
Make
ata_sg_clean()global and don't allow NCQ for internal commands.Signed-off-by Tejun Heo <[email protected]>
ACK
PATCH 4/4 libata: improve media error handling
In reply to Tejun HeoThe ATA spec says "The amount of data transferred is indeterminate" when READ/WRITE commands fail with error status. TF regs contain the address of the first sector which failed, but that's it. libata reports the reported sector to sd which assumes data upto the first failed sector was transferred successfully. This can result in data corruption.
This patch implements highlevel command-aware recovery which currently has only one recovery action - ata_eh_do_partial(). If the device reports the first failed block, it tries to transfer upto that block.
SCSI sense generation is updated such that the first failed block is reported to SCSI layer iff partial IO occurred.
--- drivers/ata/libata-eh.c | 143 +++++++++++++++++++++++++++++++++++++++++++++ drivers/ata/libata-scsi.c | 36 +++++++----- include/linux/libata.h | 2 + 3 files changed, 166 insertions(+), 15 deletions(-)
drivers/ata/libata-eh.c +143 -0
@@ -1873,6 +1873,144 @@ static int ata_eh_resume(struct ata_portreturn 0 ; } +/** +* ata_eh_do_partial - perform partial IO after media error +* @qc: target ATA command +* +* The amount of succesfully transferred data is 'indeterminate' +* after a device error. This makes media error recovery +* difficult for high level driver. This function makes sure +* data upto the first bad sector is successfully transferred. +* +* LOCKING: +* Kernel thread context (may sleep). +* +* RETURNS: +* 0 on success, -errno otherwise +*/ +static int ata_eh_do_partial (struct ata_queued_cmd *qc) +{ +struct ata_port *ap = qc->ap; +struct ata_device *dev = qc->dev; +struct ata_eh_context *ehc = &ap->eh_context; +struct ata_taskfile tf; +u64 begin, bad; +struct scatterlist *sg; +unsigned int good_bytes, n_elem, idx, len, stored_len, err_mask; +int dma_dir, rc; + +/* determine initial good blocks and dma_dir */ + begin =ata_tf_read_block (&qc->tf, qc->dev); + bad =ata_tf_read_block (&qc->result_tf, qc->dev); + +if (bad < begin || begin + qc->nsect <= bad) { +ata_dev_printk (dev,KERN_WARNING ,"bogus bad block reported " +"begin=%llu nsect=%u bad=%llu, assuming %llu\n" , + (unsigned long long )begin, qc->nsect, + (unsigned long long )bad, + (unsigned long long )begin + qc->nsect /2 ); +return 0 ; + } + + good_bytes = (bad - begin) <<9 ; + +if (!good_bytes) +return 0 ; + + dma_dir =DMA_FROM_DEVICE ; +if (qc->tf.flags &ATA_TFLAG_WRITE ) + dma_dir =DMA_TO_DEVICE ; + +/* we're gonna reuse sglist, store & clear existing mapping */ + sg = qc->__sg; + n_elem = qc->orig_n_elem; + +if (qc->flags &ATA_QCFLAG_DMAMAP ) +ata_sg_clean (qc); + +/* find boundary sg */ + len =0 ; +for (idx =0 ; idx < n_elem; idx++) { +if (len + sg[idx].length > good_bytes) +break ; + len += sg[idx].length; + } +BUG_ON (idx >= n_elem); + +/* build TF */ +ata_tf_init (dev, &tf); + rc =ata_build_tf (&tf, dev, begin, good_bytes >>9 , + qc->tf.flags & (ATA_TFLAG_WRITE |ATA_TFLAG_FUA ), +ATA_TAG_INTERNAL ); +if (rc <0 ) { +ata_dev_printk (dev,KERN_ERR ,"failed to build TF for " +"partial IO (rc=%d)\n" , rc); +return rc; + } + +/* temporarily trim sg and perform partial IO */ + stored_len = sg[idx].length; + sg[idx].length = good_bytes - len; + + err_mask =ata_exec_internal_sg (dev, &tf,NULL , dma_dir, sg, idx +1 ); + + sg[idx].length = stored_len; + +if (err_mask) { +ata_dev_printk (dev,KERN_ERR , +"partial IO failed (err_mask=0x%x)\n" , err_mask); +return -EIO ; + } + + ehc->has_partial_mask |=1 << dev->devno; +return 0 ; +} + +/** +* ata_eh_hl_recover - highlevel command-aware recovery +* @ap: target host port +* @r_failed_dev: result parameter to indicate failing device +* +* Highlevel command-aware recovery. +* +* LOCKING: +* Kernel thread context (may sleep). +* +* RETURNS: +* 0 on success, -errno otherwise +*/ +static int ata_eh_hl_recover (struct ata_port *ap, +struct ata_device **r_failed_dev) +{ +struct ata_eh_context *ehc = &ap->eh_context; +int tag; + +for (tag =0 ; tag <ATA_MAX_QUEUE ; tag++) { +struct ata_queued_cmd *qc =__ata_qc_from_tag (ap, tag); +struct ata_device *dev = qc->dev; +int rc; + +if (!(qc->flags &ATA_QCFLAG_FAILED )) +continue ; + +if (dev->class ==ATA_DEV_ATA ) { +if (!(ehc->did_partial_mask & (1 << dev->devno)) && + (qc->flags &ATA_QCFLAG_IO ) && + (qc->err_mask &AC_ERR_MEDIA ) && + !(qc->err_mask & ~(AC_ERR_MEDIA |AC_ERR_DEV ))) { + ehc->did_partial_mask |=1 << dev->devno; + rc =ata_eh_do_partial (qc); +if (rc) { + *r_failed_dev = dev; +return rc; + } + } + } + } + +return 0 ; +} +static int ata_port_nr_enabled (struct ata_port *ap) {int i, cnt =0 ;
@@ -2030,6 +2168,11 @@ static int ata_eh_recover(struct ata_por ehc->i.flags &= ~ATA_EHI_SETMODE ; } +/* perform highlevel recovery */ + rc =ata_eh_hl_recover (ap, &dev); +if (rc) + goto dev_fail; +/* suspend devices */ rc =ata_eh_suspend (ap, &dev);if (rc)
drivers/ata/libata-scsi.c +21 -15
@@ -754,13 +754,14 @@ static void ata_gen_passthru_sense(struc*/ static void ata_gen_ata_sense (struct ata_queued_cmd *qc) { +struct ata_port *ap = qc->ap;struct ata_device *dev = qc->dev; +struct ata_eh_context *ehc = &ap->eh_context;struct scsi_cmnd *cmd = qc->scsicmd;struct ata_taskfile *tf = &qc->result_tf;unsigned char *sb = cmd->sense_buffer;unsigned char *desc = sb +8 ;int verbose = qc->ap->ops->error_handler ==NULL ; -u64 block;memset (sb,0 ,SCSI_SENSE_BUFFERSIZE );
@@ -779,20 +780,25 @@ static void ata_gen_ata_sense(struct ata sb[1 ] &=0x0f ; } - block =ata_tf_read_block (&qc->result_tf, dev); - -/* information sense data descriptor */ - sb[7 ] =12 ; - desc[0 ] =0x00 ; - desc[1 ] =10 ; - - desc[2 ] |=0x80 ;/* valid */ - desc[6 ] = block >>40 ; - desc[7 ] = block >>32 ; - desc[8 ] = block >>24 ; - desc[9 ] = block >>16 ; - desc[10 ] = block >>8 ; - desc[11 ] = block; +/* If partial transfer occurred, tell upper layer how many +* bytes are completed. +*/ +if (ehc->has_partial_mask & (1 << dev->devno)) { +u64 block =ata_tf_read_block (&qc->result_tf, dev); + +/* information sense data descriptor */ + sb[7 ] =12 ; + desc[0 ] =0x00 ; + desc[1 ] =10 ; + + desc[2 ] |=0x80 ;/* valid */ + desc[6 ] = block >>40 ; + desc[7 ] = block >>32 ; + desc[8 ] = block >>24 ; + desc[9 ] = block >>16 ; + desc[10 ] = block >>8 ; + desc[11 ] = block; + } }static void ata_scsi_sdev_config (struct scsi_device *sdev)
include/linux/libata.h +2 -0
@@ -527,6 +527,8 @@ struct ata_eh_context {int tries[ATA_MAX_DEVICES ];unsigned int classes[ATA_MAX_DEVICES ];unsigned int did_probe_mask; +unsigned int did_partial_mask; +unsigned int has_partial_mask; };struct ata_port {
Signature · 1 line
1.4.3.3
Quoted 14 lines · Tejun Heo
The ATA spec says "The amount of data transferred is indeterminate" when READ/WRITE commands fail with error status. TF regs contain the address of the first sector which failed, but that's it. libata reports the reported sector to sd which assumes data upto the first failed sector was transferred successfully. This can result in data corruption.
This patch implements highlevel command-aware recovery which currently has only one recovery action -
ata_eh_do_partial(). If the device reports the first failed block, it tries to transfer upto that block.SCSI sense generation is updated such that the first failed block is reported to SCSI layer iff partial IO occurred.
Quoted 6 lines
Signed-off-by Tejun Heo <[email protected]>--- drivers/ata/libata-eh.c | 143 +++++++++++++++++++++++++++++++++++++++++++++ drivers/ata/libata-scsi.c | 36 +++++++----- include/linux/libata.h | 2 + 3 files changed, 166 insertions(+), 15 deletions(-)
(finally reviewing a patch from a 3 months ago)
ACK, please resend at your leisure
In general, I have worries about how solid partial completion really is.
I am mainly thinking of SCSI and VFS not libata, here. Your patch
looks sane enough, so let's see how things go...
Jeff