2021-05-10 01:42:28

by zhouchuangao

Subject: [PATCH] kernel/module: Use BUG_ON instead of if condition followed by BUG

This patch comes from cocinelle warning.

BUG_ON uses unlikely in if(). Through disassembly, we can see that
brk #0x800 is compiled to the end of the function.
As you can see below:
......
ffffff8008660bec: d65f03c0 ret
ffffff8008660bf0: d4210000 brk #0x800

Usually, the condition in if () is not satisfied. For the
multi-stage pipeline, we do not need to perform fetch decode
and excute operation on brk instruction.

In my opinion, this can improve the efficiency of the
multi-stage pipeline.

Signed-off-by: zhouchuangao <[email protected]>
---
kernel/module.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/kernel/module.c b/kernel/module.c
index b5dd92e..faf9114 100644
--- a/kernel/module.c
+++ b/kernel/module.c
@@ -1014,8 +1014,7 @@ void __symbol_put(const char *symbol)
};

preempt_disable();
- if (!find_symbol(&fsa))
- BUG();
+ BUG_ON(!find_symbol(&fsa));
module_put(fsa.owner);
preempt_enable();
}
--
2.7.4

2021-05-12 13:20:44

by Jessica Yu

Subject: Re: [PATCH] kernel/module: Use BUG_ON instead of if condition followed by BUG

+++ zhouchuangao [09/05/21 18:38 -0700]:
>This patch comes from cocinelle warning.

Please include the output of the cocinelle warning here as well.

See commit 56149c8cd820 ("media: pci: saa7164-core.c: replace if
(cond) BUG() with BUG_ON()") for an example. Thanks.

>BUG_ON uses unlikely in if(). Through disassembly, we can see that
>brk #0x800 is compiled to the end of the function.
>As you can see below:
> ......
> ffffff8008660bec: d65f03c0 ret
> ffffff8008660bf0: d4210000 brk #0x800
>
>Usually, the condition in if () is not satisfied. For the
>multi-stage pipeline, we do not need to perform fetch decode
>and excute operation on brk instruction.
>
>In my opinion, this can improve the efficiency of the
>multi-stage pipeline.
>
>Signed-off-by: zhouchuangao <[email protected]>
>---
> kernel/module.c | 3 +--
> 1 file changed, 1 insertion(+), 2 deletions(-)
>
>diff --git a/kernel/module.c b/kernel/module.c
>index b5dd92e..faf9114 100644
>--- a/kernel/module.c
>+++ b/kernel/module.c
>@@ -1014,8 +1014,7 @@ void __symbol_put(const char *symbol)
> };
>
> preempt_disable();
>- if (!find_symbol(&fsa))
>- BUG();
>+ BUG_ON(!find_symbol(&fsa));
> module_put(fsa.owner);
> preempt_enable();
> }
>--
>2.7.4
>