LoongArch: Remove unnecessary checks in bt_address()

[PATCH v1] LoongArch: Remove unnecessary checks in bt_address()

Posted by Tiezhu Yang 1 week, 3 days ago

According to the following function definitions, __kernel_text_address()
already checks __module_text_address(), so it should remove the check of
__module_text_address() in bt_address() at least.

int __kernel_text_address(unsigned long addr)
{
	if (kernel_text_address(addr))
		return 1;
	...
	return 0;
}

int kernel_text_address(unsigned long addr)
{
	bool no_rcu;
	int ret = 1;
	...
	if (is_module_text_address(addr))
		goto out;
	...
	return ret;
}

bool is_module_text_address(unsigned long addr)
{
	guard(rcu)();
	return __module_text_address(addr) != NULL;
}

Furthermore, since it returns ra at the end of bt_address() and check it
with __kernel_text_address() after calling bt_address(), so it can remove
the check of __kernel_text_address() in bt_address() too.

Just remove unnecessary checks, no functional changes intended.

Signed-off-by: Tiezhu Yang <yangtiezhu@loongson.cn>
---
 arch/loongarch/kernel/unwind_orc.c | 6 ------
 1 file changed, 6 deletions(-)

diff --git a/arch/loongarch/kernel/unwind_orc.c b/arch/loongarch/kernel/unwind_orc.c
index 0d5fa64a2225..3919dbd04c21 100644
--- a/arch/loongarch/kernel/unwind_orc.c
+++ b/arch/loongarch/kernel/unwind_orc.c
@@ -360,12 +360,6 @@ static inline unsigned long bt_address(unsigned long ra)
 {
 	extern unsigned long eentry;
 
-	if (__kernel_text_address(ra))
-		return ra;
-
-	if (__module_text_address(ra))
-		return ra;
-
 	if (ra >= eentry && ra < eentry +  EXCCODE_INT_END * VECSIZE) {
 		unsigned long func;
 		unsigned long type = (ra - eentry) / VECSIZE;
-- 
2.42.0

Re: [PATCH v1] LoongArch: Remove unnecessary checks in bt_address()

Posted by Huacai Chen 1 week, 2 days ago

Hi, Tiezhu,

On Tue, Dec 9, 2025 at 2:18 PM Tiezhu Yang <yangtiezhu@loongson.cn> wrote:
>
> According to the following function definitions, __kernel_text_address()
> already checks __module_text_address(), so it should remove the check of
> __module_text_address() in bt_address() at least.
>
> int __kernel_text_address(unsigned long addr)
> {
>         if (kernel_text_address(addr))
>                 return 1;
>         ...
>         return 0;
> }
>
> int kernel_text_address(unsigned long addr)
> {
>         bool no_rcu;
>         int ret = 1;
>         ...
>         if (is_module_text_address(addr))
>                 goto out;
>         ...
>         return ret;
> }
>
> bool is_module_text_address(unsigned long addr)
> {
>         guard(rcu)();
>         return __module_text_address(addr) != NULL;
> }
>
> Furthermore, since it returns ra at the end of bt_address() and check it
> with __kernel_text_address() after calling bt_address(), so it can remove
> the check of __kernel_text_address() in bt_address() too.
>
> Just remove unnecessary checks, no functional changes intended.
>
> Signed-off-by: Tiezhu Yang <yangtiezhu@loongson.cn>
> ---
>  arch/loongarch/kernel/unwind_orc.c | 6 ------
>  1 file changed, 6 deletions(-)
>
> diff --git a/arch/loongarch/kernel/unwind_orc.c b/arch/loongarch/kernel/unwind_orc.c
> index 0d5fa64a2225..3919dbd04c21 100644
> --- a/arch/loongarch/kernel/unwind_orc.c
> +++ b/arch/loongarch/kernel/unwind_orc.c
> @@ -360,12 +360,6 @@ static inline unsigned long bt_address(unsigned long ra)
>  {
>         extern unsigned long eentry;
>
> -       if (__kernel_text_address(ra))
> -               return ra;
> -
> -       if (__module_text_address(ra))
> -               return ra;
I think the correct way is to remove the __module_text_address()
condition but keep the __kernel_text_address() condition. Then return
0 at the end of this function, and remove the __kernel_text_address()
condition out of this function.


Huacai

> -
>         if (ra >= eentry && ra < eentry +  EXCCODE_INT_END * VECSIZE) {
>                 unsigned long func;
>                 unsigned long type = (ra - eentry) / VECSIZE;
> --
> 2.42.0
>

Re: [PATCH v1] LoongArch: Remove unnecessary checks in bt_address()

Posted by Tiezhu Yang 1 week, 2 days ago

On 2025/12/9 下午4:30, Huacai Chen wrote:
> Hi, Tiezhu,
> 
> On Tue, Dec 9, 2025 at 2:18 PM Tiezhu Yang <yangtiezhu@loongson.cn> wrote:
>>
...
>>          extern unsigned long eentry;
>>
>> -       if (__kernel_text_address(ra))
>> -               return ra;
>> -
>> -       if (__module_text_address(ra))
>> -               return ra;
> I think the correct way is to remove the __module_text_address()
> condition but keep the __kernel_text_address() condition. Then return
> 0 at the end of this function, and remove the __kernel_text_address()
> condition out of this function.

It can not remove the check of __kernel_text_address() after calling
bt_address() because it needs to validate the calculated address for
exception, then no need to keep the __kernel_text_address() condition
in bt_address() because it will check the PC outside bt_address().

Thanks,
Tiezhu

Re: [PATCH v1] LoongArch: Remove unnecessary checks in bt_address()

Posted by Bibo Mao 1 week, 2 days ago


On 2025/12/10 上午9:28, Tiezhu Yang wrote:
> On 2025/12/9 下午4:30, Huacai Chen wrote:
>> Hi, Tiezhu,
>>
>> On Tue, Dec 9, 2025 at 2:18 PM Tiezhu Yang <yangtiezhu@loongson.cn> 
>> wrote:
>>>
> ...
>>>          extern unsigned long eentry;
>>>
>>> -       if (__kernel_text_address(ra))
>>> -               return ra;
>>> -
>>> -       if (__module_text_address(ra))
>>> -               return ra;
>> I think the correct way is to remove the __module_text_address()
>> condition but keep the __kernel_text_address() condition. Then return
>> 0 at the end of this function, and remove the __kernel_text_address()
>> condition out of this function.
> 
> It can not remove the check of __kernel_text_address() after calling
> bt_address() because it needs to validate the calculated address for
> exception, then no need to keep the __kernel_text_address() condition
> in bt_address() because it will check the PC outside bt_address().
         state->pc = bt_address(pc);
         if (!state->pc) {
                 pr_err("cannot find unwind pc at %p\n", (void *)pc);
                 goto err;
         }

         if (!__kernel_text_address(state->pc))
                 goto err;
I guess that you both comes from different views :) one treats these 
piece of code into one, one only views function bt_address().

Especially with if (!state->pc), how could this happen?

Regards
Bibo Mao

> 
> Thanks,
> Tiezhu
>

Re: [PATCH v1] LoongArch: Remove unnecessary checks in bt_address()

Posted by Tiezhu Yang 1 week, 1 day ago

On 2025/12/10 上午10:25, Bibo Mao wrote:
> 
> 
> On 2025/12/10 上午9:28, Tiezhu Yang wrote:
>> On 2025/12/9 下午4:30, Huacai Chen wrote:
>>> Hi, Tiezhu,
>>>
>>> On Tue, Dec 9, 2025 at 2:18 PM Tiezhu Yang <yangtiezhu@loongson.cn> 
>>> wrote:
>>>>
>> ...
>>>>          extern unsigned long eentry;
>>>>
>>>> -       if (__kernel_text_address(ra))
>>>> -               return ra;
>>>> -
>>>> -       if (__module_text_address(ra))
>>>> -               return ra;
>>> I think the correct way is to remove the __module_text_address()
>>> condition but keep the __kernel_text_address() condition. Then return
>>> 0 at the end of this function, and remove the __kernel_text_address()
>>> condition out of this function.
>>
>> It can not remove the check of __kernel_text_address() after calling
>> bt_address() because it needs to validate the calculated address for
>> exception, then no need to keep the __kernel_text_address() condition
>> in bt_address() because it will check the PC outside bt_address().
>          state->pc = bt_address(pc);
>          if (!state->pc) {
>                  pr_err("cannot find unwind pc at %p\n", (void *)pc);
>                  goto err;
>          }
> 
>          if (!__kernel_text_address(state->pc))
>                  goto err;
> I guess that you both comes from different views :) one treats these 
> piece of code into one, one only views function bt_address().
> 
> Especially with if (!state->pc), how could this happen?

IMO, state->pc will be not 0 forever in practice, this check is just an
error path and can be removed too if possible.

Thanks,
Tiezhu

Re: [PATCH v1] LoongArch: Remove unnecessary checks in bt_address()

Posted by Huacai Chen 5 days, 18 hours ago

On Wed, Dec 10, 2025 at 4:00 PM Tiezhu Yang <yangtiezhu@loongson.cn> wrote:
>
> On 2025/12/10 上午10:25, Bibo Mao wrote:
> >
> >
> > On 2025/12/10 上午9:28, Tiezhu Yang wrote:
> >> On 2025/12/9 下午4:30, Huacai Chen wrote:
> >>> Hi, Tiezhu,
> >>>
> >>> On Tue, Dec 9, 2025 at 2:18 PM Tiezhu Yang <yangtiezhu@loongson.cn>
> >>> wrote:
> >>>>
> >> ...
> >>>>          extern unsigned long eentry;
> >>>>
> >>>> -       if (__kernel_text_address(ra))
> >>>> -               return ra;
> >>>> -
> >>>> -       if (__module_text_address(ra))
> >>>> -               return ra;
> >>> I think the correct way is to remove the __module_text_address()
> >>> condition but keep the __kernel_text_address() condition. Then return
> >>> 0 at the end of this function, and remove the __kernel_text_address()
> >>> condition out of this function.
> >>
> >> It can not remove the check of __kernel_text_address() after calling
> >> bt_address() because it needs to validate the calculated address for
> >> exception, then no need to keep the __kernel_text_address() condition
> >> in bt_address() because it will check the PC outside bt_address().
> >          state->pc = bt_address(pc);
> >          if (!state->pc) {
> >                  pr_err("cannot find unwind pc at %p\n", (void *)pc);
> >                  goto err;
> >          }
> >
> >          if (!__kernel_text_address(state->pc))
> >                  goto err;
> > I guess that you both comes from different views :) one treats these
> > piece of code into one, one only views function bt_address().
> >
> > Especially with if (!state->pc), how could this happen?
>
> IMO, state->pc will be not 0 forever in practice, this check is just an
> error path and can be removed too if possible.
bt_address() return ra both in "good path" and "bad path" is strange.
I still suggest using my method, but move __kernel_text_address()
after the "if (ra >= eentry && ra < eentry +  EXCCODE_INT_END *
VECSIZE)" block to verify the modified ra.

Huacai

>
> Thanks,
> Tiezhu
>

Re: [PATCH v1] LoongArch: Remove unnecessary checks in bt_address()

Posted by Tiezhu Yang 4 days, 17 hours ago

On 12/13/25 20:24, Huacai Chen wrote:
> On Wed, Dec 10, 2025 at 4:00 PM Tiezhu Yang <yangtiezhu@loongson.cn> wrote:
>>
>> On 2025/12/10 上午10:25, Bibo Mao wrote:
>>>
>>>
>>> On 2025/12/10 上午9:28, Tiezhu Yang wrote:
>>>> On 2025/12/9 下午4:30, Huacai Chen wrote:
>>>>> Hi, Tiezhu,
>>>>>
>>>>> On Tue, Dec 9, 2025 at 2:18 PM Tiezhu Yang <yangtiezhu@loongson.cn>
>>>>> wrote:
>>>>>>
>>>> ...
>>>>>>           extern unsigned long eentry;
>>>>>>
>>>>>> -       if (__kernel_text_address(ra))
>>>>>> -               return ra;
>>>>>> -
>>>>>> -       if (__module_text_address(ra))
>>>>>> -               return ra;
>>>>> I think the correct way is to remove the __module_text_address()
>>>>> condition but keep the __kernel_text_address() condition. Then return
>>>>> 0 at the end of this function, and remove the __kernel_text_address()
>>>>> condition out of this function.
>>>>
>>>> It can not remove the check of __kernel_text_address() after calling
>>>> bt_address() because it needs to validate the calculated address for
>>>> exception, then no need to keep the __kernel_text_address() condition
>>>> in bt_address() because it will check the PC outside bt_address().
>>>           state->pc = bt_address(pc);
>>>           if (!state->pc) {
>>>                   pr_err("cannot find unwind pc at %p\n", (void *)pc);
>>>                   goto err;
>>>           }
>>>
>>>           if (!__kernel_text_address(state->pc))
>>>                   goto err;
>>> I guess that you both comes from different views :) one treats these
>>> piece of code into one, one only views function bt_address().
>>>
>>> Especially with if (!state->pc), how could this happen?
>>
>> IMO, state->pc will be not 0 forever in practice, this check is just an
>> error path and can be removed too if possible.
> bt_address() return ra both in "good path" and "bad path" is strange.
> I still suggest using my method, but move __kernel_text_address()
> after the "if (ra >= eentry && ra < eentry +  EXCCODE_INT_END *
> VECSIZE)" block to verify the modified ra.

I am OK if you mean like this:

static inline unsigned long bt_address(unsigned long ra)
{
	extern unsigned long eentry;

	if (ra >= eentry && ra < eentry +  EXCCODE_INT_END * VECSIZE) {
		...
		ra = func + offset;
	}

	if (__kernel_text_address(ra))
		return ra;

	return 0;
}

bool unwind_next_frame(struct unwind_state *state)
{
	...
	state->pc = bt_address(pc);
	if (!state->pc) {
		pr_err("cannot find unwind pc at %p\n", (void *)pc);
		goto err;
	}

	return true;
	...
}

If so, I will send v2 later.

Thanks,
Tiezhu

Re: [PATCH v1] LoongArch: Remove unnecessary checks in bt_address()

Posted by Huacai Chen 4 days, 3 hours ago

On Sun, Dec 14, 2025 at 9:15 PM Tiezhu Yang <yangtiezhu@loongson.cn> wrote:
>
> On 12/13/25 20:24, Huacai Chen wrote:
> > On Wed, Dec 10, 2025 at 4:00 PM Tiezhu Yang <yangtiezhu@loongson.cn> wrote:
> >>
> >> On 2025/12/10 上午10:25, Bibo Mao wrote:
> >>>
> >>>
> >>> On 2025/12/10 上午9:28, Tiezhu Yang wrote:
> >>>> On 2025/12/9 下午4:30, Huacai Chen wrote:
> >>>>> Hi, Tiezhu,
> >>>>>
> >>>>> On Tue, Dec 9, 2025 at 2:18 PM Tiezhu Yang <yangtiezhu@loongson.cn>
> >>>>> wrote:
> >>>>>>
> >>>> ...
> >>>>>>           extern unsigned long eentry;
> >>>>>>
> >>>>>> -       if (__kernel_text_address(ra))
> >>>>>> -               return ra;
> >>>>>> -
> >>>>>> -       if (__module_text_address(ra))
> >>>>>> -               return ra;
> >>>>> I think the correct way is to remove the __module_text_address()
> >>>>> condition but keep the __kernel_text_address() condition. Then return
> >>>>> 0 at the end of this function, and remove the __kernel_text_address()
> >>>>> condition out of this function.
> >>>>
> >>>> It can not remove the check of __kernel_text_address() after calling
> >>>> bt_address() because it needs to validate the calculated address for
> >>>> exception, then no need to keep the __kernel_text_address() condition
> >>>> in bt_address() because it will check the PC outside bt_address().
> >>>           state->pc = bt_address(pc);
> >>>           if (!state->pc) {
> >>>                   pr_err("cannot find unwind pc at %p\n", (void *)pc);
> >>>                   goto err;
> >>>           }
> >>>
> >>>           if (!__kernel_text_address(state->pc))
> >>>                   goto err;
> >>> I guess that you both comes from different views :) one treats these
> >>> piece of code into one, one only views function bt_address().
> >>>
> >>> Especially with if (!state->pc), how could this happen?
> >>
> >> IMO, state->pc will be not 0 forever in practice, this check is just an
> >> error path and can be removed too if possible.
> > bt_address() return ra both in "good path" and "bad path" is strange.
> > I still suggest using my method, but move __kernel_text_address()
> > after the "if (ra >= eentry && ra < eentry +  EXCCODE_INT_END *
> > VECSIZE)" block to verify the modified ra.
>
> I am OK if you mean like this:
>
> static inline unsigned long bt_address(unsigned long ra)
> {
>         extern unsigned long eentry;
>
>         if (ra >= eentry && ra < eentry +  EXCCODE_INT_END * VECSIZE) {
>                 ...
>                 ra = func + offset;
>         }
>
>         if (__kernel_text_address(ra))
>                 return ra;
>
>         return 0;
> }
>
> bool unwind_next_frame(struct unwind_state *state)
> {
>         ...
>         state->pc = bt_address(pc);
>         if (!state->pc) {
>                 pr_err("cannot find unwind pc at %p\n", (void *)pc);
>                 goto err;
>         }
>
>         return true;
>         ...
> }
>
> If so, I will send v2 later.
Yes, this is what I want.

Huacai

>
> Thanks,
> Tiezhu
>
>