Re: "warning: context imbalance" in kernel/bpf/hashtab.c

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

 



On Sat, Nov 07, 2020 at 12:33:52PM -0800, Linus Torvalds wrote:
> On Sat, Nov 7, 2020 at 12:09 PM Luc Van Oostenryck
> <luc.vanoostenryck@xxxxxxxxx> wrote:
> >
> > Yes, once the SEL(SEL(x,C,0),C,0) is handled this all simplify to
> >         .L0:
> >                  <entry-point>
> >                  select.32   %r6 <- %arg1, $0xffffffff, $0
> >                  cbr         %arg1, .L6, .L4
> >         .L4:
> >                  call        spin_lock
> >                  # call      %r4 <- errno, %r1
> >                  call        spin_unlock
> >                  br          .L6
> >         .L6:
> >                  ret.32      %r6
> >
> > but to me, this seems a very very special case, so not much interesting.
> 
> Oh, ok, so just fixing the missing simplification does actually fix it.

This case, yes. And I suppose will solves the problem Song had.
But, for example, none of the select simplifications I posted
yesterday (not the 'move-everything-up', of course) have any effect
on v5.10-rc1. I mean, I'm sure it they have some effect on the
generated code but not on the context imbalance warnings (or any
other warning).

> And I'm not sure how special-case this is. Because I think we _do_
> actually generally doing the if-conversion "later", even if it's not a
> separate phase, simply because we end up doing it at the "OP_PHI"
> instruction, which should always end up linearizing after the branches
> that should have been simplified.

Yes, but the branch simplifications depend on the simplifications
that can be made on the condition. For the moment there is a focus
on select because it often happens when the condition is simple but
for these context imbalance the general case is something like:
	stuff
	if (arbitrary condition)
		take lock
	do other stuff
	if (same arbitrary condition)
		release lock

and the problem is (at least) twofold (but partially related):
1) recognize that the second condition is the same as the first one
2) avoid that partial optimizations of the first condition 'leak'
   into the common part because this often inhibits doing 1)
   [Sorry, if this is not very clear. I need to find some small
    examples to illustrate this].

When the condition is very simple, it is converted into a select
but very often it's more complex, involves a memory access and/or
some function calls or some asm.

 
> So sometimes the ordering can be simply due to the order we see and
> react to instructions.
> 
> > > Have we done that if_convert_phi() simplification too early?
> >
> > Absolutely. I've often observed that it inhibits other simplifications
> > related to conditionals. And, even when it doesn't, I think that many
> > select simplifications would already be handled at SETxx + CBR level
> > (like your previous SEL(x,x,0)).
> 
> Yeah, the problem with doing multiple passes, though, is that you
> inevitably end up wanting to repeat them, because a later phase will
> cause you to be able to then do earlier phases again.
> 
> So there's never any one "correct" order to do things in, and I think
> you always end up with those "oh, unlucky, we did that one
> simplification that then made another one harder" regardless of what
> you do.

Yes, sure, I think it's in general inevitable. But I think that for the
if-conversion here it wouldn't be much of a problem because, in itself,
it doesn't create other 'functional'/value-related simplifications.
It only may create an empty BB that can then be simplified away with
some branch simplifications. Maybe I'm just an optimist.

But sure, it's hard to predict exactly the effect. I should experiment
a bit to see the impact on quality, complexity and time.

-- Luc



[Index of Archives]     [Newbies FAQ]     [LKML]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Trinity Fuzzer Tool]

  Powered by Linux