Steven Rostedt <rostedt@xxxxxxxxxxx> writes: > On Sun, 2012-04-29 at 16:14 +0200, Sasha Levin wrote: > >> A fix for that could be having the sysctl modifying a different var, >> and having ftrace_enabled from that under a lock, but I'm not sure if >> it's worth the work for the cleanup. > > That was my original plan, but it seemed too much of a hassle than it > was worth, as I needed to make sure the mirrored variable was in sync > with ftrace_enabled, otherwise it could be confusing when ftrace was not > working but sysctl showed ftrace set to 1. I don't see the problem you are trying to solve with your patches. What I do see is you have ignored one of the biggest problem with the current sysctl interface in that it is not easy to plug in your own code in the cases you need to before an update is made. (Locks permission checks, etc). You have also bloated struct ctl_table for no apparent reason. This current crop of patches was just sloppy. You showed a poor choice of function names and did not preserve necessary invariants when changing the code. It looks like you exchanged something that was a bit ugly for something that straight out encourages broken behavior. So I respectfully suggest you go back to the drawing board and figure out a solution that makes this class of function much easier to write in a bug free manner. Eric -- 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