linux-arch.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Chunguang Xu <brookxu.cn@gmail.com>
To: arnd@arndb.de
Cc: rppt@kernel.org, linux-arch@vger.kernel.org,
	linux-kernel@vger.kernel.org
Subject: [PATCH 00/23] clean up the code related to ASSERT()
Date: Thu, 27 Aug 2020 18:14:05 +0800	[thread overview]
Message-ID: <cover.1598518912.git.brookxu@tencent.com> (raw)

The kernel has not yet defined ASSERT(). Indeed, BUG() and WARN() are very
clear and can cover most application scenarios. However, some applications
require more debugging information and similar behavior to assert(), which
cannot be directly provided by BUG() and WARN().

Therefore, many modules independently implement ASSERT(), and most of them
are similar, but slightly different, such as:

 #define ASSERT(expr) \
         if(!(expr)) { \
                 printk( "\n" __FILE__ ":%d: Assertion " #expr " failed!\n",__LINE__); \
                 panic(#expr); \
         }

 #define ASSERT(x)                                                       \
 do {                                                                    \
         if (!(x)) {                                                     \
                 printk(KERN_EMERG "assertion failed %s: %d: %s\n",      \
                        __FILE__, __LINE__, #x);                         \
                 BUG();                                                  \
         }                                                               \
 } while (0)

Some implementations are not optimal for instruction prediction, such as
missing unlikely():

 #define assert(expr) \
         if(!(expr)) { \
         printk( "Assertion failed! %s,%s,%s,line=%d\n",\
         #expr,__FILE__,__func__,__LINE__); \
         BUG(); \
         }

Some implementations have too little log content information, such as:

 #define ASSERT(X)                                               \
 do {                                                            \
        if (unlikely(!(X))) {                                   \
                printk(KERN_ERR "\n");                          \
                printk(KERN_ERR "XXX: Assertion failed\n");     \
                BUG();                                          \
        }                                                       \
 } while(0)

As we have seen, This makes the code redundant and inconvenient to
troubleshoot the system. Therefore, perhaps we need to define two
wrappers for BUG() and WARN_ON(), such as ASSERT_FAIL() and
ASSERT_WARN(), provide the implementation of ASSERT(), simplify the
code and facilitate problem analysis.

Maybe I missed some information, but I think there is a need to clean
up the code, maybe in other ways, and more discussion is needed here.
If this approach is reasonable, I will clean up these codes later and
issue related patches.

Chunguang Xu (23):
  include/asm-generic/bug.h: add ASSERT_FAIL() and ASSERT_WARN() wrapper
  ia64: use ASSERT_FAIL()/ASSERT_WARN() to cleanup some code
  KVM: use ASSERT_FAIL()/ASSERT_WARN() to cleanup some code
  fore200e: use ASSERT_FAIL()/ASSERT_WARN() to cleanup some code
  scsi: use ASSERT_FAIL()/ASSERT_WARN() to cleanup some code
  rxrpc: use ASSERT_FAIL()/ASSERT_WARN() to cleanup some code
  lib/mpi: use ASSERT_FAIL()/ASSERT_WARN() to cleanup some code
  jfs: use ASSERT_FAIL()/ASSERT_WARN() to cleanup some code
  cachefiles: use ASSERT_FAIL()/ASSERT_WARN() to cleanup some code
  btrfs: use ASSERT_FAIL()/ASSERT_WARN() to cleanup some code
  afs: use ASSERT_FAIL()/ASSERT_WARN() to cleanup some code
  rivafb: use ASSERT_FAIL()/ASSERT_WARN() to cleanup some code
  nvidia: use ASSERT_FAIL()/ASSERT_WARN() to cleanup some code
  fbdev/cirrusfb:: use ASSERT_FAIL()/ASSERT_WARN() to cleanup some code
  media/staging: use ASSERT_FAIL()/ASSERT_WARN() to cleanup some code
  sym53c8xx: use ASSERT_FAIL()/ASSERT_WARN() to cleanup some code
  8139too: use ASSERT_FAIL()/ASSERT_WARN() to cleanup some code
  net:hns: use ASSERT_FAIL()/ASSERT_WARN() to cleanup some code
  block/sx8: use ASSERT_FAIL()/ASSERT_WARN() to cleanup some code
  skb: use ASSERT_FAIL()/ASSERT_WARN() to cleanup some code
  ext4: use ASSERT_FAIL()/ASSERT_WARN() to cleanup some code
  rbd: use ASSERT_FAIL()/ASSERT_WARN() to cleanup some code
  ALSA: asihpi: use ASSERT_FAIL()/ASSERT_WARN() to cleanup some code

 arch/ia64/hp/common/sba_iommu.c                    |  6 +---
 arch/x86/kvm/ioapic.h                              |  9 +-----
 drivers/atm/fore200e.c                             |  6 +---
 drivers/block/rbd.c                                |  9 +-----
 drivers/block/skd_main.c                           |  8 +-----
 drivers/block/sx8.c                                |  6 +---
 drivers/net/ethernet/hisilicon/hns/hnae.h          |  8 +-----
 drivers/net/ethernet/realtek/8139too.c             |  6 +---
 drivers/scsi/megaraid/mega_common.h                | 10 ++-----
 drivers/scsi/sym53c8xx_2/sym_hipd.h                |  9 +-----
 .../pci/hive_isp_css_include/assert_support.h      |  6 +---
 drivers/video/fbdev/cirrusfb.c                     |  6 +---
 drivers/video/fbdev/nvidia/nvidia.c                |  7 +----
 drivers/video/fbdev/riva/fbdev.c                   |  7 +----
 fs/afs/internal.h                                  |  9 +-----
 fs/btrfs/ctree.h                                   | 12 +-------
 fs/cachefiles/internal.h                           |  9 +-----
 fs/ext4/mballoc.c                                  | 10 +------
 fs/jfs/jfs_debug.h                                 | 13 +--------
 include/asm-generic/bug.h                          | 33 ++++++++++++++++++++++
 lib/mpi/mpi-internal.h                             |  7 +----
 net/rxrpc/ar-internal.h                            |  8 +-----
 sound/pci/asihpi/hpidebug.h                        |  8 +-----
 23 files changed, 56 insertions(+), 156 deletions(-)

-- 
1.8.3.1


             reply	other threads:[~2020-08-27 10:14 UTC|newest]

Thread overview: 31+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-08-27 10:14 Chunguang Xu [this message]
2020-08-27 10:14 ` [PATCH 01/23] include/asm-generic/bug.h: add ASSERT_FAIL() and ASSERT_WARN() wrapper Chunguang Xu
2020-08-27 10:14 ` [PATCH 02/23] ia64: use ASSERT_FAIL()/ASSERT_WARN() to cleanup some code Chunguang Xu
2020-08-27 10:14 ` [PATCH 03/23] KVM: " Chunguang Xu
2020-08-27 10:14 ` [PATCH 04/23] fore200e: " Chunguang Xu
2020-08-27 10:14 ` [PATCH 05/23] scsi: " Chunguang Xu
2020-08-27 10:14 ` [PATCH 06/23] rxrpc: " Chunguang Xu
2020-08-27 17:10   ` kernel test robot
2020-08-27 18:03   ` kernel test robot
2020-08-28  6:05   ` kernel test robot
2020-08-27 10:14 ` [PATCH 07/23] lib/mpi: " Chunguang Xu
2020-08-27 10:14 ` [PATCH 08/23] jfs: " Chunguang Xu
2020-08-27 16:13   ` kernel test robot
2020-08-27 10:14 ` [PATCH 09/23] cachefiles: " Chunguang Xu
2020-08-28  4:30   ` kernel test robot
2020-08-27 10:14 ` [PATCH 10/23] btrfs: " Chunguang Xu
2020-08-27 10:14 ` [PATCH 11/23] afs: " Chunguang Xu
2020-08-27 16:29   ` kernel test robot
2020-08-28  9:49   ` kernel test robot
2020-08-27 10:14 ` [PATCH 12/23] rivafb: " Chunguang Xu
2020-08-27 10:14 ` [PATCH 13/23] nvidia: " Chunguang Xu
2020-08-27 10:14 ` [PATCH 14/23] fbdev/cirrusfb:: " Chunguang Xu
2020-08-27 10:14 ` [PATCH 15/23] media/staging: " Chunguang Xu
2020-08-27 10:14 ` [PATCH 16/23] sym53c8xx: " Chunguang Xu
2020-08-27 10:14 ` [PATCH 17/23] 8139too: " Chunguang Xu
2020-08-27 10:14 ` [PATCH 18/23] net:hns: " Chunguang Xu
2020-08-27 10:14 ` [PATCH 19/23] block/sx8: " Chunguang Xu
2020-08-27 10:14 ` [PATCH 20/23] skb: " Chunguang Xu
2020-08-27 10:14 ` [PATCH 21/23] ext4: " Chunguang Xu
2020-08-27 10:14 ` [PATCH 22/23] rbd: " Chunguang Xu
2020-08-27 10:14 ` [PATCH 23/23] ALSA: asihpi: " Chunguang 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=cover.1598518912.git.brookxu@tencent.com \
    --to=brookxu.cn@gmail.com \
    --cc=arnd@arndb.de \
    --cc=linux-arch@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=rppt@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;
as well as URLs for NNTP newsgroup(s).