public inbox for linux-btrfs@vger.kernel.org
 help / color / mirror / Atom feed
From: David Sterba <dsterba@suse.com>
To: linux-btrfs@vger.kernel.org
Cc: David Sterba <dsterba@suse.com>
Subject: [PATCH 0/5] Assertion and debugging helpers
Date: Wed, 16 Apr 2025 11:08:07 +0200	[thread overview]
Message-ID: <cover.1744794336.git.dsterba@suse.com> (raw)

Hi,

this is a RFC series. We need to improve debugging and logging helpers
so there's no ASSERT(0) or the convoluted
WARN_ON(IS_DEFINED(CONFIG_BTRFS_DEBUG)). This was mentioned in past
discussions so here's my proposal.

The series is only build tested, I'd like to hear some feedback if this
is going the right direction or if there are suggestions for fine
tuning.

1) Add verbose ASSERT macro, so we can print additional information when
it triggers, namely printing the values of the assertion expression.
More details in the first patch, basic pattern is something like

    VASSERT(value > limit, "value=%llu limit=%llu", value, limit);

The second patch shows it's application in volumes.c, converting about
half where it's relevant. There are about 800 assertions in fs/btrfs/
and we don't need to convert them all. This can be done incrementally
and as needed.

The verbose version is another macro, although with some preprocessor
magic it should be possible to make ASSERT take variable number of
arguments. Does not seem worth though.

2) Wrap WARN_ON(IS_DEFINED(CONFIG_BTRFS_DEBUG)) to DEBUG_WARN() with
optional message with printk format. This is used to replace the
WARN_ON(...) above and also the ASSERT(0).

The ultimate goal for me is to get rid of all ASSERT(0), it's not used
consistently and looks like it's a note to the code author. There may be
several reasons for it's use and although I've converted almost all to
DEBUG_WARN it may miss the intentions.

In some cases it may be better to add proper error handling, print a
message or warn and exit with error. Possibly the are cases where the
code cannot continue, meaning it should be a BUG_ON but this is also
something we want to convert to proper error handling.

David Sterba (5):
  btrfs: add verbose version of ASSERT
  btrfs: example use of VASSERT() in volumes.c
  btrfs: add debug build only WARN
  btrfs: convert WARN_ON(IS_ENABLED(CONFIG_BTRFS_DEBUG)) to DEBUG_WARN
  btrfs: convert ASSERT(0) to DEBUG_WARN()

 fs/btrfs/backref.c         |  4 +--
 fs/btrfs/delayed-ref.c     |  2 +-
 fs/btrfs/dev-replace.c     |  2 +-
 fs/btrfs/disk-io.c         |  2 +-
 fs/btrfs/extent-tree.c     |  2 +-
 fs/btrfs/extent_io.c       |  4 +--
 fs/btrfs/extent_map.c      |  2 +-
 fs/btrfs/free-space-tree.c | 27 +++++++++-------
 fs/btrfs/inode.c           |  6 ++--
 fs/btrfs/messages.h        | 30 ++++++++++++++++++
 fs/btrfs/qgroup.c          |  6 ++--
 fs/btrfs/relocation.c      |  4 +--
 fs/btrfs/send.c            |  4 +--
 fs/btrfs/space-info.c      |  2 +-
 fs/btrfs/tree-checker.c    |  8 ++---
 fs/btrfs/volumes.c         | 64 ++++++++++++++++++++++++--------------
 fs/btrfs/zoned.c           |  2 +-
 17 files changed, 109 insertions(+), 62 deletions(-)

-- 
2.49.0


             reply	other threads:[~2025-04-16  9:08 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-04-16  9:08 David Sterba [this message]
2025-04-16  9:08 ` [PATCH 1/5] btrfs: add verbose version of ASSERT David Sterba
2025-04-16 15:03   ` Johannes Thumshirn
2025-04-16 19:30     ` David Sterba
2025-04-16  9:08 ` [PATCH 2/5] btrfs: example use of VASSERT() in volumes.c David Sterba
2025-04-16  9:08 ` [PATCH 3/5] btrfs: add debug build only WARN David Sterba
2025-04-16  9:08 ` [PATCH 4/5] btrfs: convert WARN_ON(IS_ENABLED(CONFIG_BTRFS_DEBUG)) to DEBUG_WARN David Sterba
2025-04-16  9:08 ` [PATCH 5/5] btrfs: convert ASSERT(0) to DEBUG_WARN() David Sterba
2025-04-16 10:55 ` [PATCH 0/5] Assertion and debugging helpers Qu Wenruo
2025-04-16 20:20   ` David Sterba
2025-04-16 22:30     ` David Sterba
2025-04-16 22:44       ` Qu Wenruo

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=cover.1744794336.git.dsterba@suse.com \
    --to=dsterba@suse.com \
    --cc=linux-btrfs@vger.kernel.org \
    /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