All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Kirill A. Shutemov" <kirill@shutemov.name>
To: Peter Zijlstra <peterz@infradead.org>
Cc: torvalds@linux-foundation.org, paulmck@linux.vnet.ibm.com,
	tglx@linutronix.de, akpm@linux-foundation.org, riel@redhat.com,
	mgorman@suse.de, oleg@redhat.com, mingo@redhat.com,
	minchan@kernel.org, kamezawa.hiroyu@jp.fujitsu.com,
	viro@zeniv.linux.org.uk, laijs@cn.fujitsu.com, dave@stgolabs.net,
	linux-kernel@vger.kernel.org, linux-mm@kvack.org
Subject: Re: [RFC][PATCH 3/6] mm: VMA sequence count
Date: Thu, 23 Oct 2014 18:05:08 +0300	[thread overview]
Message-ID: <20141023150508.GA10316@node.dhcp.inet.fi> (raw)
In-Reply-To: <20141023142224.GL3219@twins.programming.kicks-ass.net>

On Thu, Oct 23, 2014 at 04:22:24PM +0200, Peter Zijlstra wrote:
> On Thu, Oct 23, 2014 at 03:36:16PM +0300, Kirill A. Shutemov wrote:
> > On Wed, Oct 22, 2014 at 03:44:16PM +0200, Peter Zijlstra wrote:
> > > On Wed, Oct 22, 2014 at 02:15:54PM +0200, Peter Zijlstra wrote:
> > > > On Wed, Oct 22, 2014 at 02:53:04PM +0300, Kirill A. Shutemov wrote:
> > > > > Em, no. In this case change_protection() will not touch the pte, since
> > > > > it's pte_none() and the pte_same() check will pass just fine.
> > > > 
> > > > Oh, that's what you meant. Yes that's a problem, yes vm_page_prot
> > > > needs wrapping too.
> > > 
> > > Maybe also vm_policy, is there anything else that can change while a vma
> > > lives?
> > 
> >  - vm_flags, obviously;
> 
> Do those ever change?

The flags which can change (probably incomplete):

 - prot-related: VM_READ, VM_WRITE, VM_EXEC -- mprotect();
 - VM_LOCKED - mlock();
 - VM_SEQ_READ, VM_RAND_READ, VM_DONTCOPY, VM_DONTDUMP, VM_HUGEPAGE,
   VM_NOHUGEPAGE, VM_MERGEABLE -- madvise();
 - VM_SOFTDIRTY -- through procfs;
 
> The only thing that jumps out is the VM_LOCKED thing and that should not
> really matter one way or the other, but sure can do.

I would not be that sure about VM_LOCKED. Consider munlock() vs. write
fault race.

static int do_wp_page(struct fault_env *fe)
        __releases(ptl)
{
...
err:
	if (old_page) {
		/*
		 * Don't let another task, with possibly unlocked vma,
		 * keep the mlocked page.
		 */
		if ((ret & VM_FAULT_WRITE) && (fe->vma->vm_flags & VM_LOCKED)) {
			lock_page(old_page);	/* LRU manipulation */
			munlock_vma_page(old_page);
			unlock_page(old_page);
		}
		page_cache_release(old_page);
	}
	return ret;
...
}

The page can leak out mlocked, iiuc.

Some other flags can be problematic too.

> In any case, yes I'll go include them.

I hope it will not hurt single-threaded workloads even more. :-/

-- 
 Kirill A. Shutemov

--
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>

WARNING: multiple messages have this Message-ID (diff)
From: "Kirill A. Shutemov" <kirill@shutemov.name>
To: Peter Zijlstra <peterz@infradead.org>
Cc: torvalds@linux-foundation.org, paulmck@linux.vnet.ibm.com,
	tglx@linutronix.de, akpm@linux-foundation.org, riel@redhat.com,
	mgorman@suse.de, oleg@redhat.com, mingo@redhat.com,
	minchan@kernel.org, kamezawa.hiroyu@jp.fujitsu.com,
	viro@zeniv.linux.org.uk, laijs@cn.fujitsu.com, dave@stgolabs.net,
	linux-kernel@vger.kernel.org, linux-mm@kvack.org
Subject: Re: [RFC][PATCH 3/6] mm: VMA sequence count
Date: Thu, 23 Oct 2014 18:05:08 +0300	[thread overview]
Message-ID: <20141023150508.GA10316@node.dhcp.inet.fi> (raw)
In-Reply-To: <20141023142224.GL3219@twins.programming.kicks-ass.net>

On Thu, Oct 23, 2014 at 04:22:24PM +0200, Peter Zijlstra wrote:
> On Thu, Oct 23, 2014 at 03:36:16PM +0300, Kirill A. Shutemov wrote:
> > On Wed, Oct 22, 2014 at 03:44:16PM +0200, Peter Zijlstra wrote:
> > > On Wed, Oct 22, 2014 at 02:15:54PM +0200, Peter Zijlstra wrote:
> > > > On Wed, Oct 22, 2014 at 02:53:04PM +0300, Kirill A. Shutemov wrote:
> > > > > Em, no. In this case change_protection() will not touch the pte, since
> > > > > it's pte_none() and the pte_same() check will pass just fine.
> > > > 
> > > > Oh, that's what you meant. Yes that's a problem, yes vm_page_prot
> > > > needs wrapping too.
> > > 
> > > Maybe also vm_policy, is there anything else that can change while a vma
> > > lives?
> > 
> >  - vm_flags, obviously;
> 
> Do those ever change?

The flags which can change (probably incomplete):

 - prot-related: VM_READ, VM_WRITE, VM_EXEC -- mprotect();
 - VM_LOCKED - mlock();
 - VM_SEQ_READ, VM_RAND_READ, VM_DONTCOPY, VM_DONTDUMP, VM_HUGEPAGE,
   VM_NOHUGEPAGE, VM_MERGEABLE -- madvise();
 - VM_SOFTDIRTY -- through procfs;
 
> The only thing that jumps out is the VM_LOCKED thing and that should not
> really matter one way or the other, but sure can do.

I would not be that sure about VM_LOCKED. Consider munlock() vs. write
fault race.

static int do_wp_page(struct fault_env *fe)
        __releases(ptl)
{
...
err:
	if (old_page) {
		/*
		 * Don't let another task, with possibly unlocked vma,
		 * keep the mlocked page.
		 */
		if ((ret & VM_FAULT_WRITE) && (fe->vma->vm_flags & VM_LOCKED)) {
			lock_page(old_page);	/* LRU manipulation */
			munlock_vma_page(old_page);
			unlock_page(old_page);
		}
		page_cache_release(old_page);
	}
	return ret;
...
}

The page can leak out mlocked, iiuc.

Some other flags can be problematic too.

> In any case, yes I'll go include them.

I hope it will not hurt single-threaded workloads even more. :-/

-- 
 Kirill A. Shutemov

  reply	other threads:[~2014-10-23 15:05 UTC|newest]

Thread overview: 88+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-10-20 21:56 [RFC][PATCH 0/6] Another go at speculative page faults Peter Zijlstra
2014-10-20 21:56 ` Peter Zijlstra
2014-10-20 21:56 ` [RFC][PATCH 1/6] mm: Dont assume page-table invariance during faults Peter Zijlstra
2014-10-20 21:56   ` Peter Zijlstra
2014-10-20 21:56 ` [RFC][PATCH 2/6] mm: Prepare for FAULT_FLAG_SPECULATIVE Peter Zijlstra
2014-10-20 21:56   ` Peter Zijlstra
2014-10-20 21:56 ` [RFC][PATCH 3/6] mm: VMA sequence count Peter Zijlstra
2014-10-20 21:56   ` Peter Zijlstra
2014-10-22 11:26   ` Kirill A. Shutemov
2014-10-22 11:26     ` Kirill A. Shutemov
2014-10-22 11:39     ` Peter Zijlstra
2014-10-22 11:39       ` Peter Zijlstra
2014-10-22 11:53       ` Kirill A. Shutemov
2014-10-22 11:53         ` Kirill A. Shutemov
2014-10-22 12:15         ` Peter Zijlstra
2014-10-22 12:15           ` Peter Zijlstra
2014-10-22 13:44           ` Peter Zijlstra
2014-10-22 13:44             ` Peter Zijlstra
2014-10-23 12:36             ` Kirill A. Shutemov
2014-10-23 12:36               ` Kirill A. Shutemov
2014-10-23 14:22               ` Peter Zijlstra
2014-10-23 14:22                 ` Peter Zijlstra
2014-10-23 15:05                 ` Kirill A. Shutemov [this message]
2014-10-23 15:05                   ` Kirill A. Shutemov
2014-10-20 21:56 ` [RFC][PATCH 4/6] SRCU free VMAs Peter Zijlstra
2014-10-20 21:56   ` Peter Zijlstra
2014-10-20 23:41   ` Linus Torvalds
2014-10-20 23:41     ` Linus Torvalds
2014-10-21  8:07     ` Peter Zijlstra
2014-10-21  8:07       ` Peter Zijlstra
2014-10-24 15:16       ` Christoph Lameter
2014-10-24 15:16         ` Christoph Lameter
2014-10-24 15:51         ` Peter Zijlstra
2014-10-24 15:51           ` Peter Zijlstra
2014-10-24 17:08           ` Christoph Lameter
2014-10-24 17:08             ` Christoph Lameter
2014-10-21  8:22     ` Peter Zijlstra
2014-10-21  8:22       ` Peter Zijlstra
2014-10-23 10:14   ` Lai Jiangshan
2014-10-23 10:14     ` Lai Jiangshan
2014-10-23 11:03     ` Peter Zijlstra
2014-10-23 11:03       ` Peter Zijlstra
2014-10-24  3:33       ` Lai Jiangshan
2014-10-24  3:33         ` Lai Jiangshan
2014-10-24  7:26         ` Peter Zijlstra
2014-10-24  7:26           ` Peter Zijlstra
2014-10-20 21:56 ` [RFC][PATCH 5/6] mm: Provide speculative fault infrastructure Peter Zijlstra
2014-10-20 21:56   ` Peter Zijlstra
2014-10-21  8:35   ` Kirill A. Shutemov
2014-10-21  8:35     ` Kirill A. Shutemov
2014-10-21 10:41     ` Peter Zijlstra
2014-10-21 10:41       ` Peter Zijlstra
2014-10-21 19:00   ` Peter Zijlstra
2014-10-21 19:00     ` Peter Zijlstra
2014-10-20 21:56 ` [RFC][PATCH 6/6] mm,x86: Add speculative pagefault handling Peter Zijlstra
2014-10-20 21:56   ` Peter Zijlstra
2014-10-21  0:07 ` [RFC][PATCH 0/6] Another go at speculative page faults Andy Lutomirski
2014-10-21  0:07   ` Andy Lutomirski
2014-10-21  8:11   ` Peter Zijlstra
2014-10-21  8:11     ` Peter Zijlstra
2014-10-21 16:23 ` Ingo Molnar
2014-10-21 16:23   ` Ingo Molnar
2014-10-21 17:09   ` Kirill A. Shutemov
2014-10-21 17:09     ` Kirill A. Shutemov
2014-10-21 17:56     ` Peter Zijlstra
2014-10-21 17:56       ` Peter Zijlstra
2014-10-23 10:40       ` Lai Jiangshan
2014-10-23 10:40         ` Lai Jiangshan
2014-10-23 11:04         ` Peter Zijlstra
2014-10-23 11:04           ` Peter Zijlstra
2014-10-24  7:54           ` Ingo Molnar
2014-10-24  7:54             ` Ingo Molnar
2014-10-24 13:14             ` Peter Zijlstra
2014-10-24 13:14               ` Peter Zijlstra
2014-10-28  5:32               ` Namhyung Kim
2014-10-28  5:32                 ` Namhyung Kim
2014-10-21 17:25   ` Peter Zijlstra
2014-10-21 17:25     ` Peter Zijlstra
2014-10-22 12:35     ` Ingo Molnar
2014-10-22 12:35       ` Ingo Molnar
2014-10-22  7:34 ` Davidlohr Bueso
2014-10-22  7:34   ` Davidlohr Bueso
2014-10-22 11:29   ` Kirill A. Shutemov
2014-10-22 11:29     ` Kirill A. Shutemov
2014-10-22 11:45     ` Peter Zijlstra
2014-10-22 11:45       ` Peter Zijlstra
2014-10-22 11:55       ` Kirill A. Shutemov
2014-10-22 11:55         ` Kirill A. Shutemov

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20141023150508.GA10316@node.dhcp.inet.fi \
    --to=kirill@shutemov.name \
    --cc=akpm@linux-foundation.org \
    --cc=dave@stgolabs.net \
    --cc=kamezawa.hiroyu@jp.fujitsu.com \
    --cc=laijs@cn.fujitsu.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=mgorman@suse.de \
    --cc=minchan@kernel.org \
    --cc=mingo@redhat.com \
    --cc=oleg@redhat.com \
    --cc=paulmck@linux.vnet.ibm.com \
    --cc=peterz@infradead.org \
    --cc=riel@redhat.com \
    --cc=tglx@linutronix.de \
    --cc=torvalds@linux-foundation.org \
    --cc=viro@zeniv.linux.org.uk \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.