Re: [PATCH 09/10] vhost, mm: make sure that oom_reaper doesn't reap memory read by vhost
From: Michal Hocko <mhocko@kernel.org>
Date: 2016-08-12 09:42:42
Sorry to bother you Paul but I would be really greatful if you could comment on this, please! On Sun 31-07-16 11:44:38, Michal Hocko wrote:
On Fri 29-07-16 20:57:44, Michael S. Tsirkin wrote:quoted
On Fri, Jul 29, 2016 at 03:35:29PM +0200, Michal Hocko wrote:quoted
On Fri 29-07-16 16:14:10, Michael S. Tsirkin wrote:quoted
On Fri, Jul 29, 2016 at 08:04:22AM +0200, Michal Hocko wrote:quoted
On Thu 28-07-16 23:41:53, Michael S. Tsirkin wrote:quoted
On Thu, Jul 28, 2016 at 09:42:33PM +0200, Michal Hocko wrote:[...]quoted
quoted
quoted
quoted
and the reader would hit a page fault + * if it stumbled over a reaped memory.This last point I don't get. flag read could bypass data read if that happens data read could happen after unmap yes it might get a PF but you handle that, correct?The point I've tried to make is that if the reader really page faults then get_user will imply the full barrier already. If get_user didn't page fault then the state of the flag is not really important because the reaper shouldn't have touched it. Does it make more sense now or I've missed your question?Can task flag read happen before the get_user pagefault?Do you mean? get_user_mm() temp = false <- test_bit(MMF_UNSTABLE, &mm->flags) ret = __get_user(x, ptr) #PF if (!ret && temp) # misses the flag The code is basically doing if (!__get_user() && test_bit(MMF_UNSTABLE, &mm->flags)) so test_bit part of the conditional cannot be evaluated before __get_user() part is done. Compiler cannot reorder two depending subconditions AFAIK.But maybe the CPU can.Are you sure? How does that differ from if (ptr && ptr->something) construct? Let's CC Paul. Just to describe the situation. We have the following situation: #define __get_user_mm(mm, x, ptr) \ ({ \ int ___gu_err = __get_user(x, ptr); \ if (!___gu_err && test_bit(MMF_UNSTABLE, &mm->flags)) \ ___gu_err = -EFAULT; \ ___gu_err; \ }) and the oom reaper doing: set_bit(MMF_UNSTABLE, &mm->flags); for (vma = mm->mmap ; vma; vma = vma->vm_next) { unmap_page_range I assume that write memory barrier between set_bit and unmap_page_range is not really needed because unmapping should already imply the memory barrier. A read memory barrier between __get_user and test_bit shouldn't be really needed because we can tolerate a stale value if __get_user didn't #PF because we haven't unmapped that address obviously. If we unmapped it then __get_user would #PF and that should imply a full memory barrier as well. Now the question is whether a CPU can speculate and read the flag before we issue the #PF.
-- Michal Hocko SUSE Labs -- To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to majordomo@kvack.org. For more info on Linux MM, see: http://www.linux-mm.org/ . Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>