Re: [PATCH v2 1/8] ata: remove reference to non-existing error_handler()

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 20/07/2023 01:42, Niklas Cassel wrote:
From: Hannes Reinecke <hare@xxxxxxx>

With commit 65a15d6560df ("scsi: ipr: Remove SATA support") all
libata drivers now have the error_handler() callback provided,
so we can stop checking for non-existing error_handler callback.

Signed-off-by: Hannes Reinecke <hare@xxxxxxx>
[niklas: fixed review comments, rebased, solved conflicts during rebase,
fixed bug that unconditionally dumped all QCs, removed the now unused
function ata_dump_status(), removed the now unreachable failure paths in
atapi_qc_complete(), removed the non-EH function to request ATAPI sense]
Signed-off-by: Niklas Cassel <niklas.cassel@xxxxxxx>

ata_qc_from_tag() still has a ap->ops->error_handler check, right?

---
  drivers/ata/libata-core.c | 209 +++++++++++++++-----------------------
  drivers/ata/libata-eh.c   | 150 ++++++++++++---------------
  drivers/ata/libata-sata.c |   7 +-
  drivers/ata/libata-scsi.c | 142 ++------------------------
  drivers/ata/libata-sff.c  |  30 ++----
  5 files changed, 166 insertions(+), 372 deletions(-)

...


/*
-	 * For new EH, all qcs are finished in one of three ways -
+	 * For EH, all qcs are finished in one of three ways -
  	 * normal completion, error completion, and SCSI timeout.
  	 * Both completions can race against SCSI timeout.  When normal
  	 * completion wins, the qc never reaches EH.  When error
@@ -659,94 +656,89 @@ EXPORT_SYMBOL(ata_scsi_cmd_error_handler);
  void ata_scsi_port_error_handler(struct Scsi_Host *host, struct ata_port *ap)
  {
  	unsigned long flags;
+	struct ata_link *link;
/* invoke error handler */

Is this comment only really relevant when we may not previously invoked the error handler?

-	if (ap->ops->error_handler) {
-		struct ata_link *link;
- /* acquire EH ownership */
-		ata_eh_acquire(ap);
+	/* acquire EH ownership */
+	ata_eh_acquire(ap);
   repeat:
-		/* kill fast drain timer */
-		del_timer_sync(&ap->fastdrain_timer);
+	/* kill fast drain timer */
+	del_timer_sync(&ap->fastdrain_timer);
- /* process port resume request */
-		ata_eh_handle_port_resume(ap);
+	/* process port resume request */
+	ata_eh_handle_port_resume(ap);
- /* fetch & clear EH info */
-		spin_lock_irqsave(ap->lock, flags);
+	/* fetch & clear EH info */
+	spin_lock_irqsave(ap->lock, flags);

...

   *	ata_to_sense_error - convert ATA error to SCSI error
   *	@id: ATA device number
@@ -904,7 +863,6 @@ static void ata_gen_passthru_sense(struct ata_queued_cmd *qc)
  	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;
  	u8 sense_key, asc, ascq;
memset(sb, 0, SCSI_SENSE_BUFFERSIZE);
@@ -916,7 +874,7 @@ static void ata_gen_passthru_sense(struct ata_queued_cmd *qc)
  	if (qc->err_mask ||
  	    tf->status & (ATA_BUSY | ATA_DF | ATA_ERR | ATA_DRQ)) {
  		ata_to_sense_error(qc->ap->print_id, tf->status, tf->error,
-				   &sense_key, &asc, &ascq, verbose);
+				   &sense_key, &asc, &ascq, false);
  		ata_scsi_set_sense(qc->dev, cmd, sense_key, asc, ascq);
  	} else {
  		/*
@@ -999,7 +957,6 @@ static void ata_gen_ata_sense(struct ata_queued_cmd *qc)
  	struct scsi_cmnd *cmd = qc->scsicmd;
  	struct ata_taskfile *tf = &qc->result_tf;
  	unsigned char *sb = cmd->sense_buffer;
-	int verbose = qc->ap->ops->error_handler == NULL;
  	u64 block;
  	u8 sense_key, asc, ascq;
@@ -1017,7 +974,7 @@ static void ata_gen_ata_sense(struct ata_queued_cmd *qc)
  	if (qc->err_mask ||
  	    tf->status & (ATA_BUSY | ATA_DF | ATA_ERR | ATA_DRQ)) {
  		ata_to_sense_error(qc->ap->print_id, tf->status, tf->error,
-				   &sense_key, &asc, &ascq, verbose);
+				   &sense_key, &asc, &ascq, false);

Please check this - AFAICS, we only ever pass false for @verbose arg now (so it would not be needed, and ata_to_sense_error() may be simplified)

  		ata_scsi_set_sense(dev, cmd, sense_key, asc, ascq);
  	} else {
  		/* Could not decode error */
@@ -1179,9 +1136,6 @@ void ata_scsi_slave_destroy(struct scsi_device *sdev)
  	unsigned long flags;
  	struct ata_device *dev;
- if (!ap->ops->error_handler)




[Index of Archives]     [Linux Filesystems]     [Linux SCSI]     [Linux RAID]     [Git]     [Kernel Newbies]     [Linux Newbie]     [Security]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Samba]     [Device Mapper]

  Powered by Linux