Thread (47 messages) 47 messages, 5 authors, 2016-08-24

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>
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help