All of lore.kernel.org
 help / color / mirror / Atom feed
From: Denis Efremov <efremov@linux.com>
To: Andy Whitcroft <apw@canonical.com>, Joe Perches <joe@perches.com>
Cc: Andrew Morton <akpm@linux-foundation.org>, linux-kernel@vger.kernel.org
Subject: Re: [PATCH v2] checkpatch: check for nested unlikely calls
Date: Wed, 28 Aug 2019 18:36:53 +0300	[thread overview]
Message-ID: <9655ddea-a98d-073c-f130-e9345517f44f@linux.com> (raw)
In-Reply-To: <20190828133256.13630-1-efremov@linux.com>

On 8/28/19 4:32 PM, Denis Efremov wrote:
> IS_ERR, IS_ERR_OR_NULL, IS_ERR_VALUE already contain unlikely optimization
> internally. Thus, there is no point in calling these functions under
> likely/unlikely.

It looks like this rule could be extended with this list:
CHECK_DATA_CORRUPTION
GLOCK_BUG_ON
SPIN_BUG_ON
RWLOCK_BUG_ON
DCCP_BUG_ON
GEM_BUG_ON
BUG_ON
WARN
WARN_TAINT
WARN_ON_ONCE
WARN_ONCE
WARN_TAINT_ONCE
WARN_ON_SMP

However, grep shows that maybe only BUG_ON, WARN_ON, WARN, WARN_ON_ONCE worth checking:
git grep 'likely(\s*\(CHECK_DATA_CORRUPTION\|GLOCK_BUG_ON\|SPIN_BUG_ON\|RWLOCK_BUG_ON\|DCCP_BUG_ON\|GEM_BUG_ON\|BUG_ON\|WARN\|WARN_TAINT\|WARN_ON_ONCE\|WARN_ONCE\|WARN_TAINT_ONCE\|WARN_ON_SMP\)' .
drivers/gpu/drm/msm/disp/mdp5/mdp5_ctl.c:       if (unlikely(WARN_ON(!mixer))) {
drivers/gpu/drm/msm/disp/mdp5/mdp5_ctl.c:       if (unlikely(WARN_ON(ctl_cfg->count > MAX_CTL))) {
drivers/gpu/drm/msm/disp/mdp_format.c:  if (unlikely(WARN_ON(type >= CSC_MAX)))
drivers/net/ethernet/mellanox/mlx5/core/en_accel/ktls_tx.c:     if (unlikely(WARN_ON_ONCE(tls_ctx->netdev != netdev)))
drivers/net/wimax/i2400m/tx.c:          if (unlikely(WARN_ON(pad_buf == NULL
drivers/xen/events/events_base.c:       if (unlikely(WARN(irq >= nr_irqs, "Invalid irq %d!\n", irq)))
fs/open.c:      if (unlikely(WARN_ON(!f->f_op))) {
fs/xfs/xfs_buf.c:       if (unlikely(WARN_ON(!bp->b_ops || !bp->b_ops->magic[idx])))
fs/xfs/xfs_buf.c:       if (unlikely(WARN_ON(!bp->b_ops || !bp->b_ops->magic16[idx])))

> +# nested likely/unlikely calls
		if ($line =~ /\b(?:(?:un)?likely)\s*\(\s*!?\s*((?:IS_ERR(?:_OR_NULL|_VALUE)?)|(?:BUG_ON|WARN(?:_ON(?:_ONCE)?)?)))/) {
or maybe even to match all possible WARNs:
		if ($line =~ /\b(?:(?:un)?likely)\s*\(\s*!?\s*((?:IS_ERR(?:_OR_NULL|_VALUE)?)|(?:BUG_ON|WARN)))/) {
> +			WARN("LIKELY_MISUSE",
> +			     "nested (un)?likely calls, unlikely already used in $1 internally\n" . $herecurr);
> +		}

Any suggestions for v3?


Thanks,
Denis


      reply	other threads:[~2019-08-28 15:36 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-08-27 16:55 [PATCH] checkpatch: check for nested (un)?likely calls Denis Efremov
2019-08-27 17:21 ` Joe Perches
2019-08-27 17:32   ` Denis Efremov
2019-08-28 13:32 ` [PATCH v2] checkpatch: check for nested unlikely calls Denis Efremov
2019-08-28 15:36   ` Denis Efremov [this message]

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=9655ddea-a98d-073c-f130-e9345517f44f@linux.com \
    --to=efremov@linux.com \
    --cc=akpm@linux-foundation.org \
    --cc=apw@canonical.com \
    --cc=joe@perches.com \
    --cc=linux-kernel@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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.