lkml.org 
[lkml]   [2009]   [Feb]   [8]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
Date
From
SubjectRe: 2.6.28.4 regression: mmap fails if mlockall used
On Sun, 8 Feb 2009, Sami Farin wrote:
> On Sun, Feb 08, 2009 at 18:25:45 +0000, Hugh Dickins wrote:
> > On Sun, 8 Feb 2009, Sami Farin wrote:
> >
> > > 2.6.28.2 + gcc-4.3.2-7 works.
> > > 2.6.28.4 + gcc-4.4.0-0.16 does not work.
> > > I run x86_64 SMP kernel.
> >
> > If it's really a bug, in kernel or gcc, then it will help to know
> > how 2.6.28.4 + gcc-4.3.2-7 behaves. And are you using the respective
> > version of gcc to build both the kernel and the a.out?
>
> Yes, I used the same gcc for both of them.
> I noticed ntpd (started with -m for mlockall) did not work with 2.6.28.4:
> getpwnam, getaddrinfo, and maybe others failed. ntpd was originally compiled
> with gcc 4.3.2-7, but using gcc 4.4.0-0.16 did not change anything.
>
> > > # strace ./a.out ntp
> > > 12:10:14.780726 mmap(NULL, 2147624, PROT_READ|PROT_EXEC, MAP_PRIVATE|MAP_DENYWRITE, 3, 0) = -1 EFAULT (Bad address) <0.000038>
> >
> > I wonder where that 2147624 originates from. Because EFAULT is exactly
>
> yeah I snipped a bit too much...:
>
> 21:01:54.543468 open("/lib64/libnss_files.so.2", O_RDONLY) = 3 <0.000034>
> 21:01:54.543562 read(3, "\177ELF\2\1\1\0\0\0\0\0\0\0\0\0\3\0>\0\1\0\0\0@ \0\0\0\0\0\0@\0\0\0\0\0\0\0\230\352\0\0\0\0\0\0\0\0\0\0@\0008\0\t\0@\0!\0 \0\6\0\0\0\5\0\0\0@\0\0\0\0\0\0\0@\0\0\0\0\0\0\0@\0\0\0\0\0\0\0\370\1\0\0\0\0\0\0\370\1\0\0\0\0\0\0\10\0\0\0\0\0\0\0\3\0\0\0\4\0\0\0\340"..., 832) = 832 <0.000016>
> 21:01:54.543683 fstat(3, {st_dev=makedev(8, 6), st_ino=101893687, st_mode=S_IFREG|0755, st_nlink=1, st_uid=0, st_gid=0, st_blksize=4096, st_blocks=128, st_size=62168, st_atime=2008/11/01-00:18:43, st_mtime=2008/11/01-00:18:43, st_ctime=2008/11/06-23:46:26}) = 0 <0.000012>
> 21:01:54.543791 mmap(NULL, 2147624, PROT_READ|PROT_EXEC, MAP_PRIVATE|MAP_DENYWRITE, 3, 0) = -1 EFAULT (Bad address) <0.000046>

Right, st_size=62168 but it's mapping 2147624, so it's not surprising
that an EFAULT comes into it if we're mlocking (but see below, you're
perfectly correct).

In my case I don't actually see that happening in the getpwnam() after
the mlockall(), but I can see a similar instance earlier, while it's
mmaping /lib64/libc.so.6.

At first I was very puzzled, then remembered: it does the one oversized
mmap from the file in order to reserve contiguous virtual memory space,
then follows it up with MAP_FIXED mmaps to replace the beyond-EOF parts
with what it actually wants in there. Fair enough: it could be done
differently, but this is an efficient and accepted way to do it.

>
> > what you get on an mmap of a file, following an mlockall(MCL_FUTURE),
> > if the file is actually a page or more shorter than the size given:
> > the mlocking tries to fault in a non-existent page of the file, if
> > in userspace you'd get SIGBUS, but within the kernel it's EFAULT
> > returned from the mmap.
> >
> > My suspicion is that the 2147624 is just wrong: is it a filesize,
>
> I haven't looked at glibc where it pulls the value.
> But that mmap calls succeeds if mlockall is not called.
>
> Yes, bug can also be in gcc, but I'd bet my euros (but not very many)
> on mlock changes introduced in 2.6.28.2 --> 2.6.28.4.

You are perfectly correct. The 2.6.28 code was careful to hide
the -EFAULT (or other) locking error from higher levels - and we
can see why that's necessary, given MCL_FUTURE and this technique
for reserving space with one oversized mapping from file. But
the 2.6.28.4 code is mistakenly passing the error back on up.

>
> If I don't hear others crying about mlockall in 2.6.28.4
> in a week or so, I may bother trying older gcc with 2.6.28.4,
> but not right now..

There may be some tears, but you've really helped to sooth this.

>
> > but the file gets truncated before the mmap? or is it the size given
> > in an ELF section perhaps, but the file actually not that big?
> > Any ENOSPC in that filesystem recently?
>
> No ENOSPC.
>
> > > 12:10:14.780809 close(3) = 0 <0.000012>
> > > 12:10:14.780856 munmap(0x7f3476e0d000, 421232) = 0 <0.000145>
> > > 12:10:14.781054 write(2, "./a.out: getpwnam failed: Success\n"..., 34./a.out: getpwnam failed: Success
> > > ) = 34 <0.000015>
> > >
> > > I can do malloc(3000000), then mmap call is
> > > 12:50:20.694207 mmap(NULL, 3002368, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_ANONYMOUS, -1, 0) = 0x7f8a8d16b000 <0.003078>
> >
> > Whereas in the case of anonymous, we don't have an underlying object
> > to fault in (or create the object in response to the mmap), so no
> > such problem.
> >
> > I didn't manage to reproduce this here, but I wasn't using the same
> > version of gcc nor (I'd guess!) your kernel config nor your a.out.
>
> To be sure: you tried to reproduce by compiling the attached file
> on 2.6.28.4 kernel?

Silly me missed the attachment, thanks for pointing it out: as I said
above, in my case it didn't actually show the problem (I guess because
my getpwnam() can ignore the network), but stracing it certainly helped
to clarify the issue.

>
> Thanks for looking at this...!

More thanks to you for reporting it. Here's a patch against 2.6.28.4
(or applies at offset to current linux-2.6 git), please test and report
back when you've a moment:


[PATCH] mm: fix error case in mlock downgrade reversion

Commit 27421e211a39784694b597dbf35848b88363c248, Manually revert
"mlock: downgrade mmap sem while populating mlocked regions", has
introduced its own regression: __mlock_vma_pages_range() may report
an error (for example, -EFAULT from trying to lock down pages from
beyond EOF), but mlock_vma_pages_range() must hide that from its
callers as before.

Reported-by: Sami Farin <safari-kernel@safari.iki.fi>
Signed-off-by: Hugh Dickins <hugh@veritas.com>
Cc: stable@kernel.org
---

mm/mlock.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)

--- 2.6.28.4/mm/mlock.c 2009-02-07 01:00:40.000000000 +0000
+++ linux/mm/mlock.c 2009-02-08 20:12:38.000000000 +0000
@@ -310,7 +310,10 @@ long mlock_vma_pages_range(struct vm_are
is_vm_hugetlb_page(vma) ||
vma == get_gate_vma(current))) {

- return __mlock_vma_pages_range(vma, start, end, 1);
+ __mlock_vma_pages_range(vma, start, end, 1);
+
+ /* Hide errors from mmap() and other callers */
+ return 0;
}

/*

\
 
 \ /
  Last update: 2009-02-08 22:01    [W:0.560 / U:0.064 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site