2018-09-07 17:28:55

by Igor Stoppa

[permalink] [raw]
Subject: [PATCH] ethernet: hnae: add unlikely() to assert()

The assert() condition is likely to be true.

Signed-off-by: Igor Stoppa <[email protected]>
Cc: huangdaode <[email protected]>
Cc: Yisen Zhuang <[email protected]>
Cc: Salil Mehta <[email protected]>
Cc: "David S. Miller" <[email protected]>
Cc: [email protected]
Cc: [email protected]
---
drivers/net/ethernet/hisilicon/hns/hnae.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/hisilicon/hns/hnae.h b/drivers/net/ethernet/hisilicon/hns/hnae.h
index 08a750fb60c4..bd3c180a3fe9 100644
--- a/drivers/net/ethernet/hisilicon/hns/hnae.h
+++ b/drivers/net/ethernet/hisilicon/hns/hnae.h
@@ -47,7 +47,7 @@
#ifndef assert
#define assert(expr) \
do { \
- if (!(expr)) { \
+ if (unlikely(!(expr))) { \
pr_err("Assertion failed! %s, %s, %s, line %d\n", \
#expr, __FILE__, __func__, __LINE__); \
} \
--
2.17.1



2018-09-07 21:49:22

by David Miller

[permalink] [raw]
Subject: Re: [PATCH] ethernet: hnae: add unlikely() to assert()

From: Igor Stoppa <[email protected]>
Date: Fri, 7 Sep 2018 20:26:50 +0300

> The assert() condition is likely to be true.

Worse than that is that drivers should not be definiting their own
private "assert()" macro.

I'd rather have that fixed instead. We have tons of standard kernel
facilities that do what they are trying to do here, without overloading
the standard C namespace in this way.

2018-09-08 15:05:15

by Igor Stoppa

[permalink] [raw]
Subject: Re: [PATCH] ethernet: hnae: add unlikely() to assert()



On 08/09/18 00:46, David Miller wrote:
> From: Igor Stoppa <[email protected]>
> Date: Fri, 7 Sep 2018 20:26:50 +0300
>
>> The assert() condition is likely to be true.
>
> Worse than that is that drivers should not be definiting their own
> private "assert()" macro.
>
> I'd rather have that fixed instead. We have tons of standard kernel
> facilities that do what they are trying to do here, without overloading
> the standard C namespace in this way.

I've converted into WARN() what passed the compile test.
In one case it didn't even compile :-(

I could only compile-test.

--
igor