* [PATCH] perf: rework memory accounting in perf_mmap()
@ 2019-09-04 21:46 Song Liu
2019-09-16 19:43 ` Song Liu
` (2 more replies)
0 siblings, 3 replies; 8+ messages in thread
From: Song Liu @ 2019-09-04 21:46 UTC (permalink / raw)
To: linux-kernel; +Cc: kernel-team, Song Liu, Jie Meng, Peter Zijlstra
perf_mmap() always increases user->locked_vm. As a result, "extra" could
grow bigger than "user_extra", which doesn't make sense. Here is an
example case:
Note: Assume "user_lock_limit" is very small.
| # of perf_mmap calls |vma->vm_mm->pinned_vm|user->locked_vm|
| 0 | 0 | 0 |
| 1 | user_extra | user_extra |
| 2 | 3 * user_extra | 2 * user_extra|
| 3 | 6 * user_extra | 3 * user_extra|
| 4 | 10 * user_extra | 4 * user_extra|
Fix this by maintaining proper user_extra and extra.
Reported-by: Hechao Li <hechaol@fb.com>
Cc: Jie Meng <jmeng@fb.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Signed-off-by: Song Liu <songliubraving@fb.com>
---
kernel/events/core.c | 17 +++++++++++++++--
1 file changed, 15 insertions(+), 2 deletions(-)
diff --git a/kernel/events/core.c b/kernel/events/core.c
index 0463c1151bae..a0120bdbce17 100644
--- a/kernel/events/core.c
+++ b/kernel/events/core.c
@@ -5585,7 +5585,8 @@ static void perf_mmap_close(struct vm_area_struct *vma)
* undo the VM accounting.
*/
- atomic_long_sub((size >> PAGE_SHIFT) + 1, &mmap_user->locked_vm);
+ atomic_long_sub((size >> PAGE_SHIFT) + 1 - mmap_locked,
+ &mmap_user->locked_vm);
atomic64_sub(mmap_locked, &vma->vm_mm->pinned_vm);
free_uid(mmap_user);
@@ -5729,8 +5730,20 @@ static int perf_mmap(struct file *file, struct vm_area_struct *vma)
user_locked = atomic_long_read(&user->locked_vm) + user_extra;
- if (user_locked > user_lock_limit)
+ if (user_locked <= user_lock_limit) {
+ /* charge all to locked_vm */
+ } else if (atomic_long_read(&user->locked_vm) >= user_lock_limit) {
+ /* charge all to pinned_vm */
+ extra = user_extra;
+ user_extra = 0;
+ } else {
+ /*
+ * charge locked_vm until it hits user_lock_limit;
+ * charge the rest from pinned_vm
+ */
extra = user_locked - user_lock_limit;
+ user_extra -= extra;
+ }
lock_limit = rlimit(RLIMIT_MEMLOCK);
lock_limit >>= PAGE_SHIFT;
--
2.17.1
^ permalink raw reply related [flat|nested] 8+ messages in thread* Re: [PATCH] perf: rework memory accounting in perf_mmap()
2019-09-04 21:46 [PATCH] perf: rework memory accounting in perf_mmap() Song Liu
@ 2019-09-16 19:43 ` Song Liu
2019-09-16 20:10 ` Hechao Li
2019-09-30 9:02 ` Peter Zijlstra
2019-10-09 12:59 ` [tip: perf/urgent] perf/core: Rework " tip-bot2 for Song Liu
2 siblings, 1 reply; 8+ messages in thread
From: Song Liu @ 2019-09-16 19:43 UTC (permalink / raw)
To: LKML, Peter Zijlstra; +Cc: Kernel Team, Jie Meng, Hechao Li
Hi Peter,
> On Sep 4, 2019, at 2:46 PM, Song Liu <songliubraving@fb.com> wrote:
>
> perf_mmap() always increases user->locked_vm. As a result, "extra" could
> grow bigger than "user_extra", which doesn't make sense. Here is an
> example case:
>
> Note: Assume "user_lock_limit" is very small.
> | # of perf_mmap calls |vma->vm_mm->pinned_vm|user->locked_vm|
> | 0 | 0 | 0 |
> | 1 | user_extra | user_extra |
> | 2 | 3 * user_extra | 2 * user_extra|
> | 3 | 6 * user_extra | 3 * user_extra|
> | 4 | 10 * user_extra | 4 * user_extra|
>
> Fix this by maintaining proper user_extra and extra.
>
> Reported-by: Hechao Li <hechaol@fb.com>
> Cc: Jie Meng <jmeng@fb.com>
> Cc: Peter Zijlstra <peterz@infradead.org>
> Signed-off-by: Song Liu <songliubraving@fb.com>
Could you please share your feedbacks/comments on this one?
Thanks,
Song
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] perf: rework memory accounting in perf_mmap()
2019-09-16 19:43 ` Song Liu
@ 2019-09-16 20:10 ` Hechao Li
2019-09-30 5:33 ` Song Liu
0 siblings, 1 reply; 8+ messages in thread
From: Hechao Li @ 2019-09-16 20:10 UTC (permalink / raw)
To: Song Liu; +Cc: LKML, Peter Zijlstra, Kernel Team, Jie Meng
Song Liu <songliubraving@fb.com> wrote on Mon [2019-Sep-16 12:43:16 -0700]:
> Hi Peter,
>
> > On Sep 4, 2019, at 2:46 PM, Song Liu <songliubraving@fb.com> wrote:
> >
> > perf_mmap() always increases user->locked_vm. As a result, "extra" could
> > grow bigger than "user_extra", which doesn't make sense. Here is an
> > example case:
> >
> > Note: Assume "user_lock_limit" is very small.
> > | # of perf_mmap calls |vma->vm_mm->pinned_vm|user->locked_vm|
> > | 0 | 0 | 0 |
> > | 1 | user_extra | user_extra |
> > | 2 | 3 * user_extra | 2 * user_extra|
> > | 3 | 6 * user_extra | 3 * user_extra|
> > | 4 | 10 * user_extra | 4 * user_extra|
> >
> > Fix this by maintaining proper user_extra and extra.
> >
> > Reported-by: Hechao Li <hechaol@fb.com>
> > Cc: Jie Meng <jmeng@fb.com>
> > Cc: Peter Zijlstra <peterz@infradead.org>
> > Signed-off-by: Song Liu <songliubraving@fb.com>
>
> Could you please share your feedbacks/comments on this one?
>
> Thanks,
> Song
The change looks good to me. Thanks, Song.
Reviewed-By: Hechao Li <hechaol@fb.com>
Hechao
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] perf: rework memory accounting in perf_mmap()
2019-09-16 20:10 ` Hechao Li
@ 2019-09-30 5:33 ` Song Liu
0 siblings, 0 replies; 8+ messages in thread
From: Song Liu @ 2019-09-30 5:33 UTC (permalink / raw)
To: Hechao Li; +Cc: LKML, Peter Zijlstra, Kernel Team, Jie Meng
> On Sep 16, 2019, at 1:10 PM, Hechao Li <hechaol@fb.com> wrote:
>
> Song Liu <songliubraving@fb.com> wrote on Mon [2019-Sep-16 12:43:16 -0700]:
>> Hi Peter,
>>
>>> On Sep 4, 2019, at 2:46 PM, Song Liu <songliubraving@fb.com> wrote:
>>>
>>> perf_mmap() always increases user->locked_vm. As a result, "extra" could
>>> grow bigger than "user_extra", which doesn't make sense. Here is an
>>> example case:
>>>
>>> Note: Assume "user_lock_limit" is very small.
>>> | # of perf_mmap calls |vma->vm_mm->pinned_vm|user->locked_vm|
>>> | 0 | 0 | 0 |
>>> | 1 | user_extra | user_extra |
>>> | 2 | 3 * user_extra | 2 * user_extra|
>>> | 3 | 6 * user_extra | 3 * user_extra|
>>> | 4 | 10 * user_extra | 4 * user_extra|
>>>
>>> Fix this by maintaining proper user_extra and extra.
>>>
>>> Reported-by: Hechao Li <hechaol@fb.com>
>>> Cc: Jie Meng <jmeng@fb.com>
>>> Cc: Peter Zijlstra <peterz@infradead.org>
>>> Signed-off-by: Song Liu <songliubraving@fb.com>
>>
>> Could you please share your feedbacks/comments on this one?
>>
>> Thanks,
>> Song
>
> The change looks good to me. Thanks, Song.
>
> Reviewed-By: Hechao Li <hechaol@fb.com>
Thanks Hechao!
Hi Peter,
Does this fix look good to you?
Thanks,
Song
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] perf: rework memory accounting in perf_mmap()
2019-09-04 21:46 [PATCH] perf: rework memory accounting in perf_mmap() Song Liu
2019-09-16 19:43 ` Song Liu
@ 2019-09-30 9:02 ` Peter Zijlstra
2019-10-07 16:31 ` Song Liu
2019-10-09 12:59 ` [tip: perf/urgent] perf/core: Rework " tip-bot2 for Song Liu
2 siblings, 1 reply; 8+ messages in thread
From: Peter Zijlstra @ 2019-09-30 9:02 UTC (permalink / raw)
To: Song Liu; +Cc: linux-kernel, kernel-team, Jie Meng
On Wed, Sep 04, 2019 at 02:46:18PM -0700, Song Liu wrote:
> perf_mmap() always increases user->locked_vm. As a result, "extra" could
> grow bigger than "user_extra", which doesn't make sense. Here is an
> example case:
>
> Note: Assume "user_lock_limit" is very small.
> | # of perf_mmap calls |vma->vm_mm->pinned_vm|user->locked_vm|
> | 0 | 0 | 0 |
> | 1 | user_extra | user_extra |
> | 2 | 3 * user_extra | 2 * user_extra|
> | 3 | 6 * user_extra | 3 * user_extra|
> | 4 | 10 * user_extra | 4 * user_extra|
>
> Fix this by maintaining proper user_extra and extra.
Aah, indeed.
Also, this code is unreadable (which is mostly my own fault I suppose)
:/
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] perf: rework memory accounting in perf_mmap()
2019-09-30 9:02 ` Peter Zijlstra
@ 2019-10-07 16:31 ` Song Liu
2019-10-07 16:56 ` Peter Zijlstra
0 siblings, 1 reply; 8+ messages in thread
From: Song Liu @ 2019-10-07 16:31 UTC (permalink / raw)
To: Peter Zijlstra; +Cc: open list, Kernel Team, Jie Meng, Hechao Li
Hi Peter,
> On Sep 30, 2019, at 2:02 AM, Peter Zijlstra <peterz@infradead.org> wrote:
>
> On Wed, Sep 04, 2019 at 02:46:18PM -0700, Song Liu wrote:
>> perf_mmap() always increases user->locked_vm. As a result, "extra" could
>> grow bigger than "user_extra", which doesn't make sense. Here is an
>> example case:
>>
>> Note: Assume "user_lock_limit" is very small.
>> | # of perf_mmap calls |vma->vm_mm->pinned_vm|user->locked_vm|
>> | 0 | 0 | 0 |
>> | 1 | user_extra | user_extra |
>> | 2 | 3 * user_extra | 2 * user_extra|
>> | 3 | 6 * user_extra | 3 * user_extra|
>> | 4 | 10 * user_extra | 4 * user_extra|
>>
>> Fix this by maintaining proper user_extra and extra.
>
> Aah, indeed.
Thanks for the feedback!
>
> Also, this code is unreadable (which is mostly my own fault I suppose)
> :/
How does this patch look to you? Is it ready to merge?
Thanks,
Song
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] perf: rework memory accounting in perf_mmap()
2019-10-07 16:31 ` Song Liu
@ 2019-10-07 16:56 ` Peter Zijlstra
0 siblings, 0 replies; 8+ messages in thread
From: Peter Zijlstra @ 2019-10-07 16:56 UTC (permalink / raw)
To: Song Liu; +Cc: open list, Kernel Team, Jie Meng, Hechao Li
On Mon, Oct 07, 2019 at 04:31:37PM +0000, Song Liu wrote:
> Hi Peter,
>
> > On Sep 30, 2019, at 2:02 AM, Peter Zijlstra <peterz@infradead.org> wrote:
> >
> > On Wed, Sep 04, 2019 at 02:46:18PM -0700, Song Liu wrote:
> >> perf_mmap() always increases user->locked_vm. As a result, "extra" could
> >> grow bigger than "user_extra", which doesn't make sense. Here is an
> >> example case:
> >>
> >> Note: Assume "user_lock_limit" is very small.
> >> | # of perf_mmap calls |vma->vm_mm->pinned_vm|user->locked_vm|
> >> | 0 | 0 | 0 |
> >> | 1 | user_extra | user_extra |
> >> | 2 | 3 * user_extra | 2 * user_extra|
> >> | 3 | 6 * user_extra | 3 * user_extra|
> >> | 4 | 10 * user_extra | 4 * user_extra|
> >>
> >> Fix this by maintaining proper user_extra and extra.
> >
> > Aah, indeed.
>
> Thanks for the feedback!
>
> >
> > Also, this code is unreadable (which is mostly my own fault I suppose)
> > :/
>
> How does this patch look to you? Is it ready to merge?
Yes, I queued it, I'll get it into tip soon. It got held up due to me
working on some other patches, sorry about that.
^ permalink raw reply [flat|nested] 8+ messages in thread
* [tip: perf/urgent] perf/core: Rework memory accounting in perf_mmap()
2019-09-04 21:46 [PATCH] perf: rework memory accounting in perf_mmap() Song Liu
2019-09-16 19:43 ` Song Liu
2019-09-30 9:02 ` Peter Zijlstra
@ 2019-10-09 12:59 ` tip-bot2 for Song Liu
2 siblings, 0 replies; 8+ messages in thread
From: tip-bot2 for Song Liu @ 2019-10-09 12:59 UTC (permalink / raw)
To: linux-tip-commits
Cc: Hechao Li, Song Liu, Peter Zijlstra (Intel), kernel-team,
Jie Meng, Linus Torvalds, Thomas Gleixner, Ingo Molnar,
Borislav Petkov, linux-kernel
The following commit has been merged into the perf/urgent branch of tip:
Commit-ID: d44248a41337731a111374822d7d4451b64e73e4
Gitweb: https://git.kernel.org/tip/d44248a41337731a111374822d7d4451b64e73e4
Author: Song Liu <songliubraving@fb.com>
AuthorDate: Wed, 04 Sep 2019 14:46:18 -07:00
Committer: Ingo Molnar <mingo@kernel.org>
CommitterDate: Wed, 09 Oct 2019 12:44:12 +02:00
perf/core: Rework memory accounting in perf_mmap()
perf_mmap() always increases user->locked_vm. As a result, "extra" could
grow bigger than "user_extra", which doesn't make sense. Here is an
example case:
(Note: Assume "user_lock_limit" is very small.)
| # of perf_mmap calls |vma->vm_mm->pinned_vm|user->locked_vm|
| 0 | 0 | 0 |
| 1 | user_extra | user_extra |
| 2 | 3 * user_extra | 2 * user_extra|
| 3 | 6 * user_extra | 3 * user_extra|
| 4 | 10 * user_extra | 4 * user_extra|
Fix this by maintaining proper user_extra and extra.
Reviewed-By: Hechao Li <hechaol@fb.com>
Reported-by: Hechao Li <hechaol@fb.com>
Signed-off-by: Song Liu <songliubraving@fb.com>
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Cc: <kernel-team@fb.com>
Cc: Jie Meng <jmeng@fb.com>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Link: https://lkml.kernel.org/r/20190904214618.3795672-1-songliubraving@fb.com
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
kernel/events/core.c | 17 +++++++++++++++--
1 file changed, 15 insertions(+), 2 deletions(-)
diff --git a/kernel/events/core.c b/kernel/events/core.c
index f953dd1..2b8265a 100644
--- a/kernel/events/core.c
+++ b/kernel/events/core.c
@@ -5668,7 +5668,8 @@ again:
* undo the VM accounting.
*/
- atomic_long_sub((size >> PAGE_SHIFT) + 1, &mmap_user->locked_vm);
+ atomic_long_sub((size >> PAGE_SHIFT) + 1 - mmap_locked,
+ &mmap_user->locked_vm);
atomic64_sub(mmap_locked, &vma->vm_mm->pinned_vm);
free_uid(mmap_user);
@@ -5812,8 +5813,20 @@ accounting:
user_locked = atomic_long_read(&user->locked_vm) + user_extra;
- if (user_locked > user_lock_limit)
+ if (user_locked <= user_lock_limit) {
+ /* charge all to locked_vm */
+ } else if (atomic_long_read(&user->locked_vm) >= user_lock_limit) {
+ /* charge all to pinned_vm */
+ extra = user_extra;
+ user_extra = 0;
+ } else {
+ /*
+ * charge locked_vm until it hits user_lock_limit;
+ * charge the rest from pinned_vm
+ */
extra = user_locked - user_lock_limit;
+ user_extra -= extra;
+ }
lock_limit = rlimit(RLIMIT_MEMLOCK);
lock_limit >>= PAGE_SHIFT;
^ permalink raw reply related [flat|nested] 8+ messages in thread
end of thread, other threads:[~2019-10-09 12:59 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2019-09-04 21:46 [PATCH] perf: rework memory accounting in perf_mmap() Song Liu
2019-09-16 19:43 ` Song Liu
2019-09-16 20:10 ` Hechao Li
2019-09-30 5:33 ` Song Liu
2019-09-30 9:02 ` Peter Zijlstra
2019-10-07 16:31 ` Song Liu
2019-10-07 16:56 ` Peter Zijlstra
2019-10-09 12:59 ` [tip: perf/urgent] perf/core: Rework " tip-bot2 for Song Liu
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.