bpf.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: jianghaoran <jianghaoran@kylinos.cn>
To: Hengqi Chen <hengqi.chen@gmail.com>
Cc: loongarch@lists.linux.dev, bpf@vger.kernel.org,
	kernel@xen0n.name,  chenhuacai@kernel.org,
	yangtiezhu@loongson.cn, jolsa@kernel.org, haoluo@google.com,
	 sdf@fomichev.me, kpsingh@kernel.org, john.fastabend@gmail.com,
	 yonghong.song@linux.dev, song@kernel.org, eddyz87@gmail.com,
	martin.lau@linux.dev,  andrii@kernel.org, daniel@iogearbox.net,
	ast@kernel.org
Subject: re:[PATCH v2 1/2] LoongArch: BPF: Optimize the calculation method of jmp_offset in the emit_bpf_tail_call function
Date: Tue, 22 Jul 2025 17:34:37 +0800	[thread overview]
Message-ID: <4443b6e55d74fd2b78e6790b0719b32e70a0df00.camel@kylinos.cn> (raw)
In-Reply-To: <CAEyhmHRUQV5JROOO+PyuZoLuFRrVJ-eeYH5hMf9gtVXW18aa8w@mail.gmail.com>





在 2025-07-16星期三的 09:31 +0800,Hengqi Chen写道:
> Hi Haoran,
> 
> On Tue, Jul 8, 2025 at 3:19 PM Haoran Jiang <
jianghaoran@kylinos.cn> > wrote:
> > 
> > The extra pass of bpf_int_jit_compile() skips JIT context initialization
> > which essentially skips offset calculation leaving out_offset = -1,
> > the jmp_offset in emit_bpf_tail_call is calculated
> > by #define jmp_offset (out_offset - (cur_offset)) is a negative number,
> > which does not meet expectations.The final generated assembly as follow.
> > 
> 
> "does not meet expectations" ? Simply "is wrong" ?
> 
> The subject line should be something like:
>   Fix jump offset calculation in tailcall
> 
> It's a fix, not optimization.
> 
> Other than that, feel free to add:
> Reviewed-by: Hengqi Chen <
hengqi.chen@gmail.com> >
I will make the modification in the next version.

> > 54:     bgeu            $a2, $t1, -8        # 0x0000004c
> > 58:     addi.d          $a6, $s5, -1
> > 5c:     bltz            $a6, -16            # 0x0000004c
> > 60:     alsl.d          $t2, $a2, $a1, 0x3
> > 64:     ld.d            $t2, $t2, 264
> > 68:     beq             $t2, $zero, -28     # 0x0000004c
> > 
> > Before apply this patch, the follow test case will reveal soft
> > lock issues.
> > 
> > cd tools/testing/selftests/bpf/
> > ./test_progs --allow=tailcalls/tailcall_bpf2bpf_1
> > 
> > dmesg:
> > watchdog: BUG: soft lockup - CPU#2 stuck for 26s!
> > [test_progs:25056]
> > 
> > Fixes: 5dc615520c4d ("LoongArch: Add BPF JIT support")
> > Signed-off-by: Haoran Jiang <
> > jianghaoran@kylinos.cn
> > >
> > ---
> >  arch/loongarch/net/bpf_jit.c | 21 ++++++---------------
> >  1 file changed, 6 insertions(+), 15 deletions(-)
> > 
> > diff --git a/arch/loongarch/net/bpf_jit.c
> > b/arch/loongarch/net/bpf_jit.c
> > index fa1500d4aa3e..5061bfc978f2 100644
> > --- a/arch/loongarch/net/bpf_jit.c
> > +++ b/arch/loongarch/net/bpf_jit.c
> > @@ -208,9 +208,7 @@ bool bpf_jit_supports_far_kfunc_call(void)
> >         return true;
> >  }
> > 
> > -/* initialized on the first pass of build_body() */
> > -static int out_offset = -1;
> > -static int emit_bpf_tail_call(struct jit_ctx *ctx)
> > +static int emit_bpf_tail_call(struct jit_ctx *ctx, int insn)
> >  {
> >         int off;
> >         u8 tcc = tail_call_reg(ctx);
> > @@ -220,9 +218,10 @@ static int emit_bpf_tail_call(struct
> > jit_ctx *ctx)
> >         u8 t2 = LOONGARCH_GPR_T2;
> >         u8 t3 = LOONGARCH_GPR_T3;
> >         const int idx0 = ctx->idx;
> > +       int tc_ninsn = 0;
> > 
> >  #define cur_offset (ctx->idx - idx0)
> > -#define jmp_offset (out_offset - (cur_offset))
> > +#define jmp_offset (tc_ninsn - (cur_offset))
> > 
> >         /*
> >          * a0: &ctx
> > @@ -232,6 +231,8 @@ static int emit_bpf_tail_call(struct
> > jit_ctx *ctx)
> >          * if (index >= array->map.max_entries)
> >          *       goto out;
> >          */
> > +       tc_ninsn = insn ? ctx->offset[insn+1] - ctx-
> > >offset[insn] :
> > +               ctx->offset[0];
> >         off = offsetof(struct bpf_array, map.max_entries);
> >         emit_insn(ctx, ldwu, t1, a1, off);
> >         /* bgeu $a2, $t1, jmp_offset */
> > @@ -263,15 +264,6 @@ static int emit_bpf_tail_call(struct
> > jit_ctx *ctx)
> >         emit_insn(ctx, ldd, t3, t2, off);
> >         __build_epilogue(ctx, true);
> > 
> > -       /* out: */
> > -       if (out_offset == -1)
> > -               out_offset = cur_offset;
> > -       if (cur_offset != out_offset) {
> > -               pr_err_once("tail_call out_offset = %d,
> > expected %d!\n",
> > -                           cur_offset, out_offset);
> > -               return -1;
> > -       }
> > -
> >         return 0;
> > 
> >  toofar:
> > @@ -916,7 +908,7 @@ static int build_insn(const struct bpf_insn
> > *insn, struct jit_ctx *ctx, bool ext
> >         /* tail call */
> >         case BPF_JMP | BPF_TAIL_CALL:
> >                 mark_tail_call(ctx);
> > -               if (emit_bpf_tail_call(ctx) < 0)
> > +               if (emit_bpf_tail_call(ctx, i) < 0)
> >                         return -EINVAL;
> >                 break;
> > 
> > @@ -1342,7 +1334,6 @@ struct bpf_prog
> > *bpf_int_jit_compile(struct bpf_prog *prog)
> >         if (tmp_blinded)
> >                 bpf_jit_prog_release_other(prog, prog ==
> > orig_prog ? tmp : orig_prog);
> > 
> > -       out_offset = -1;
> > 
> >         return prog;
> > 
> > --
> > 2.43.0
> > 
> > 


  reply	other threads:[~2025-07-22  9:34 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-07-08  7:18 [PATCH v2 0/2] Fix two tailcall-related issues Haoran Jiang
2025-07-08  7:18 ` [PATCH v2 1/2] LoongArch: BPF: Optimize the calculation method of jmp_offset in the emit_bpf_tail_call function Haoran Jiang
2025-07-16  1:31   ` Hengqi Chen
2025-07-22  9:34     ` jianghaoran [this message]
2025-07-08  7:18 ` [PATCH v2 2/2] LoongArch: BPF: Fix tailcall hierarchy Haoran Jiang
2025-07-16  2:28   ` Hengqi Chen
2025-07-22  9:56     ` re:[PATCH " jianghaoran

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=4443b6e55d74fd2b78e6790b0719b32e70a0df00.camel@kylinos.cn \
    --to=jianghaoran@kylinos.cn \
    --cc=andrii@kernel.org \
    --cc=ast@kernel.org \
    --cc=bpf@vger.kernel.org \
    --cc=chenhuacai@kernel.org \
    --cc=daniel@iogearbox.net \
    --cc=eddyz87@gmail.com \
    --cc=haoluo@google.com \
    --cc=hengqi.chen@gmail.com \
    --cc=john.fastabend@gmail.com \
    --cc=jolsa@kernel.org \
    --cc=kernel@xen0n.name \
    --cc=kpsingh@kernel.org \
    --cc=loongarch@lists.linux.dev \
    --cc=martin.lau@linux.dev \
    --cc=sdf@fomichev.me \
    --cc=song@kernel.org \
    --cc=yangtiezhu@loongson.cn \
    --cc=yonghong.song@linux.dev \
    /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).