Re: [PATCH bpf-next] bpf, mips: No need to use min() to get MAX_TAIL_CALL_CNT

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Fri, Aug 26, 2022 at 6:18 PM Daniel Borkmann <daniel@xxxxxxxxxxxxx> wrote:
>
> On 8/26/22 4:12 AM, Tiezhu Yang wrote:
> > MAX_TAIL_CALL_CNT is 33, so min(MAX_TAIL_CALL_CNT, 0xffff) is always
> > MAX_TAIL_CALL_CNT, it is better to use MAX_TAIL_CALL_CNT directly.
> >
> > Signed-off-by: Tiezhu Yang <yangtiezhu@xxxxxxxxxxx>
> > ---
> >   arch/mips/net/bpf_jit_comp32.c | 2 +-
> >   arch/mips/net/bpf_jit_comp64.c | 2 +-
> >   2 files changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/arch/mips/net/bpf_jit_comp32.c b/arch/mips/net/bpf_jit_comp32.c
> > index 83c975d..8fee671 100644
> > --- a/arch/mips/net/bpf_jit_comp32.c
> > +++ b/arch/mips/net/bpf_jit_comp32.c
> > @@ -1381,7 +1381,7 @@ void build_prologue(struct jit_context *ctx)
> >        * 16-byte area in the parent's stack frame. On a tail call, the
> >        * calling function jumps into the prologue after these instructions.
> >        */
> > -     emit(ctx, ori, MIPS_R_T9, MIPS_R_ZERO, min(MAX_TAIL_CALL_CNT, 0xffff));
>
> I presume this is the max that can be encoded, right? Maybe just convert this
> to a BUILD_BUG_ON(MAX_TAIL_CALL_CNT > 0xffff) with a comment on why the assertion
> is there?

Correct. The min() is there for a reason. In the unlikely event that
the TCC limit is raised to more than 16 bits, it is clamped to the
maximum value allowed for the generated code (0xffff). One can argue
that it is better fail to compile instead of degrading gracefully, but
some kind check should be there IMO.

>
> > +     emit(ctx, ori, MIPS_R_T9, MIPS_R_ZERO, MAX_TAIL_CALL_CNT);
> >       emit(ctx, sw, MIPS_R_T9, 0, MIPS_R_SP);
> >
> >       /*
> > diff --git a/arch/mips/net/bpf_jit_comp64.c b/arch/mips/net/bpf_jit_comp64.c
> > index 6475828..ac175af 100644
> > --- a/arch/mips/net/bpf_jit_comp64.c
> > +++ b/arch/mips/net/bpf_jit_comp64.c
> > @@ -552,7 +552,7 @@ void build_prologue(struct jit_context *ctx)
> >        * On a tail call, the calling function jumps into the prologue
> >        * after this instruction.
> >        */
> > -     emit(ctx, ori, tc, MIPS_R_ZERO, min(MAX_TAIL_CALL_CNT, 0xffff));
> > +     emit(ctx, ori, tc, MIPS_R_ZERO, MAX_TAIL_CALL_CNT);
> >
> >       /* === Entry-point for tail calls === */
> >
> >
>



[Index of Archives]     [LKML Archive]     [Linux ARM Kernel]     [Linux ARM]     [Git]     [Yosemite News]     [Linux SCSI]     [Linux Hams]

  Powered by Linux