Linux-mm Archive on lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v3] fs/proc/task_mmu: fix lost maps entries above 2^63
@ 2026-10-13 15:21 Stian Halseth
  2026-10-13 15:28 ` Lorenzo Stoakes (ARM)
  0 siblings, 1 reply; 3+ messages in thread
From: Stian Halseth @ 2026-10-13 15:21 UTC (permalink / raw)
  To: Andrew Morton
  Cc: David Hildenbrand, Suren Baghdasaryan, Liam R. Howlett,
	Lorenzo Stoakes, Vlastimil Babka, Jann Horn, Pedro Falcato,
	Andreas Larsson, linux-mm, linux-fsdevel, sparclinux,
	linux-kernel, Stian Halseth, stable

When a read of /proc/pid/{maps,smaps,numa_maps} resumes, m_start()
rewinds *ppos to priv->last_pos so that the vma which did not fit into
the previous buffer is shown again. It only does so for last_addr > 0,
meaning to exclude the start of the file and SENTINEL_VMA_GATE, but
last_addr is a signed loff_t holding a vm_end. sparc64 maps shared
libraries, anonymous memory and the stack above 0x8000000000000000, so
the rewind does not happen there and one entry is lost at every read()
boundary.

Reading the maps file of a process with read sizes from 64 to 8192
bytes in steps of 64 loses lines at 126 of the 128 sizes, smaps at 123
and numa_maps at 97, and the proc-maps-race selftest fails 4 of its 6
tests. Python's sys.remote_exec() reads maps with fgets() and fails
intermittently when it misses the libpython data mapping.

Skip the rewind only for 0 and the two sentinels instead, with an
addr_is_sentinel() helper.

Fixes: 5631da56c9a87 ("fs/proc/task_mmu: read proc/pid/maps under per-vma lock")
Closes: https://github.com/sparclinux/issues/issues/111
Cc: stable@vger.kernel.org
Acked-by: David Hildenbrand (Arm) <david@kernel.org>
Signed-off-by: Stian Halseth <stian@itx.no>
---

Notes:
    v3: drop the inline from addr_is_sentinel(), add David's Acked-by.
    v2: https://lore.kernel.org/all/20261013150712.1827315-1-stian@itx.no/
    v2: addr_is_sentinel() covering both sentinels, as the comment says
    (David Hildenbrand). No functional change: m_start() returns early on
    SENTINEL_VMA_END. Re-tested on the T4 (7.2.10): maps and smaps read at
    128 sizes, 200 and 1000 mappings, no lost lines.
    v1: https://lore.kernel.org/all/20261011162019.922436-1-stian@itx.no/
    
    Tested on a SPARC T7-1 (sun4v, 7.3-rc5), a T4-1 (7.2.9) and a Sun Fire
    V240 (sun4u, 7.3-rc5): no lost lines at any read size afterwards, and
    Python's sys.remote_exec() test passes 20/20. On an amd64 KVM guest maps,
    smaps, numa_maps and the proc selftests are unchanged.

 fs/proc/task_mmu.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/fs/proc/task_mmu.c b/fs/proc/task_mmu.c
index 5c54aebe21182..24c2a5079f080 100644
--- a/fs/proc/task_mmu.c
+++ b/fs/proc/task_mmu.c
@@ -32,6 +32,11 @@
 #define SENTINEL_VMA_END	-1
 #define SENTINEL_VMA_GATE	-2
 
+static bool addr_is_sentinel(loff_t addr)
+{
+	return addr == SENTINEL_VMA_END || addr == SENTINEL_VMA_GATE;
+}
+
 #define SEQ_PUT_DEC(str, val) \
 		seq_put_decimal_ull_width(m, str, (val) << (PAGE_SHIFT-10), 8)
 void task_mem(struct seq_file *m, struct mm_struct *mm)
@@ -339,7 +344,7 @@ static void *m_start(struct seq_file *m, loff_t *ppos)
 	 * Reset current position if last_addr was set before
 	 * and it's not a sentinel.
 	 */
-	if (last_addr > 0)
+	if (last_addr && !addr_is_sentinel(last_addr))
 		*ppos = last_addr = priv->last_pos;
 	vma_iter_init(&priv->iter, mm, (unsigned long)last_addr);
 	hold_task_mempolicy(priv);
-- 
2.43.0



^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH v3] fs/proc/task_mmu: fix lost maps entries above 2^63
  2026-10-13 15:21 [PATCH v3] fs/proc/task_mmu: fix lost maps entries above 2^63 Stian Halseth
@ 2026-10-13 15:28 ` Lorenzo Stoakes (ARM)
  2026-10-13 15:34   ` Stian Halseth
  0 siblings, 1 reply; 3+ messages in thread
From: Lorenzo Stoakes (ARM) @ 2026-10-13 15:28 UTC (permalink / raw)
  To: Stian Halseth
  Cc: Andrew Morton, David Hildenbrand, Suren Baghdasaryan,
	Liam R. Howlett, Vlastimil Babka, Jann Horn, Pedro Falcato,
	Andreas Larsson, linux-mm, linux-fsdevel, sparclinux,
	linux-kernel, stable

On Tue, Oct 13, 2026 at 05:21:06PM +0200, Stian Halseth wrote:
> When a read of /proc/pid/{maps,smaps,numa_maps} resumes, m_start()
> rewinds *ppos to priv->last_pos so that the vma which did not fit into
> the previous buffer is shown again. It only does so for last_addr > 0,
                                     ^
This could do with a newline between paragraphs.

> meaning to exclude the start of the file and SENTINEL_VMA_GATE, but
> last_addr is a signed loff_t holding a vm_end. sparc64 maps shared
                                                ^
                                          Here too.

> libraries, anonymous memory and the stack above 0x8000000000000000, so
> the rewind does not happen there and one entry is lost at every read()
> boundary.
>
> Reading the maps file of a process with read sizes from 64 to 8192
> bytes in steps of 64 loses lines at 126 of the 128 sizes, smaps at 123
> and numa_maps at 97, and the proc-maps-race selftest fails 4 of its 6
> tests. Python's sys.remote_exec() reads maps with fgets() and fails
> intermittently when it misses the libpython data mapping.

These numbers are hard to understand it's like playing numberwang a little
bit ;)

It'd be good to simplify this by saying something like 'testing revealed
that lines are not correctly read from /proc/$pid/[maps, smaps, numa_maps]
etc.' or at least something that's a little more readable.

>
> Skip the rewind only for 0 and the two sentinels instead, with an
> addr_is_sentinel() helper.
>
> Fixes: 5631da56c9a87 ("fs/proc/task_mmu: read proc/pid/maps under per-vma lock")
> Closes: https://github.com/sparclinux/issues/issues/111
> Cc: stable@vger.kernel.org
> Acked-by: David Hildenbrand (Arm) <david@kernel.org>
> Signed-off-by: Stian Halseth <stian@itx.no>

Please stop respinning so fast thanks :)

We generally want a ~week between respins for newer contributors. At least
a day.

5 minutes is a bit much...

This looks reasonable to me, but I want Suren to have a closer look.

Acked-by: Lorenzo Stoakes (ARM) <ljs@kernel.org>

> diff --git a/fs/proc/task_mmu.c b/fs/proc/task_mmu.c
> index 5c54aebe21182..24c2a5079f080 100644
> --- a/fs/proc/task_mmu.c
> +++ b/fs/proc/task_mmu.c

> @@ -339,7 +344,7 @@ static void *m_start(struct seq_file *m, loff_t *ppos)
>  	 * Reset current position if last_addr was set before
>  	 * and it's not a sentinel.
>  	 */
> -	if (last_addr > 0)
> +	if (last_addr && !addr_is_sentinel(last_addr))

I think last_addr > 0 was fine, but it's not really a big deal.

--
Cheers, Lorenzo


^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH v3] fs/proc/task_mmu: fix lost maps entries above 2^63
  2026-10-13 15:28 ` Lorenzo Stoakes (ARM)
@ 2026-10-13 15:34   ` Stian Halseth
  0 siblings, 0 replies; 3+ messages in thread
From: Stian Halseth @ 2026-10-13 15:34 UTC (permalink / raw)
  To: Lorenzo Stoakes (ARM)
  Cc: Andrew Morton, David Hildenbrand, Suren Baghdasaryan,
	Liam R. Howlett, Vlastimil Babka, Jann Horn, Pedro Falcato,
	Andreas Larsson, linux-mm, linux-fsdevel, sparclinux,
	linux-kernel, stable

[-- Attachment #1: Type: text/plain, Size: 361 bytes --]

On Tue, 2026-10-13 at 16:28 +0100, Lorenzo Stoakes (ARM) wrote:
> 
> Please stop respinning so fast thanks :)
> 
> We generally want a ~week between respins for newer contributors. At
> least a day.

Noted, sorry for the noise. I'll fix the commit message as you suggest
and wait for Suren before sending anything else.


Thanks for the ack,
Stian

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2026-10-13 15:34 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-10-13 15:21 [PATCH v3] fs/proc/task_mmu: fix lost maps entries above 2^63 Stian Halseth
2026-10-13 15:28 ` Lorenzo Stoakes (ARM)
2026-10-13 15:34   ` Stian Halseth

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox