LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
* [patch 1/2] mm, doc: cleanup and clarify munmap behavior for hugetlb memory
@ 2015-03-26 23:23 David Rientjes
  2015-03-26 23:23 ` [patch 2/2] mm, selftests: test return value of munmap for MAP_HUGETLB memory David Rientjes
                   ` (2 more replies)
  0 siblings, 3 replies; 14+ messages in thread
From: David Rientjes @ 2015-03-26 23:23 UTC (permalink / raw)
  To: Andrew Morton, Jonathan Corbet
  Cc: Davide Libenzi, Luiz Capitulino, Shuah Khan, Hugh Dickins,
	Andrea Arcangeli, Joern Engel, Jianguo Wu, Eric B Munson,
	linux-mm, linux-kernel, linux-api, linux-doc

munmap(2) of hugetlb memory requires a length that is hugepage aligned,
otherwise it may fail.  Add this to the documentation.

This also cleans up the documentation and separates it into logical
units: one part refers to MAP_HUGETLB and another part refers to
requirements for shared memory segments.

Signed-off-by: David Rientjes <rientjes@google.com>
---
 Documentation/vm/hugetlbpage.txt | 21 +++++++++++++--------
 1 file changed, 13 insertions(+), 8 deletions(-)

diff --git a/Documentation/vm/hugetlbpage.txt b/Documentation/vm/hugetlbpage.txt
--- a/Documentation/vm/hugetlbpage.txt
+++ b/Documentation/vm/hugetlbpage.txt
@@ -289,15 +289,20 @@ file systems, write system calls are not.
 Regular chown, chgrp, and chmod commands (with right permissions) could be
 used to change the file attributes on hugetlbfs.
 
-Also, it is important to note that no such mount command is required if the
+Also, it is important to note that no such mount command is required if
 applications are going to use only shmat/shmget system calls or mmap with
-MAP_HUGETLB.  Users who wish to use hugetlb page via shared memory segment
-should be a member of a supplementary group and system admin needs to
-configure that gid into /proc/sys/vm/hugetlb_shm_group.  It is possible for
-same or different applications to use any combination of mmaps and shm*
-calls, though the mount of filesystem will be required for using mmap calls
-without MAP_HUGETLB.  For an example of how to use mmap with MAP_HUGETLB see
-map_hugetlb.c.
+MAP_HUGETLB.  For an example of how to use mmap with MAP_HUGETLB see map_hugetlb
+below.
+
+Users who wish to use hugetlb memory via shared memory segment should be a
+member of a supplementary group and system admin needs to configure that gid
+into /proc/sys/vm/hugetlb_shm_group.  It is possible for same or different
+applications to use any combination of mmaps and shm* calls, though the mount of
+filesystem will be required for using mmap calls without MAP_HUGETLB.
+
+When using munmap(2) to unmap hugetlb memory, the length specified must be
+hugepage aligned, otherwise it will fail with errno set to EINVAL.
+
 
 Examples
 ========

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

* [patch 2/2] mm, selftests: test return value of munmap for MAP_HUGETLB memory
  2015-03-26 23:23 [patch 1/2] mm, doc: cleanup and clarify munmap behavior for hugetlb memory David Rientjes
@ 2015-03-26 23:23 ` David Rientjes
  2015-03-26 23:52   ` Michael Ellerman
  2015-03-27 13:58 ` [patch 1/2] mm, doc: cleanup and clarify munmap behavior for hugetlb memory Eric B Munson
  2015-03-30  1:35 ` Hugh Dickins
  2 siblings, 1 reply; 14+ messages in thread
From: David Rientjes @ 2015-03-26 23:23 UTC (permalink / raw)
  To: Andrew Morton
  Cc: Jonathan Corbet, Davide Libenzi, Luiz Capitulino, Shuah Khan,
	Hugh Dickins, Andrea Arcangeli, Joern Engel, Jianguo Wu,
	Eric B Munson, linux-mm, linux-kernel, linux-api, linux-doc

When MAP_HUGETLB memory is unmapped, the length must be hugepage aligned,
otherwise it fails with -EINVAL.

All tests currently behave correctly, but it's better to explcitly test
the return value for completeness and document the requirement,
especially if users copy map_hugetlb.c as a sample implementation.

Signed-off-by: David Rientjes <rientjes@google.com>
---
 tools/testing/selftests/powerpc/mm/hugetlb_vs_thp_test.c | 8 ++++++--
 tools/testing/selftests/vm/hugetlbfstest.c               | 4 +++-
 tools/testing/selftests/vm/map_hugetlb.c                 | 6 +++++-
 3 files changed, 14 insertions(+), 4 deletions(-)

diff --git a/tools/testing/selftests/powerpc/mm/hugetlb_vs_thp_test.c b/tools/testing/selftests/powerpc/mm/hugetlb_vs_thp_test.c
--- a/tools/testing/selftests/powerpc/mm/hugetlb_vs_thp_test.c
+++ b/tools/testing/selftests/powerpc/mm/hugetlb_vs_thp_test.c
@@ -21,9 +21,13 @@ static int test_body(void)
 		 * Typically the mmap will fail because no huge pages are
 		 * allocated on the system. But if there are huge pages
 		 * allocated the mmap will succeed. That's fine too, we just
-		 * munmap here before continuing.
+		 * munmap here before continuing.  munmap() length of
+		 * MAP_HUGETLB memory must be hugepage aligned.
 		 */
-		munmap(addr, SIZE);
+		if (munmap(addr, SIZE)) {
+			perror("munmap");
+			return 1;
+		}
 	}
 
 	p = mmap(addr, SIZE, PROT_READ | PROT_WRITE,
diff --git a/tools/testing/selftests/vm/hugetlbfstest.c b/tools/testing/selftests/vm/hugetlbfstest.c
--- a/tools/testing/selftests/vm/hugetlbfstest.c
+++ b/tools/testing/selftests/vm/hugetlbfstest.c
@@ -34,6 +34,7 @@ static void do_mmap(int fd, int extra_flags, int unmap)
 	int *p;
 	int flags = MAP_PRIVATE | MAP_POPULATE | extra_flags;
 	u64 before, after;
+	int ret;
 
 	before = read_rss();
 	p = mmap(NULL, length, PROT_READ | PROT_WRITE, flags, fd, 0);
@@ -44,7 +45,8 @@ static void do_mmap(int fd, int extra_flags, int unmap)
 			!"rss didn't grow as expected");
 	if (!unmap)
 		return;
-	munmap(p, length);
+	ret = munmap(p, length);
+	assert(!ret || !"munmap returned an unexpected error");
 	after = read_rss();
 	assert(llabs(after - before) < 0x40000 ||
 			!"rss didn't shrink as expected");
diff --git a/tools/testing/selftests/vm/map_hugetlb.c b/tools/testing/selftests/vm/map_hugetlb.c
--- a/tools/testing/selftests/vm/map_hugetlb.c
+++ b/tools/testing/selftests/vm/map_hugetlb.c
@@ -73,7 +73,11 @@ int main(void)
 	write_bytes(addr);
 	ret = read_bytes(addr);
 
-	munmap(addr, LENGTH);
+	/* munmap() length of MAP_HUGETLB memory must be hugepage aligned */
+	if (munmap(addr, LENGTH)) {
+		perror("munmap");
+		exit(1);
+	}
 
 	return ret;
 }

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

* Re: [patch 2/2] mm, selftests: test return value of munmap for MAP_HUGETLB memory
  2015-03-26 23:23 ` [patch 2/2] mm, selftests: test return value of munmap for MAP_HUGETLB memory David Rientjes
@ 2015-03-26 23:52   ` Michael Ellerman
  0 siblings, 0 replies; 14+ messages in thread
From: Michael Ellerman @ 2015-03-26 23:52 UTC (permalink / raw)
  To: David Rientjes
  Cc: Andrew Morton, Jonathan Corbet, Davide Libenzi, Luiz Capitulino,
	Shuah Khan, Hugh Dickins, Andrea Arcangeli, Joern Engel,
	Jianguo Wu, Eric B Munson, linux-mm, linux-kernel, linux-api,
	linux-doc

On Thu, 2015-03-26 at 16:23 -0700, David Rientjes wrote:
> When MAP_HUGETLB memory is unmapped, the length must be hugepage aligned,
> otherwise it fails with -EINVAL.
> 
> All tests currently behave correctly, but it's better to explcitly test
> the return value for completeness and document the requirement,
> especially if users copy map_hugetlb.c as a sample implementation.
> 
> Signed-off-by: David Rientjes <rientjes@google.com>
> ---
>  tools/testing/selftests/powerpc/mm/hugetlb_vs_thp_test.c | 8 ++++++--
> 
> diff --git a/tools/testing/selftests/powerpc/mm/hugetlb_vs_thp_test.c b/tools/testing/selftests/powerpc/mm/hugetlb_vs_thp_test.c
> --- a/tools/testing/selftests/powerpc/mm/hugetlb_vs_thp_test.c
> +++ b/tools/testing/selftests/powerpc/mm/hugetlb_vs_thp_test.c
> @@ -21,9 +21,13 @@ static int test_body(void)
>  		 * Typically the mmap will fail because no huge pages are
>  		 * allocated on the system. But if there are huge pages
>  		 * allocated the mmap will succeed. That's fine too, we just
> -		 * munmap here before continuing.
> +		 * munmap here before continuing.  munmap() length of
> +		 * MAP_HUGETLB memory must be hugepage aligned.
>  		 */
> -		munmap(addr, SIZE);
> +		if (munmap(addr, SIZE)) {
> +			perror("munmap");
> +			return 1;
> +		}
>  	}
>  
>  	p = mmap(addr, SIZE, PROT_READ | PROT_WRITE,

Acked-by: Michael Ellerman <mpe@ellerman.id.au>

cheers



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

* Re: [patch 1/2] mm, doc: cleanup and clarify munmap behavior for hugetlb memory
  2015-03-26 23:23 [patch 1/2] mm, doc: cleanup and clarify munmap behavior for hugetlb memory David Rientjes
  2015-03-26 23:23 ` [patch 2/2] mm, selftests: test return value of munmap for MAP_HUGETLB memory David Rientjes
@ 2015-03-27 13:58 ` Eric B Munson
  2015-03-28  1:37   ` David Rientjes
  2015-03-30  1:35 ` Hugh Dickins
  2 siblings, 1 reply; 14+ messages in thread
From: Eric B Munson @ 2015-03-27 13:58 UTC (permalink / raw)
  To: David Rientjes
  Cc: Andrew Morton, Jonathan Corbet, Davide Libenzi, Luiz Capitulino,
	Shuah Khan, Hugh Dickins, Andrea Arcangeli, Joern Engel,
	Jianguo Wu, linux-mm, linux-kernel, linux-api, linux-doc

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

On Thu, 26 Mar 2015, David Rientjes wrote:

> munmap(2) of hugetlb memory requires a length that is hugepage aligned,
> otherwise it may fail.  Add this to the documentation.
> 
> This also cleans up the documentation and separates it into logical
> units: one part refers to MAP_HUGETLB and another part refers to
> requirements for shared memory segments.
> 
> Signed-off-by: David Rientjes <rientjes@google.com>
> ---

If this is the route we are going to take, this behavoir needs to be
called out prominently in the mmap/munmap man page.


>  Documentation/vm/hugetlbpage.txt | 21 +++++++++++++--------
>  1 file changed, 13 insertions(+), 8 deletions(-)
> 
> diff --git a/Documentation/vm/hugetlbpage.txt b/Documentation/vm/hugetlbpage.txt
> --- a/Documentation/vm/hugetlbpage.txt
> +++ b/Documentation/vm/hugetlbpage.txt
> @@ -289,15 +289,20 @@ file systems, write system calls are not.
>  Regular chown, chgrp, and chmod commands (with right permissions) could be
>  used to change the file attributes on hugetlbfs.
>  
> -Also, it is important to note that no such mount command is required if the
> +Also, it is important to note that no such mount command is required if
>  applications are going to use only shmat/shmget system calls or mmap with
> -MAP_HUGETLB.  Users who wish to use hugetlb page via shared memory segment
> -should be a member of a supplementary group and system admin needs to
> -configure that gid into /proc/sys/vm/hugetlb_shm_group.  It is possible for
> -same or different applications to use any combination of mmaps and shm*
> -calls, though the mount of filesystem will be required for using mmap calls
> -without MAP_HUGETLB.  For an example of how to use mmap with MAP_HUGETLB see
> -map_hugetlb.c.
> +MAP_HUGETLB.  For an example of how to use mmap with MAP_HUGETLB see map_hugetlb
> +below.
> +
> +Users who wish to use hugetlb memory via shared memory segment should be a
> +member of a supplementary group and system admin needs to configure that gid
> +into /proc/sys/vm/hugetlb_shm_group.  It is possible for same or different
> +applications to use any combination of mmaps and shm* calls, though the mount of
> +filesystem will be required for using mmap calls without MAP_HUGETLB.
> +
> +When using munmap(2) to unmap hugetlb memory, the length specified must be
> +hugepage aligned, otherwise it will fail with errno set to EINVAL.
> +
>  
>  Examples
>  ========
> 
> --
> 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>

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

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

* Re: [patch 1/2] mm, doc: cleanup and clarify munmap behavior for hugetlb memory
  2015-03-27 13:58 ` [patch 1/2] mm, doc: cleanup and clarify munmap behavior for hugetlb memory Eric B Munson
@ 2015-03-28  1:37   ` David Rientjes
  0 siblings, 0 replies; 14+ messages in thread
From: David Rientjes @ 2015-03-28  1:37 UTC (permalink / raw)
  To: Eric B Munson
  Cc: Andrew Morton, Jonathan Corbet, Davide Libenzi, Luiz Capitulino,
	Shuah Khan, Hugh Dickins, Andrea Arcangeli, Joern Engel,
	Jianguo Wu, linux-mm, linux-kernel, linux-api, linux-doc

On Fri, 27 Mar 2015, Eric B Munson wrote:

> > munmap(2) of hugetlb memory requires a length that is hugepage aligned,
> > otherwise it may fail.  Add this to the documentation.
> > 
> > This also cleans up the documentation and separates it into logical
> > units: one part refers to MAP_HUGETLB and another part refers to
> > requirements for shared memory segments.
> > 
> > Signed-off-by: David Rientjes <rientjes@google.com>
> > ---
> 
> If this is the route we are going to take, this behavoir needs to be
> called out prominently in the mmap/munmap man page.
> 

Yeah, that was my next step, but before we get mtk involved I was trying 
to get this merged since man2/mmap.2 already has a 
.I Documentation/vm/hugetlbpage.txt for MAP_HUGETLB so the man page patch 
can simply reference this addition to the file as justification.

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

* Re: [patch 1/2] mm, doc: cleanup and clarify munmap behavior for hugetlb memory
  2015-03-26 23:23 [patch 1/2] mm, doc: cleanup and clarify munmap behavior for hugetlb memory David Rientjes
  2015-03-26 23:23 ` [patch 2/2] mm, selftests: test return value of munmap for MAP_HUGETLB memory David Rientjes
  2015-03-27 13:58 ` [patch 1/2] mm, doc: cleanup and clarify munmap behavior for hugetlb memory Eric B Munson
@ 2015-03-30  1:35 ` Hugh Dickins
  2015-03-30 14:23   ` Eric B Munson
  2015-04-02 22:40   ` David Rientjes
  2 siblings, 2 replies; 14+ messages in thread
From: Hugh Dickins @ 2015-03-30  1:35 UTC (permalink / raw)
  To: David Rientjes
  Cc: Andrew Morton, Jonathan Corbet, Davide Libenzi, Luiz Capitulino,
	Shuah Khan, Hugh Dickins, Andrea Arcangeli, Joern Engel,
	Jianguo Wu, Eric B Munson, linux-mm, linux-kernel, linux-api,
	linux-doc

On Thu, 26 Mar 2015, David Rientjes wrote:

> munmap(2) of hugetlb memory requires a length that is hugepage aligned,
> otherwise it may fail.  Add this to the documentation.

Thanks for taking this on, David.  But although munmap(2) is the one
Davide called out, it goes beyond that, doesn't it?  To mprotect and
madvise and ...

I don't want to work out the list myself: is_vm_hugetlb_page() is
special-cased all over, and different syscalls react differently.

Which is another reason why, like you, I much prefer not to interfere
with the long established behavior: it would be very easy to introduce
bugs and worse inconsistencies.

And mprotect(2) is a good example of why we should not mess around
with the long established API here: changing an mprotect from failing
on a particular size to acting on a larger size is not a safe change.

Eric, I apologize for bringing you in to the discussion, and then
ignoring your input.  I understand that you would like MAP_HUGETLB
to behave more understandably.  We can all agree that the existing
behavior is unsatisfying.  But it's many years too late now to 
change it around - and I suspect that a full exercise to do so would
actually discover some good reasons why the original choices were made.

> 
> This also cleans up the documentation and separates it into logical
> units: one part refers to MAP_HUGETLB and another part refers to
> requirements for shared memory segments.
> 
> Signed-off-by: David Rientjes <rientjes@google.com>
> ---
>  Documentation/vm/hugetlbpage.txt | 21 +++++++++++++--------
>  1 file changed, 13 insertions(+), 8 deletions(-)
> 
> diff --git a/Documentation/vm/hugetlbpage.txt b/Documentation/vm/hugetlbpage.txt
> --- a/Documentation/vm/hugetlbpage.txt
> +++ b/Documentation/vm/hugetlbpage.txt
> @@ -289,15 +289,20 @@ file systems, write system calls are not.
>  Regular chown, chgrp, and chmod commands (with right permissions) could be
>  used to change the file attributes on hugetlbfs.
>  
> -Also, it is important to note that no such mount command is required if the
> +Also, it is important to note that no such mount command is required if
>  applications are going to use only shmat/shmget system calls or mmap with
> -MAP_HUGETLB.  Users who wish to use hugetlb page via shared memory segment
> -should be a member of a supplementary group and system admin needs to
> -configure that gid into /proc/sys/vm/hugetlb_shm_group.  It is possible for
> -same or different applications to use any combination of mmaps and shm*
> -calls, though the mount of filesystem will be required for using mmap calls
> -without MAP_HUGETLB.  For an example of how to use mmap with MAP_HUGETLB see
> -map_hugetlb.c.
> +MAP_HUGETLB.  For an example of how to use mmap with MAP_HUGETLB see map_hugetlb
> +below.
> +
> +Users who wish to use hugetlb memory via shared memory segment should be a
> +member of a supplementary group and system admin needs to configure that gid
> +into /proc/sys/vm/hugetlb_shm_group.  It is possible for same or different
> +applications to use any combination of mmaps and shm* calls, though the mount of
> +filesystem will be required for using mmap calls without MAP_HUGETLB.
> +
> +When using munmap(2) to unmap hugetlb memory, the length specified must be
> +hugepage aligned, otherwise it will fail with errno set to EINVAL.

Perhaps just adding something like "The same is true for mprotect(2)
and other such memory system calls." is good enough for here.

> +
>  
>  Examples
>  ========

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

* Re: [patch 1/2] mm, doc: cleanup and clarify munmap behavior for hugetlb memory
  2015-03-30  1:35 ` Hugh Dickins
@ 2015-03-30 14:23   ` Eric B Munson
  2015-03-30 20:23     ` Hugh Dickins
  2015-04-02 22:40   ` David Rientjes
  1 sibling, 1 reply; 14+ messages in thread
From: Eric B Munson @ 2015-03-30 14:23 UTC (permalink / raw)
  To: Hugh Dickins
  Cc: David Rientjes, Andrew Morton, Jonathan Corbet, Davide Libenzi,
	Luiz Capitulino, Shuah Khan, Andrea Arcangeli, Joern Engel,
	Jianguo Wu, linux-mm, linux-kernel, linux-api, linux-doc

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

On Sun, 29 Mar 2015, Hugh Dickins wrote:

> On Thu, 26 Mar 2015, David Rientjes wrote:
> 
> > munmap(2) of hugetlb memory requires a length that is hugepage aligned,
> > otherwise it may fail.  Add this to the documentation.
> 
> Thanks for taking this on, David.  But although munmap(2) is the one
> Davide called out, it goes beyond that, doesn't it?  To mprotect and
> madvise and ...
> 
> I don't want to work out the list myself: is_vm_hugetlb_page() is
> special-cased all over, and different syscalls react differently.
> 
> Which is another reason why, like you, I much prefer not to interfere
> with the long established behavior: it would be very easy to introduce
> bugs and worse inconsistencies.
> 
> And mprotect(2) is a good example of why we should not mess around
> with the long established API here: changing an mprotect from failing
> on a particular size to acting on a larger size is not a safe change.
> 
> Eric, I apologize for bringing you in to the discussion, and then
> ignoring your input.  I understand that you would like MAP_HUGETLB
> to behave more understandably.  We can all agree that the existing
> behavior is unsatisfying.  But it's many years too late now to 
> change it around - and I suspect that a full exercise to do so would
> actually discover some good reasons why the original choices were made.

No worries, my main concern was avoiding the confusion that led me down
the rabbit hole of compaction and mlock.  As long as the documentation,
man pages, and the code all agree I am satisfied.  I would have
preferred to make the code match the docs, but I understand that
changing the code now introduces a risk of breaking userspace.

It is charitable of you to assume that there were good reasons for the
original decision.  But as the author of the code in question, I suspect
the omission was one of my own inexperience.

Eric

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

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

* Re: [patch 1/2] mm, doc: cleanup and clarify munmap behavior for hugetlb memory
  2015-03-30 14:23   ` Eric B Munson
@ 2015-03-30 20:23     ` Hugh Dickins
  0 siblings, 0 replies; 14+ messages in thread
From: Hugh Dickins @ 2015-03-30 20:23 UTC (permalink / raw)
  To: Eric B Munson
  Cc: Hugh Dickins, David Rientjes, Andrew Morton, Jonathan Corbet,
	Davide Libenzi, Luiz Capitulino, Shuah Khan, Andrea Arcangeli,
	Joern Engel, Jianguo Wu, linux-mm, linux-kernel, linux-api,
	linux-doc

On Mon, 30 Mar 2015, Eric B Munson wrote:
> On Sun, 29 Mar 2015, Hugh Dickins wrote:
> > 
> > Eric, I apologize for bringing you in to the discussion, and then
> > ignoring your input.  I understand that you would like MAP_HUGETLB
> > to behave more understandably.  We can all agree that the existing
> > behavior is unsatisfying.  But it's many years too late now to 
> > change it around - and I suspect that a full exercise to do so would
> > actually discover some good reasons why the original choices were made.
> 
> No worries, my main concern was avoiding the confusion that led me down
> the rabbit hole of compaction and mlock.  As long as the documentation,
> man pages, and the code all agree I am satisfied.  I would have
> preferred to make the code match the docs, but I understand that
> changing the code now introduces a risk of breaking userspace.
> 
> It is charitable of you to assume that there were good reasons for the
> original decision.  But as the author of the code in question, I suspect
> the omission was one of my own inexperience.

No, you are both too modest and too arrogant :)

You were extending the existing hugetlbfs infrastructure to be
accessible through a MAP_HUGETLB interface.  You therefore inherited
the defects (some probably necessary, others perhaps not) of the
original hugetlbfs implementation, which is where this disagreeable
behaviour comes from.

If you were to ask for MAP_HUGETLB to behave differently from mapping
hugetlbfs here, I would shout no.  For a start, we'd have to add a
VM_HUGETLB2 flag so that each place that tests VM_HUGETLB (usually
through is_vm_hugetlb_page(vma) - sic) could decide how to behave
instead.

I for one have neither time nor inclination to write or review
any such patch.

Hugh

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

* Re: [patch 1/2] mm, doc: cleanup and clarify munmap behavior for hugetlb memory
  2015-03-30  1:35 ` Hugh Dickins
  2015-03-30 14:23   ` Eric B Munson
@ 2015-04-02 22:40   ` David Rientjes
  2015-04-02 22:50     ` [patch -mm] mm, doc: cleanup and clarify munmap behavior for hugetlb memory fix David Rientjes
  1 sibling, 1 reply; 14+ messages in thread
From: David Rientjes @ 2015-04-02 22:40 UTC (permalink / raw)
  To: Hugh Dickins
  Cc: Andrew Morton, Jonathan Corbet, Davide Libenzi, Luiz Capitulino,
	Shuah Khan, Andrea Arcangeli, Joern Engel, Jianguo Wu,
	Eric B Munson, linux-mm, linux-kernel, linux-api, linux-doc

On Sun, 29 Mar 2015, Hugh Dickins wrote:

> > munmap(2) of hugetlb memory requires a length that is hugepage aligned,
> > otherwise it may fail.  Add this to the documentation.
> 
> Thanks for taking this on, David.  But although munmap(2) is the one
> Davide called out, it goes beyond that, doesn't it?  To mprotect and
> madvise and ...
> 

Yes, good point, munmap(2) isn't special in this case, the alignment to 
the native page size of the platform should apply to madvise, mbind, 
mincore, mlock, mprotect, remap_file_pages, etc.

I'd hesitate to compile any authoritative list on the behavior in 
Documentation/vm/hugetlbpage.txt since it would exclude future extensions, 
but I'll update it to be more inclusive of other mm syscalls rather than 
specify only munmap(2).

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

* [patch -mm] mm, doc: cleanup and clarify munmap behavior for hugetlb memory fix
  2015-04-02 22:40   ` David Rientjes
@ 2015-04-02 22:50     ` David Rientjes
  2015-04-03  1:05       ` Hugh Dickins
  2015-04-04  9:34       ` Jonathan Corbet
  0 siblings, 2 replies; 14+ messages in thread
From: David Rientjes @ 2015-04-02 22:50 UTC (permalink / raw)
  To: Andrew Morton
  Cc: Hugh Dickins, Jonathan Corbet, Davide Libenzi, Luiz Capitulino,
	Shuah Khan, Andrea Arcangeli, Joern Engel, Jianguo Wu,
	Eric B Munson, linux-mm, linux-kernel, linux-api, linux-doc

Don't only specify munmap(2) behavior with respect the hugetlb memory, all 
other syscalls get naturally aligned to the native page size of the 
processor.  Rather, pick out munmap(2) as a specific example.

Signed-off-by: David Rientjes <rientjes@google.com>
---
 Documentation/vm/hugetlbpage.txt | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/Documentation/vm/hugetlbpage.txt b/Documentation/vm/hugetlbpage.txt
index 1270fb1..030977f 100644
--- a/Documentation/vm/hugetlbpage.txt
+++ b/Documentation/vm/hugetlbpage.txt
@@ -313,8 +313,11 @@ into /proc/sys/vm/hugetlb_shm_group.  It is possible for same or different
 applications to use any combination of mmaps and shm* calls, though the mount of
 filesystem will be required for using mmap calls without MAP_HUGETLB.
 
-When using munmap(2) to unmap hugetlb memory, the length specified must be
-hugepage aligned, otherwise it will fail with errno set to EINVAL.
+Syscalls that operate on memory backed by hugetlb pages only have their lengths
+aligned to the native page size of the processor; they will normally fail with
+errno set to EINVAL or exclude hugetlb pages that extend beyond the length if
+not hugepage aligned.  For example, munmap(2) will fail if memory is backed by
+a hugetlb page and the length is smaller than the hugepage size.
 
 
 Examples

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

* Re: [patch -mm] mm, doc: cleanup and clarify munmap behavior for hugetlb memory fix
  2015-04-02 22:50     ` [patch -mm] mm, doc: cleanup and clarify munmap behavior for hugetlb memory fix David Rientjes
@ 2015-04-03  1:05       ` Hugh Dickins
  2015-04-04  9:34       ` Jonathan Corbet
  1 sibling, 0 replies; 14+ messages in thread
From: Hugh Dickins @ 2015-04-03  1:05 UTC (permalink / raw)
  To: David Rientjes
  Cc: Andrew Morton, Hugh Dickins, Jonathan Corbet, Davide Libenzi,
	Luiz Capitulino, Shuah Khan, Andrea Arcangeli, Joern Engel,
	Jianguo Wu, Eric B Munson, linux-mm, linux-kernel, linux-api,
	linux-doc

On Thu, 2 Apr 2015, David Rientjes wrote:

> Don't only specify munmap(2) behavior with respect the hugetlb memory, all 
> other syscalls get naturally aligned to the native page size of the 
> processor.  Rather, pick out munmap(2) as a specific example.
> 
> Signed-off-by: David Rientjes <rientjes@google.com>

Thanks, yes, good wording: it is best to be a bit vague here,
since each msyscall takes the approach most convenient for it.

Acked-by: Hugh Dickins <hughd@google.com>

> ---
>  Documentation/vm/hugetlbpage.txt | 7 +++++--
>  1 file changed, 5 insertions(+), 2 deletions(-)
> 
> diff --git a/Documentation/vm/hugetlbpage.txt b/Documentation/vm/hugetlbpage.txt
> index 1270fb1..030977f 100644
> --- a/Documentation/vm/hugetlbpage.txt
> +++ b/Documentation/vm/hugetlbpage.txt
> @@ -313,8 +313,11 @@ into /proc/sys/vm/hugetlb_shm_group.  It is possible for same or different
>  applications to use any combination of mmaps and shm* calls, though the mount of
>  filesystem will be required for using mmap calls without MAP_HUGETLB.
>  
> -When using munmap(2) to unmap hugetlb memory, the length specified must be
> -hugepage aligned, otherwise it will fail with errno set to EINVAL.
> +Syscalls that operate on memory backed by hugetlb pages only have their lengths
> +aligned to the native page size of the processor; they will normally fail with
> +errno set to EINVAL or exclude hugetlb pages that extend beyond the length if
> +not hugepage aligned.  For example, munmap(2) will fail if memory is backed by
> +a hugetlb page and the length is smaller than the hugepage size.
>  
>  
>  Examples

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

* Re: [patch -mm] mm, doc: cleanup and clarify munmap behavior for hugetlb memory fix
  2015-04-02 22:50     ` [patch -mm] mm, doc: cleanup and clarify munmap behavior for hugetlb memory fix David Rientjes
  2015-04-03  1:05       ` Hugh Dickins
@ 2015-04-04  9:34       ` Jonathan Corbet
  2015-04-09 19:46         ` David Rientjes
  1 sibling, 1 reply; 14+ messages in thread
From: Jonathan Corbet @ 2015-04-04  9:34 UTC (permalink / raw)
  To: David Rientjes
  Cc: Andrew Morton, Hugh Dickins, Davide Libenzi, Luiz Capitulino,
	Shuah Khan, Andrea Arcangeli, Joern Engel, Jianguo Wu,
	Eric B Munson, linux-mm, linux-kernel, linux-api, linux-doc

On Thu, 2 Apr 2015 15:50:15 -0700 (PDT)
David Rientjes <rientjes@google.com> wrote:

> Don't only specify munmap(2) behavior with respect the hugetlb memory, all 
> other syscalls get naturally aligned to the native page size of the 
> processor.  Rather, pick out munmap(2) as a specific example.

So I was going to apply this to the docs tree, but it doesn't even come
close.  What tree was this patch generated against?

Thanks,

jon

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

* Re: [patch -mm] mm, doc: cleanup and clarify munmap behavior for hugetlb memory fix
  2015-04-04  9:34       ` Jonathan Corbet
@ 2015-04-09 19:46         ` David Rientjes
  2015-04-11 13:26           ` Jonathan Corbet
  0 siblings, 1 reply; 14+ messages in thread
From: David Rientjes @ 2015-04-09 19:46 UTC (permalink / raw)
  To: Jonathan Corbet
  Cc: Andrew Morton, Hugh Dickins, Davide Libenzi, Luiz Capitulino,
	Shuah Khan, Andrea Arcangeli, Joern Engel, Jianguo Wu,
	Eric B Munson, linux-mm, linux-kernel, linux-api, linux-doc

On Sat, 4 Apr 2015, Jonathan Corbet wrote:

> On Thu, 2 Apr 2015 15:50:15 -0700 (PDT)
> David Rientjes <rientjes@google.com> wrote:
> 
> > Don't only specify munmap(2) behavior with respect the hugetlb memory, all 
> > other syscalls get naturally aligned to the native page size of the 
> > processor.  Rather, pick out munmap(2) as a specific example.
> 
> So I was going to apply this to the docs tree, but it doesn't even come
> close.  What tree was this patch generated against?
> 

Sorry, it's not intended to go through the docs tree, it's a patch to fix 
mm-doc-cleanup-and-clarify-munmap-behavior-for-hugetlb-memory.patch in 
-mm.  It's been merged into that tree, but I would still appreciate your 
ack!

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

* Re: [patch -mm] mm, doc: cleanup and clarify munmap behavior for hugetlb memory fix
  2015-04-09 19:46         ` David Rientjes
@ 2015-04-11 13:26           ` Jonathan Corbet
  0 siblings, 0 replies; 14+ messages in thread
From: Jonathan Corbet @ 2015-04-11 13:26 UTC (permalink / raw)
  To: David Rientjes
  Cc: Andrew Morton, Hugh Dickins, Davide Libenzi, Luiz Capitulino,
	Shuah Khan, Andrea Arcangeli, Joern Engel, Jianguo Wu,
	Eric B Munson, linux-mm, linux-kernel, linux-api, linux-doc

On Thu, 9 Apr 2015 12:46:09 -0700 (PDT)
David Rientjes <rientjes@google.com> wrote:

> It's been merged into that tree, but I would still appreciate your 
> ack!

Easy enough.

Acked-by: Jonathan Corbet <corbet@lwn.net>

jon

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

end of thread, other threads:[~2015-04-11 13:26 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-03-26 23:23 [patch 1/2] mm, doc: cleanup and clarify munmap behavior for hugetlb memory David Rientjes
2015-03-26 23:23 ` [patch 2/2] mm, selftests: test return value of munmap for MAP_HUGETLB memory David Rientjes
2015-03-26 23:52   ` Michael Ellerman
2015-03-27 13:58 ` [patch 1/2] mm, doc: cleanup and clarify munmap behavior for hugetlb memory Eric B Munson
2015-03-28  1:37   ` David Rientjes
2015-03-30  1:35 ` Hugh Dickins
2015-03-30 14:23   ` Eric B Munson
2015-03-30 20:23     ` Hugh Dickins
2015-04-02 22:40   ` David Rientjes
2015-04-02 22:50     ` [patch -mm] mm, doc: cleanup and clarify munmap behavior for hugetlb memory fix David Rientjes
2015-04-03  1:05       ` Hugh Dickins
2015-04-04  9:34       ` Jonathan Corbet
2015-04-09 19:46         ` David Rientjes
2015-04-11 13:26           ` Jonathan Corbet

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).