On Fri, Feb 19, 2016 at 05:05:43PM -0500, Oleg Drokin wrote: > If we only change some_random_stuff to some_random_stuff() at location 2 - the warning also reappears and it's a false positive. > Even if we replace some_random_stuff with a at 2, warning reappears, and even if we only replace some_random_stuff at 1 with a - it reappears again. > But those all are false positives, not false negatives, so I guess we are fine. Ah.... I will look at this next week. > > > Wow. Sorry. We're really talking at cross purposes. I'm describing > > the flow analysis for drivers/staging/lustre/lustre/llite/lloop.c::loop_thread() > > not the test file. > > > > In loop_thread() we have a double unlock if "lo->lo_state == > > LLOOP_RUNDOWN", but that can only happen in another thread so Smatch > > (incorrectly) says it is impossible and ignores it. > > Ah, I see. now it all makes sense esp. since we assigned lo->lo_state earlier on in the function. > > I imagine it's possible to invalidate a whole struct state (except locks?) if > a lock within that structure was used as that implies there are other threads > at least potentially in place that mess with the structure content (though we don't > really know what parts of it). > I guess this will have it's good and bad sides, since we cannot really know which > parts of the structs really could be accessed by parallel threads so it might as well > lead to worse results than this false negative, so probably just needs to be tried or perhaps even > be retained as an option. I feel like the threads issue is maybe solveable, but probably it will take a year or two to do it properly... 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