Message ID | 20211221072131.46673-66-hare@suse.de |
---|---|
State | New |
Headers | show |
Series | libata: rework logging, take II | expand |
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 --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); }
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(-)