From: Vladimir Davydov <vdavydov@parallels.com>
To: Andrew Morton <akpm@linux-foundation.org>
Cc: oleg@redhat.com, rientjes@google.com, cl@linux.com,
linux-mm@kvack.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH 2/3] fork: reset mm->pinned_vm
Date: Fri, 20 Jun 2014 11:38:38 +0400 [thread overview]
Message-ID: <20140620073838.GA5387@esperanza> (raw)
In-Reply-To: <20140619135820.57c4934dd613c5e723f9ca82@linux-foundation.org>
On Thu, Jun 19, 2014 at 01:58:20PM -0700, Andrew Morton wrote:
> On Thu, 19 Jun 2014 13:07:47 +0400 Vladimir Davydov <vdavydov@parallels.com> wrote:
>
> > mm->pinned_vm counts pages of mm's address space that were permanently
> > pinned in memory by increasing their reference counter. The counter was
> > introduced by commit bc3e53f682d9 ("mm: distinguish between mlocked and
> > pinned pages"), while before it locked_vm had been used for such pages.
> >
> > Obviously, we should reset the counter on fork if !CLONE_VM, just like
> > we do with locked_vm, but currently we don't. Let's fix it.
> >
> > ...
> >
> > --- a/kernel/fork.c
> > +++ b/kernel/fork.c
> > @@ -534,6 +534,7 @@ static struct mm_struct *mm_init(struct mm_struct *mm, struct task_struct *p)
> > atomic_long_set(&mm->nr_ptes, 0);
> > mm->map_count = 0;
> > mm->locked_vm = 0;
> > + mm->pinned_vm = 0;
> > memset(&mm->rss_stat, 0, sizeof(mm->rss_stat));
> > spin_lock_init(&mm->page_table_lock);
> > mm_init_cpumask(mm);
>
> What are the runtime effects of this? I think it is only
> "/proc/pid/status:VmPin is screwed up", because we don't use vm_pinned
> in rlimit checks. Yes?
Hmm, ib_umem_get[infiniband] and perf_mmap still check pinned_vm against
RLIMIT_MEMLOCK. It's left from the times when pinned pages were
accounted under locked_vm, but today it looks wrong. It isn't clear to
me how we should deal with it.
And BTW, we still have some drivers accounting pinned pages under
mm->locked_vm - this is what commit bc3e53f682d9 was fighting against.
It's infiniband/usnic and vfio.
Thanks.
--
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: Vladimir Davydov <vdavydov@parallels.com>
To: Andrew Morton <akpm@linux-foundation.org>
Cc: <oleg@redhat.com>, <rientjes@google.com>, <cl@linux.com>,
<linux-mm@kvack.org>, <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH 2/3] fork: reset mm->pinned_vm
Date: Fri, 20 Jun 2014 11:38:38 +0400 [thread overview]
Message-ID: <20140620073838.GA5387@esperanza> (raw)
In-Reply-To: <20140619135820.57c4934dd613c5e723f9ca82@linux-foundation.org>
On Thu, Jun 19, 2014 at 01:58:20PM -0700, Andrew Morton wrote:
> On Thu, 19 Jun 2014 13:07:47 +0400 Vladimir Davydov <vdavydov@parallels.com> wrote:
>
> > mm->pinned_vm counts pages of mm's address space that were permanently
> > pinned in memory by increasing their reference counter. The counter was
> > introduced by commit bc3e53f682d9 ("mm: distinguish between mlocked and
> > pinned pages"), while before it locked_vm had been used for such pages.
> >
> > Obviously, we should reset the counter on fork if !CLONE_VM, just like
> > we do with locked_vm, but currently we don't. Let's fix it.
> >
> > ...
> >
> > --- a/kernel/fork.c
> > +++ b/kernel/fork.c
> > @@ -534,6 +534,7 @@ static struct mm_struct *mm_init(struct mm_struct *mm, struct task_struct *p)
> > atomic_long_set(&mm->nr_ptes, 0);
> > mm->map_count = 0;
> > mm->locked_vm = 0;
> > + mm->pinned_vm = 0;
> > memset(&mm->rss_stat, 0, sizeof(mm->rss_stat));
> > spin_lock_init(&mm->page_table_lock);
> > mm_init_cpumask(mm);
>
> What are the runtime effects of this? I think it is only
> "/proc/pid/status:VmPin is screwed up", because we don't use vm_pinned
> in rlimit checks. Yes?
Hmm, ib_umem_get[infiniband] and perf_mmap still check pinned_vm against
RLIMIT_MEMLOCK. It's left from the times when pinned pages were
accounted under locked_vm, but today it looks wrong. It isn't clear to
me how we should deal with it.
And BTW, we still have some drivers accounting pinned pages under
mm->locked_vm - this is what commit bc3e53f682d9 was fighting against.
It's infiniband/usnic and vfio.
Thanks.
next prev parent reply other threads:[~2014-06-20 7:38 UTC|newest]
Thread overview: 12+ messages / expand[flat|nested] mbox.gz Atom feed top
2014-06-19 9:07 [PATCH 1/3] fork/exec: cleanup mm initialization Vladimir Davydov
2014-06-19 9:07 ` Vladimir Davydov
2014-06-19 9:07 ` [PATCH 2/3] fork: reset mm->pinned_vm Vladimir Davydov
2014-06-19 9:07 ` Vladimir Davydov
2014-06-19 20:58 ` Andrew Morton
2014-06-19 20:58 ` Andrew Morton
2014-06-20 7:38 ` Vladimir Davydov [this message]
2014-06-20 7:38 ` Vladimir Davydov
2014-06-19 9:07 ` [PATCH 3/3] fork: copy mm's vm usage counters under mmap_sem Vladimir Davydov
2014-06-19 9:07 ` Vladimir Davydov
2014-06-19 18:15 ` [PATCH 1/3] fork/exec: cleanup mm initialization Oleg Nesterov
2014-06-19 18:15 ` Oleg Nesterov
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=20140620073838.GA5387@esperanza \
--to=vdavydov@parallels.com \
--cc=akpm@linux-foundation.org \
--cc=cl@linux.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-mm@kvack.org \
--cc=oleg@redhat.com \
--cc=rientjes@google.com \
/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.