Discussion:
[Libunwind-devel] mincore_validate fails sometimes
Daniel Vassdal
7 years ago
Permalink
We have an issue where on Yocto Morty 2.2 mincore_validate() sometimes unexpectedly returns -1.
This seems to happen most often directly after the system has been booted.

The check that fails is
if (!(mvec[i] & 1)) return -1;
The reason I suspect this is a libunwind problem is that when HAVE_MINCORE isn't defined, it works well.
This workaround is sufficient for us, but in case this indeed is a bug I figured I'd let you guys know.

Best regards,
Daniel Vassdal
Meshtech AS
Dave Watson
7 years ago
Permalink
Post by Daniel Vassdal
We have an issue where on Yocto Morty 2.2 mincore_validate() sometimes unexpectedly returns -1.
This seems to happen most often directly after the system has been booted.
Do you know what the errno is when it returns -1? Is there some condition we should be checking for?
Post by Daniel Vassdal
The check that fails is
if (!(mvec[i] & 1)) return -1;
The reason I suspect this is a libunwind problem is that when HAVE_MINCORE isn't defined, it works well.
This workaround is sufficient for us, but in case this indeed is a bug I figured I'd let you guys know.
Daniel Vassdal
7 years ago
Permalink
Hi Dave,
Thanks for your reply.

Maybe I wasn't being entirely clear; the call to mincore() itself succeeded, i.e. returned 0.
However, when the result array is checked, one or more of the pages are not in memory.
we should also check that the pages are mapped, through the passed mvec array. This patch
also adds this verification.
for (i = 0; i < (len + PAGE_SIZE - 1) / PAGE_SIZE; i++)
{
if (!(mvec[i] & 1)) return -1;
}
mincore() returns a vector that indicates whether pages of the
calling process's virtual memory are resident in core (RAM), and so
will not cause a disk access (page fault) if referenced.
Is this what the author intended to check?

Furthermore, the man page states hat mincore() can return
ENOMEM addr to addr + length contained unmapped memory.
Thus it looks to me as if mincore() already returns an error code for what the comment says the block above is checking for.
This is a little out of my comfort zone so I may be talking nonsense, but maybe have a look at it?

- Daniel

-----Original Message-----
From: Dave Watson [mailto:***@fb.com]
Sent: 16 January 2018 19:18
To: Daniel Vassdal <***@meshtech.no>
Cc: Libunwind-***@nongnu.org
Subject: Re: [Libunwind-devel] mincore_validate fails sometimes
We have an issue where on Yocto Morty 2.2 mincore_validate() sometimes unexpectedly returns -1.
This seems to happen most often directly after the system has been booted.
Do you know what the errno is when it returns -1? Is there some condition we should be checking for?
The check that fails is
if (!(mvec[i] & 1)) return -1;
The reason I suspect this is a libunwind problem is that when HAVE_MINCORE isn't defined, it works well.
This workaround is sufficient for us, but in case this indeed is a bug I figured I'd let you guys know.
Dave Watson
7 years ago
Permalink
Post by Daniel Vassdal
Hi Dave,
Thanks for your reply.
Maybe I wasn't being entirely clear; the call to mincore() itself succeeded, i.e. returned 0.
However, when the result array is checked, one or more of the pages are not in memory.
Ah, ok.
Post by Daniel Vassdal
we should also check that the pages are mapped, through the passed mvec array. This patch
also adds this verification.
for (i = 0; i < (len + PAGE_SIZE - 1) / PAGE_SIZE; i++)
{
if (!(mvec[i] & 1)) return -1;
}
mincore() returns a vector that indicates whether pages of the
calling process's virtual memory are resident in core (RAM), and so
will not cause a disk access (page fault) if referenced.
Is this what the author intended to check?
Yes, it's meant to check that the pages are mapped in. Do you think
the above code is incorrect?
Post by Daniel Vassdal
Furthermore, the man page states hat mincore() can return
ENOMEM addr to addr + length contained unmapped memory.
Thus it looks to me as if mincore() already returns an error code for what the comment says the block above is checking for.
This is a little out of my comfort zone so I may be talking nonsense, but maybe have a look at it?
I think this means ENOMEM is returned only if *all* pages are
unmapped, but the vector must be checked if only some of the pages are
unmapped.
Daniel Vassdal
7 years ago
Permalink
Sorry for the late reply.
Yes, it's meant to check that the pages are mapped in. Do you think the above code is incorrect?
Yes, I think that it is incorrect, as the mincore() result array doesn't indicate if the page is mapped or not, the return code does.

...
I think this means ENOMEM is returned only if *all* pages are unmapped, but the vector must be checked if only some of the pages are unmapped.
This is wrong. If we consult the kernel source code, we see that for each page do_mincore() is called.
https://github.com/torvalds/linux/blob/master/mm/mincore.c#L251-L270

Here it is checked that the page is indeed mapped. If any page in the range isn't, ENOMEM is returned.
https://github.com/torvalds/linux/blob/master/mm/mincore.c#L189-L196

And thus, if the point is to check that the pages are mapped and valid, checking only the return value of mincore() is sufficient.
Checking the result array is wrong unless we really care if the memory is currently in core or not.
And even then it is dubious, as, according to the documentation, the result may be stale even before the function has returned, unless the memory is mlocked into place. And if we know it is, why are we checking?
https://github.com/torvalds/linux/blob/master/mm/mincore.c#L209-L212


- DV

-----Original Message-----
From: Dave Watson [mailto:***@fb.com]
Sent: 17 January 2018 16:59
To: Daniel Vassdal <***@meshtech.no>
Cc: Libunwind-***@nongnu.org
Subject: Re: [Libunwind-devel] mincore_validate fails sometimes
Hi Dave,
Thanks for your reply.
Maybe I wasn't being entirely clear; the call to mincore() itself succeeded, i.e. returned 0.
However, when the result array is checked, one or more of the pages are not in memory.
Ah, ok.
we should also check that the pages are mapped, through the passed
mvec array. This patch also adds this verification.
for (i = 0; i < (len + PAGE_SIZE - 1) / PAGE_SIZE; i++)
{
if (!(mvec[i] & 1)) return -1;
}
mincore() returns a vector that indicates whether pages of the
calling process's virtual memory are resident in core (RAM), and so
will not cause a disk access (page fault) if referenced.
Is this what the author intended to check?
Yes, it's meant to check that the pages are mapped in. Do you think the above code is incorrect?
Furthermore, the man page states hat mincore() can return
ENOMEM addr to addr + length contained unmapped memory.
Thus it looks to me as if mincore() already returns an error code for what the comment says the block above is checking for.
This is a little out of my comfort zone so I may be talking nonsense, but maybe have a look at it?
I think this means ENOMEM is returned only if *all* pages are unmapped, but the vector must be checked if only some of the pages are unmapped.
Dave Watson
7 years ago
Permalink
...
Indeed it does look like you are correct, and we should only check the
return value, not the array. Have you verified this fixes your issue?
Would you like to send a patch?

Thanks!
Daniel Vassdal
7 years ago
Permalink
Indeed it does look like you are correct, and we should only check the return value, not the array. Have you verified this fixes your issue?
Would you like to send a patch?
I tested it and it solved the issue, but as I figured I'd get some more eyes on it I ended up just disabling mincore for the build.
Since then we've upgraded to Poky 2.4, which uses v1.2 rather than bc8698f which Poky 2.2 uses.
We haven't run into any issues with this version, and when checking the source, v1.2 is before the original check of the result vector was added while bc8698f is after.

I submitted a PR on GitHub.
https://github.com/libunwind/libunwind/pull/64

- DV

Loading...