On Thu 26-03-15 13:53:48, Michal Hocko wrote: > On Wed 25-03-15 02:17:08, Johannes Weiner wrote: > > Disabling the OOM killer needs to exclude allocators from entering, > > not existing victims from exiting. > > The idea was that exit_oom_victim doesn't miss a waiter. > > exit_oom_victim is doing > atomic_dec_return(&oom_victims) && oom_killer_disabled) > > so there is a full (implicit) memory barrier befor oom_killer_disabled > check. The other part is trickier. oom_killer_disable does: > oom_killer_disabled = true; > up_write(&oom_sem); > > wait_event(oom_victims_wait, !atomic_read(&oom_victims)); > > up_write doesn't guarantee a full memory barrier AFAICS in > Documentation/memory-barriers.txt (although the generic and x86 > implementations seem to implement it as a full barrier) but wait_event > implies the full memory barrier (prepare_to_wait_event does spin > lock&unlock) before checking the condition in the slow path. This should > be sufficient and docummented... > > /* > * We do not need to hold oom_sem here because oom_killer_disable > * guarantees that oom_killer_disabled chage is visible before > * the waiter is put into sleep (prepare_to_wait_event) so > * we cannot miss a wake up. > */ > > in unmark_oom_victim() OK, I can see that the next patch removes oom_killer_disabled completely. The dependency won't be there and so the concerns about the memory barriers. Is there any reason why the ordering is done this way? It would sound more logical to me. -- Michal Hocko SUSE Labs -- 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
