On Thu, Feb 18, 2016 at 05:35:47PM -0500, Oleg Drokin wrote:
>
> On Feb 18, 2016, at 5:04 PM, Dan Carpenter wrote:
>
> > On Wed, Feb 17, 2016 at 10:47:45AM -0500, Oleg Drokin wrote:
> >> Does not matter, I noticed the lack of the function call and changed it to be one and did not
> >> see any difference in smatch output.
> >>
> >
> > For me it works... I get a double unlock bug.
>
> I think we are talking about different things.
> I get double unlock bug too every time. Unless your patch is applied and then I don't.
>
I'm totally lost. What do you want it to do?
> >> After all this is just a small attempt at distilling down the actual real world code
> >> (see live example in drivers/staging/lustre/lustre/llite/lloop.c::loop_thread() )
> >
> > It's saying that that loop is a forever loop because we set
> > "lo->lo_state = LLOOP_BOUND;" at the start of the function and only
> > exit when it is set to LLOOP_RUNDOWN which I guess hapens in another
> > thread?
> >
> > Smatch doesn't understand about threads... I don't even have a roadmap
> > in my head how to handle this situation.
>
> I understand.
> That's why it should always be complaining about the double unlock, right? With your patch
> that tracks impossible states it no longer does this.
>
No. It's working (if you ignore threads).
// not locked.
goto out;
up();
// state is unlocked
for (;;) {
if (impossible)
break;
}
// state is impossible
out:
// state is merged (impossible + start_state) so calling up is
// fine
up();
> >>> If you unlock a (&start_state + &unlocked) then it probably should
> >>> generate a warning but it doesn't... Check_locking is one of the oldest
> >>> remaining checks and it needs to be re-written.
> >> I actually noticed it has several modes of operation, sometimes just warning about
> >> inconsistent locking and sometimes actually elaborating that here, here and here
> >> we return locking, and there, thee and there we return unlocked.
> > Yeah. It's not beautiful. The main thing though is that it doesn't do
> > cross function analysis…
>
> The other parts of the code do, as in they know what function changes
> what, don't they?
Yeah. Because the states are named after variables but for check
the states are named "mutex:&var".
regards,
dan carpenter
--
To unsubscribe from this list: send the line "unsubscribe smatch" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
