On Wed 30-08-17 19:49:04, Andrea Arcangeli wrote: > Hello Michal, > > On Wed, Aug 30, 2017 at 10:46:00AM +0200, Michal Hocko wrote: > > + * TODO: we really want to get rid of this ugly hack and make sure that > > + * notifiers cannot block for unbounded amount of time and add > > + * mmu_notifier_invalidate_range_{start,end} around unmap_page_range > > KVM already should be ok in that respect. However the major reason to > prefer mmu_notifier_invalidate_range_start/end is those can block and > schedule waiting for stuff happening behind the PCI bus easily. So I'm > not sure if the TODO is good idea to keep. Long term, I was thinking about a flag to reflect that all registered notifiers are oom safe (aka they do not depend on memory allocations or any locks which depend on an allocation) and then we can call into notifiers. So the check would end up if (!mm_has_safe_notifiers(mm)) ... > > + */ > > + if (mm_has_notifiers(mm)) { > > + schedule_timeout_idle(HZ); > > Why the schedule_timeout? What's the difference with the OOM > reaper going to sleep again in the main loop instead? Well, this is what I had initially - basically to return false here and rely on oom_reap_task to retry. But my current understanding is that mm_has_notifiers is likely to be a semi-permanent state (once set it won't likely go away) so I figured it would be better to simply wait here and fail right away. If my assumption is not correct then I will simply return false here. > > > + goto unlock_oom; > > + } > > mm_has_notifiers stops changing after obtaining the mmap_sem for > reading. See the do_mmu_notifier_register. So it's better to put the > mm_has_notifiers check immediately after the below: > > > if (!down_read_trylock(&mm->mmap_sem)) { > > ret = false; > > trace_skip_task_reaping(tsk->pid); > > If we succeed taking the mmap_sem for reading then we read a stable > value out of mm_has_notifiers and be sure it won't be set from under > us. OK, I will move it. Thanks! -- Michal Hocko SUSE Labs -- To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to majordomo@xxxxxxxxx. For more info on Linux MM, see: http://www.linux-mm.org/ . Don't email: <a href=mailto:"dont@xxxxxxxxx"> email@xxxxxxxxx </a>