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:
> > On Fri, Jul 29, 2016 at 03:35:29PM +0200, Michal Hocko wrote:
> > > On Fri 29-07-16 16:14:10, Michael S. Tsirkin wrote:
> > > > On Fri, Jul 29, 2016 at 08:04:22AM +0200, Michal Hocko wrote:
> > > > > On Thu 28-07-16 23:41:53, Michael S. Tsirkin wrote:
> > > > > > On Thu, Jul 28, 2016 at 09:42:33PM +0200, Michal Hocko wrote:
> > > [...]
> > > > > > > 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@xxxxxxxxx. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@xxxxxxxxx";> email@xxxxxxxxx </a>