On Tue, Dec 29, 2015 at 08:30:19PM -0500, Nicholas Krause wrote: > This fixes rcu locking for the else block in the function > terminate_walk by calling rcu_read_lock at the beginning > of this code in order to protect against concurrent writers > here in this else block while allowing multiple readers to > be able to read concurrently here. > > Signed-off-by: Nicholas Krause <xerofoify@xxxxxxxxx> Patch is complete garbage, and so's the vague "commit message". "concurrent writers"? Writers to _what_? We do not touch *ANY* potentially shared data objects from that point to the place where rcu_read_unlock() is done. All the 3 lines until rcu_read_unlock(). And yes, I can reconstruct your "thinking process" here - playing psychoproctologist isn't hard, just unpleasant: * Og see rcu_read_unlock() * Og see no rcu_read_lock() * Og add rcu_read_lock() * Og slap boilerplate "explanation" thingy in "commit message" thingy * Og send * Og contributed What you have not done after the second step is * post "why is that rcu_read_unlock() in terminate_walk() unpaired?" *OR* if you are willing to spend a few minutes to answer that question yourself: * check if rcu_read_lock() might have been already done * look around for apparently unpaired rcu_read_lock and rcu_read_unlock * see a couple of such rcu_read_lock() in different cases of path_init() and several rcu_read_unlock() in different cases of unlazy_walk(), unlazy_link() and terminate_walk() * note that these rcu_read_lock() in path_init() happen when there's LOOKUP_RCU in flags * see that it happens in *all* success cases of path_init() with LOOKUP_RCU in flags and only in those. * guess that LOOKUP_RCU might have something to do with that, especially since that case in terminate_walk() is *also* after finding LOOKUP_RCU, only in nd->flags this time. And it's followed by removing LOOKUP_RCU from there. * observe that nd->flags is set by path_init() and LOOKUP_RCU ends up there exactly in cases when rcu_read_lock() had been done. * look where else do we remove LOOKUP_RCU. See it done in unlazy_walk(), unlazy_link() - exact same places where we have apparently unpaired rcu_read_unlock(). * observe that rcu_read_unlock() happens on all paths in unlazy_walk(), and LOOKUP_RCU removal is unconditional there. * observe that rcu_read_unlock() and LOOKUP_RCU removal in unlazy_link() happen in the same branch, right next to each other (the other branch calls unlazy_walk()) * come to obvious hypothesis - that LOOKUP_RCU in nd->flags corresponds to doing the pathname resolution under rcu_read_lock() and dropping rcu_read_lock() goes along with removing LOOKUP_RCU from there. * verify it (observations above cover most of that work, but rechecking with explicit hypothesis to verify doesn't hurt). You don't change your MO, do you? You see something, don't spend any effort trying to understand what's there or asking what's going on; instead you post random crap patches, let the subsystem maintainers deal with it, nod thanks to any explanations of the reasons you were wrong and go on doing the same fucking thing again and again, despite any attempts of explanations. When people start filtering you out, you come up with yet another email address and continue the same damn thing from it. In your world it apparently constitutes "contributions" and you don't give a flying fuck about any evidence to the contrary... *plonk* *plonk* -- To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
