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@xxxxxxx>
> ---
> 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);
> }
--
Damien Le Moal
Western Digital Research
