driverdev-devel.linuxdriverproject.org archive mirror
 help / color / mirror / Atom feed
From: Coiby Xu <coiby.xu@gmail.com>
To: devel@driverdev.osuosl.org
Cc: Benjamin Poirier <benjamin.poirier@gmail.com>,
	Willem de Bruijn <willemdebruijn.kernel@gmail.com>,
	Shung-Hsi Yu <shung-hsi.yu@suse.com>,
	Dan Carpenter <dan.carpenter@oracle.com>
Subject: [PATCH v3 0/8] staging: qlge: Re-writing the debugging features
Date: Fri, 16 Oct 2020 19:53:59 +0800	[thread overview]
Message-ID: <20201016115407.170821-1-coiby.xu@gmail.com> (raw)

This patch set aims to avoid dumping registers, data structures and
coredump to dmesg and also to reduce the code size of the qlge driver.

As pointed out by Benjamin [1],

> At 2000 lines, qlge_dbg.c alone is larger than some entire ethernet
> drivers. Most of what it does is dump kernel data structures or pci
> memory mapped registers to dmesg. There are better facilities for that.
> My thinking is not simply to delete qlge_dbg.c but to replace it, making
> sure that most of the same information is still available. For data
> structures, crash or drgn can be used; possibly with a script for the
> latter which formats the data. For pci registers, they should be
> included in the ethtool register dump and a patch added to ethtool to
> pretty print them. That's what other drivers like e1000e do. For the
> "coredump", devlink health can be used.

So the debugging features are re-written following Benjamin's advice,
   - dump kernel data structures in drgn
   - use devlink to do coredump which also includes device status and
     general registers

For the usage examples of supported devlink commands, please check the
last patch.

[1] https://lkml.org/lkml/2020/6/30/19

v2 -> v3
- Fix newly introduced resource leak [Dan Carpenter]
- Fix bugs after using struct qlge_adapter as the private data struct of devlink
- Add qlge_ prefix for structures not having a prefix and fix a left-over ql_adapter [Benjamin Poirier]

v1 -> v2
- Call devlink_free when register_netdev fails [Willem de Bruijn]
- "scripts/checkpatch.pl --strict" for changes [Dan Carpente]
- Declares variables in "Reverse Christmas Tree" [Dan Carpente]
- Use the sizeof() directly  [Dan Carpente]
- Add SPDX-License-Identifier to qlge_devlink.{c,h}
- Rename ql_* to qlge_* [Benjamin Poirier]
- Update drivers/staging/qlge/TODO [Benjamin Poirier]
- struct qlge_adapter is now used as the private data struct of
  devlink instead of net_device [Benjamin Poirier]

RFC -> v1
 - select NET_DEVLINK in Kconfig [Benjamin Poirier]
 - Don't do a coredump when the interface is down [Shung-Hsi Yu]
 - Remove stray newlines [Benjamin Poirier]
 - force_coredump for devlink
 - Remove mpi_core_to_log which will output the coredump to the kernel
   ring buffer
 - Put drgn script under Documentation [Benjamin Poirier]
 - Rename qlge_health.* to qlge_devlink.*

Coiby Xu (8):
  staging: qlge: use qlge_* prefix to avoid namespace clashes with other
    qlogic drivers
  staging: qlge: Initialize devlink health dump framework
  staging: qlge: re-write qlge_init_device
  staging: qlge: coredump via devlink health reporter
  staging: qlge: support force_coredump option for devlink health dump
  staging: qlge: remove mpi_core_to_log which sends coredump to the
    kernel ring buffer
  staging: qlge: clean up debugging code in the QL_ALL_DUMP ifdef land
  staging: qlge: add documentation for debugging qlge

 .../networking/device_drivers/index.rst       |    1 +
 .../device_drivers/qlogic/index.rst           |   18 +
 .../networking/device_drivers/qlogic/qlge.rst |  118 ++
 MAINTAINERS                                   |    6 +
 drivers/staging/qlge/Kconfig                  |    1 +
 drivers/staging/qlge/Makefile                 |    2 +-
 drivers/staging/qlge/TODO                     |   10 -
 drivers/staging/qlge/qlge.h                   |  244 +--
 drivers/staging/qlge/qlge_dbg.c               | 1650 +++++------------
 drivers/staging/qlge/qlge_devlink.c           |  156 ++
 drivers/staging/qlge/qlge_devlink.h           |    9 +
 drivers/staging/qlge/qlge_ethtool.c           |  234 ++-
 drivers/staging/qlge/qlge_main.c              | 1375 +++++++-------
 drivers/staging/qlge/qlge_mpi.c               |  356 ++--
 14 files changed, 1866 insertions(+), 2314 deletions(-)
 create mode 100644 Documentation/networking/device_drivers/qlogic/index.rst
 create mode 100644 Documentation/networking/device_drivers/qlogic/qlge.rst
 create mode 100644 drivers/staging/qlge/qlge_devlink.c
 create mode 100644 drivers/staging/qlge/qlge_devlink.h

--
2.28.0

_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

             reply	other threads:[~2020-10-16 11:56 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-10-16 11:53 Coiby Xu [this message]
2020-10-16 11:54 ` [PATCH v3 1/8] staging: qlge: use qlge_* prefix to avoid namespace clashes with other qlogic drivers Coiby Xu
2020-10-16 11:54 ` [PATCH v3 2/8] staging: qlge: Initialize devlink health dump framework Coiby Xu
2020-10-20  8:57   ` Shung-Hsi Yu
2020-10-20 10:27     ` Shung-Hsi Yu
2020-10-20 10:46       ` Coiby Xu
2020-10-16 11:54 ` [PATCH v3 3/8] staging: qlge: re-write qlge_init_device Coiby Xu
2020-10-16 11:54 ` [PATCH v3 4/8] staging: qlge: coredump via devlink health reporter Coiby Xu
2020-10-16 11:54 ` [PATCH v3 5/8] staging: qlge: support force_coredump option for devlink health dump Coiby Xu
2020-10-16 11:54 ` [PATCH v3 6/8] staging: qlge: remove mpi_core_to_log which sends coredump to the kernel ring buffer Coiby Xu
2020-10-16 11:54 ` [PATCH v3 7/8] staging: qlge: clean up debugging code in the QL_ALL_DUMP ifdef land Coiby Xu
2020-10-16 11:54 ` [PATCH v3 8/8] staging: qlge: add documentation for debugging qlge Coiby Xu

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20201016115407.170821-1-coiby.xu@gmail.com \
    --to=coiby.xu@gmail.com \
    --cc=benjamin.poirier@gmail.com \
    --cc=dan.carpenter@oracle.com \
    --cc=devel@driverdev.osuosl.org \
    --cc=shung-hsi.yu@suse.com \
    --cc=willemdebruijn.kernel@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).