On 12/21/21 16:20, Hannes Reinecke wrote:

Hi all,

after some prodding from individual persons I've resurrected my
patchset to put libata logging on a even keel, and use structured
logging for everything.
So this patch does away with DPRINTK, ATA_DEBUG or ata_msg_XXX()
calls, and moves everything over to structured logging
(ie the dev_XXX() calls).
Additionally I've added tracepoints to trace command flow,
error handling, host state machine etc.

So everything is looking far saner now.

As usual, comments and reviews are welcome.

I know that the device names suck. Blame Tejun.

I applied this series, temporarily, to the branch for-5.17-logging, with
the fixes I commented about.

After some light testing, seems OK, but need to hammer this a little more.

My main concern is the change in patch 12: using the dev_xxx() macros
directly changes the messages from being all prefixed with "ataX.X" to
different flavors (devX.X, linkX.X, portX.X, etc). While I do like the
code simplification, this makes reading dmesg to track how a device is
behaving very hard.

Defining dev_fmt() everywhere the ata_{port,link,dev}_XXX functions are
used would solve this, but that is annoying as almost all drivers would
need to define that. Looking at the dev_XXX() macros, I do not see an
easy solution. Any idea ?

Note: If you resend a new version, please base it off for-5.17-logging.

Cheers !

Changes to the original submission:
- Converted all printk() and pr_XXX() calls, too
- Included reviews from Damien
- Included reviews from Sergey

Changes to v2:
- Rebase to ata/for-5.17
- Include reviews from Sergey

Hannes Reinecke (68):
  libata: remove pointless debugging messages
  libata: whitespace cleanup
  libata: Add ata_port_classify() helper
  libata: move ata_dump_id() to dynamic debugging
  libata: sanitize ATA_HORKAGE_DUMP_ID
  libata: add reset tracepoints
  libata: add qc_prep tracepoint
  libata: tracepoints for bus-master DMA
  libata-sff: tracepoints for HSM state machine
  libata-scsi: drop DPRINTK calls for cdb translation
  libata: add tracepoints for ATA error handling
  libata: move ata_{port,link,dev}_dbg to standard dev_XXX() macros
  libata: revamp ata_get_cmd_descript()
  libata: move DPRINTK to ata debugging
  sata_mv: kill 'port' argument in mv_dump_all_regs()
  sata_mv: replace DPRINTK with dynamic debugging
  pata_octeon_cf: remove DPRINTK() macro in interrupt context
  pdc_adma: Remove DPRINTK call
  sata_fsl: move DPRINTK to ata debugging
  sata_rcar: replace DPRINTK() with ata_port_dbg()
  sata_qstor: replace DPRINTK() with ata_port_dbg()
  pata_pdc2027x: Replace PDPRINTK() with standard ata logging
  libata: remove pointless VPRINTK() calls
  ahci: Drop pointless VPRINTK() calls and convert the remaining ones
  pdc_adma: Drop pointless VPRINTK() calls and remove disabled NCQ
    debugging
  pata_octeon_cf: Drop pointless VPRINTK() calls and convert the
    remaining one
  pata_via: Drop pointless VPRINTK() calls
  sata_promise: Drop pointless VPRINTK() calls and convert the remaining
    ones
  sata_qstor: Drop pointless VPRINTK() calls
  sata_rcar: Drop pointless VPRINTK() calls
  sata_inic162x: Drop pointless VPRINTK() calls
  sata_mv: Drop pointless VPRINTK() call and convert the remaining one
  sata_nv: drop pointless VPRINTK() calls and convert remaining ones
  sata_fsl: convert VPRINTK() calls to ata_port_dbg()
  sata_sil: Drop pointless VPRINTK() calls
  sata_sx4: Drop pointless VPRINTK() calls and convert the remaining
    ones
  sata_sx4: add module parameter 'dimm_test'
  libata: drop ata_msg_error() and ata_msg_intr()
  libata: drop ata_msg_ctl()
  libata: drop ata_msg_malloc()
  libata: drop ata_msg_warn()
  libata: drop ata_msg_probe()
  libata: drop ata_msg_info()
  libata: drop ata_msg_drv()
  libata: remove 'new' ata message handling
  libata: remove debug compilation switches
  pata_atp867x: convert printk() calls
  pata_cmd640: convert printk() calls
  pata_cmd64x: convert printk() calls
  pata_cs5520: convert printk() calls
  pata_cs5536: convert printk() calls
  pata_cypress: convert printk() calls
  pata_it821x: convert printk() calls
  pata_marvell: convert printk() calls
  pata_rz1000: convert printk() calls
  pata_serverworks: convert printk() calls
  pata_sil680: convert printk() calls
  sata_sx4: convert printk() calls
  sata_mv: convert remaining printk() to structured logging
  pata_hpt37x: convert pr_XXX() calls
  pata_octeon_cf: Replace pr_XXX() calls with structured logging
  pata_hpt3x2n: convert pr_XXX() calls
  sata_gemini: convert pr_err() calls
  pata_hpt366: convert pr_warn() calls
  libata-scsi: rework ata_dump_status to avoid using pr_cont()
  sata_dwc_460ex: drop DEBUG_NCQ
  sata_dwc_460ex: remove 'check_status' argument
  sata_dwc_460ex: Remove debug compile options

 drivers/ata/Kconfig             |  12 -
 drivers/ata/acard-ahci.c        |   4 -
 drivers/ata/ahci.c              |  13 +-
 drivers/ata/ahci_qoriq.c        |   4 -
 drivers/ata/ahci_xgene.c        |   4 -
 drivers/ata/ata_piix.c          |  11 +-
 drivers/ata/libahci.c           |  33 +--
 drivers/ata/libata-acpi.c       |  69 +++---
 drivers/ata/libata-core.c       | 225 ++++-------------
 drivers/ata/libata-eh.c         |  72 +++---
 drivers/ata/libata-pmp.c        |   8 -
 drivers/ata/libata-sata.c       |   5 -
 drivers/ata/libata-scsi.c       | 111 ++-------
 drivers/ata/libata-sff.c        |  88 +++----
 drivers/ata/libata-trace.c      |  47 ++++
 drivers/ata/libata-transport.c  |  45 +++-
 drivers/ata/libata.h            |   5 +-
 drivers/ata/pata_arasan_cf.c    |   3 +
 drivers/ata/pata_atp867x.c      |  29 +--
 drivers/ata/pata_cmd640.c       |   2 +-
 drivers/ata/pata_cmd64x.c       |   4 +-
 drivers/ata/pata_cs5520.c       |   4 +-
 drivers/ata/pata_cs5536.c       |   4 +-
 drivers/ata/pata_cypress.c      |   2 +-
 drivers/ata/pata_ep93xx.c       |   1 -
 drivers/ata/pata_hpt366.c       |   5 +-
 drivers/ata/pata_hpt37x.c       |  20 +-
 drivers/ata/pata_hpt3x2n.c      |  12 +-
 drivers/ata/pata_it821x.c       |  43 ++--
 drivers/ata/pata_ixp4xx_cf.c    |   6 +-
 drivers/ata/pata_marvell.c      |   9 +-
 drivers/ata/pata_octeon_cf.c    |  48 +---
 drivers/ata/pata_pdc2027x.c     |  71 +++---
 drivers/ata/pata_pdc202xx_old.c |   2 -
 drivers/ata/pata_rz1000.c       |   4 +-
 drivers/ata/pata_serverworks.c  |   4 +-
 drivers/ata/pata_sil680.c       |   9 +-
 drivers/ata/pata_via.c          |  12 -
 drivers/ata/pdc_adma.c          |  33 +--
 drivers/ata/sata_dwc_460ex.c    | 120 ++-------
 drivers/ata/sata_fsl.c          | 165 +++++--------
 drivers/ata/sata_gemini.c       |   4 +-
 drivers/ata/sata_inic162x.c     |   4 +-
 drivers/ata/sata_mv.c           | 130 +++++-----
 drivers/ata/sata_nv.c           |  54 ++---
 drivers/ata/sata_promise.c      |  31 +--
 drivers/ata/sata_qstor.c        |  15 +-
 drivers/ata/sata_rcar.c         |  26 +-
 drivers/ata/sata_sil.c          |   1 -
 drivers/ata/sata_sil24.c        |   5 +-
 drivers/ata/sata_sx4.c          | 148 ++++--------
 include/linux/libata.h          |  99 ++------
 include/trace/events/libata.h   | 416 +++++++++++++++++++++++++++++++-
 53 files changed, 1050 insertions(+), 1251 deletions(-)

-- 
Damien Le Moal
Western Digital Research