diff mbox series

[65/68] libata-scsi: rework ata_dump_status to avoid using pr_cont()

Message ID 20211221072131.46673-66-hare@suse.de
State New
Headers show
Series libata: rework logging, take II | expand

Commit Message

Hannes Reinecke Dec. 21, 2021, 7:21 a.m. UTC
pr_cont() has the problem that individual calls will be disrupted
under high load, causing each call to end up on a single line and
thereby mangling the output.
So rework ata_dump_status() to have just one call to ata_port_warn()
and avoid this problem.

Signed-off-by: Hannes Reinecke <hare@suse.de>
---
 drivers/ata/libata-scsi.c | 49 ++++++++++++++++++---------------------
 1 file changed, 22 insertions(+), 27 deletions(-)

Comments

Damien Le Moal Dec. 30, 2021, 5:13 a.m. UTC | #1
On 12/21/21 16:21, Hannes Reinecke wrote:
> pr_cont() has the problem that individual calls will be disrupted
> under high load, causing each call to end up on a single line and
> thereby mangling the output.
> So rework ata_dump_status() to have just one call to ata_port_warn()
> and avoid this problem.
> 
> Signed-off-by: Hannes Reinecke <hare@suse.de>
> ---
>  drivers/ata/libata-scsi.c | 49 ++++++++++++++++++---------------------
>  1 file changed, 22 insertions(+), 27 deletions(-)
> 
> diff --git a/drivers/ata/libata-scsi.c b/drivers/ata/libata-scsi.c
> index 11fb046e3035..d72852ac9ca3 100644
> --- a/drivers/ata/libata-scsi.c
> +++ b/drivers/ata/libata-scsi.c
> @@ -678,37 +678,32 @@ static void ata_qc_set_pc_nbytes(struct ata_queued_cmd *qc)
>   *	LOCKING:
>   *	inherited from caller
>   */
> -static void ata_dump_status(unsigned id, struct ata_taskfile *tf)
> +static void ata_dump_status(struct ata_port *ap, struct ata_taskfile *tf)

You forgot to fix the kdoc comments, which caused a compile warning with
W=1. Fixed that.

>  {
>  	u8 stat = tf->command, err = tf->feature;
>  
> -	pr_warn("ata%u: status=0x%02x { ", id, stat);
>  	if (stat & ATA_BUSY) {
> -		pr_cont("Busy }\n");	/* Data is not valid in this case */
> +		ata_port_warn(ap, "status=0x%02x {Busy} ", stat);
>  	} else {
> -		if (stat & ATA_DRDY)	pr_cont("DriveReady ");
> -		if (stat & ATA_DF)	pr_cont("DeviceFault ");
> -		if (stat & ATA_DSC)	pr_cont("SeekComplete ");
> -		if (stat & ATA_DRQ)	pr_cont("DataRequest ");
> -		if (stat & ATA_CORR)	pr_cont("CorrectedError ");
> -		if (stat & ATA_SENSE)	pr_cont("Sense ");
> -		if (stat & ATA_ERR)	pr_cont("Error ");
> -		pr_cont("}\n");
> -
> -		if (err) {
> -			pr_warn("ata%u: error=0x%02x { ", id, err);
> -			if (err & ATA_ABORTED)	pr_cont("DriveStatusError ");
> -			if (err & ATA_ICRC) {
> -				if (err & ATA_ABORTED)
> -						pr_cont("BadCRC ");
> -				else		pr_cont("Sector ");
> -			}
> -			if (err & ATA_UNC)	pr_cont("UncorrectableError ");
> -			if (err & ATA_IDNF)	pr_cont("SectorIdNotFound ");
> -			if (err & ATA_TRK0NF)	pr_cont("TrackZeroNotFound ");
> -			if (err & ATA_AMNF)	pr_cont("AddrMarkNotFound ");
> -			pr_cont("}\n");
> -		}
> +		ata_port_warn(ap, "status=0x%02x { %s%s%s%s%s%s%s} ", stat,
> +			      stat & ATA_DRDY ? "DriveReady " : "",
> +			      stat & ATA_DF ? "DeviceFault " : "",
> +			      stat & ATA_DSC ? "SeekComplete " : "",
> +			      stat & ATA_DRQ ? "DataRequest " : "",
> +			      stat & ATA_CORR ? "CorrectedError " : "",
> +			      stat & ATA_SENSE ? "Sense " : "",
> +			      stat & ATA_ERR ? "Error " : "");
> +		if (err)
> +			ata_port_warn(ap, "error=0x%02x {%s%s%s%s%s%s", err,
> +				      err & ATA_ABORTED ?
> +				      "DriveStatusError " : "",
> +				      err & ATA_ICRC ?
> +				      (err & ATA_ABORTED ?
> +				       "BadCRC " : "Sector ") : "",
> +				      err & ATA_UNC ? "UncorrectableError " : "",
> +				      err & ATA_IDNF ? "SectorIdNotFound " : "",
> +				      err & ATA_TRK0NF ? "TrackZeroNotFound " : "",
> +				      err & ATA_AMNF ? "AddrMarkNotFound " : "");
>  	}
>  }
>  
> @@ -1662,7 +1657,7 @@ static void ata_scsi_qc_complete(struct ata_queued_cmd *qc)
>  		cmd->result = SAM_STAT_GOOD;
>  
>  	if (need_sense && !ap->ops->error_handler)
> -		ata_dump_status(ap->print_id, &qc->result_tf);
> +		ata_dump_status(ap, &qc->result_tf);
>  
>  	ata_qc_done(qc);
>  }
diff mbox series

Patch

diff --git a/drivers/ata/libata-scsi.c b/drivers/ata/libata-scsi.c
index 11fb046e3035..d72852ac9ca3 100644
--- a/drivers/ata/libata-scsi.c
+++ b/drivers/ata/libata-scsi.c
@@ -678,37 +678,32 @@  static void ata_qc_set_pc_nbytes(struct ata_queued_cmd *qc)
  *	LOCKING:
  *	inherited from caller
  */
-static void ata_dump_status(unsigned id, struct ata_taskfile *tf)
+static void ata_dump_status(struct ata_port *ap, struct ata_taskfile *tf)
 {
 	u8 stat = tf->command, err = tf->feature;
 
-	pr_warn("ata%u: status=0x%02x { ", id, stat);
 	if (stat & ATA_BUSY) {
-		pr_cont("Busy }\n");	/* Data is not valid in this case */
+		ata_port_warn(ap, "status=0x%02x {Busy} ", stat);
 	} else {
-		if (stat & ATA_DRDY)	pr_cont("DriveReady ");
-		if (stat & ATA_DF)	pr_cont("DeviceFault ");
-		if (stat & ATA_DSC)	pr_cont("SeekComplete ");
-		if (stat & ATA_DRQ)	pr_cont("DataRequest ");
-		if (stat & ATA_CORR)	pr_cont("CorrectedError ");
-		if (stat & ATA_SENSE)	pr_cont("Sense ");
-		if (stat & ATA_ERR)	pr_cont("Error ");
-		pr_cont("}\n");
-
-		if (err) {
-			pr_warn("ata%u: error=0x%02x { ", id, err);
-			if (err & ATA_ABORTED)	pr_cont("DriveStatusError ");
-			if (err & ATA_ICRC) {
-				if (err & ATA_ABORTED)
-						pr_cont("BadCRC ");
-				else		pr_cont("Sector ");
-			}
-			if (err & ATA_UNC)	pr_cont("UncorrectableError ");
-			if (err & ATA_IDNF)	pr_cont("SectorIdNotFound ");
-			if (err & ATA_TRK0NF)	pr_cont("TrackZeroNotFound ");
-			if (err & ATA_AMNF)	pr_cont("AddrMarkNotFound ");
-			pr_cont("}\n");
-		}
+		ata_port_warn(ap, "status=0x%02x { %s%s%s%s%s%s%s} ", stat,
+			      stat & ATA_DRDY ? "DriveReady " : "",
+			      stat & ATA_DF ? "DeviceFault " : "",
+			      stat & ATA_DSC ? "SeekComplete " : "",
+			      stat & ATA_DRQ ? "DataRequest " : "",
+			      stat & ATA_CORR ? "CorrectedError " : "",
+			      stat & ATA_SENSE ? "Sense " : "",
+			      stat & ATA_ERR ? "Error " : "");
+		if (err)
+			ata_port_warn(ap, "error=0x%02x {%s%s%s%s%s%s", err,
+				      err & ATA_ABORTED ?
+				      "DriveStatusError " : "",
+				      err & ATA_ICRC ?
+				      (err & ATA_ABORTED ?
+				       "BadCRC " : "Sector ") : "",
+				      err & ATA_UNC ? "UncorrectableError " : "",
+				      err & ATA_IDNF ? "SectorIdNotFound " : "",
+				      err & ATA_TRK0NF ? "TrackZeroNotFound " : "",
+				      err & ATA_AMNF ? "AddrMarkNotFound " : "");
 	}
 }
 
@@ -1662,7 +1657,7 @@  static void ata_scsi_qc_complete(struct ata_queued_cmd *qc)
 		cmd->result = SAM_STAT_GOOD;
 
 	if (need_sense && !ap->ops->error_handler)
-		ata_dump_status(ap->print_id, &qc->result_tf);
+		ata_dump_status(ap, &qc->result_tf);
 
 	ata_qc_done(qc);
 }