2.6.33 pagemap endless read loop

An LTP run on x86-64/2.6.33-rc3 run right now results in "proc01" hanging

lr-x------ 1 root root 64 2010-01-10 02:28 7 -> /proc/2679/task/2679/pagemap

strace shows an endless loop of

read(7, "\0\0\0\0\0\0\0\6\0\0\0\0\0\0\0\6\0\0\0\0\0\0\0\6\0\0\0"..., 1024) = 1024 read(7, "\0\0\0\0\0\0\0\6\0\0\0\0\0\0\0\6\0\0\0\0\0\0\0\6\0\0\0"..., 1024) = 1024 read(7, "\0\0\0\0\0\0\0\6\0\0\0\0\0\0\0\6\0\0\0\0\0\0\0\6\0\0\0"..., 1024) = 1024 read(7, "\0\0\0\0\0\0\0\6\0\0\0\0\0\0\0\6\0\0\0\0\0\0\0\6\0\0\0"..., 1024) = 1024 read(7, "\0\0\0\0\0\0\0\6\0\0\0\0\0\0\0\6\0\0\0\0\0\0\0\6\0\0\0"..., 1024) = 1024 read(7, "\0\0\0\0\0\0\0\6\0\0\0\0\0\0\0\6\0\0\0\0\0\0\0\6\0\0\0"..., 1024) = 1024 read(7, "\0\0\0\0\0\0\0\6\0\0\0\0\0\0\0\6\0\0\0\0\0\0\0\6\0\0\0"..., 1024) = 10

cat of that file hangs in the same way so it seems like a kernel bug. A cat of /proc/2679/pagemap also hangs, in fact any pagemap cat seems to.

I tried to revert the latest change from Horiguchi-san:

commit 5dc37642cbce34619e4588a9f0bdad1d2f870956

Author: Naoya Horiguchi <[email protected]>
Date:   Mon Dec 14 18:00:01 2009 -0800

mm hugetlb: add hugepage support to pagemap

but that didn't fix it, so it must be something else.

Haven't done further bisect or anything, should be easy to reproduce.

-Andi

[email protected] -- Speaking for myself only.

Are you sure?

On my 32bit machine, it runs for a long time, but finally exits.

of a process, on x86-32, it should be 4G/4K = 1024*1024 pages, also that when you 'cat' it, tty layer is involved to display it too.

Live like a child, think like the god.

I don't know if it's really endless, but it's doing a good emulation of it if it's not.

That was on 64bit.

proc01 had accumulated several minutes of CPU time when I stopped it.

Only the mapped ones surely?

Right now it looks like it dumps all the holes and that obviously will never finish on 64bit which has 47bits of address space.

-Andi

[email protected] -- Speaking for myself only.

Takes forever if you whack .pagemap_pte_hole too, just spends it's life in pagemap_pte_range() instead.

start_vaddr:0x0 end_vaddr:0x7ffffffff000

Hm, zillion paces ain't a walk.

-Mike

Hmm, could you please try:

time cat /proc/self/pagemap >/dev/null

? Is it much faster than:

time cat /proc/self/pagemap

?

Thanks!

Live like a child, think like the god.

I stopped it after a minute of CPU time...

Obviously reading 47bits of zeroes take a long time.

-Andi

[email protected] -- Speaking for myself only.

Yeah, but this just proves it is not endless, doesn't it? :)

Thanks.

How about this trick?
Tests show that cp/cat will exit if read nothing :)

---
pagemap: early return on unmapped areas

This helps stop dumb cp/cat early instead of loop for days:

http://bugzilla.kernel.org/show_bug.cgi?id=15041

This also serves as an optimization to normal pagemap users (eg. page-types).

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

@@ -586,6 +586,10 @@ static int pagemap_pte_hole(unsigned lon
 	struct pagemapread *pm = walk->private;
 	unsigned long addr;
 	int err = 0;
+
+	if ((end - start) / PAGE_SIZE > pm->end - pm->out)
+		return PM_END_OF_BUFFER;
+
 	for (addr = start; addr < end; addr += PAGE_SIZE) {
 		err = add_to_pagemap(addr, PM_NOT_PRESENT, pm);
 		if (err)

Yes seems like a reasonable idea.

-Andi

Thanks!

I just changed the ">" to ">=", because it happen to make a difference for cp/cat, which does 4k sized read:

4k read buffer = 4k/8 pages = 512 pages = NR_PMD_PAGES

So the change to ">=" makes cp exit immediately (4k buffer matches exactly one PMD hole).

before

        dd if=/proc/$$/pagemap of=/dev/null bs=4k
        512+0 records in
        512+0 records out
        2097152 bytes (2.1 MB) copied, 0.0225796 s, 92.9 MB/s

after
dd if=/proc/$$/pagemap of=/dev/null bs=4k 0+0 records in
0+0 records out
0 bytes (0 B) copied, 2.8029e-05 s, 0.0 kB/s

page-types still works (only faster;) with this patch.

Thanks,
Fengguang
---
pagemap: early return on unmapped areas

This helps stop dumb cp/cat early instead of loop for days:

http://bugzilla.kernel.org/show_bug.cgi?id=15041

This also serves as an optimization to normal pagemap users (eg. page-types).

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

@@ -586,6 +586,10 @@ static int pagemap_pte_hole(unsigned lon
 	struct pagemapread *pm = walk->private;
 	unsigned long addr;
 	int err = 0;
+
+	if ((end - start) / PAGE_SIZE >= pm->end - pm->out)
+		return PM_END_OF_BUFFER;
+
 	for (addr = start; addr < end; addr += PAGE_SIZE) {
 		err = add_to_pagemap(addr, PM_NOT_PRESENT, pm);
 		if (err)

Have you guys lost your minds?

I regularly used the exact standard tool you just broke above to test pagemap when I was developing it. There's no doubt that this patch (or any similarly misguided ones) WILL regress existing naive or not intended to be portable 32-bit users. And if I was some poor future bastard trying to write a program against pagemap with your patch, odds of getting a concussion banging my head against the wall trying to figure out its highly non-file-like behavior would be quite high.

Why are we even still on this. There's no regression. LTP + pagemap + 64-bit has always behaved thus. Nor are large virtual files new. And LTP's n00b assumption that it can just read all of any file in /proc has always been wrong. LTP had the exact same problem on 64-bit /proc/kcore (how long has that existed?) and finally fixed it in 2005.

http://selenic.com : development and support for Mercurial and Linux