From: Andrew Morton <akpm@linux-foundation.org>
To: Johannes Weiner <hannes@cmpxchg.org>
Cc: Shaohua Li <shli@fb.com>,
Siddhesh Poyarekar <siddhesh.poyarekar@gmail.com>,
linux-mm@kvack.org, linux-kernel@vger.kernel.org,
kernel-team@fb.com
Subject: Re: [PATCH] proc: revert /proc/<pid>/maps [stack:TID] annotation
Date: Tue, 19 Jan 2016 14:14:30 -0800 [thread overview]
Message-ID: <20160119141430.8ff9c464.akpm@linux-foundation.org> (raw)
In-Reply-To: <1453226559-17322-1-git-send-email-hannes@cmpxchg.org>
On Tue, 19 Jan 2016 13:02:39 -0500 Johannes Weiner <hannes@cmpxchg.org> wrote:
> b764375 ("procfs: mark thread stack correctly in proc/<pid>/maps")
> added [stack:TID] annotation to /proc/<pid>/maps. Finding the task of
> a stack VMA requires walking the entire thread list, turning this into
> quadratic behavior: a thousand threads means a thousand stacks, so the
> rendering of /proc/<pid>/maps needs to look at a million threads. The
> cost is not in proportion to the usefulness as described in the patch.
>
> Drop the [stack:TID] annotation to make /proc/<pid>/maps (and
> /proc/<pid>/numa_maps) usable again for higher thread counts.
>
> The [stack] annotation inside /proc/<pid>/task/<tid>/maps is retained,
> as identifying the stack VMA there is an O(1) operation.
Four years ago, ouch.
Any thoughts on the obvious back-compatibility concerns? ie, why did
Siddhesh implement this in the first place? My bad for not ensuring
that the changelog told us this.
https://lkml.org/lkml/2012/1/14/25 has more info:
: Memory mmaped by glibc for a thread stack currently shows up as a
: simple anonymous map, which makes it difficult to differentiate between
: memory usage of the thread on stack and other dynamic allocation.
: Since glibc already uses MAP_STACK to request this mapping, the
: attached patch uses this flag to add additional VM_STACK_FLAGS to the
: resulting vma so that the mapping is treated as a stack and not any
: regular anonymous mapping. Also, one may use vm_flags to decide if a
: vma is a stack.
But even that doesn't really tell us what the actual *value* of the
patch is to end-users.
I note that this patch is a partial revert - the smaps and numa_maps
parts of b764375 remain in place. What's up with that?
--
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: Andrew Morton <akpm@linux-foundation.org>
To: Johannes Weiner <hannes@cmpxchg.org>
Cc: Shaohua Li <shli@fb.com>,
Siddhesh Poyarekar <siddhesh.poyarekar@gmail.com>,
linux-mm@kvack.org, linux-kernel@vger.kernel.org,
kernel-team@fb.com
Subject: Re: [PATCH] proc: revert /proc/<pid>/maps [stack:TID] annotation
Date: Tue, 19 Jan 2016 14:14:30 -0800 [thread overview]
Message-ID: <20160119141430.8ff9c464.akpm@linux-foundation.org> (raw)
In-Reply-To: <1453226559-17322-1-git-send-email-hannes@cmpxchg.org>
On Tue, 19 Jan 2016 13:02:39 -0500 Johannes Weiner <hannes@cmpxchg.org> wrote:
> b764375 ("procfs: mark thread stack correctly in proc/<pid>/maps")
> added [stack:TID] annotation to /proc/<pid>/maps. Finding the task of
> a stack VMA requires walking the entire thread list, turning this into
> quadratic behavior: a thousand threads means a thousand stacks, so the
> rendering of /proc/<pid>/maps needs to look at a million threads. The
> cost is not in proportion to the usefulness as described in the patch.
>
> Drop the [stack:TID] annotation to make /proc/<pid>/maps (and
> /proc/<pid>/numa_maps) usable again for higher thread counts.
>
> The [stack] annotation inside /proc/<pid>/task/<tid>/maps is retained,
> as identifying the stack VMA there is an O(1) operation.
Four years ago, ouch.
Any thoughts on the obvious back-compatibility concerns? ie, why did
Siddhesh implement this in the first place? My bad for not ensuring
that the changelog told us this.
https://lkml.org/lkml/2012/1/14/25 has more info:
: Memory mmaped by glibc for a thread stack currently shows up as a
: simple anonymous map, which makes it difficult to differentiate between
: memory usage of the thread on stack and other dynamic allocation.
: Since glibc already uses MAP_STACK to request this mapping, the
: attached patch uses this flag to add additional VM_STACK_FLAGS to the
: resulting vma so that the mapping is treated as a stack and not any
: regular anonymous mapping. Also, one may use vm_flags to decide if a
: vma is a stack.
But even that doesn't really tell us what the actual *value* of the
patch is to end-users.
I note that this patch is a partial revert - the smaps and numa_maps
parts of b764375 remain in place. What's up with that?
next prev parent reply other threads:[~2016-01-19 22:14 UTC|newest]
Thread overview: 24+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-01-19 18:02 [PATCH] proc: revert /proc/<pid>/maps [stack:TID] annotation Johannes Weiner
2016-01-19 18:02 ` Johannes Weiner
2016-01-19 22:14 ` Andrew Morton [this message]
2016-01-19 22:14 ` Andrew Morton
2016-01-19 23:30 ` Kirill A. Shutemov
2016-01-19 23:30 ` Kirill A. Shutemov
2016-01-20 3:21 ` Siddhesh Poyarekar
2016-01-20 3:21 ` Siddhesh Poyarekar
2016-01-19 23:38 ` Johannes Weiner
2016-01-19 23:38 ` Johannes Weiner
2016-01-20 3:17 ` Siddhesh Poyarekar
2016-01-20 3:17 ` Siddhesh Poyarekar
2016-01-20 5:27 ` Andrew Morton
2016-01-20 5:27 ` Andrew Morton
2016-01-21 10:49 ` [PATCH] proc: add missing 'mm' variable in nommu is_stack() Arnd Bergmann
2016-01-21 10:49 ` Arnd Bergmann
-- strict thread matches above, loose matches on Subject: below --
2016-01-25 21:30 [PATCH] proc: revert /proc/<pid>/maps [stack:TID] annotation Colin Cross
2016-01-25 21:30 ` Colin Cross
2016-01-25 23:14 ` Kirill A. Shutemov
2016-01-25 23:14 ` Kirill A. Shutemov
2016-01-25 23:53 ` Colin Cross
2016-01-25 23:53 ` Colin Cross
2016-01-28 10:25 ` Kirill A. Shutemov
2016-01-28 10:25 ` 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=20160119141430.8ff9c464.akpm@linux-foundation.org \
--to=akpm@linux-foundation.org \
--cc=hannes@cmpxchg.org \
--cc=kernel-team@fb.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-mm@kvack.org \
--cc=shli@fb.com \
--cc=siddhesh.poyarekar@gmail.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.