[PATCH] Subtract min_free_kbytes from dirtyable memory

[PATCH] Subtract min_free_kbytes from dirtyable memory

[paul.szabo]

When calculating amount of dirtyable memory, min_free_kbytes should be
subtracted because it is not intended for dirty pages.

Using an "extern int" because that is the only interface to some such
sysctl values.

(This patch does not solve the PAE OOM issue.)

Paul Szabo   psz@maths.usyd.edu.au   http://www.maths.usyd.edu.au/u/psz/
School of Mathematics and Statistics   University of Sydney    Australia

Reported-by: Paul Szabo <psz@maths.usyd.edu.au>
Reference: http://bugs.debian.org/695182
Signed-off-by: Paul Szabo <psz@maths.usyd.edu.au>

--- mm/page-writeback.c.old	2012-12-06 22:20:40.000000000 +1100
+++ mm/page-writeback.c	2013-01-21 13:57:05.000000000 +1100
@@ -343,12 +343,16 @@
 unsigned long determine_dirtyable_memory(void)
 {
 	unsigned long x;
+	extern int min_free_kbytes;
 
 	x = global_page_state(NR_FREE_PAGES) + global_reclaimable_pages();
 
 	if (!vm_highmem_is_dirtyable)
 		x -= highmem_dirtyable_memory(x);
 
+	/* Subtract min_free_kbytes */
+	x -= min(x, min_free_kbytes >> (PAGE_SHIFT - 10));
+
 	return x + 1;	/* Ensure that we never return 0 */
 }

--
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>

Re: [PATCH] Subtract min_free_kbytes from dirtyable memory

[Rik van Riel]

On 01/20/2013 10:15 PM, paul.szabo@sydney.edu.au wrote:
> When calculating amount of dirtyable memory, min_free_kbytes should be
> subtracted because it is not intended for dirty pages.
>
> Using an "extern int" because that is the only interface to some such
> sysctl values.
>
> (This patch does not solve the PAE OOM issue.)
>
> Paul Szabo   psz@maths.usyd.edu.au   http://www.maths.usyd.edu.au/u/psz/
> School of Mathematics and Statistics   University of Sydney    Australia
>
> Reported-by: Paul Szabo <psz@maths.usyd.edu.au>
> Reference: http://bugs.debian.org/695182
> Signed-off-by: Paul Szabo <psz@maths.usyd.edu.au>

Acked-by: Rik van Riel <riel@redhat.com>


-- 
All rights reversed

--
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>

Re: [PATCH] Subtract min_free_kbytes from dirtyable memory

[Andrew Morton]

On Mon, 21 Jan 2013 14:15:49 +1100
paul.szabo@sydney.edu.au wrote:

> When calculating amount of dirtyable memory, min_free_kbytes should be
> subtracted because it is not intended for dirty pages.

Makes sense.

> Using an "extern int" because that is the only interface to some such
> sysctl values.

urgh, not that way.  Let's do it properly:

From: Andrew Morton <akpm@linux-foundation.org>
Subject: page-writebackc-subtract-min_free_kbytes-from-dirtyable-memory-fix

fix up min_free_kbytes extern declarations

Cc: Paul Szabo <psz@maths.usyd.edu.au>
Cc: Rik van Riel <riel@redhat.com>
Cc: Wu Fengguang <fengguang.wu@intel.com>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
---

 include/linux/mm.h  |    3 +++
 kernel/sysctl.c     |    1 -
 mm/huge_memory.c    |    1 -
 mm/page-writeback.c |    1 -
 4 files changed, 3 insertions(+), 3 deletions(-)

--- a/mm/page-writeback.c~page-writebackc-subtract-min_free_kbytes-from-dirtyable-memory-fix
+++ a/mm/page-writeback.c
@@ -233,7 +233,6 @@ static unsigned long highmem_dirtyable_m
 static unsigned long global_dirtyable_memory(void)
 {
 	unsigned long x;
-	extern int min_free_kbytes;
 
 	x = global_page_state(NR_FREE_PAGES) + global_reclaimable_pages();
 	x -= min(x, dirty_balance_reserve);
--- a/include/linux/mm.h~page-writebackc-subtract-min_free_kbytes-from-dirtyable-memory-fix
+++ a/include/linux/mm.h
@@ -1387,6 +1387,9 @@ extern void setup_per_cpu_pageset(void);
 extern void zone_pcp_update(struct zone *zone);
 extern void zone_pcp_reset(struct zone *zone);
 
+/* page_alloc.c */
+extern int min_free_kbytes;
+
 /* nommu.c */
 extern atomic_long_t mmap_pages_allocated;
 extern int nommu_shrink_inode_mappings(struct inode *, size_t, size_t);
--- a/mm/huge_memory.c~page-writebackc-subtract-min_free_kbytes-from-dirtyable-memory-fix
+++ a/mm/huge_memory.c
@@ -105,7 +105,6 @@ static int set_recommended_min_free_kbyt
 	struct zone *zone;
 	int nr_zones = 0;
 	unsigned long recommended_min;
-	extern int min_free_kbytes;
 
 	if (!khugepaged_enabled())
 		return 0;
--- a/kernel/sysctl.c~page-writebackc-subtract-min_free_kbytes-from-dirtyable-memory-fix
+++ a/kernel/sysctl.c
@@ -104,7 +104,6 @@ extern char core_pattern[];
 extern unsigned int core_pipe_limit;
 #endif
 extern int pid_max;
-extern int min_free_kbytes;
 extern int pid_max_min, pid_max_max;
 extern int sysctl_drop_caches;
 extern int percpu_pagelist_fraction;
_


> (This patch does not solve the PAE OOM issue.)
> 
> Paul Szabo   psz@maths.usyd.edu.au   http://www.maths.usyd.edu.au/u/psz/
> School of Mathematics and Statistics   University of Sydney    Australia
> 
> Reported-by: Paul Szabo <psz@maths.usyd.edu.au>

Reported-by isn't needed in such cases.  It is assumed that finder==fixer.

> Reference: http://bugs.debian.org/695182
> Signed-off-by: Paul Szabo <psz@maths.usyd.edu.au>
> 
> --- mm/page-writeback.c.old	2012-12-06 22:20:40.000000000 +1100
> +++ mm/page-writeback.c	2013-01-21 13:57:05.000000000 +1100

Please prepare patches in `patch -p1' form.  This should be covered in
Documentation/SubmittingPatches, but isn't. 
Documentation/applying-patches.txt mentions it.

> @@ -343,12 +343,16 @@
>  unsigned long determine_dirtyable_memory(void)

You appear to be patching an old kernel.  But the change is still
applicable, to global_dirtyable_memory().

>  {
>  	unsigned long x;
> +	extern int min_free_kbytes;
>  
>  	x = global_page_state(NR_FREE_PAGES) + global_reclaimable_pages();
>  
>  	if (!vm_highmem_is_dirtyable)
>  		x -= highmem_dirtyable_memory(x);
>  
> +	/* Subtract min_free_kbytes */
> +	x -= min(x, min_free_kbytes >> (PAGE_SHIFT - 10));

Generates

mm/page-writeback.c:244: warning: comparison of distinct pointer types lacks a cast

because of the problematic min(int, unsigned long).  min_free_kbytes
should have an unsigned (long?) type, but I can't be bothered fixing
that right now..

From: Andrew Morton <akpm@linux-foundation.org>
Subject: page-writebackc-subtract-min_free_kbytes-from-dirtyable-memory-fix-fix

fix min() warning

Cc: Paul Szabo <psz@maths.usyd.edu.au>
Cc: Rik van Riel <riel@redhat.com>
Cc: Wu Fengguang <fengguang.wu@intel.com>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
---

 mm/page-writeback.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

--- a/mm/page-writeback.c~page-writebackc-subtract-min_free_kbytes-from-dirtyable-memory-fix-fix
+++ a/mm/page-writeback.c
@@ -241,7 +241,7 @@ static unsigned long global_dirtyable_me
 		x -= highmem_dirtyable_memory(x);
 
 	/* Subtract min_free_kbytes */
-	x -= min(x, min_free_kbytes >> (PAGE_SHIFT - 10));
+	x -= min_t(unsigned long, x, min_free_kbytes >> (PAGE_SHIFT - 10));
 
 	return x + 1;	/* Ensure that we never return 0 */
 }
_

--
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>

Re: [PATCH] Subtract min_free_kbytes from dirtyable memory

[Minchan Kim]

On Mon, Jan 21, 2013 at 02:15:49PM +1100, paul.szabo@sydney.edu.au wrote:
> When calculating amount of dirtyable memory, min_free_kbytes should be
> subtracted because it is not intended for dirty pages.

So what's the effect for user?
It would be better to include that in description if possible.

> 
> Using an "extern int" because that is the only interface to some such
> sysctl values.
> 
> (This patch does not solve the PAE OOM issue.)
> 
> Paul Szabo   psz@maths.usyd.edu.au   http://www.maths.usyd.edu.au/u/psz/
> School of Mathematics and Statistics   University of Sydney    Australia
> 
> Reported-by: Paul Szabo <psz@maths.usyd.edu.au>
> Reference: http://bugs.debian.org/695182
> Signed-off-by: Paul Szabo <psz@maths.usyd.edu.au>
> 
> --- mm/page-writeback.c.old	2012-12-06 22:20:40.000000000 +1100
> +++ mm/page-writeback.c	2013-01-21 13:57:05.000000000 +1100
> @@ -343,12 +343,16 @@
>  unsigned long determine_dirtyable_memory(void)
>  {
>  	unsigned long x;
> +	extern int min_free_kbytes;
>  
>  	x = global_page_state(NR_FREE_PAGES) + global_reclaimable_pages();
>  
>  	if (!vm_highmem_is_dirtyable)
>  		x -= highmem_dirtyable_memory(x);
>  
> +	/* Subtract min_free_kbytes */
> +	x -= min(x, min_free_kbytes >> (PAGE_SHIFT - 10));

It seems you saw old kernel.
Current kernel includes following logic.

static unsigned long global_dirtyable_memory(void)
{
        unsigned long x;

        x = global_page_state(NR_FREE_PAGES) + global_reclaimable_pages();
        x -= min(x, dirty_balance_reserve);

        if (!vm_highmem_is_dirtyable)
                x -= highmem_dirtyable_memory(x);

        return x + 1;   /* Ensure that we never return 0 */
}

And dirty_lanace_reserve already includes high_wmark_pages.
Look at calculate_totalreserve_pages.

So I think we don't need this patch.
Thanks.

> +
>  	return x + 1;	/* Ensure that we never return 0 */
>  }
> 
> --
> 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>

-- 
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/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH] Subtract min_free_kbytes from dirtyable memory

[paul.szabo]

Dear Minchan,

> So what's the effect for user?

Sorry I have no idea.

The kernel seems to work well without this patch; or in fact not so
well, PAE crashing with spurious OOM. In my fruitless efforts of
avoiding OOM by sensible choices of sysctl tunables, I noticed that
maybe the treatment of min_free_kbytes was not right. Getting this
right did not help in avoiding OOM.

> It seems you saw old kernel.

Yes I have Debian on my machines. :-)

> Current kernel includes following logic.
> 
> static unsigned long global_dirtyable_memory(void)
> {
>         unsigned long x;
> 
>         x = global_page_state(NR_FREE_PAGES) + global_reclaimable_pages();
>         x -= min(x, dirty_balance_reserve);
> 
>         if (!vm_highmem_is_dirtyable)
>                 x -= highmem_dirtyable_memory(x);
> 
>         return x + 1;   /* Ensure that we never return 0 */
> }
> 
> And dirty_lanace_reserve already includes high_wmark_pages.
> Look at calculate_totalreserve_pages.
> 
> So I think we don't need this patch.
> Thanks.

Presumably, dirty_balance_reserve takes min_free_kbytes into account?
Then I agree that this patch is not needed on those newer kernels.

A question: what is the use or significance of vm_highmem_is_dirtyable?
It seems odd that it would be used in setting limits or threshholds, but
not used in decisions where to put dirty things. Is that so, is that as
should be? What is the recommended setting of highmem_is_dirtyable?

Thanks, Paul

Paul Szabo   psz@maths.usyd.edu.au   http://www.maths.usyd.edu.au/u/psz/
School of Mathematics and Statistics   University of Sydney    Australia

--
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>

Re: [PATCH] Subtract min_free_kbytes from dirtyable memory

[paul.szabo]

Dear Minchan,

> So what's the effect for user?
> ...
> It seems you saw old kernel.
> ...
> Current kernel includes ...
> So I think we don't need this patch.

As I understand now, my patch is "right" and needed for older kernels;
for newer kernels, the issue has been fixed in equivalent ways; it was
an oversight that the change was not backported; and any justification
you need, you can get from those "later better" patches.

I asked:

  A question: what is the use or significance of vm_highmem_is_dirtyable?
  It seems odd that it would be used in setting limits or threshholds, but
  not used in decisions where to put dirty things. Is that so, is that as
  should be? What is the recommended setting of highmem_is_dirtyable?

The silence is deafening. I guess highmem_is_dirtyable is an aberration.

Thanks, Paul

Paul Szabo   psz@maths.usyd.edu.au   http://www.maths.usyd.edu.au/u/psz/
School of Mathematics and Statistics   University of Sydney    Australia

--
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>

Re: [PATCH] Subtract min_free_kbytes from dirtyable memory

[Minchan Kim]

On Fri, Jan 25, 2013 at 08:53:24PM +1100, paul.szabo@sydney.edu.au wrote:
> Dear Minchan,
> 
> > So what's the effect for user?
> > ...
> > It seems you saw old kernel.
> > ...
> > Current kernel includes ...
> > So I think we don't need this patch.
> 
> As I understand now, my patch is "right" and needed for older kernels;
> for newer kernels, the issue has been fixed in equivalent ways; it was
> an oversight that the change was not backported; and any justification
> you need, you can get from those "later better" patches.

I don't know your problem because you didn't write down your problem in
changelog. Anyway, If you want to apply it into older kernel,
please read Documentation/stable_kernel_rules.txt.

In summary,

1. Define your problem.
2. Apply your fix to see the problem goes away in older kernel.
3. If so, write the problem and effect in changelog
4. Send it to stable maintainers and mm maintainer

That's all.

> 
> I asked:
> 
>   A question: what is the use or significance of vm_highmem_is_dirtyable?
>   It seems odd that it would be used in setting limits or threshholds, but
>   not used in decisions where to put dirty things. Is that so, is that as
>   should be? What is the recommended setting of highmem_is_dirtyable?
> 
> The silence is deafening. I guess highmem_is_dirtyable is an aberration.

I hope this helps you find primary reason of your problem.
git show 195cf453


> 
> Thanks, Paul
> 
> Paul Szabo   psz@maths.usyd.edu.au   http://www.maths.usyd.edu.au/u/psz/
> School of Mathematics and Statistics   University of Sydney    Australia
> 
> --
> 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>

-- 
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/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: Bug#695182: [PATCH] Subtract min_free_kbytes from dirtyable memory

[Ben Hutchings]

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

On Fri, 2013-01-25 at 20:53 +1100, paul.szabo@sydney.edu.au wrote:
> Dear Minchan,
> 
> > So what's the effect for user?
> > ...
> > It seems you saw old kernel.
> > ...
> > Current kernel includes ...
> > So I think we don't need this patch.
> 
> As I understand now, my patch is "right" and needed for older kernels;
> for newer kernels, the issue has been fixed in equivalent ways; it was
> an oversight that the change was not backported; and any justification
> you need, you can get from those "later better" patches.
[...]

If you can identify where it was fixed then your patch for older
versions should go to stable with a reference to the upstream fix (see
Documentation/stable_kernel_rules.txt).

Ben.

-- 
Ben Hutchings
Q.  Which is the greater problem in the world today, ignorance or apathy?
A.  I don't know and I couldn't care less.

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 828 bytes --]

Re: Bug#695182: [PATCH] Subtract min_free_kbytes from dirtyable memory

[paul.szabo]

Dear Ben,

> If you can identify where it was fixed then ...

Sorry I cannot do that. I have no idea where kernel changelogs are kept.

I am happy to do some work. Please do not call me lazy.

Cheers, Paul

Paul Szabo   psz@maths.usyd.edu.au   http://www.maths.usyd.edu.au/u/psz/
School of Mathematics and Statistics   University of Sydney    Australia

--
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>

Re: [PATCH] Subtract min_free_kbytes from dirtyable memory

[Jonathan Nieder]

Hi Paul,

paul.szabo@sydney.edu.au wrote:
> Dear Ben,

>> If you can identify where it was fixed then ...
>
> Sorry I cannot do that. I have no idea where kernel changelogs are kept.

Here are some tools.

  # prerequisite:
  apt-get install git; # as root

  # to get the kernel history:
  git clone \
    https://kernel.googlesource.com/pub/scm/linux/kernel/git/torvalds/linux.git
  cd linux

  # to view the changelog:
  git log v3.2..

  # to grep change descriptions:
  git log --grep=min_free_kbytes v3.2..

  # to view the patches corresponding to changes:
  git log --patch v3.2.. -- mm/

  # graphical interface
  apt-get install gitk; # as root
  gitk v3.2.. -- mm

  # web interface:
  w3m http://git.kernel.org/?p=linux/kernel/git/torvalds/linux.git

The "exploring git history" section of the git user manual has more
details:

  http://git-htmldocs.googlecode.com/git/user-manual.html#exploring-git-history

Thanks,
Jonathan

--
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>

Re: [PATCH] Subtract min_free_kbytes from dirtyable memory

[Theodore Ts'o]

(In the teach a person to fish category...)

If you know the file and line number where a bug/regression was
introduced, the "git blame" command is a great tool for identifying
the commit which changed a given line of code.  Then use "git tag
--contains <commit it>" to see when a particular commit was introduced
into the mainline kernel.

					- Ted

--
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>