Re: [PATCH 1/1] mm: add dirty_highmem option

Andrew Morton
2007-11-27T04:54:49Z

On Thu, 22 Nov 2007 14:42:04 +1100 Bron Gondwana <brong@fastmail.fm> wrote:

> /*
> + * free highmem will not be subtracted from the total free memory
> + * for calculating free ratios if vm_dirty_highmem is true
> + */
> +int vm_dirty_highmem;

One would expect that setting dirty_highmem to true would cause highmem to
be accounted in dirty-memory calculations. However with this change
reality is in fact the inverse of that.

So how about this?

Documentation/filesystems/proc.txt | 4 ++--
mm/page-writeback.c | 8 ++++----
2 files changed, 6 insertions(+), 6 deletions(-)

diff -puN Documentation/filesystems/proc.txt~mm-add-dirty_highmem-option-fix Documentation/filesystems/proc.txt
--- a/Documentation/filesystems/proc.txt~mm-add-dirty_highmem-option-fix
+++ a/Documentation/filesystems/proc.txt
@@ -1265,8 +1265,8 @@ Contains, as a boolean, a switch to allo
part of the "available" memory against which the dirty ratios will be
applied.

-Setting this to 1 can be useful on 32 bit machines where you want to make
-random changes within an MMAPed file that is larger than your available
+Setting this to 0 (false) can be useful on 32 bit machines where you wish to
+make random changes within an MMAPed file that is larger than your available
lowmem, however it is potentially dangerous and has serious bounce-buffer
issues.

diff -puN mm/page-writeback.c~mm-add-dirty_highmem-option-fix mm/page-writeback.c
--- a/mm/page-writeback.c~mm-add-dirty_highmem-option-fix
+++ a/mm/page-writeback.c
@@ -69,10 +69,10 @@ static inline long sync_writeback_pages(
int dirty_background_ratio = 5;

/*
- * free highmem will not be subtracted from the total free memory
- * for calculating free ratios if vm_dirty_highmem is true
+ * free highmem will be subtracted from the total free memory for calculating
+ * free ratios if vm_dirty_highmem is true
*/
-int vm_dirty_highmem;
+int vm_dirty_highmem = 1;

/*
* The generator of dirty data starts writeback at this percentage
@@ -293,7 +293,7 @@ static unsigned long determine_dirtyable
x = global_page_state(NR_FREE_PAGES)
+ global_page_state(NR_INACTIVE)
+ global_page_state(NR_ACTIVE);
- if (!vm_dirty_highmem)
+ if (vm_dirty_highmem)
x -= highmem_dirtyable_memory(x);
return x + 1; /* Ensure that we never return 0 */
}
_

(I dropped the already-merged part of your patch)

(I fixed a build error in kernel/sysctl.c: "one" was defined twice when
suitable config options were set).

(It's an unpleasing patch, btw. But it's an unpleasant problem and at least
this way people can tell us "hey, I did <this> and it started to work")