[PATCH 1/1] mmap: Don't count shmem pages as free in __vm_enough_memory

[PATCH 1/1] mmap: Don't count shmem pages as free in __vm_enough_memory

[Dmitry Fink]

shmem pages can't be reclaimed and if they are swapped out
that doesn't affect the overall available memory in the system,
so don't count them along with the rest of the file backed pages.

Signed-off-by: Dmitry Fink <dmitry.fink@palm.com>
---
 mm/mmap.c |    7 +++++++
 1 files changed, 7 insertions(+), 0 deletions(-)

diff --git a/mm/mmap.c b/mm/mmap.c
index b88624f..3a34dc2 100644
--- a/mm/mmap.c
+++ b/mm/mmap.c
@@ -119,6 +119,13 @@ int __vm_enough_memory(struct mm_struct *mm, long pages, int cap_sys_admin)
 		unsigned long n;
 
 		free = global_page_state(NR_FILE_PAGES);
+
+		/* shmem pages shouldn't be counted as free in this
+		 * case, they can't be purged, only swapped out, and
+		 * that won't affect the overall amount of available
+		 * memory in the system. */
+		free -= global_page_state(NR_SHMEM);
+
 		free += nr_swap_pages;
 
 		/*
-- 
1.6.0.4

--
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/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH 1/1] mmap: Don't count shmem pages as free in __vm_enough_memory

[Minchan Kim]

On Mon, Jul 4, 2011 at 4:39 AM, Dmitry Fink <finikk@gmail.com> wrote:
> shmem pages can't be reclaimed and if they are swapped out
> that doesn't affect the overall available memory in the system,
> so don't count them along with the rest of the file backed pages.
>
> Signed-off-by: Dmitry Fink <dmitry.fink@palm.com>
Reviewed-by: Minchan Kim <minchan.kim@gmail.com>

I am not sure the description is good. :(
But I think this patch is reasonable.

In swapless system,guessing overcommit can have a problem.
And in current implementation of OVERCOMMIT_GUESS, we consider anon
pages as empty space of swap so shmem pages should be accounted by
that.

-- 
Kind regards,
Minchan Kim

--
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/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH 1/1] mmap: Don't count shmem pages as free in __vm_enough_memory

[Dmitry Fink (Palm GBU)]

If I understand the logic correctly, even systems with swap set to
OVERCOMMIT_GUESS are equally affected,
what we are trying to do here is count the amount of immediately available
and
"potentially" available space both in memory and in swap. shmem is not
immediately
available, but it is not potentially available either, even if we swap it
out, it will
just be relocated from memory into swap, total amount of immediate and
potentially
available memory is not going to be affected, so we shouldn't count it as
available
in the first place.

Dmitry

On 7/3/11 5:43 PM, "Minchan Kim" <minchan.kim@gmail.com> wrote:

>On Mon, Jul 4, 2011 at 4:39 AM, Dmitry Fink <finikk@gmail.com> wrote:
>> shmem pages can't be reclaimed and if they are swapped out
>> that doesn't affect the overall available memory in the system,
>> so don't count them along with the rest of the file backed pages.
>>
>> Signed-off-by: Dmitry Fink <dmitry.fink@palm.com>
>Reviewed-by: Minchan Kim <minchan.kim@gmail.com>
>
>I am not sure the description is good. :(
>But I think this patch is reasonable.
>
>In swapless system,guessing overcommit can have a problem.
>And in current implementation of OVERCOMMIT_GUESS, we consider anon
>pages as empty space of swap so shmem pages should be accounted by
>that.
>
>-- 
>Kind regards,
>Minchan Kim

--
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/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH 1/1] mmap: Don't count shmem pages as free in __vm_enough_memory

[Minchan Kim]

On Mon, Jul 4, 2011 at 12:10 PM, Dmitry Fink (Palm GBU)
<Dmitry.Fink@palm.com> wrote:
> If I understand the logic correctly, even systems with swap set to
> OVERCOMMIT_GUESS are equally affected,
> what we are trying to do here is count the amount of immediately available
> and
> "potentially" available space both in memory and in swap. shmem is not
> immediately
> available, but it is not potentially available either, even if we swap it
> out, it will
> just be relocated from memory into swap, total amount of immediate and
> potentially
> available memory is not going to be affected, so we shouldn't count it as
> available
> in the first place.

Agree. I think this is good one rather than old description.

-- 
Kind regards,
Minchan Kim

--
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/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH 1/1] mmap: Don't count shmem pages as free in __vm_enough_memory

[Hugh Dickins]

On Sun, 3 Jul 2011, Dmitry Fink wrote:

> shmem pages can't be reclaimed and if they are swapped out
> that doesn't affect the overall available memory in the system,
> so don't count them along with the rest of the file backed pages.
> 
> Signed-off-by: Dmitry Fink <dmitry.fink@palm.com>

That's a good point.  You can add
Acked-by: Hugh Dickins <hughd@google.com>
if you'll let me burble on for a while...

1. Your block comment style differs from kernel coding style, and
   the nearby comments do follow kernel coding style: please adjust.

2. If you're wondering why this was missed for so long, it's simply
   that we didn't have that separate NR_SHMEM count until 2.6.32.

3. There's a danger that this change will fail some large mappings
   that were allowed before; but I guess we run that risk every
   time we release a larger kernel than before, so let's grant you
   the patch... but it might have to be reverted if anyone complains.

4. i915 GEM uses shmem pages that _can_ (sometimes) be freed under
   memory pressure; but then, on the other side, some of the pages
   counted as "free" may actually be locked or pinned in some way.

5. The OVERCOMMIT_GUESS estimation is at best something of a joke
   (a thousand concurrent tasks would each be allowed to make their
   own separate maximal mappings), and any change appears to dignify
   it somewhat; but you are right, so let's do it.

6. I was worried about additional overhead, and puzzled where the
   actual free pages get counted: ah, lower down, with a comment that
   nr_free_pages() is very expensive on large systems... and what does
   nr_free_pages() do?  It does global_page_state(NR_FREE_PAGES): so
   does that imply that your additional global_page_state(NR_SHMEM)
   will be expensive>?  No, the comment, and the peculiar placing of
   the nr_free_pages() call, date from when it was a loop over all
   zones (hence all nodes) in the system.  Now, tell me to go away
   and make a separate patch of my own if you wish, fair enough;
   but I suggest you tidy that up too (and clearer if it explicitly
   says global_page_state(NR_FREE_PAGES) rather than nr_free_pages()):

   free = global_page_state(NR_FREE_PAGES);
   free += global_page_state(NR_FILE_PAGES);
   etc.

7. There's an almost identical copy of this code in mm/nommu.c:
   please update that one too to keep them in synch.  I suppose it
   would be better to keep one copy of it somewhere else, but by
   now I've probably exhausted your patience, plus I've a nasty
   feeling that if I suggest somewhere, I'll be tricking you
   into a build error with this or that config.  Another time...

You can see why I don't like reviewing more than one-line changes,
can't you :-?

Thanks,
Hugh

> ---
>  mm/mmap.c |    7 +++++++
>  1 files changed, 7 insertions(+), 0 deletions(-)
> 
> diff --git a/mm/mmap.c b/mm/mmap.c
> index b88624f..3a34dc2 100644
> --- a/mm/mmap.c
> +++ b/mm/mmap.c
> @@ -119,6 +119,13 @@ int __vm_enough_memory(struct mm_struct *mm, long pages, int cap_sys_admin)
>  		unsigned long n;
>  
>  		free = global_page_state(NR_FILE_PAGES);
> +
> +		/* shmem pages shouldn't be counted as free in this
> +		 * case, they can't be purged, only swapped out, and
> +		 * that won't affect the overall amount of available
> +		 * memory in the system. */
> +		free -= global_page_state(NR_SHMEM);
> +
>  		free += nr_swap_pages;
>  
>  		/*
> -- 
> 1.6.0.4

--
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/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

[PATCH] mmap: Fix and tidy up overcommit page arithmetic

[Dmitry Fink]

- shmem pages are not immediately available, but they are not
potentially available either, even if we swap them out, they will
just relocate from memory into swap, total amount of immediate and
potentially available memory is not going to be affected, so we
shouldn't count them as potentially free in the first place.

- nr_free_pages() is not an expensive operation anymore, there is
no need to split the decision making in two halves and repeat code.

Signed-off-by: Dmitry Fink <dmitry.fink@palm.com>
Reviewed-by: Minchan Kim <minchan.kim@gmail.com>
Acked-by: Hugh Dickins <hughd@google.com>
---
 mm/mmap.c  |   33 +++++++++++++--------------------
 mm/nommu.c |   29 +++++++++++------------------
 2 files changed, 24 insertions(+), 38 deletions(-)

diff --git a/mm/mmap.c b/mm/mmap.c
index d49736f..6b26cc3 100644
--- a/mm/mmap.c
+++ b/mm/mmap.c
@@ -124,7 +124,16 @@ int __vm_enough_memory(struct mm_struct *mm, long pages, int cap_sys_admin)
 	if (sysctl_overcommit_memory == OVERCOMMIT_GUESS) {
 		unsigned long n;
 
-		free = global_page_state(NR_FILE_PAGES);
+		free = global_page_state(NR_FREE_PAGES);
+		free += global_page_state(NR_FILE_PAGES);
+
+		/* shmem pages shouldn't be counted as free in this
+		 * case, they can't be purged, only swapped out, and
+		 * that won't affect the overall amount of available
+		 * memory in the system.
+		 */
+		free -= global_page_state(NR_SHMEM);
+
 		free += nr_swap_pages;
 
 		/*
@@ -136,34 +145,18 @@ int __vm_enough_memory(struct mm_struct *mm, long pages, int cap_sys_admin)
 		free += global_page_state(NR_SLAB_RECLAIMABLE);
 
 		/*
-		 * Leave the last 3% for root
-		 */
-		if (!cap_sys_admin)
-			free -= free / 32;
-
-		if (free > pages)
-			return 0;
-
-		/*
-		 * nr_free_pages() is very expensive on large systems,
-		 * only call if we're about to fail.
-		 */
-		n = nr_free_pages();
-
-		/*
 		 * Leave reserved pages. The pages are not for anonymous pages.
 		 */
-		if (n <= totalreserve_pages)
+		if (free <= totalreserve_pages)
 			goto error;
 		else
-			n -= totalreserve_pages;
+			free -= totalreserve_pages;
 
 		/*
 		 * Leave the last 3% for root
 		 */
 		if (!cap_sys_admin)
-			n -= n / 32;
-		free += n;
+			free -= free / 32;
 
 		if (free > pages)
 			return 0;
diff --git a/mm/nommu.c b/mm/nommu.c
index 9edc897..510048d 100644
--- a/mm/nommu.c
+++ b/mm/nommu.c
@@ -1887,7 +1887,16 @@ int __vm_enough_memory(struct mm_struct *mm, long pages, int cap_sys_admin)
 	if (sysctl_overcommit_memory == OVERCOMMIT_GUESS) {
 		unsigned long n;
 
-		free = global_page_state(NR_FILE_PAGES);
+		free = global_page_state(NR_FREE_PAGES);
+		free += global_page_state(NR_FILE_PAGES);
+
+		/* shmem pages shouldn't be counted as free in this
+		 * case, they can't be purged, only swapped out, and
+		 * that won't affect the overall amount of available
+		 * memory in the system.
+		 */
+		free -= global_page_state(NR_SHMEM);
+
 		free += nr_swap_pages;
 
 		/*
@@ -1899,21 +1908,6 @@ int __vm_enough_memory(struct mm_struct *mm, long pages, int cap_sys_admin)
 		free += global_page_state(NR_SLAB_RECLAIMABLE);
 
 		/*
-		 * Leave the last 3% for root
-		 */
-		if (!cap_sys_admin)
-			free -= free / 32;
-
-		if (free > pages)
-			return 0;
-
-		/*
-		 * nr_free_pages() is very expensive on large systems,
-		 * only call if we're about to fail.
-		 */
-		n = nr_free_pages();
-
-		/*
 		 * Leave reserved pages. The pages are not for anonymous pages.
 		 */
 		if (n <= totalreserve_pages)
@@ -1925,8 +1919,7 @@ int __vm_enough_memory(struct mm_struct *mm, long pages, int cap_sys_admin)
 		 * Leave the last 3% for root
 		 */
 		if (!cap_sys_admin)
-			n -= n / 32;
-		free += n;
+			free -= free / 32;
 
 		if (free > pages)
 			return 0;
-- 
1.7.6

--
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/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

[PATCH] mmap: Fix and tidy up overcommit page arithmetic

[Dmitry Fink]

- shmem pages are not immediately available, but they are not
potentially available either, even if we swap them out, they will
just relocate from memory into swap, total amount of immediate and
potentially available memory is not going to be affected, so we
shouldn't count them as potentially free in the first place.

- nr_free_pages() is not an expensive operation anymore, there is
no need to split the decision making in two halves and repeat code.

Signed-off-by: Dmitry Fink <dmitry.fink@palm.com>
Reviewed-by: Minchan Kim <minchan.kim@gmail.com>
Acked-by: Hugh Dickins <hughd@google.com>
---
 mm/mmap.c  |   33 ++++++++++++---------------------
 mm/nommu.c |   33 ++++++++++++---------------------
 2 files changed, 24 insertions(+), 42 deletions(-)

diff --git a/mm/mmap.c b/mm/mmap.c
index d49736f..b6ed22e 100644
--- a/mm/mmap.c
+++ b/mm/mmap.c
@@ -122,9 +122,16 @@ int __vm_enough_memory(struct mm_struct *mm, long pages, int cap_sys_admin)
 		return 0;
 
 	if (sysctl_overcommit_memory == OVERCOMMIT_GUESS) {
-		unsigned long n;
+		free = global_page_state(NR_FREE_PAGES);
+		free += global_page_state(NR_FILE_PAGES);
+
+		/* shmem pages shouldn't be counted as free in this
+		 * case, they can't be purged, only swapped out, and
+		 * that won't affect the overall amount of available
+		 * memory in the system.
+		 */
+		free -= global_page_state(NR_SHMEM);
 
-		free = global_page_state(NR_FILE_PAGES);
 		free += nr_swap_pages;
 
 		/*
@@ -136,34 +143,18 @@ int __vm_enough_memory(struct mm_struct *mm, long pages, int cap_sys_admin)
 		free += global_page_state(NR_SLAB_RECLAIMABLE);
 
 		/*
-		 * Leave the last 3% for root
-		 */
-		if (!cap_sys_admin)
-			free -= free / 32;
-
-		if (free > pages)
-			return 0;
-
-		/*
-		 * nr_free_pages() is very expensive on large systems,
-		 * only call if we're about to fail.
-		 */
-		n = nr_free_pages();
-
-		/*
 		 * Leave reserved pages. The pages are not for anonymous pages.
 		 */
-		if (n <= totalreserve_pages)
+		if (free <= totalreserve_pages)
 			goto error;
 		else
-			n -= totalreserve_pages;
+			free -= totalreserve_pages;
 
 		/*
 		 * Leave the last 3% for root
 		 */
 		if (!cap_sys_admin)
-			n -= n / 32;
-		free += n;
+			free -= free / 32;
 
 		if (free > pages)
 			return 0;
diff --git a/mm/nommu.c b/mm/nommu.c
index 9edc897..54017d7 100644
--- a/mm/nommu.c
+++ b/mm/nommu.c
@@ -1885,9 +1885,16 @@ int __vm_enough_memory(struct mm_struct *mm, long pages, int cap_sys_admin)
 		return 0;
 
 	if (sysctl_overcommit_memory == OVERCOMMIT_GUESS) {
-		unsigned long n;
+		free = global_page_state(NR_FREE_PAGES);
+		free += global_page_state(NR_FILE_PAGES);
+
+		/* shmem pages shouldn't be counted as free in this
+		 * case, they can't be purged, only swapped out, and
+		 * that won't affect the overall amount of available
+		 * memory in the system.
+		 */
+		free -= global_page_state(NR_SHMEM);
 
-		free = global_page_state(NR_FILE_PAGES);
 		free += nr_swap_pages;
 
 		/*
@@ -1899,34 +1906,18 @@ int __vm_enough_memory(struct mm_struct *mm, long pages, int cap_sys_admin)
 		free += global_page_state(NR_SLAB_RECLAIMABLE);
 
 		/*
-		 * Leave the last 3% for root
-		 */
-		if (!cap_sys_admin)
-			free -= free / 32;
-
-		if (free > pages)
-			return 0;
-
-		/*
-		 * nr_free_pages() is very expensive on large systems,
-		 * only call if we're about to fail.
-		 */
-		n = nr_free_pages();
-
-		/*
 		 * Leave reserved pages. The pages are not for anonymous pages.
 		 */
-		if (n <= totalreserve_pages)
+		if (free <= totalreserve_pages)
 			goto error;
 		else
-			n -= totalreserve_pages;
+			free -= totalreserve_pages;
 
 		/*
 		 * Leave the last 3% for root
 		 */
 		if (!cap_sys_admin)
-			n -= n / 32;
-		free += n;
+			free -= free / 32;
 
 		if (free > pages)
 			return 0;
-- 
1.7.6

--
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/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH] mmap: Fix and tidy up overcommit page arithmetic

[Minchan Kim]

On Sun, Jul 10, 2011 at 5:55 AM, Dmitry Fink <dmitry.fink@palm.com> wrote:
> - shmem pages are not immediately available, but they are not
> potentially available either, even if we swap them out, they will
> just relocate from memory into swap, total amount of immediate and
> potentially available memory is not going to be affected, so we
> shouldn't count them as potentially free in the first place.
>
> - nr_free_pages() is not an expensive operation anymore, there is
> no need to split the decision making in two halves and repeat code.
>
> Signed-off-by: Dmitry Fink <dmitry.fink@palm.com>
> Reviewed-by: Minchan Kim <minchan.kim@gmail.com>
> Acked-by: Hugh Dickins <hughd@google.com>
> ---
>  mm/mmap.c  |   33 ++++++++++++---------------------
>  mm/nommu.c |   33 ++++++++++++---------------------
>  2 files changed, 24 insertions(+), 42 deletions(-)
>
> diff --git a/mm/mmap.c b/mm/mmap.c
> index d49736f..b6ed22e 100644
> --- a/mm/mmap.c
> +++ b/mm/mmap.c
> @@ -122,9 +122,16 @@ int __vm_enough_memory(struct mm_struct *mm, long pages, int cap_sys_admin)
>                return 0;
>
>        if (sysctl_overcommit_memory == OVERCOMMIT_GUESS) {
> -               unsigned long n;
> +               free = global_page_state(NR_FREE_PAGES);
> +               free += global_page_state(NR_FILE_PAGES);
> +
> +               /* shmem pages shouldn't be counted as free in this

Nitpick.
You didn't correct comment style. It's not a linux kernel coding style.

-- 
Kind regards,
Minchan Kim

--
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/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

[PATCH] mmap: Fix and tidy up overcommit page arithmetic

[Dmitry Fink]

- shmem pages are not immediately available, but they are not
potentially available either, even if we swap them out, they will
just relocate from memory into swap, total amount of immediate and
potentially available memory is not going to be affected, so we
shouldn't count them as potentially free in the first place.

- nr_free_pages() is not an expensive operation anymore, there is
no need to split the decision making in two halves and repeat code.

Signed-off-by: Dmitry Fink <dmitry.fink@palm.com>
Reviewed-by: Minchan Kim <minchan.kim@gmail.com>
Acked-by: Hugh Dickins <hughd@google.com>
---
 mm/mmap.c  |   34 +++++++++++++---------------------
 mm/nommu.c |   34 +++++++++++++---------------------
 2 files changed, 26 insertions(+), 42 deletions(-)

diff --git a/mm/mmap.c b/mm/mmap.c
index d49736f..a65efd4 100644
--- a/mm/mmap.c
+++ b/mm/mmap.c
@@ -122,9 +122,17 @@ int __vm_enough_memory(struct mm_struct *mm, long pages, int cap_sys_admin)
 		return 0;
 
 	if (sysctl_overcommit_memory == OVERCOMMIT_GUESS) {
-		unsigned long n;
+		free = global_page_state(NR_FREE_PAGES);
+		free += global_page_state(NR_FILE_PAGES);
+
+		/*
+		 * shmem pages shouldn't be counted as free in this
+		 * case, they can't be purged, only swapped out, and
+		 * that won't affect the overall amount of available
+		 * memory in the system.
+		 */
+		free -= global_page_state(NR_SHMEM);
 
-		free = global_page_state(NR_FILE_PAGES);
 		free += nr_swap_pages;
 
 		/*
@@ -136,34 +144,18 @@ int __vm_enough_memory(struct mm_struct *mm, long pages, int cap_sys_admin)
 		free += global_page_state(NR_SLAB_RECLAIMABLE);
 
 		/*
-		 * Leave the last 3% for root
-		 */
-		if (!cap_sys_admin)
-			free -= free / 32;
-
-		if (free > pages)
-			return 0;
-
-		/*
-		 * nr_free_pages() is very expensive on large systems,
-		 * only call if we're about to fail.
-		 */
-		n = nr_free_pages();
-
-		/*
 		 * Leave reserved pages. The pages are not for anonymous pages.
 		 */
-		if (n <= totalreserve_pages)
+		if (free <= totalreserve_pages)
 			goto error;
 		else
-			n -= totalreserve_pages;
+			free -= totalreserve_pages;
 
 		/*
 		 * Leave the last 3% for root
 		 */
 		if (!cap_sys_admin)
-			n -= n / 32;
-		free += n;
+			free -= free / 32;
 
 		if (free > pages)
 			return 0;
diff --git a/mm/nommu.c b/mm/nommu.c
index 9edc897..76f2b4b 100644
--- a/mm/nommu.c
+++ b/mm/nommu.c
@@ -1885,9 +1885,17 @@ int __vm_enough_memory(struct mm_struct *mm, long pages, int cap_sys_admin)
 		return 0;
 
 	if (sysctl_overcommit_memory == OVERCOMMIT_GUESS) {
-		unsigned long n;
+		free = global_page_state(NR_FREE_PAGES);
+		free += global_page_state(NR_FILE_PAGES);
+
+		/*
+		 * shmem pages shouldn't be counted as free in this
+		 * case, they can't be purged, only swapped out, and
+		 * that won't affect the overall amount of available
+		 * memory in the system.
+		 */
+		free -= global_page_state(NR_SHMEM);
 
-		free = global_page_state(NR_FILE_PAGES);
 		free += nr_swap_pages;
 
 		/*
@@ -1899,34 +1907,18 @@ int __vm_enough_memory(struct mm_struct *mm, long pages, int cap_sys_admin)
 		free += global_page_state(NR_SLAB_RECLAIMABLE);
 
 		/*
-		 * Leave the last 3% for root
-		 */
-		if (!cap_sys_admin)
-			free -= free / 32;
-
-		if (free > pages)
-			return 0;
-
-		/*
-		 * nr_free_pages() is very expensive on large systems,
-		 * only call if we're about to fail.
-		 */
-		n = nr_free_pages();
-
-		/*
 		 * Leave reserved pages. The pages are not for anonymous pages.
 		 */
-		if (n <= totalreserve_pages)
+		if (free <= totalreserve_pages)
 			goto error;
 		else
-			n -= totalreserve_pages;
+			free -= totalreserve_pages;
 
 		/*
 		 * Leave the last 3% for root
 		 */
 		if (!cap_sys_admin)
-			n -= n / 32;
-		free += n;
+			free -= free / 32;
 
 		if (free > pages)
 			return 0;
-- 
1.7.6

--
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/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH] mmap: Fix and tidy up overcommit page arithmetic

[Hugh Dickins]

On Sun, 10 Jul 2011, Dmitry Fink wrote:

> - shmem pages are not immediately available, but they are not
> potentially available either, even if we swap them out, they will
> just relocate from memory into swap, total amount of immediate and
> potentially available memory is not going to be affected, so we
> shouldn't count them as potentially free in the first place.
> 
> - nr_free_pages() is not an expensive operation anymore, there is
> no need to split the decision making in two halves and repeat code.
> 
> Signed-off-by: Dmitry Fink <dmitry.fink@palm.com>
> Reviewed-by: Minchan Kim <minchan.kim@gmail.com>
> Acked-by: Hugh Dickins <hughd@google.com>

Just right: thanks a lot for redoing it this way, Dmitry.

> ---
>  mm/mmap.c  |   34 +++++++++++++---------------------
>  mm/nommu.c |   34 +++++++++++++---------------------
>  2 files changed, 26 insertions(+), 42 deletions(-)
> 
> diff --git a/mm/mmap.c b/mm/mmap.c
> index d49736f..a65efd4 100644
> --- a/mm/mmap.c
> +++ b/mm/mmap.c
> @@ -122,9 +122,17 @@ int __vm_enough_memory(struct mm_struct *mm, long pages, int cap_sys_admin)
>  		return 0;
>  
>  	if (sysctl_overcommit_memory == OVERCOMMIT_GUESS) {
> -		unsigned long n;
> +		free = global_page_state(NR_FREE_PAGES);
> +		free += global_page_state(NR_FILE_PAGES);
> +
> +		/*
> +		 * shmem pages shouldn't be counted as free in this
> +		 * case, they can't be purged, only swapped out, and
> +		 * that won't affect the overall amount of available
> +		 * memory in the system.
> +		 */
> +		free -= global_page_state(NR_SHMEM);
>  
> -		free = global_page_state(NR_FILE_PAGES);
>  		free += nr_swap_pages;
>  
>  		/*
> @@ -136,34 +144,18 @@ int __vm_enough_memory(struct mm_struct *mm, long pages, int cap_sys_admin)
>  		free += global_page_state(NR_SLAB_RECLAIMABLE);
>  
>  		/*
> -		 * Leave the last 3% for root
> -		 */
> -		if (!cap_sys_admin)
> -			free -= free / 32;
> -
> -		if (free > pages)
> -			return 0;
> -
> -		/*
> -		 * nr_free_pages() is very expensive on large systems,
> -		 * only call if we're about to fail.
> -		 */
> -		n = nr_free_pages();
> -
> -		/*
>  		 * Leave reserved pages. The pages are not for anonymous pages.
>  		 */
> -		if (n <= totalreserve_pages)
> +		if (free <= totalreserve_pages)
>  			goto error;
>  		else
> -			n -= totalreserve_pages;
> +			free -= totalreserve_pages;
>  
>  		/*
>  		 * Leave the last 3% for root
>  		 */
>  		if (!cap_sys_admin)
> -			n -= n / 32;
> -		free += n;
> +			free -= free / 32;
>  
>  		if (free > pages)
>  			return 0;
> diff --git a/mm/nommu.c b/mm/nommu.c
> index 9edc897..76f2b4b 100644
> --- a/mm/nommu.c
> +++ b/mm/nommu.c
> @@ -1885,9 +1885,17 @@ int __vm_enough_memory(struct mm_struct *mm, long pages, int cap_sys_admin)
>  		return 0;
>  
>  	if (sysctl_overcommit_memory == OVERCOMMIT_GUESS) {
> -		unsigned long n;
> +		free = global_page_state(NR_FREE_PAGES);
> +		free += global_page_state(NR_FILE_PAGES);
> +
> +		/*
> +		 * shmem pages shouldn't be counted as free in this
> +		 * case, they can't be purged, only swapped out, and
> +		 * that won't affect the overall amount of available
> +		 * memory in the system.
> +		 */
> +		free -= global_page_state(NR_SHMEM);
>  
> -		free = global_page_state(NR_FILE_PAGES);
>  		free += nr_swap_pages;
>  
>  		/*
> @@ -1899,34 +1907,18 @@ int __vm_enough_memory(struct mm_struct *mm, long pages, int cap_sys_admin)
>  		free += global_page_state(NR_SLAB_RECLAIMABLE);
>  
>  		/*
> -		 * Leave the last 3% for root
> -		 */
> -		if (!cap_sys_admin)
> -			free -= free / 32;
> -
> -		if (free > pages)
> -			return 0;
> -
> -		/*
> -		 * nr_free_pages() is very expensive on large systems,
> -		 * only call if we're about to fail.
> -		 */
> -		n = nr_free_pages();
> -
> -		/*
>  		 * Leave reserved pages. The pages are not for anonymous pages.
>  		 */
> -		if (n <= totalreserve_pages)
> +		if (free <= totalreserve_pages)
>  			goto error;
>  		else
> -			n -= totalreserve_pages;
> +			free -= totalreserve_pages;
>  
>  		/*
>  		 * Leave the last 3% for root
>  		 */
>  		if (!cap_sys_admin)
> -			n -= n / 32;
> -		free += n;
> +			free -= free / 32;
>  
>  		if (free > pages)
>  			return 0;
> -- 
> 1.7.6

--
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/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>