On 04/15/2014 08:55 AM, Johannes Sixt wrote: > Am 4/14/2014 15:54, schrieb Michael Haggerty: >> The function remove_lock_file_on_signal() is used as a signal handler. >> It is not realistic to make the signal handler conform strictly to the >> C standard, which is very restrictive about what a signal handler is >> allowed to do. But let's increase the likelihood that it will work: >> >> The lock_file_list global variable and several fields from struct >> lock_file are used by the signal handler. Declare those values >> "volatile" to increase the chance that the signal handler will see a >> valid object state. > > Yes, it's important that the signal handler sees a valid object state, and > "volatile" can help here. But I think the reason why it helps is not > obvious, and it should be mentioned in the commit message: > > It is not so much that "volatile" forces the compiler to lay down each > access of the variable coded in C in the assembly code, but more > importantly, that "volatile" disallows any re-ordering of these accesses. > Then: > > - 'lock->active = 1' must be the last assignment during setup > > - 'lock->active = 0' must be the first assignment during tear-down. > > - Ideally, all members of struct lock_file should be "volatile". > > The last point is important because the compiler is allowed to re-order > accesses to non-"volatile" variables across "volatile" accesses. I say > "ideally" because if filename were defined "volatile filename[PATH_MAX]", > strcpy() could not be used anymore. OTOH, it is unlikely that a compiler > re-orders a strcpy() call across other expressions, and we can get away > without "volatile" in the "filename" case in practice. Thanks for the clarification. I will edit the commit message to better explain the rationale. >> Suggested-by: Johannes Sixt <j.sixt@xxxxxxxxxxxxx> > > Not a big deal, but just in case you re-roll again and you do not forget: > > Johannes Sixt <j6t@xxxxxxxx> > > is preferred. ACK. Michael -- Michael Haggerty mhagger@xxxxxxxxxxxx http://softwareswirl.blogspot.com/ -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html