[PATCH] remove throttle_vm_writeout()

[PATCH] remove throttle_vm_writeout()

[Miklos Szeredi]

This in preparation for the writable mmap patches for fuse.  I know it
conflicts with

  writeback-remove-unnecessary-wait-in-throttle_vm_writeout.patch

but if this function is to be removed, it doesn't make much sense to
fix it first ;)
---

From: Miklos Szeredi <mszeredi@suse.cz>

By relying on the global diry limits, this can cause a deadlock when
devices are stacked.

If the stacking is done through a fuse filesystem, the __GFP_FS,
__GFP_IO tests won't help: the process doing the allocation doesn't
have any special flag.

So why exactly does this function exist?

Direct reclaim does not _increase_ the number of dirty pages in the
system, so rate limiting it seems somewhat pointless.

There are two cases:

1) File backed pages -> file

  dirty + writeback count remains constant

2) Anonymous pages -> swap

  writeback count increases, dirty balancing will hold back file
  writeback in favor of swap

So the real question is: does case 2 need rate limiting, or is it OK
to let the device queue fill with swap pages as fast as possible?

Signed-off-by: Miklos Szeredi <mszeredi@suse.cz>
---

Index: linux/include/linux/writeback.h
===================================================================
--- linux.orig/include/linux/writeback.h	2007-10-02 16:55:03.000000000 +0200
+++ linux/include/linux/writeback.h	2007-10-04 13:40:33.000000000 +0200
@@ -94,7 +94,6 @@ static inline void inode_sync_wait(struc
 int wakeup_pdflush(long nr_pages);
 void laptop_io_completion(void);
 void laptop_sync_completion(void);
-void throttle_vm_writeout(gfp_t gfp_mask);
 
 /* These are exported to sysctl. */
 extern int dirty_background_ratio;
Index: linux/mm/page-writeback.c
===================================================================
--- linux.orig/mm/page-writeback.c	2007-10-02 16:55:03.000000000 +0200
+++ linux/mm/page-writeback.c	2007-10-04 13:40:33.000000000 +0200
@@ -497,37 +497,6 @@ void balance_dirty_pages_ratelimited_nr(
 }
 EXPORT_SYMBOL(balance_dirty_pages_ratelimited_nr);
 
-void throttle_vm_writeout(gfp_t gfp_mask)
-{
-	long background_thresh;
-	long dirty_thresh;
-
-	if ((gfp_mask & (__GFP_FS|__GFP_IO)) != (__GFP_FS|__GFP_IO)) {
-		/*
-		 * The caller might hold locks which can prevent IO completion
-		 * or progress in the filesystem.  So we cannot just sit here
-		 * waiting for IO to complete.
-		 */
-		congestion_wait(WRITE, HZ/10);
-		return;
-	}
-
-        for ( ; ; ) {
-		get_dirty_limits(&background_thresh, &dirty_thresh, NULL, NULL);
-
-                /*
-                 * Boost the allowable dirty threshold a bit for page
-                 * allocators so they don't get DoS'ed by heavy writers
-                 */
-                dirty_thresh += dirty_thresh / 10;      /* wheeee... */
-
-                if (global_page_state(NR_UNSTABLE_NFS) +
-			global_page_state(NR_WRITEBACK) <= dirty_thresh)
-                        	break;
-                congestion_wait(WRITE, HZ/10);
-        }
-}
-
 /*
  * writeback at least _min_pages, and keep writing until the amount of dirty
  * memory is less than the background threshold, or until we're all clean.
Index: linux/mm/vmscan.c
===================================================================
--- linux.orig/mm/vmscan.c	2007-10-02 16:55:03.000000000 +0200
+++ linux/mm/vmscan.c	2007-10-04 13:40:33.000000000 +0200
@@ -1184,7 +1184,6 @@ static unsigned long shrink_zone(int pri
 		}
 	}
 
-	throttle_vm_writeout(sc->gfp_mask);
 	return nr_reclaimed;
 }
 

--
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] remove throttle_vm_writeout()

[Peter Zijlstra]

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

On Thu, 2007-10-04 at 14:25 +0200, Miklos Szeredi wrote:
> This in preparation for the writable mmap patches for fuse.  I know it
> conflicts with
> 
>   writeback-remove-unnecessary-wait-in-throttle_vm_writeout.patch
> 
> but if this function is to be removed, it doesn't make much sense to
> fix it first ;)
> ---
> 
> From: Miklos Szeredi <mszeredi@suse.cz>
> 
> By relying on the global diry limits, this can cause a deadlock when
> devices are stacked.
> 
> If the stacking is done through a fuse filesystem, the __GFP_FS,
> __GFP_IO tests won't help: the process doing the allocation doesn't
> have any special flag.
> 
> So why exactly does this function exist?
> 
> Direct reclaim does not _increase_ the number of dirty pages in the
> system, so rate limiting it seems somewhat pointless.
> 
> There are two cases:
> 
> 1) File backed pages -> file
> 
>   dirty + writeback count remains constant
> 
> 2) Anonymous pages -> swap
> 
>   writeback count increases, dirty balancing will hold back file
>   writeback in favor of swap
> 
> So the real question is: does case 2 need rate limiting, or is it OK
> to let the device queue fill with swap pages as fast as possible?

Because balance_dirty_pages() maintains:

 nr_dirty + nr_unstable + nr_writeback < 
	total_dirty + nr_cpus * ratelimit_pages

throttle_vm_writeout() _should_ not deadlock on that, unless you're
caught in the error term: nr_cpus * ratelimit_pages. 

Which can only happen when it is larger than 10% of dirty_thresh.

Which is even more unlikely since it doesn't account nr_dirty (as I
think it should).

As for 2), yes I think having a limit on the total number of pages in
flight is a good thing. But that said, there might be better ways to do
that.




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

Re: [PATCH] remove throttle_vm_writeout()

[Miklos Szeredi]

> > 1) File backed pages -> file
> > 
> >   dirty + writeback count remains constant
> > 
> > 2) Anonymous pages -> swap
> > 
> >   writeback count increases, dirty balancing will hold back file
> >   writeback in favor of swap
> > 
> > So the real question is: does case 2 need rate limiting, or is it OK
> > to let the device queue fill with swap pages as fast as possible?
> 
> > Because balance_dirty_pages() maintains:
> 
>  nr_dirty + nr_unstable + nr_writeback < 
> 	total_dirty + nr_cpus * ratelimit_pages
> 
> throttle_vm_writeout() _should_ not deadlock on that, unless you're
> caught in the error term: nr_cpus * ratelimit_pages. 

And it does get caught on that in small memory machines.  This
deadlock is easily reproducable on a 32MB UML instance.  I haven't yet
tested with the per-bdi patches, but I don't think they make a
difference in this case.

> Which can only happen when it is larger than 10% of dirty_thresh.
> 
> Which is even more unlikely since it doesn't account nr_dirty (as I
> think it should).

I think nr_dirty is totally irrelevant.  Since we don't care about
case 1), and in case 2) nr_dirty doesn't play any role.

> As for 2), yes I think having a limit on the total number of pages in
> flight is a good thing.

Why?

> But that said, there might be better ways to do that.

Sure, if we do need to globally limit the number of under-writeback
pages, then I think we need to do it independently of the dirty
accounting.

Miklos

--
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] remove throttle_vm_writeout()

[Peter Zijlstra]

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

On Thu, 2007-10-04 at 15:00 +0200, Miklos Szeredi wrote:
> > > 1) File backed pages -> file
> > > 
> > >   dirty + writeback count remains constant
> > > 
> > > 2) Anonymous pages -> swap
> > > 
> > >   writeback count increases, dirty balancing will hold back file
> > >   writeback in favor of swap
> > > 
> > > So the real question is: does case 2 need rate limiting, or is it OK
> > > to let the device queue fill with swap pages as fast as possible?
> > 
> > > Because balance_dirty_pages() maintains:
> > 
> >  nr_dirty + nr_unstable + nr_writeback < 
> > 	total_dirty + nr_cpus * ratelimit_pages
> > 
> > throttle_vm_writeout() _should_ not deadlock on that, unless you're
> > caught in the error term: nr_cpus * ratelimit_pages. 
> 
> And it does get caught on that in small memory machines.  This
> deadlock is easily reproducable on a 32MB UML instance.  

Ah, yes, for those that is indeed easily doable.

> I haven't yet
> tested with the per-bdi patches, but I don't think they make a
> difference in this case.

Correct, they would not.

> > Which can only happen when it is larger than 10% of dirty_thresh.
> > 
> > Which is even more unlikely since it doesn't account nr_dirty (as I
> > think it should).
> 
> I think nr_dirty is totally irrelevant.  Since we don't care about
> case 1), and in case 2) nr_dirty doesn't play any role.

Ah, but its correct to have since we compare against dirty_thresh, which
is defined to be a unit of nr_dirty + nr_unstable + nr_writeback. if we
take one of these out, then we get an undefined amount of space extra.

> > As for 2), yes I think having a limit on the total number of pages in
> > flight is a good thing.
> 
> Why?

for my swapping over network thingies I need to put a bound on the
amount of outgoing traffic in flight because that bounds the amount of
memory consumed by the sending side.

> > But that said, there might be better ways to do that.
> 
> Sure, if we do need to globally limit the number of under-writeback
> pages, then I think we need to do it independently of the dirty
> accounting.

It need not be global, it could be per BDI as well, but yes.

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

Re: [PATCH] remove throttle_vm_writeout()

[Miklos Szeredi]

> > > Which can only happen when it is larger than 10% of dirty_thresh.
> > > 
> > > Which is even more unlikely since it doesn't account nr_dirty (as I
> > > think it should).
> > 
> > I think nr_dirty is totally irrelevant.  Since we don't care about
> > case 1), and in case 2) nr_dirty doesn't play any role.
> 
> Ah, but its correct to have since we compare against dirty_thresh, which
> is defined to be a unit of nr_dirty + nr_unstable + nr_writeback. if we
> take one of these out, then we get an undefined amount of space extra.

Yeah, I guess the point of the function was to limit nr_write to
_anything_ smaller than the total memory.

> > > As for 2), yes I think having a limit on the total number of pages in
> > > flight is a good thing.
> > 
> > Why?
> 
> for my swapping over network thingies I need to put a bound on the
> amount of outgoing traffic in flight because that bounds the amount of
> memory consumed by the sending side.

I guess you will have some request queue with limited length, no?

The main problem seems to be if devices use up all the reserved memory
for queuing write requests.  Limiting the in-flight pages is a very
crude way to solve this, the assumptions are:

O: overhead as a fraction of the request size
T: total memory
R: reserved memory
T-R: may be full of anon pages

so if (T-R)*O > R  we are in trouble.

if we limit the writeback memory to L and L*O < R we are OK.  But we
don't know O (it's device dependent).  We can make an estimate
calculate L based on that, but that will be a number totally
independent of the dirty threshold.

> > > But that said, there might be better ways to do that.
> > 
> > Sure, if we do need to globally limit the number of under-writeback
> > pages, then I think we need to do it independently of the dirty
> > accounting.
> 
> It need not be global, it could be per BDI as well, but yes.

For per-bdi limits we have the queue length.

Miklod

--
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] remove throttle_vm_writeout()

[Peter Zijlstra]

On Thu, 2007-10-04 at 15:49 +0200, Miklos Szeredi wrote:
> > > > Which can only happen when it is larger than 10% of dirty_thresh.
> > > > 
> > > > Which is even more unlikely since it doesn't account nr_dirty (as I
> > > > think it should).
> > > 
> > > I think nr_dirty is totally irrelevant.  Since we don't care about
> > > case 1), and in case 2) nr_dirty doesn't play any role.
> > 
> > Ah, but its correct to have since we compare against dirty_thresh, which
> > is defined to be a unit of nr_dirty + nr_unstable + nr_writeback. if we
> > take one of these out, then we get an undefined amount of space extra.
> 
> Yeah, I guess the point of the function was to limit nr_write to
> _anything_ smaller than the total memory.

*grin*, crude :-/

> > > > As for 2), yes I think having a limit on the total number of pages in
> > > > flight is a good thing.
> > > 
> > > Why?
> > 
> > for my swapping over network thingies I need to put a bound on the
> > amount of outgoing traffic in flight because that bounds the amount of
> > memory consumed by the sending side.
> 
> I guess you will have some request queue with limited length, no?

See below.

> The main problem seems to be if devices use up all the reserved memory
> for queuing write requests.  Limiting the in-flight pages is a very
> crude way to solve this, the assumptions are:
> 
> O: overhead as a fraction of the request size
> T: total memory
> R: reserved memory
> T-R: may be full of anon pages
> 
> so if (T-R)*O > R  we are in trouble.
> 
> if we limit the writeback memory to L and L*O < R we are OK.  But we
> don't know O (it's device dependent).  We can make an estimate
> calculate L based on that, but that will be a number totally
> independent of the dirty threshold.

Yeah, I'm guestimating O on a per device basis, but I agree that the
current ratio limiting is quite crude. I'm not at all sorry to see
throttle_vm_writeback() go, I just wanted to make a point that what it
does is not quite without merrit - we agree that it can be done better
differently.

> > > > But that said, there might be better ways to do that.
> > > 
> > > Sure, if we do need to globally limit the number of under-writeback
> > > pages, then I think we need to do it independently of the dirty
> > > accounting.
> > 
> > It need not be global, it could be per BDI as well, but yes.
> 
> For per-bdi limits we have the queue length.

Agreed, except for:

static int may_write_to_queue(struct backing_dev_info *bdi)
{
	if (current->flags & PF_SWAPWRITE)
		return 1;
	if (!bdi_write_congested(bdi))
		return 1;
	if (bdi == current->backing_dev_info)
		return 1;
	return 0;
}

Which will write to congested queues. Anybody know why?

--
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] remove throttle_vm_writeout()

[Andrew Morton]

On Thu, 04 Oct 2007 18:47:07 +0200 Peter Zijlstra <a.p.zijlstra@chello.nl> wrote:

> > > > > But that said, there might be better ways to do that.
> > > > 
> > > > Sure, if we do need to globally limit the number of under-writeback
> > > > pages, then I think we need to do it independently of the dirty
> > > > accounting.
> > > 
> > > It need not be global, it could be per BDI as well, but yes.
> > 
> > For per-bdi limits we have the queue length.
> 
> Agreed, except for:
> 
> static int may_write_to_queue(struct backing_dev_info *bdi)
> {
> 	if (current->flags & PF_SWAPWRITE)
> 		return 1;
> 	if (!bdi_write_congested(bdi))
> 		return 1;
> 	if (bdi == current->backing_dev_info)
> 		return 1;
> 	return 0;
> }
> 
> Which will write to congested queues. Anybody know why?


commit c4e2d7ddde9693a4c05da7afd485db02c27a7a09
Author: akpm <akpm>
Date:   Sun Dec 22 01:07:33 2002 +0000

    [PATCH] Give kswapd writeback higher priority than pdflush
    
    The `low latency page reclaim' design works by preventing page
    allocators from blocking on request queues (and by preventing them from
    blocking against writeback of individual pages, but that is immaterial
    here).
    
    This has a problem under some situations.  pdflush (or a write(2)
    caller) could be saturating the queue with highmem pages.  This
    prevents anyone from writing back ZONE_NORMAL pages.  We end up doing
    enormous amounts of scenning.
    
    A test case is to mmap(MAP_SHARED) almost all of a 4G machine's memory,
    then kill the mmapping applications.  The machine instantly goes from
    0% of memory dirty to 95% or more.  pdflush kicks in and starts writing
    the least-recently-dirtied pages, which are all highmem.  The queue is
    congested so nobody will write back ZONE_NORMAL pages.  kswapd chews
    50% of the CPU scanning past dirty ZONE_NORMAL pages and page reclaim
    efficiency (pages_reclaimed/pages_scanned) falls to 2%.
    
    So this patch changes the policy for kswapd.  kswapd may use all of a
    request queue, and is prepared to block on request queues.
    
    What will now happen in the above scenario is:
    
    1: The page alloctor scans some pages, fails to reclaim enough
       memory and takes a nap in blk_congetion_wait().
    
    2: kswapd() will scan the ZONE_NORMAL LRU and will start writing
       back pages.  (These pages will be rotated to the tail of the
       inactive list at IO-completion interrupt time).
    
       This writeback will saturate the queue with ZONE_NORMAL pages.
       Conveniently, pdflush will avoid the congested queues.  So we end up
       writing the correct pages.
    
    In this test, kswapd CPU utilisation falls from 50% to 2%, page reclaim
    efficiency rises from 2% to 40% and things are generally a lot happier.
    
    
    The downside is that kswapd may now do a lot less page reclaim,
    increasing page allocation latency, causing more direct reclaim,
    increasing lock contention in the VM, etc.  But I have not been able to
    demonstrate that in testing.
    
    
    The other problem is that there is only one kswapd, and there are lots
    of disks.  That is a generic problem - without being able to co-opt
    user processes we don't have enough threads to keep lots of disks saturated.
    
    One fix for this would be to add an additional "really congested"
    threshold in the request queues, so kswapd can still perform
    nonblocking writeout.  This gives kswapd priority over pdflush while
    allowing kswapd to feed many disk queues.  I doubt if this will be
    called for.
    
    BKrev: 3e051055aitHp3bZBPSqmq21KGs5aQ

diff --git a/include/linux/swap.h b/include/linux/swap.h
index c635f39..9ab0209 100644
--- a/include/linux/swap.h
+++ b/include/linux/swap.h
@@ -7,6 +7,7 @@ #include <linux/kdev_t.h>
 #include <linux/linkage.h>
 #include <linux/mmzone.h>
 #include <linux/list.h>
+#include <linux/sched.h>
 #include <asm/atomic.h>
 #include <asm/page.h>
 
@@ -14,6 +15,11 @@ #define SWAP_FLAG_PREFER	0x8000	/* set i
 #define SWAP_FLAG_PRIO_MASK	0x7fff
 #define SWAP_FLAG_PRIO_SHIFT	0
 
+static inline int current_is_kswapd(void)
+{
+	return current->flags & PF_KSWAPD;
+}
+
 /*
  * MAX_SWAPFILES defines the maximum number of swaptypes: things which can
  * be swapped to.  The swap type and the offset into that swap type are
diff --git a/mm/vmscan.c b/mm/vmscan.c
index aeab1e3..a8b9d2c 100644
--- a/mm/vmscan.c
+++ b/mm/vmscan.c
@@ -204,6 +204,19 @@ static inline int is_page_cache_freeable
 	return page_count(page) - !!PagePrivate(page) == 2;
 }
 
+static int may_write_to_queue(struct backing_dev_info *bdi)
+{
+	if (current_is_kswapd())
+		return 1;
+	if (current_is_pdflush())	/* This is unlikely, but why not... */
+		return 1;
+	if (!bdi_write_congested(bdi))
+		return 1;
+	if (bdi == current->backing_dev_info)
+		return 1;
+	return 0;
+}
+
 /*
  * shrink_list returns the number of reclaimed pages
  */
@@ -303,8 +316,6 @@ #endif /* CONFIG_SWAP */
 		 * See swapfile.c:page_queue_congested().
 		 */
 		if (PageDirty(page)) {
-			struct backing_dev_info *bdi;
-
 			if (!is_page_cache_freeable(page))
 				goto keep_locked;
 			if (!mapping)
@@ -313,9 +324,7 @@ #endif /* CONFIG_SWAP */
 				goto activate_locked;
 			if (!may_enter_fs)
 				goto keep_locked;
-			bdi = mapping->backing_dev_info;
-			if (bdi != current->backing_dev_info &&
-					bdi_write_congested(bdi))
+			if (!may_write_to_queue(mapping->backing_dev_info))
 				goto keep_locked;
 			write_lock(&mapping->page_lock);
 			if (test_clear_page_dirty(page)) {
@@ -424,7 +433,7 @@ keep:
 	if (pagevec_count(&freed_pvec))
 		__pagevec_release_nonlru(&freed_pvec);
 	mod_page_state(pgsteal, ret);
-	if (current->flags & PF_KSWAPD)
+	if (current_is_kswapd())
 		mod_page_state(kswapd_steal, ret);
 	mod_page_state(pgactivate, pgactivate);
 	return ret;

--
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] remove throttle_vm_writeout()

[Peter Zijlstra]

On Thu, 2007-10-04 at 10:46 -0700, Andrew Morton wrote:
> On Thu, 04 Oct 2007 18:47:07 +0200 Peter Zijlstra <a.p.zijlstra@chello.nl> wrote:

> > static int may_write_to_queue(struct backing_dev_info *bdi)
> > {
> > 	if (current->flags & PF_SWAPWRITE)
> > 		return 1;
> > 	if (!bdi_write_congested(bdi))
> > 		return 1;
> > 	if (bdi == current->backing_dev_info)
> > 		return 1;
> > 	return 0;
> > }
> > 
> > Which will write to congested queues. Anybody know why?

OK, I guess I could have found that :-/

> commit c4e2d7ddde9693a4c05da7afd485db02c27a7a09
> Author: akpm <akpm>
> Date:   Sun Dec 22 01:07:33 2002 +0000
> 
>     [PATCH] Give kswapd writeback higher priority than pdflush
>     
>     The `low latency page reclaim' design works by preventing page
>     allocators from blocking on request queues (and by preventing them from
>     blocking against writeback of individual pages, but that is immaterial
>     here).
>     
>     This has a problem under some situations.  pdflush (or a write(2)
>     caller) could be saturating the queue with highmem pages.  This
>     prevents anyone from writing back ZONE_NORMAL pages.  We end up doing
>     enormous amounts of scenning.
>     
>     A test case is to mmap(MAP_SHARED) almost all of a 4G machine's memory,
>     then kill the mmapping applications.  The machine instantly goes from
>     0% of memory dirty to 95% or more. 

With dirty page tracking this is not supposed to happen anymore.

>  pdflush kicks in and starts writing
>     the least-recently-dirtied pages, which are all highmem. 

with highmem >> normal, and user pages preferring highmem, this will
likely still be true.

>  The queue is
>     congested so nobody will write back ZONE_NORMAL pages.  kswapd chews
>     50% of the CPU scanning past dirty ZONE_NORMAL pages and page reclaim
>     efficiency (pages_reclaimed/pages_scanned) falls to 2%.

So, the problem is a heavy writer vs swap. Which is still possible.
   
>     So this patch changes the policy for kswapd.  kswapd may use all of a
>     request queue, and is prepared to block on request queues.

So request queue's have a limit above the congestion level on which they
will block?

NFS doesn't have that AFAIK

>     What will now happen in the above scenario is:
>     
>     1: The page alloctor scans some pages, fails to reclaim enough
>        memory and takes a nap in blk_congetion_wait().
>     
>     2: kswapd() will scan the ZONE_NORMAL LRU and will start writing
>        back pages.  (These pages will be rotated to the tail of the
>        inactive list at IO-completion interrupt time).
>     
>        This writeback will saturate the queue with ZONE_NORMAL pages.
>        Conveniently, pdflush will avoid the congested queues.  So we end up
>        writing the correct pages.
>     
>     In this test, kswapd CPU utilisation falls from 50% to 2%, page reclaim
>     efficiency rises from 2% to 40% and things are generally a lot happier.
>     
>     
>     The downside is that kswapd may now do a lot less page reclaim,
>     increasing page allocation latency, causing more direct reclaim,
>     increasing lock contention in the VM, etc.  But I have not been able to
>     demonstrate that in testing.
>     
>     
>     The other problem is that there is only one kswapd, and there are lots
>     of disks.  That is a generic problem - without being able to co-opt
>     user processes we don't have enough threads to keep lots of disks saturated.
>     
>     One fix for this would be to add an additional "really congested"
>     threshold in the request queues, so kswapd can still perform
>     nonblocking writeout.  This gives kswapd priority over pdflush while
>     allowing kswapd to feed many disk queues.  I doubt if this will be
>     called for.

I could do that.

--
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] remove throttle_vm_writeout()

[Andrew Morton]

On Thu, 04 Oct 2007 20:10:10 +0200
Peter Zijlstra <a.p.zijlstra@chello.nl> wrote:

> 
> On Thu, 2007-10-04 at 10:46 -0700, Andrew Morton wrote:
> > On Thu, 04 Oct 2007 18:47:07 +0200 Peter Zijlstra <a.p.zijlstra@chello.nl> wrote:
> 
> > > static int may_write_to_queue(struct backing_dev_info *bdi)
> > > {
> > > 	if (current->flags & PF_SWAPWRITE)
> > > 		return 1;
> > > 	if (!bdi_write_congested(bdi))
> > > 		return 1;
> > > 	if (bdi == current->backing_dev_info)
> > > 		return 1;
> > > 	return 0;
> > > }
> > > 
> > > Which will write to congested queues. Anybody know why?
> 
> OK, I guess I could have found that :-/

Nice changelog, if I do say so myself ;)

> >     One fix for this would be to add an additional "really congested"
> >     threshold in the request queues, so kswapd can still perform
> >     nonblocking writeout.  This gives kswapd priority over pdflush while
> >     allowing kswapd to feed many disk queues.  I doubt if this will be
> >     called for.
> 
> I could do that.

I guess first you'd need to be able to reproduce the problem which that
patch fixed, then check that it remains fixed.

Sigh.  That problem was fairly subtle.  We could re-break reclaim in
this way and not find out about it for six months.  There's a lesson here. 
Several.  

--
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] remove throttle_vm_writeout()

[Fengguang Wu]

On Thu, Oct 04, 2007 at 10:46:50AM -0700, Andrew Morton wrote:
> On Thu, 04 Oct 2007 18:47:07 +0200 Peter Zijlstra <a.p.zijlstra@chello.nl> wrote:
> 
> > > > > > But that said, there might be better ways to do that.
> > > > > 
> > > > > Sure, if we do need to globally limit the number of under-writeback
> > > > > pages, then I think we need to do it independently of the dirty
> > > > > accounting.
> > > > 
> > > > It need not be global, it could be per BDI as well, but yes.
> > > 
> > > For per-bdi limits we have the queue length.
> > 
> > Agreed, except for:
> > 
> > static int may_write_to_queue(struct backing_dev_info *bdi)
> > {
> > 	if (current->flags & PF_SWAPWRITE)
> > 		return 1;
> > 	if (!bdi_write_congested(bdi))
> > 		return 1;
> > 	if (bdi == current->backing_dev_info)
> > 		return 1;
> > 	return 0;
> > }
> > 
> > Which will write to congested queues. Anybody know why?
> 
> 
> commit c4e2d7ddde9693a4c05da7afd485db02c27a7a09
> Author: akpm <akpm>
> Date:   Sun Dec 22 01:07:33 2002 +0000
> 
>     [PATCH] Give kswapd writeback higher priority than pdflush
>     
>     The `low latency page reclaim' design works by preventing page
>     allocators from blocking on request queues (and by preventing them from
>     blocking against writeback of individual pages, but that is immaterial
>     here).
>     
>     This has a problem under some situations.  pdflush (or a write(2)
>     caller) could be saturating the queue with highmem pages.  This
>     prevents anyone from writing back ZONE_NORMAL pages.  We end up doing
>     enormous amounts of scenning.

Sorry, I cannot not understand it. We now have balanced aging between
zones. So the page allocations are expected to distribute proportionally
between ZONE_HIGHMEM and ZONE_NORMAL?

>     A test case is to mmap(MAP_SHARED) almost all of a 4G machine's memory,
>     then kill the mmapping applications.  The machine instantly goes from
>     0% of memory dirty to 95% or more.  pdflush kicks in and starts writing
>     the least-recently-dirtied pages, which are all highmem.  The queue is
>     congested so nobody will write back ZONE_NORMAL pages.  kswapd chews
>     50% of the CPU scanning past dirty ZONE_NORMAL pages and page reclaim
>     efficiency (pages_reclaimed/pages_scanned) falls to 2%.
>     
>     So this patch changes the policy for kswapd.  kswapd may use all of a
>     request queue, and is prepared to block on request queues.
>     
>     What will now happen in the above scenario is:
>     
>     1: The page alloctor scans some pages, fails to reclaim enough
>        memory and takes a nap in blk_congetion_wait().
>     
>     2: kswapd() will scan the ZONE_NORMAL LRU and will start writing
>        back pages.  (These pages will be rotated to the tail of the
>        inactive list at IO-completion interrupt time).
>     
>        This writeback will saturate the queue with ZONE_NORMAL pages.
>        Conveniently, pdflush will avoid the congested queues.  So we end up
>        writing the correct pages.
>     
>     In this test, kswapd CPU utilisation falls from 50% to 2%, page reclaim
>     efficiency rises from 2% to 40% and things are generally a lot happier.

We may see the same problem and improvement in the absent of 'all
writeback goes to one zone' assumption.

The problem could be:
- dirty_thresh is exceeded, so balance_dirty_pages() starts syncing
  data and quickly _congests_ the queue;
- dirty pages are slowly but continuously turned into clean pages by
  balance_dirty_pages(), but they still stay in the same place in LRU;
- the zones are mostly dirty/writeback pages, kswapd has a hard time
  finding the randomly distributed clean pages;
- kswapd cannot do the writeout because the queue is congested!

The improvement could be:
- kswapd is now explicitly preferred to do the writeout;
- the pages written by kswapd will be rotated and easy for kswapd to reclaim;
- it becomes possible for kswapd to wait for the congested queue,
  instead of doing the vmscan like mad.

The congestion wait looks like a pretty natural way to throttle the kswapd.
Instead of doing the vmscan at 1000MB/s and actually freeing pages at
60MB/s(about the write throughput), kswapd will be relaxed to do vmscan at
maybe 150MB/s.

Fengguang
---
>     The downside is that kswapd may now do a lot less page reclaim,
>     increasing page allocation latency, causing more direct reclaim,
>     increasing lock contention in the VM, etc.  But I have not been able to
>     demonstrate that in testing.
>     
>     
>     The other problem is that there is only one kswapd, and there are lots
>     of disks.  That is a generic problem - without being able to co-opt
>     user processes we don't have enough threads to keep lots of disks saturated.
>     
>     One fix for this would be to add an additional "really congested"
>     threshold in the request queues, so kswapd can still perform
>     nonblocking writeout.  This gives kswapd priority over pdflush while
>     allowing kswapd to feed many disk queues.  I doubt if this will be
>     called for.
>     
>     BKrev: 3e051055aitHp3bZBPSqmq21KGs5aQ
> 
> diff --git a/include/linux/swap.h b/include/linux/swap.h
> index c635f39..9ab0209 100644
> --- a/include/linux/swap.h
> +++ b/include/linux/swap.h
> @@ -7,6 +7,7 @@ #include <linux/kdev_t.h>
>  #include <linux/linkage.h>
>  #include <linux/mmzone.h>
>  #include <linux/list.h>
> +#include <linux/sched.h>
>  #include <asm/atomic.h>
>  #include <asm/page.h>
>  
> @@ -14,6 +15,11 @@ #define SWAP_FLAG_PREFER	0x8000	/* set i
>  #define SWAP_FLAG_PRIO_MASK	0x7fff
>  #define SWAP_FLAG_PRIO_SHIFT	0
>  
> +static inline int current_is_kswapd(void)
> +{
> +	return current->flags & PF_KSWAPD;
> +}
> +
>  /*
>   * MAX_SWAPFILES defines the maximum number of swaptypes: things which can
>   * be swapped to.  The swap type and the offset into that swap type are
> diff --git a/mm/vmscan.c b/mm/vmscan.c
> index aeab1e3..a8b9d2c 100644
> --- a/mm/vmscan.c
> +++ b/mm/vmscan.c
> @@ -204,6 +204,19 @@ static inline int is_page_cache_freeable
>  	return page_count(page) - !!PagePrivate(page) == 2;
>  }
>  
> +static int may_write_to_queue(struct backing_dev_info *bdi)
> +{
> +	if (current_is_kswapd())
> +		return 1;
> +	if (current_is_pdflush())	/* This is unlikely, but why not... */
> +		return 1;
> +	if (!bdi_write_congested(bdi))
> +		return 1;
> +	if (bdi == current->backing_dev_info)
> +		return 1;
> +	return 0;
> +}
> +
>  /*
>   * shrink_list returns the number of reclaimed pages
>   */
> @@ -303,8 +316,6 @@ #endif /* CONFIG_SWAP */
>  		 * See swapfile.c:page_queue_congested().
>  		 */
>  		if (PageDirty(page)) {
> -			struct backing_dev_info *bdi;
> -
>  			if (!is_page_cache_freeable(page))
>  				goto keep_locked;
>  			if (!mapping)
> @@ -313,9 +324,7 @@ #endif /* CONFIG_SWAP */
>  				goto activate_locked;
>  			if (!may_enter_fs)
>  				goto keep_locked;
> -			bdi = mapping->backing_dev_info;
> -			if (bdi != current->backing_dev_info &&
> -					bdi_write_congested(bdi))
> +			if (!may_write_to_queue(mapping->backing_dev_info))
>  				goto keep_locked;
>  			write_lock(&mapping->page_lock);
>  			if (test_clear_page_dirty(page)) {
> @@ -424,7 +433,7 @@ keep:
>  	if (pagevec_count(&freed_pvec))
>  		__pagevec_release_nonlru(&freed_pvec);
>  	mod_page_state(pgsteal, ret);
> -	if (current->flags & PF_KSWAPD)
> +	if (current_is_kswapd())
>  		mod_page_state(kswapd_steal, ret);
>  	mod_page_state(pgactivate, pgactivate);
>  	return ret;
> 

--
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] remove throttle_vm_writeout()

[Andrew Morton]

On Fri, 5 Oct 2007 20:30:28 +0800
Fengguang Wu <wfg@mail.ustc.edu.cn> wrote:

> > commit c4e2d7ddde9693a4c05da7afd485db02c27a7a09
> > Author: akpm <akpm>
> > Date:   Sun Dec 22 01:07:33 2002 +0000
> > 
> >     [PATCH] Give kswapd writeback higher priority than pdflush
> >     
> >     The `low latency page reclaim' design works by preventing page
> >     allocators from blocking on request queues (and by preventing them from
> >     blocking against writeback of individual pages, but that is immaterial
> >     here).
> >     
> >     This has a problem under some situations.  pdflush (or a write(2)
> >     caller) could be saturating the queue with highmem pages.  This
> >     prevents anyone from writing back ZONE_NORMAL pages.  We end up doing
> >     enormous amounts of scenning.
> 
> Sorry, I cannot not understand it. We now have balanced aging between
> zones. So the page allocations are expected to distribute proportionally
> between ZONE_HIGHMEM and ZONE_NORMAL?

Sure, but we don't have one disk queue per disk per zone!  The queue is
shared by all the zones.  So if writeback from one zone has filled the
queue up, the kernel can't write back data from another zone.

(Well, it can, by blocking in get_request_wait(), but that causes long and
uncontrollable latencies).

> >     A test case is to mmap(MAP_SHARED) almost all of a 4G machine's memory,
> >     then kill the mmapping applications.  The machine instantly goes from
> >     0% of memory dirty to 95% or more.  pdflush kicks in and starts writing
> >     the least-recently-dirtied pages, which are all highmem.  The queue is
> >     congested so nobody will write back ZONE_NORMAL pages.  kswapd chews
> >     50% of the CPU scanning past dirty ZONE_NORMAL pages and page reclaim
> >     efficiency (pages_reclaimed/pages_scanned) falls to 2%.
> >     
> >     So this patch changes the policy for kswapd.  kswapd may use all of a
> >     request queue, and is prepared to block on request queues.
> >     
> >     What will now happen in the above scenario is:
> >     
> >     1: The page alloctor scans some pages, fails to reclaim enough
> >        memory and takes a nap in blk_congetion_wait().
> >     
> >     2: kswapd() will scan the ZONE_NORMAL LRU and will start writing
> >        back pages.  (These pages will be rotated to the tail of the
> >        inactive list at IO-completion interrupt time).
> >     
> >        This writeback will saturate the queue with ZONE_NORMAL pages.
> >        Conveniently, pdflush will avoid the congested queues.  So we end up
> >        writing the correct pages.
> >     
> >     In this test, kswapd CPU utilisation falls from 50% to 2%, page reclaim
> >     efficiency rises from 2% to 40% and things are generally a lot happier.
> 
> We may see the same problem and improvement in the absent of 'all
> writeback goes to one zone' assumption.
> 
> The problem could be:
> - dirty_thresh is exceeded, so balance_dirty_pages() starts syncing
>   data and quickly _congests_ the queue;

Or someone ran fsync(), or pdflush is writing back data because it exceeded
dirty_writeback_centisecs, etc.

> - dirty pages are slowly but continuously turned into clean pages by
>   balance_dirty_pages(), but they still stay in the same place in LRU;
> - the zones are mostly dirty/writeback pages, kswapd has a hard time
>   finding the randomly distributed clean pages;
> - kswapd cannot do the writeout because the queue is congested!
> 
> The improvement could be:
> - kswapd is now explicitly preferred to do the writeout;
> - the pages written by kswapd will be rotated and easy for kswapd to reclaim;
> - it becomes possible for kswapd to wait for the congested queue,
>   instead of doing the vmscan like mad.

Yeah.  In 2.4 and early 2.5, page-reclaim (both direct reclaim and kswapd,
iirc) would throttle by waiting on writeout of a particular page.  This was
a poor design, because writeback against a *particular* page can take
anywhere from one millisecond to thirty seconds to complete, depending upon
where the disk head is and all that stuff.

The critical change I made was to switch the throttling algorithm from
"wait for one page to get written" to "wait for _any_ page to get written".
 Becaue reclaim really doesn't care _which_ page got written: we want to
wake up and start scanning again when _any_ page got written.

That's what congestion_wait() does.

It is pretty crude.  It could be that writeback completed against pages which
aren't in the correct zone, or it could be that some other task went and
allocated the just-cleaned pages before this task can get running and
reclaim them, or it could be that the just-written-back pages weren't
reclaimable after all, etc.

It would take a mind-boggling amount of logic and locking to make all this
100% accurate and the need has never been demonstrated.  So page reclaim
presently should be viewed as a polling algorithm, where the rate of
polling is paced by the rate at which the IO system can retire writes.

> The congestion wait looks like a pretty natural way to throttle the kswapd.
> Instead of doing the vmscan at 1000MB/s and actually freeing pages at
> 60MB/s(about the write throughput), kswapd will be relaxed to do vmscan at
> maybe 150MB/s.

Something like that.

The critical numbers to watch are /proc/vmstat's *scan* and *steal*.  Look:

akpm:/usr/src/25> uptime
 10:08:14 up 10 days, 16:46, 15 users,  load average: 0.02, 0.05, 0.04
akpm:/usr/src/25> grep steal /proc/vmstat
pgsteal_dma 0
pgsteal_dma32 0
pgsteal_normal 0
pgsteal_high 0
pginodesteal 0
kswapd_steal 1218698
kswapd_inodesteal 266847
akpm:/usr/src/25> grep scan /proc/vmstat      
pgscan_kswapd_dma 0
pgscan_kswapd_dma32 1246816
pgscan_kswapd_normal 0
pgscan_kswapd_high 0
pgscan_direct_dma 0
pgscan_direct_dma32 448
pgscan_direct_normal 0
pgscan_direct_high 0
slabs_scanned 2881664

Ignore kswapd_inodesteal and slabs_scanned.  We see that this machine has
scanned 1246816+448 pages and has reclaimed (stolen) 1218698 pages.  That's
a reclaim success rate of 97.7%, which is pretty damn good - this machine
is just a lightly-loaded 3GB desktop.

When testing reclaim, it is critical that this ratio be monitored (vmmon.c
from ext3-tools is a vmstat-like interface to /proc/vmstat).  If the
reclaim efficiency falls below, umm, 25% then things are getting into some
trouble.

Actually, 25% is still pretty good.  We scan 4 pages for each reclaimed
page, but the amount of wall time which that takes is vastly less than the
time to write one page, bearing in mind that these things tend to be seeky
as hell.  But still, keeping an eye on the reclaim efficiency is just your
basic starting point for working on page reclaim.


--
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] remove throttle_vm_writeout()

[Fengguang Wu]

On Fri, Oct 05, 2007 at 10:20:05AM -0700, Andrew Morton wrote:
> On Fri, 5 Oct 2007 20:30:28 +0800
> Fengguang Wu <wfg@mail.ustc.edu.cn> wrote:
> 
> > > commit c4e2d7ddde9693a4c05da7afd485db02c27a7a09
> > > Author: akpm <akpm>
> > > Date:   Sun Dec 22 01:07:33 2002 +0000
> > > 
> > >     [PATCH] Give kswapd writeback higher priority than pdflush
> > >     
> > >     The `low latency page reclaim' design works by preventing page
> > >     allocators from blocking on request queues (and by preventing them from
> > >     blocking against writeback of individual pages, but that is immaterial
> > >     here).
> > >     
> > >     This has a problem under some situations.  pdflush (or a write(2)
> > >     caller) could be saturating the queue with highmem pages.  This
> > >     prevents anyone from writing back ZONE_NORMAL pages.  We end up doing
> > >     enormous amounts of scenning.
> > 
> > Sorry, I cannot not understand it. We now have balanced aging between
> > zones. So the page allocations are expected to distribute proportionally
> > between ZONE_HIGHMEM and ZONE_NORMAL?
> 
> Sure, but we don't have one disk queue per disk per zone!  The queue is
> shared by all the zones.  So if writeback from one zone has filled the
> queue up, the kernel can't write back data from another zone.

Hmm, that's a problem. But I guess when one zone is full, other zones
will not be far away... It's a "sooner or later" problem.

> (Well, it can, by blocking in get_request_wait(), but that causes long and
> uncontrollable latencies).

I guess PF_SWAPWRITE processes still have good probability to stuck in
get_request_wait().  Because balance_dirty_pages() are allowed to
disregard the congestion.  It will be exhausting the available request
slots all the time.

Signed-off-by: Fengguang Wu <wfg@mail.ustc.edu.cn>
---
 mm/page-writeback.c |    1 +
 1 file changed, 1 insertion(+)

--- linux-2.6.23-rc8-mm2.orig/mm/page-writeback.c
+++ linux-2.6.23-rc8-mm2/mm/page-writeback.c
@@ -400,6 +400,7 @@ static void balance_dirty_pages(struct a
 			.sync_mode	= WB_SYNC_NONE,
 			.older_than_this = NULL,
 			.nr_to_write	= write_chunk,
+			.nonblocking	= 1,
 			.range_cyclic	= 1,
 		};
 

> > >     A test case is to mmap(MAP_SHARED) almost all of a 4G machine's memory,
> > >     then kill the mmapping applications.  The machine instantly goes from
> > >     0% of memory dirty to 95% or more.  pdflush kicks in and starts writing
> > >     the least-recently-dirtied pages, which are all highmem.  The queue is
> > >     congested so nobody will write back ZONE_NORMAL pages.  kswapd chews
> > >     50% of the CPU scanning past dirty ZONE_NORMAL pages and page reclaim
> > >     efficiency (pages_reclaimed/pages_scanned) falls to 2%.
> > >     
> > >     So this patch changes the policy for kswapd.  kswapd may use all of a
> > >     request queue, and is prepared to block on request queues.
> > >     
> > >     What will now happen in the above scenario is:
> > >     
> > >     1: The page alloctor scans some pages, fails to reclaim enough
> > >        memory and takes a nap in blk_congetion_wait().
> > >     
> > >     2: kswapd() will scan the ZONE_NORMAL LRU and will start writing
> > >        back pages.  (These pages will be rotated to the tail of the
> > >        inactive list at IO-completion interrupt time).
> > >     
> > >        This writeback will saturate the queue with ZONE_NORMAL pages.
> > >        Conveniently, pdflush will avoid the congested queues.  So we end up
> > >        writing the correct pages.
> > >     
> > >     In this test, kswapd CPU utilisation falls from 50% to 2%, page reclaim
> > >     efficiency rises from 2% to 40% and things are generally a lot happier.
> > 
> > We may see the same problem and improvement in the absent of 'all
> > writeback goes to one zone' assumption.
> > 
> > The problem could be:
> > - dirty_thresh is exceeded, so balance_dirty_pages() starts syncing
> >   data and quickly _congests_ the queue;
> 
> Or someone ran fsync(), or pdflush is writing back data because it exceeded
> dirty_writeback_centisecs, etc.

Ah, yes.

> > - dirty pages are slowly but continuously turned into clean pages by
> >   balance_dirty_pages(), but they still stay in the same place in LRU;
> > - the zones are mostly dirty/writeback pages, kswapd has a hard time
> >   finding the randomly distributed clean pages;
> > - kswapd cannot do the writeout because the queue is congested!
> > 
> > The improvement could be:
> > - kswapd is now explicitly preferred to do the writeout;
> > - the pages written by kswapd will be rotated and easy for kswapd to reclaim;
> > - it becomes possible for kswapd to wait for the congested queue,
> >   instead of doing the vmscan like mad.
> 
> Yeah.  In 2.4 and early 2.5, page-reclaim (both direct reclaim and kswapd,
> iirc) would throttle by waiting on writeout of a particular page.  This was
> a poor design, because writeback against a *particular* page can take
> anywhere from one millisecond to thirty seconds to complete, depending upon
> where the disk head is and all that stuff.
> 
> The critical change I made was to switch the throttling algorithm from
> "wait for one page to get written" to "wait for _any_ page to get written".
>  Becaue reclaim really doesn't care _which_ page got written: we want to
> wake up and start scanning again when _any_ page got written.

That must be a big improvement!

> That's what congestion_wait() does.
> 
> It is pretty crude.  It could be that writeback completed against pages which
> aren't in the correct zone, or it could be that some other task went and
> allocated the just-cleaned pages before this task can get running and
> reclaim them, or it could be that the just-written-back pages weren't
> reclaimable after all, etc.
> 
> It would take a mind-boggling amount of logic and locking to make all this
> 100% accurate and the need has never been demonstrated.  So page reclaim
> presently should be viewed as a polling algorithm, where the rate of
> polling is paced by the rate at which the IO system can retire writes.

Yeah. So the polling overheads are limited.

> > The congestion wait looks like a pretty natural way to throttle the kswapd.
> > Instead of doing the vmscan at 1000MB/s and actually freeing pages at
> > 60MB/s(about the write throughput), kswapd will be relaxed to do vmscan at
> > maybe 150MB/s.
> 
> Something like that.
> 
> The critical numbers to watch are /proc/vmstat's *scan* and *steal*.  Look:
> 
> akpm:/usr/src/25> uptime
>  10:08:14 up 10 days, 16:46, 15 users,  load average: 0.02, 0.05, 0.04
> akpm:/usr/src/25> grep steal /proc/vmstat
> pgsteal_dma 0
> pgsteal_dma32 0
> pgsteal_normal 0
> pgsteal_high 0
> pginodesteal 0
> kswapd_steal 1218698
> kswapd_inodesteal 266847
> akpm:/usr/src/25> grep scan /proc/vmstat      
> pgscan_kswapd_dma 0
> pgscan_kswapd_dma32 1246816
> pgscan_kswapd_normal 0
> pgscan_kswapd_high 0
> pgscan_direct_dma 0
> pgscan_direct_dma32 448
> pgscan_direct_normal 0
> pgscan_direct_high 0
> slabs_scanned 2881664
> 
> Ignore kswapd_inodesteal and slabs_scanned.  We see that this machine has
> scanned 1246816+448 pages and has reclaimed (stolen) 1218698 pages.  That's
> a reclaim success rate of 97.7%, which is pretty damn good - this machine
> is just a lightly-loaded 3GB desktop.
> 
> When testing reclaim, it is critical that this ratio be monitored (vmmon.c
> from ext3-tools is a vmstat-like interface to /proc/vmstat).  If the
> reclaim efficiency falls below, umm, 25% then things are getting into some
> trouble.

Nice tool!
I've been watching the raw numbers by writing scripts. This one can be
written as:

#!/bin/bash

while true; do
	while read a b
	do
		eval $a=$b
	done < /proc/vmstat

	uptime=$(</proc/uptime)

	scan=$((
		pgscan_kswapd_dma +
		pgscan_kswapd_dma32 +
		pgscan_kswapd_normal +
		pgscan_kswapd_high +
		pgscan_direct_dma +
		pgscan_direct_dma32 +
		pgscan_direct_normal +
		pgscan_direct_high
		))

	steal=$((
		pgsteal_dma +
		pgsteal_dma32 +
		pgsteal_normal +
		pgsteal_high
		))

	ratio=$((100*steal/(scan+1)))

	echo -e "$uptime\t$ratio%\t$steal\t$scan"
        sleep 1;
done

Not surprisingly, I see nice numbers on my desktop:

9517.99 9368.60 96%     1898452 1961536

> Actually, 25% is still pretty good.  We scan 4 pages for each reclaimed
> page, but the amount of wall time which that takes is vastly less than the
> time to write one page, bearing in mind that these things tend to be seeky
> as hell.  But still, keeping an eye on the reclaim efficiency is just your
> basic starting point for working on page reclaim.

Thank you for the nice tip :-)

Fengguang

--
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] remove throttle_vm_writeout()

[David Chinner]

On Fri, Oct 05, 2007 at 08:30:28PM +0800, Fengguang Wu wrote:
> The improvement could be:
> - kswapd is now explicitly preferred to do the writeout;

Careful. kswapd is much less efficient at writeout than pdflush
because it does not do low->high offset writeback per address space.
It just flushes the pages in LRU order and that turns writeback into
a non-sequential mess. I/O sizes decrease substantially and
throughput falls through the floor.

So if you want kswapd to take over all the writeback, it needs to do
writeback in the same manner as the background flushes. i.e.  by
grabbing page->mapping and flushing that in sequential order rather
than just the page on the end of the LRU....

I documented the effect of kswapd taking over writeback in this
paper (section 5.3):

http://oss.sgi.com/projects/xfs/papers/ols2006/ols-2006-paper.pdf

Cheers,

Dave.
-- 
Dave Chinner
Principal Engineer
SGI Australian Software Group

--
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] remove throttle_vm_writeout()

[Fengguang Wu]

On Mon, Oct 08, 2007 at 09:54:33AM +1000, David Chinner wrote:
> On Fri, Oct 05, 2007 at 08:30:28PM +0800, Fengguang Wu wrote:
> > The improvement could be:
> > - kswapd is now explicitly preferred to do the writeout;
> 
> Careful. kswapd is much less efficient at writeout than pdflush
> because it does not do low->high offset writeback per address space.
> It just flushes the pages in LRU order and that turns writeback into
> a non-sequential mess. I/O sizes decrease substantially and
> throughput falls through the floor.
> 
> So if you want kswapd to take over all the writeback, it needs to do
> writeback in the same manner as the background flushes. i.e.  by
> grabbing page->mapping and flushing that in sequential order rather
> than just the page on the end of the LRU....
> 
> I documented the effect of kswapd taking over writeback in this
> paper (section 5.3):
> 
> http://oss.sgi.com/projects/xfs/papers/ols2006/ols-2006-paper.pdf

Ah, indeed. That means introducing a new "really congested" threshold
for kswapd is *dangerous*. I realized this later on, and am now
heading for another direction.

The basic idea is to
- rotate pdflush issued writeback pages for kswapd;
- use the more precise zone_rotate_wait() to throttle kswapd.

The code is a quick hack and not tested yet.
Early comments are more than welcome.

Fengguang
---
 include/linux/mmzone.h |    1 +
 mm/filemap.c           |    5 ++++-
 mm/page_alloc.c        |    1 +
 mm/swap.c              |   13 +++++++++++++
 mm/vmscan.c            |   12 ++++++++++--
 5 files changed, 29 insertions(+), 3 deletions(-)

--- linux-2.6.23-rc8-mm2.orig/include/linux/mmzone.h
+++ linux-2.6.23-rc8-mm2/include/linux/mmzone.h
@@ -316,6 +316,7 @@ struct zone {
 	wait_queue_head_t	* wait_table;
 	unsigned long		wait_table_hash_nr_entries;
 	unsigned long		wait_table_bits;
+	wait_queue_head_t	wait_rotate;
 
 	/*
 	 * Discontig memory support fields.
--- linux-2.6.23-rc8-mm2.orig/mm/filemap.c
+++ linux-2.6.23-rc8-mm2/mm/filemap.c
@@ -558,12 +558,15 @@ EXPORT_SYMBOL(unlock_page);
  */
 void end_page_writeback(struct page *page)
 {
-	if (!TestClearPageReclaim(page) || rotate_reclaimable_page(page)) {
+	int r = 1;
+	if (!TestClearPageReclaim(page) || (r = rotate_reclaimable_page(page))) {
 		if (!test_clear_page_writeback(page))
 			BUG();
 	}
 	smp_mb__after_clear_bit();
 	wake_up_page(page, PG_writeback);
+	if (!r)
+		wake_up(&page_zone(page)->wait_rotate);
 }
 EXPORT_SYMBOL(end_page_writeback);
 
--- linux-2.6.23-rc8-mm2.orig/mm/page_alloc.c
+++ linux-2.6.23-rc8-mm2/mm/page_alloc.c
@@ -3482,6 +3482,7 @@ static void __meminit free_area_init_cor
 		zone->prev_priority = DEF_PRIORITY;
 
 		zone_pcp_init(zone);
+		init_waitqueue_head(&zone->wait_rotate);
 		INIT_LIST_HEAD(&zone->active_list);
 		INIT_LIST_HEAD(&zone->inactive_list);
 		zone->nr_scan_active = 0;
--- linux-2.6.23-rc8-mm2.orig/mm/vmscan.c
+++ linux-2.6.23-rc8-mm2/mm/vmscan.c
@@ -50,6 +50,7 @@
 struct scan_control {
 	/* Incremented by the number of inactive pages that were scanned */
 	unsigned long nr_scanned;
+	unsigned long nr_dirty_writeback;
 
 	/* This context's GFP mask */
 	gfp_t gfp_mask;
@@ -558,8 +559,10 @@ static unsigned long shrink_page_list(st
 			case PAGE_ACTIVATE:
 				goto activate_locked;
 			case PAGE_SUCCESS:
-				if (PageWriteback(page) || PageDirty(page))
+				if (PageWriteback(page) || PageDirty(page)) {
+					sc->nr_dirty_writeback++;
 					goto keep;
+				}
 				/*
 				 * A synchronous write - probably a ramdisk.  Go
 				 * ahead and try to reclaim the page.
@@ -620,6 +623,10 @@ keep_locked:
 keep:
 		list_add(&page->lru, &ret_pages);
 		VM_BUG_ON(PageLRU(page));
+		if (PageLocked(page) && PageWriteback(page)) {
+			SetPageReclaim(page);
+			sc->nr_dirty_writeback++;
+		}
 	}
 	list_splice(&ret_pages, page_list);
 	if (pagevec_count(&freed_pvec))
@@ -1184,7 +1191,8 @@ static unsigned long shrink_zone(int pri
 		}
 	}
 
-	throttle_vm_writeout(sc->gfp_mask);
+	if (!nr_reclaimed && sc->nr_dirty_writeback)
+		zone_rotate_wait(zone, HZ/100);
 	return nr_reclaimed;
 }
 
--- linux-2.6.23-rc8-mm2.orig/mm/swap.c
+++ linux-2.6.23-rc8-mm2/mm/swap.c
@@ -174,6 +174,19 @@ int rotate_reclaimable_page(struct page 
 	return 0;
 }
 
+long zone_rotate_wait(struct zone* z, long timeout)
+{
+	long ret;
+	DEFINE_WAIT(wait);
+	wait_queue_head_t *wqh = &z->wait_rotate;
+
+	prepare_to_wait(wqh, &wait, TASK_UNINTERRUPTIBLE);
+	ret = io_schedule_timeout(timeout);
+	finish_wait(wqh, &wait);
+	return ret;
+}
+EXPORT_SYMBOL(zone_rotate_wait);
+
 /*
  * FIXME: speed this up?
  */

--
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] remove throttle_vm_writeout()

[Miklos Szeredi]

> Yeah, I'm guestimating O on a per device basis, but I agree that the
> current ratio limiting is quite crude. I'm not at all sorry to see
> throttle_vm_writeback() go, I just wanted to make a point that what it
> does is not quite without merrit - we agree that it can be done better
> differently.

Yes.  So what is it to be?

Is limiting by device queues enough?

Or do we need some global limit?

If so, the cleanest way I see is to separately account and limit
swap-writeback pages, so the global counters don't interfere with the
limiting.

This shouldn't be hard to do, as we have the per-bdi writeback
counting infrastructure already, and also a pseudo bdi for swap in
swapper_space.backing_dev_info.

Miklos

--
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] remove throttle_vm_writeout()

[Andrew Morton]

On Thu, 04 Oct 2007 14:25:22 +0200
Miklos Szeredi <miklos@szeredi.hu> wrote:

> From: Miklos Szeredi <mszeredi@suse.cz>
> 
> By relying on the global diry limits, this can cause a deadlock when
> devices are stacked.
> 
> If the stacking is done through a fuse filesystem, the __GFP_FS,
> __GFP_IO tests won't help: the process doing the allocation doesn't
> have any special flag.

This description of the bug-which-is-being-fixed is nowhere near adequate
enough for a reviewer to understand the problem.  This makes it hard to
suggest alternative fixes.

> So why exactly does this function exist?

That's described in the changelog for the patch which added
throttle_vm_writeout().  Unsurprisingly ;)

> Direct reclaim does not _increase_ the number of dirty pages in the
> system, so rate limiting it seems somewhat pointless.
> 
> There are two cases:
> 
> 1) File backed pages -> file
> 
>   dirty + writeback count remains constant
> 
> 2) Anonymous pages -> swap
> 
>   writeback count increases, dirty balancing will hold back file
>   writeback in favor of swap
> 
> So the real question is: does case 2 need rate limiting, or is it OK
> to let the device queue fill with swap pages as fast as possible?

None of the above.

    [PATCH] vm: pageout throttling
    
    With silly pageout testcases it is possible to place huge amounts of memory
    under I/O.  With a large request queue (CFQ uses 8192 requests) it is
    possible to place _all_ memory under I/O at the same time.
    
    This means that all memory is pinned and unreclaimable and the VM gets
    upset and goes oom.
    
    The patch limits the amount of memory which is under pageout writeout to be
    a little more than the amount of memory at which balance_dirty_pages()
    callers will synchronously throttle.
    
    This means that heavy pageout activity can starve heavy writeback activity
    completely, but heavy writeback activity will not cause starvation of
    pageout.  Because we don't want a simple `dd' to be causing excessive
    latencies in page reclaim.

afaict that problem is still there.  It is possible to get all of
ZONE_NORMAL dirty on a highmem machine.  With a large queue (or lots of
queues), vmscan can them place all of ZONE_NORMAL under IO.

It could be that we've fixed this problem via other means in the interrim,
but from a quick peek to seems to me that the scanner will still do a 100%
CPU burn when all of a zone's pages are under writeback.

throttle_vm_writeout() should be a per-zone thing, I guess.  Perhaps fixing
that would fix your deadlock.  That's doubtful, but I don't know anything
about your deadlock so I cannot say.

--
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] remove throttle_vm_writeout()

[Miklos Szeredi]

> None of the above.
> 
>     [PATCH] vm: pageout throttling
>     
>     With silly pageout testcases it is possible to place huge amounts of memory
>     under I/O.  With a large request queue (CFQ uses 8192 requests) it is
>     possible to place _all_ memory under I/O at the same time.
>     
>     This means that all memory is pinned and unreclaimable and the VM gets
>     upset and goes oom.
>     
>     The patch limits the amount of memory which is under pageout writeout to be
>     a little more than the amount of memory at which balance_dirty_pages()
>     callers will synchronously throttle.
>     
>     This means that heavy pageout activity can starve heavy writeback activity
>     completely, but heavy writeback activity will not cause starvation of
>     pageout.  Because we don't want a simple `dd' to be causing excessive
>     latencies in page reclaim.
> 
> afaict that problem is still there.  It is possible to get all of
> ZONE_NORMAL dirty on a highmem machine.  With a large queue (or lots of
> queues), vmscan can them place all of ZONE_NORMAL under IO.
> 
> It could be that we've fixed this problem via other means in the interrim,
> but from a quick peek to seems to me that the scanner will still do a 100%
> CPU burn when all of a zone's pages are under writeback.

Ah, OK.

I did read the changelog, but you added quite a bit of translation ;)

> throttle_vm_writeout() should be a per-zone thing, I guess.  Perhaps fixing
> that would fix your deadlock.  That's doubtful, but I don't know anything
> about your deadlock so I cannot say.

No, doing the throttling per-zone won't in itself fix the deadlock.

Here's a deadlock example:

Total memory = 32M
/proc/sys/vm/dirty_ratio = 10
dirty_threshold = 3M
ratelimit_pages = 1M

Some program dirties 4M (dirty_threshold + ratelimit_pages) of mmap on
a fuse fs.  Page balancing is called which turns all these into
writeback pages.

Then userspace filesystem gets a write request, and tries to allocate
memory needed to complete the writeout.

That will possibly trigger direct reclaim, and throttle_vm_writeout()
will be called.  That will block until nr_writeback goes below 3.3M
(dirty_threshold + 10%).  But since all 4M of writeback is from the
fuse fs, that will never happen.

Does that explain it better?

Miklos

--
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] remove throttle_vm_writeout()

[Andrew Morton]

On Fri, 05 Oct 2007 00:39:16 +0200
Miklos Szeredi <miklos@szeredi.hu> wrote:

> > throttle_vm_writeout() should be a per-zone thing, I guess.  Perhaps fixing
> > that would fix your deadlock.  That's doubtful, but I don't know anything
> > about your deadlock so I cannot say.
> 
> No, doing the throttling per-zone won't in itself fix the deadlock.
> 
> Here's a deadlock example:
> 
> Total memory = 32M
> /proc/sys/vm/dirty_ratio = 10
> dirty_threshold = 3M
> ratelimit_pages = 1M
> 
> Some program dirties 4M (dirty_threshold + ratelimit_pages) of mmap on
> a fuse fs.  Page balancing is called which turns all these into
> writeback pages.
> 
> Then userspace filesystem gets a write request, and tries to allocate
> memory needed to complete the writeout.
> 
> That will possibly trigger direct reclaim, and throttle_vm_writeout()
> will be called.  That will block until nr_writeback goes below 3.3M
> (dirty_threshold + 10%).  But since all 4M of writeback is from the
> fuse fs, that will never happen.
> 
> Does that explain it better?
> 

yup, thanks.

This is a somewhat general problem: a userspace process is in the IO path. 
Userspace block drivers, for example - pretty much anything which involves
kernel->userspace upcalls for storage applications.

I solved it once in the past by marking the userspace process as
PF_MEMALLOC and I beleive that others have implemented the same hack.

I suspect that what we need is a general solution, and that the solution
will involve explicitly telling the kernel that this process is one which
actually cleans memory and needs special treatment.

Because I bet there will be other corner-cases where such a process needs
kernel help, and there might be optimisation opportunities as well.

Problem is, any such mark-me-as-special syscall would need to be
privileged, and FUSE servers presently don't require special perms (do
they?)

--
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] remove throttle_vm_writeout()

[Miklos Szeredi]

> This is a somewhat general problem: a userspace process is in the IO path. 
> Userspace block drivers, for example - pretty much anything which involves
> kernel->userspace upcalls for storage applications.
> 
> I solved it once in the past by marking the userspace process as
> PF_MEMALLOC and I beleive that others have implemented the same hack.
> 
> I suspect that what we need is a general solution, and that the solution
> will involve explicitly telling the kernel that this process is one which
> actually cleans memory and needs special treatment.
> 
> Because I bet there will be other corner-cases where such a process needs
> kernel help, and there might be optimisation opportunities as well.
> 
> Problem is, any such mark-me-as-special syscall would need to be
> privileged, and FUSE servers presently don't require special perms (do
> they?)

No, and that's a rather important feature, that I'd rather not give
up.  But with the dirty limiting, the memory cleaning really shouldn't
be a problem, as there is plenty of memory _not_ used for dirty file
data, that the filesystem can use during the writeback.

So the only thing the kernel should be careful about, is not to block
on an allocation if not strictly necessary.

Actually a trivial fix for this problem could be to just tweak the
thresholds, so to make the above scenario impossible.  Although I'm
still not convinced, this patch is perfect, because the dirty
threshold can actually change in time...

Index: linux/mm/page-writeback.c
===================================================================
--- linux.orig/mm/page-writeback.c      2007-10-05 00:31:01.000000000 +0200
+++ linux/mm/page-writeback.c   2007-10-05 00:50:11.000000000 +0200
@@ -515,6 +515,12 @@ void throttle_vm_writeout(gfp_t gfp_mask
         for ( ; ; ) {
                get_dirty_limits(&background_thresh, &dirty_thresh, NULL, NULL);

+               /*
+                * Make sure the theshold is over the hard limit of
+                * dirty_thresh + ratelimit_pages * nr_cpus
+                */
+               dirty_thresh += ratelimit_pages * num_online_cpus();
+
                 /*
                  * Boost the allowable dirty threshold a bit for page
                  * allocators so they don't get DoS'ed by heavy writers


--
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] remove throttle_vm_writeout()

[Andrew Morton]

On Fri, 05 Oct 2007 01:26:12 +0200
Miklos Szeredi <miklos@szeredi.hu> wrote:

> > This is a somewhat general problem: a userspace process is in the IO path. 
> > Userspace block drivers, for example - pretty much anything which involves
> > kernel->userspace upcalls for storage applications.
> > 
> > I solved it once in the past by marking the userspace process as
> > PF_MEMALLOC and I beleive that others have implemented the same hack.
> > 
> > I suspect that what we need is a general solution, and that the solution
> > will involve explicitly telling the kernel that this process is one which
> > actually cleans memory and needs special treatment.
> > 
> > Because I bet there will be other corner-cases where such a process needs
> > kernel help, and there might be optimisation opportunities as well.
> > 
> > Problem is, any such mark-me-as-special syscall would need to be
> > privileged, and FUSE servers presently don't require special perms (do
> > they?)
> 
> No, and that's a rather important feature, that I'd rather not give
> up.

Can fuse do it?  Perhaps the fs can diddle the server's task_struct at
registration time?

>  But with the dirty limiting, the memory cleaning really shouldn't
> be a problem, as there is plenty of memory _not_ used for dirty file
> data, that the filesystem can use during the writeback.

I don't think I understand that.  Sure, it _shouldn't_ be a problem.  But it
_is_.  That's what we're trying to fix, isn't it?

> So the only thing the kernel should be careful about, is not to block
> on an allocation if not strictly necessary.
> 
> Actually a trivial fix for this problem could be to just tweak the
> thresholds, so to make the above scenario impossible.  Although I'm
> still not convinced, this patch is perfect, because the dirty
> threshold can actually change in time...
> 
> Index: linux/mm/page-writeback.c
> ===================================================================
> --- linux.orig/mm/page-writeback.c      2007-10-05 00:31:01.000000000 +0200
> +++ linux/mm/page-writeback.c   2007-10-05 00:50:11.000000000 +0200
> @@ -515,6 +515,12 @@ void throttle_vm_writeout(gfp_t gfp_mask
>          for ( ; ; ) {
>                 get_dirty_limits(&background_thresh, &dirty_thresh, NULL, NULL);
> 
> +               /*
> +                * Make sure the theshold is over the hard limit of
> +                * dirty_thresh + ratelimit_pages * nr_cpus
> +                */
> +               dirty_thresh += ratelimit_pages * num_online_cpus();
> +
>                  /*
>                   * Boost the allowable dirty threshold a bit for page
>                   * allocators so they don't get DoS'ed by heavy writers

I can probably kind of guess what you're trying to do here.  But if
ratelimit_pages * num_online_cpus() exceeds the size of the offending zone
then things might go bad.

--
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] remove throttle_vm_writeout()

[Miklos Szeredi]

> > > This is a somewhat general problem: a userspace process is in the IO path. 
> > > Userspace block drivers, for example - pretty much anything which involves
> > > kernel->userspace upcalls for storage applications.
> > > 
> > > I solved it once in the past by marking the userspace process as
> > > PF_MEMALLOC and I beleive that others have implemented the same hack.
> > > 
> > > I suspect that what we need is a general solution, and that the solution
> > > will involve explicitly telling the kernel that this process is one which
> > > actually cleans memory and needs special treatment.
> > > 
> > > Because I bet there will be other corner-cases where such a process needs
> > > kernel help, and there might be optimisation opportunities as well.
> > > 
> > > Problem is, any such mark-me-as-special syscall would need to be
> > > privileged, and FUSE servers presently don't require special perms (do
> > > they?)
> > 
> > No, and that's a rather important feature, that I'd rather not give
> > up.
> 
> Can fuse do it?  Perhaps the fs can diddle the server's task_struct at
> registration time?

No, it's futile.  What if another process is involved (ssh in case of
sshfs), etc.

> >  But with the dirty limiting, the memory cleaning really shouldn't
> > be a problem, as there is plenty of memory _not_ used for dirty file
> > data, that the filesystem can use during the writeback.
> 
> I don't think I understand that.  Sure, it _shouldn't_ be a problem.  But it
> _is_.  That's what we're trying to fix, isn't it?

The problem, I believe is in the memory allocation code, not in fuse.

In the example, memory allocation may be blocking indefinitely,
because we have 4MB under writeback, even though 28MB can still be
made available.  And that _should_ be fixable.

> > So the only thing the kernel should be careful about, is not to block
> > on an allocation if not strictly necessary.
> > 
> > Actually a trivial fix for this problem could be to just tweak the
> > thresholds, so to make the above scenario impossible.  Although I'm
> > still not convinced, this patch is perfect, because the dirty
> > threshold can actually change in time...
> > 
> > Index: linux/mm/page-writeback.c
> > ===================================================================
> > --- linux.orig/mm/page-writeback.c      2007-10-05 00:31:01.000000000 +0200
> > +++ linux/mm/page-writeback.c   2007-10-05 00:50:11.000000000 +0200
> > @@ -515,6 +515,12 @@ void throttle_vm_writeout(gfp_t gfp_mask
> >          for ( ; ; ) {
> >                 get_dirty_limits(&background_thresh, &dirty_thresh, NULL, NULL);
> > 
> > +               /*
> > +                * Make sure the theshold is over the hard limit of
> > +                * dirty_thresh + ratelimit_pages * nr_cpus
> > +                */
> > +               dirty_thresh += ratelimit_pages * num_online_cpus();
> > +
> >                  /*
> >                   * Boost the allowable dirty threshold a bit for page
> >                   * allocators so they don't get DoS'ed by heavy writers
> 
> I can probably kind of guess what you're trying to do here.  But if
> ratelimit_pages * num_online_cpus() exceeds the size of the offending zone
> then things might go bad.

I think the admin can do quite a bit of other damage, by setting
dirty_ratio too high.

Maybe this writeback throttling should just have a fixed limit of 80%
ZONE_NORMAL, and limit dirty_ratio to something like 50%.

Miklos

--
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] remove throttle_vm_writeout()

[Andrew Morton]

On Fri, 05 Oct 2007 02:12:30 +0200 Miklos Szeredi <miklos@szeredi.hu> wrote:

> > 
> > I don't think I understand that.  Sure, it _shouldn't_ be a problem.  But it
> > _is_.  That's what we're trying to fix, isn't it?
> 
> The problem, I believe is in the memory allocation code, not in fuse.

fuse is trying to do something which page reclaim was not designed for. 
Stuff broke.

> In the example, memory allocation may be blocking indefinitely,
> because we have 4MB under writeback, even though 28MB can still be
> made available.  And that _should_ be fixable.

Well yes.  But we need to work out how, without re-breaking the thing which
throttle_vm_writeout() fixed.

> > > So the only thing the kernel should be careful about, is not to block
> > > on an allocation if not strictly necessary.
> > > 
> > > Actually a trivial fix for this problem could be to just tweak the
> > > thresholds, so to make the above scenario impossible.  Although I'm
> > > still not convinced, this patch is perfect, because the dirty
> > > threshold can actually change in time...
> > > 
> > > Index: linux/mm/page-writeback.c
> > > ===================================================================
> > > --- linux.orig/mm/page-writeback.c      2007-10-05 00:31:01.000000000 +0200
> > > +++ linux/mm/page-writeback.c   2007-10-05 00:50:11.000000000 +0200
> > > @@ -515,6 +515,12 @@ void throttle_vm_writeout(gfp_t gfp_mask
> > >          for ( ; ; ) {
> > >                 get_dirty_limits(&background_thresh, &dirty_thresh, NULL, NULL);
> > > 
> > > +               /*
> > > +                * Make sure the theshold is over the hard limit of
> > > +                * dirty_thresh + ratelimit_pages * nr_cpus
> > > +                */
> > > +               dirty_thresh += ratelimit_pages * num_online_cpus();
> > > +
> > >                  /*
> > >                   * Boost the allowable dirty threshold a bit for page
> > >                   * allocators so they don't get DoS'ed by heavy writers
> > 
> > I can probably kind of guess what you're trying to do here.  But if
> > ratelimit_pages * num_online_cpus() exceeds the size of the offending zone
> > then things might go bad.
> 
> I think the admin can do quite a bit of other damage, by setting
> dirty_ratio too high.
> 
> Maybe this writeback throttling should just have a fixed limit of 80%
> ZONE_NORMAL, and limit dirty_ratio to something like 50%.

Bear in mind that the same problem will occur for the 16MB ZONE_DMA, and
we cannot limit the system-wide dirty-memory threshold to 12MB.

iow, throttle_vm_writeout() needs to become zone-aware.  Then it only
throttles when, say, 80% of ZONE_FOO is under writeback.

Except I don't think that'll fix the problem 100%: if your fuse kernel
component somehow manages to put 80% of ZONE_FOO under writeback (and
remmeber this might be only 12MB on a 16GB machine) then we get stuck again
- the fuse server process (is that the correct terminology, btw?) ends up
waiting upon itself.

I'll think about it a bit.

--
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] remove throttle_vm_writeout()

[Peter Zijlstra]

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

On Thu, 2007-10-04 at 17:48 -0700, Andrew Morton wrote:
> On Fri, 05 Oct 2007 02:12:30 +0200 Miklos Szeredi <miklos@szeredi.hu> wrote:
> 
> > > 
> > > I don't think I understand that.  Sure, it _shouldn't_ be a problem.  But it
> > > _is_.  That's what we're trying to fix, isn't it?
> > 
> > The problem, I believe is in the memory allocation code, not in fuse.
> 
> fuse is trying to do something which page reclaim was not designed for. 
> Stuff broke.
> 
> > In the example, memory allocation may be blocking indefinitely,
> > because we have 4MB under writeback, even though 28MB can still be
> > made available.  And that _should_ be fixable.
> 
> Well yes.  But we need to work out how, without re-breaking the thing which
> throttle_vm_writeout() fixed.

I'm thinking the really_congested thing will also fix this. By only
allowing a limited amount of extra writeback.

> > > > So the only thing the kernel should be careful about, is not to block
> > > > on an allocation if not strictly necessary.
> > > > 
> > > > Actually a trivial fix for this problem could be to just tweak the
> > > > thresholds, so to make the above scenario impossible.  Although I'm
> > > > still not convinced, this patch is perfect, because the dirty
> > > > threshold can actually change in time...
> > > > 
> > > > Index: linux/mm/page-writeback.c
> > > > ===================================================================
> > > > --- linux.orig/mm/page-writeback.c      2007-10-05 00:31:01.000000000 +0200
> > > > +++ linux/mm/page-writeback.c   2007-10-05 00:50:11.000000000 +0200
> > > > @@ -515,6 +515,12 @@ void throttle_vm_writeout(gfp_t gfp_mask
> > > >          for ( ; ; ) {
> > > >                 get_dirty_limits(&background_thresh, &dirty_thresh, NULL, NULL);
> > > > 
> > > > +               /*
> > > > +                * Make sure the theshold is over the hard limit of
> > > > +                * dirty_thresh + ratelimit_pages * nr_cpus
> > > > +                */
> > > > +               dirty_thresh += ratelimit_pages * num_online_cpus();
> > > > +
> > > >                  /*
> > > >                   * Boost the allowable dirty threshold a bit for page
> > > >                   * allocators so they don't get DoS'ed by heavy writers
> > > 
> > > I can probably kind of guess what you're trying to do here.  But if
> > > ratelimit_pages * num_online_cpus() exceeds the size of the offending zone
> > > then things might go bad.
> > 
> > I think the admin can do quite a bit of other damage, by setting
> > dirty_ratio too high.
> > 
> > Maybe this writeback throttling should just have a fixed limit of 80%
> > ZONE_NORMAL, and limit dirty_ratio to something like 50%.
> 
> Bear in mind that the same problem will occur for the 16MB ZONE_DMA, and
> we cannot limit the system-wide dirty-memory threshold to 12MB.
> 
> iow, throttle_vm_writeout() needs to become zone-aware.  Then it only
> throttles when, say, 80% of ZONE_FOO is under writeback.

As it stand 110% of dirty limit can already be larger than say zone_dma
(and likely is), so that is not a new bug - and I don't think its the
thing Miklos runs into.

The problem Miklos is seeing (and I, just in a different form), is that
throttle_vm_writeout() gets stuck because balance_dirty_pages() gets
called once every ratelimit_pages (per cpu). So we can have nr_cpus *
ratelimit_pages extra.....

/me thinks

ok I confused myself.

by calling balance_dirty_pages() once every ratelimit_pages (per cpu)
allows for nr_cpus() * ratelimit_pages extra _dirty_ pages. But
balance_dirty_pages() will make it:
  nr_dirty + nr_unstable + nr_writeback < thresh

So even if it writes out all of the dirty pages, we still have:
  nr_unstable + nr_writeback < thresh

So at any one time nr_writeback should not exceed thresh. But it does!?

So how do we end up with more writeback pages than that? should we teach
pdflush about these limits as well?


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

Re: [PATCH] remove throttle_vm_writeout()

[Miklos Szeredi]

> So how do we end up with more writeback pages than that? should we teach
> pdflush about these limits as well?

Ugh.

I think we should rather fix vmscan to not spin when all pages of a
zone are already under writeout.  Which is the _real_ problem,
according to Andrew.

Miklos

--
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] remove throttle_vm_writeout()

[Peter Zijlstra]

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

On Fri, 2007-10-05 at 11:22 +0200, Miklos Szeredi wrote:
> > So how do we end up with more writeback pages than that? should we teach
> > pdflush about these limits as well?
> 
> Ugh.
> 
> I think we should rather fix vmscan to not spin when all pages of a
> zone are already under writeout.  Which is the _real_ problem,
> according to Andrew.



diff --git a/include/linux/writeback.h b/include/linux/writeback.h
index 4ef4d22..eff2438 100644
--- a/include/linux/writeback.h
+++ b/include/linux/writeback.h
@@ -88,7 +88,7 @@ static inline void wait_on_inode(struct inode *inode)
 int wakeup_pdflush(long nr_pages);
 void laptop_io_completion(void);
 void laptop_sync_completion(void);
-void throttle_vm_writeout(gfp_t gfp_mask);
+void throttle_vm_writeout(struct zone *zone, gfp_t gfp_mask);
 
 /* These are exported to sysctl. */
 extern int dirty_background_ratio;
diff --git a/mm/page-writeback.c b/mm/page-writeback.c
index eec1481..f949997 100644
--- a/mm/page-writeback.c
+++ b/mm/page-writeback.c
@@ -326,11 +326,8 @@ void balance_dirty_pages_ratelimited_nr(struct address_space *mapping,
 }
 EXPORT_SYMBOL(balance_dirty_pages_ratelimited_nr);
 
-void throttle_vm_writeout(gfp_t gfp_mask)
+void throttle_vm_writeout(struct zone *zone, gfp_t gfp_mask)
 {
-	long background_thresh;
-	long dirty_thresh;
-
 	if ((gfp_mask & (__GFP_FS|__GFP_IO)) != (__GFP_FS|__GFP_IO)) {
 		/*
 		 * The caller might hold locks which can prevent IO completion
@@ -342,17 +339,16 @@ void throttle_vm_writeout(gfp_t gfp_mask)
 	}
 
         for ( ; ; ) {
-		get_dirty_limits(&background_thresh, &dirty_thresh, NULL);
+		unsigned long thresh = zone_page_state(zone, NR_ACTIVE) +
+			zone_page_state(zone, NR_INACTIVE);
 
-                /*
-                 * Boost the allowable dirty threshold a bit for page
-                 * allocators so they don't get DoS'ed by heavy writers
-                 */
-                dirty_thresh += dirty_thresh / 10;      /* wheeee... */
+		/*
+		 * wait when 75% of the zone's pages are under writeback
+		 */
+		thresh -= thresh >> 2;
+		if (zone_page_state(zone, NR_WRITEBACK) < thresh)
+			break;
 
-                if (global_page_state(NR_UNSTABLE_NFS) +
-			global_page_state(NR_WRITEBACK) <= dirty_thresh)
-                        	break;
                 congestion_wait(WRITE, HZ/10);
         }
 }
diff --git a/mm/vmscan.c b/mm/vmscan.c
index 1be5a63..7dd6bd9 100644
--- a/mm/vmscan.c
+++ b/mm/vmscan.c
@@ -948,7 +948,7 @@ static unsigned long shrink_zone(int priority, struct zone *zone,
 		}
 	}
 
-	throttle_vm_writeout(sc->gfp_mask);
+	throttle_vm_writeout(zone, sc->gfp_mask);
 
 	atomic_dec(&zone->reclaim_in_progress);
 	return nr_reclaimed;


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

Re: [PATCH] remove throttle_vm_writeout()

[Miklos Szeredi]

> diff --git a/include/linux/writeback.h b/include/linux/writeback.h
> index 4ef4d22..eff2438 100644
> --- a/include/linux/writeback.h
> +++ b/include/linux/writeback.h
> @@ -88,7 +88,7 @@ static inline void wait_on_inode(struct inode *inode)
>  int wakeup_pdflush(long nr_pages);
>  void laptop_io_completion(void);
>  void laptop_sync_completion(void);
> -void throttle_vm_writeout(gfp_t gfp_mask);
> +void throttle_vm_writeout(struct zone *zone, gfp_t gfp_mask);
>  
>  /* These are exported to sysctl. */
>  extern int dirty_background_ratio;
> diff --git a/mm/page-writeback.c b/mm/page-writeback.c
> index eec1481..f949997 100644
> --- a/mm/page-writeback.c
> +++ b/mm/page-writeback.c
> @@ -326,11 +326,8 @@ void balance_dirty_pages_ratelimited_nr(struct address_space *mapping,
>  }
>  EXPORT_SYMBOL(balance_dirty_pages_ratelimited_nr);
>  
> -void throttle_vm_writeout(gfp_t gfp_mask)
> +void throttle_vm_writeout(struct zone *zone, gfp_t gfp_mask)
>  {
> -	long background_thresh;
> -	long dirty_thresh;
> -
>  	if ((gfp_mask & (__GFP_FS|__GFP_IO)) != (__GFP_FS|__GFP_IO)) {
>  		/*
>  		 * The caller might hold locks which can prevent IO completion
> @@ -342,17 +339,16 @@ void throttle_vm_writeout(gfp_t gfp_mask)
>  	}
>  
>          for ( ; ; ) {
> -		get_dirty_limits(&background_thresh, &dirty_thresh, NULL);
> +		unsigned long thresh = zone_page_state(zone, NR_ACTIVE) +
> +			zone_page_state(zone, NR_INACTIVE);
>  
> -                /*
> -                 * Boost the allowable dirty threshold a bit for page
> -                 * allocators so they don't get DoS'ed by heavy writers
> -                 */
> -                dirty_thresh += dirty_thresh / 10;      /* wheeee... */
> +		/*
> +		 * wait when 75% of the zone's pages are under writeback
> +		 */
> +		thresh -= thresh >> 2;
> +		if (zone_page_state(zone, NR_WRITEBACK) < thresh)
> +			break;
>  
> -                if (global_page_state(NR_UNSTABLE_NFS) +
> -			global_page_state(NR_WRITEBACK) <= dirty_thresh)
> -                        	break;
>                  congestion_wait(WRITE, HZ/10);
>          }
>  }
> diff --git a/mm/vmscan.c b/mm/vmscan.c
> index 1be5a63..7dd6bd9 100644
> --- a/mm/vmscan.c
> +++ b/mm/vmscan.c
> @@ -948,7 +948,7 @@ static unsigned long shrink_zone(int priority, struct zone *zone,
>  		}
>  	}
>  
> -	throttle_vm_writeout(sc->gfp_mask);
> +	throttle_vm_writeout(zone, sc->gfp_mask);
>  
>  	atomic_dec(&zone->reclaim_in_progress);
>  	return nr_reclaimed;
> 
> 

I think that's an improvement in all respects.

However it still does not generally address the deadlock scenario: if
there's a small DMA zone, and fuse manages to put all of those pages
under writeout, then there's trouble.

But it's not really fuse specific.  If it was a normal filesystem that
did that, and it needed a GFP_DMA allocation for writeout, it is in
trouble also, as that allocation would fail (at least no deadlock).

Or is GFP_DMA never used by fs/io writeout paths?

Miklos

--
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] remove throttle_vm_writeout()

[Miklos Szeredi]

> I think that's an improvement in all respects.
> 
> However it still does not generally address the deadlock scenario: if
> there's a small DMA zone, and fuse manages to put all of those pages
> under writeout, then there's trouble.

And the only way to solve that AFAICS, is to make sure fuse never uses
more than e.g. 50% of _any_ zone for page cache.  And that may need
some tweaking in the allocator...

Miklos

--
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] remove throttle_vm_writeout()

[John Stoffel]

>> I think that's an improvement in all respects.
>> 
>> However it still does not generally address the deadlock scenario: if
>> there's a small DMA zone, and fuse manages to put all of those pages
>> under writeout, then there's trouble.

Miklos> And the only way to solve that AFAICS, is to make sure fuse
Miklos> never uses more than e.g. 50% of _any_ zone for page cache.
Miklos> And that may need some tweaking in the allocator...

So what happens if I have three different FUSE mounts, all under heavy
write pressure?  It's not a FUSE problem, it's a VM problem as far as
I can see.   All I did was extrapolate from the 50% number (where did
that come from?) and triple it to go over 100%, since we obviously
shouldn't take 100% of any zone, right?

So the real cure is to have some way to rate limit Zone usage, making
it harder and harder to allocate in a zone as the zone gets more and
more full.  But how do you do this in a non-deadlocky way?  

Buy hey, I'm not that knowledgeable about the VM.  

--
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] remove throttle_vm_writeout()

[Peter Zijlstra]

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

On Fri, 2007-10-05 at 12:27 +0200, Miklos Szeredi wrote:
> > diff --git a/include/linux/writeback.h b/include/linux/writeback.h
> > index 4ef4d22..eff2438 100644
> > --- a/include/linux/writeback.h
> > +++ b/include/linux/writeback.h
> > @@ -88,7 +88,7 @@ static inline void wait_on_inode(struct inode *inode)
> >  int wakeup_pdflush(long nr_pages);
> >  void laptop_io_completion(void);
> >  void laptop_sync_completion(void);
> > -void throttle_vm_writeout(gfp_t gfp_mask);
> > +void throttle_vm_writeout(struct zone *zone, gfp_t gfp_mask);
> >  
> >  /* These are exported to sysctl. */
> >  extern int dirty_background_ratio;
> > diff --git a/mm/page-writeback.c b/mm/page-writeback.c
> > index eec1481..f949997 100644
> > --- a/mm/page-writeback.c
> > +++ b/mm/page-writeback.c
> > @@ -326,11 +326,8 @@ void balance_dirty_pages_ratelimited_nr(struct address_space *mapping,
> >  }
> >  EXPORT_SYMBOL(balance_dirty_pages_ratelimited_nr);
> >  
> > -void throttle_vm_writeout(gfp_t gfp_mask)
> > +void throttle_vm_writeout(struct zone *zone, gfp_t gfp_mask)
> >  {
> > -	long background_thresh;
> > -	long dirty_thresh;
> > -
> >  	if ((gfp_mask & (__GFP_FS|__GFP_IO)) != (__GFP_FS|__GFP_IO)) {
> >  		/*
> >  		 * The caller might hold locks which can prevent IO completion
> > @@ -342,17 +339,16 @@ void throttle_vm_writeout(gfp_t gfp_mask)
> >  	}
> >  
> >          for ( ; ; ) {
> > -		get_dirty_limits(&background_thresh, &dirty_thresh, NULL);
> > +		unsigned long thresh = zone_page_state(zone, NR_ACTIVE) +
> > +			zone_page_state(zone, NR_INACTIVE);
> >  
> > -                /*
> > -                 * Boost the allowable dirty threshold a bit for page
> > -                 * allocators so they don't get DoS'ed by heavy writers
> > -                 */
> > -                dirty_thresh += dirty_thresh / 10;      /* wheeee... */
> > +		/*
> > +		 * wait when 75% of the zone's pages are under writeback
> > +		 */
> > +		thresh -= thresh >> 2;
> > +		if (zone_page_state(zone, NR_WRITEBACK) < thresh)
> > +			break;
> >  
> > -                if (global_page_state(NR_UNSTABLE_NFS) +
> > -			global_page_state(NR_WRITEBACK) <= dirty_thresh)
> > -                        	break;
> >                  congestion_wait(WRITE, HZ/10);
> >          }
> >  }
> > diff --git a/mm/vmscan.c b/mm/vmscan.c
> > index 1be5a63..7dd6bd9 100644
> > --- a/mm/vmscan.c
> > +++ b/mm/vmscan.c
> > @@ -948,7 +948,7 @@ static unsigned long shrink_zone(int priority, struct zone *zone,
> >  		}
> >  	}
> >  
> > -	throttle_vm_writeout(sc->gfp_mask);
> > +	throttle_vm_writeout(zone, sc->gfp_mask);
> >  
> >  	atomic_dec(&zone->reclaim_in_progress);
> >  	return nr_reclaimed;
> > 
> > 
> 
> I think that's an improvement in all respects.
> 
> However it still does not generally address the deadlock scenario: if
> there's a small DMA zone, and fuse manages to put all of those pages
> under writeout, then there's trouble.
> 
> But it's not really fuse specific.  If it was a normal filesystem that
> did that, and it needed a GFP_DMA allocation for writeout, it is in
> trouble also, as that allocation would fail (at least no deadlock).
> 
> Or is GFP_DMA never used by fs/io writeout paths?

I agree that its not complete. (hence the lack of sign-off etc.)

'normally' writeback pages just need an interrupt to signal the stuff is
written back. ie. the writeback completion path is atomic. 

 [ result of the thinking below -- the above is esp. true for swap pages
   - so maybe we should ensure that !swap traffic can never exceed this
   75% - that way, swap can always us get out of a tight spot ]

Maybe we should make that a requirement, although I see how that becomes
rather hard for FUSE (which is where Andrews PF_MEMALLOC suggestion
comes from - but I really dislike PF_MEMALLOC exposed to userspace).

Limiting FUSE to say 50% (suggestion from your other email) sounds like
a horrible hack to me. - Need more time to think on this.

.. o O [ process of thinking ]

While I think, I might as well tell the problem I had with
throttle_vm_writeout(), which is nfs unstable pages. Those don't go away
by waiting for it - as throttle_vm_writeout() does. So once you have an
excess of those, you're stuck as well.

In this patch I totally ignored unstable, but I'm not sure that's the
proper thing to do, I'd need to figure out what happens to an unstable
page when passed into pageout() - or if its passed to pageout at all.

If unstable pages would be passed to pageout(), and it would properly
convert them to writeback and clean them, then there is nothing wrong.

(Trond?)



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

Re: [PATCH] remove throttle_vm_writeout()

[Miklos Szeredi]

> Limiting FUSE to say 50% (suggestion from your other email) sounds like
> a horrible hack to me. - Need more time to think on this.

I don't really understand all that page balancing stuff, but I think
this will probably never or very rarely happen, because the allocator
will prefer the bigger zones, and the dirty page limiting will not let
the bigger zones get too full of dirty pages.

And even it can happen, it's not necessarily a fuse-only thing.

It makes tons of sense to make sure, that we don't fully dirty _any_
specialized zone.  One special zone group are the low-memory pages.
And currently balance_dirty_pages() makes sure we don't fill that up
with dirty file backed pages.  So something like that should make
sense for other special zones like DMA as well.

I'm not saying it's trivial, or even possible to implement, just
thinking...

Miklos

--
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] remove throttle_vm_writeout()

[Trond Myklebust]

On Fri, 2007-10-05 at 12:57 +0200, Peter Zijlstra wrote:
> In this patch I totally ignored unstable, but I'm not sure that's the
> proper thing to do, I'd need to figure out what happens to an unstable
> page when passed into pageout() - or if its passed to pageout at all.
> 
> If unstable pages would be passed to pageout(), and it would properly
> convert them to writeback and clean them, then there is nothing wrong.

Why would we want to do that? That would be a hell of a lot of work
(locking pages, setting flags, unlocking pages, ...) for absolutely no
reason.

Unstable writes are writes which have been sent to the server, but which
haven't been written to disk on the server. A single RPC command is then
sent (COMMIT) which basically tells the server to call fsync(). After
that is successful, we can free up the pages, but we do that with no
extra manipulation of the pages themselves: no page locks, just removal
from the NFS private radix tree, and freeing up of the NFS private
structures.

We only need to touch the pages again in the unlikely case that the
COMMIT fails because the server has rebooted. In this case we have to
resend the writes, and so the pages are marked as dirty, so we can go
through the whole writepages() rigmarole again...

So, no. I don't see sending pages through pageout() as being at all
helpful.

Trond

--
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] remove throttle_vm_writeout()

[Peter Zijlstra]

On Fri, 2007-10-05 at 13:50 -0400, Trond Myklebust wrote:
> On Fri, 2007-10-05 at 12:57 +0200, Peter Zijlstra wrote:
> > In this patch I totally ignored unstable, but I'm not sure that's the
> > proper thing to do, I'd need to figure out what happens to an unstable
> > page when passed into pageout() - or if its passed to pageout at all.
> > 
> > If unstable pages would be passed to pageout(), and it would properly
> > convert them to writeback and clean them, then there is nothing wrong.
> 
> Why would we want to do that? That would be a hell of a lot of work
> (locking pages, setting flags, unlocking pages, ...) for absolutely no
> reason.
> 
> Unstable writes are writes which have been sent to the server, but which
> haven't been written to disk on the server. A single RPC command is then
> sent (COMMIT) which basically tells the server to call fsync(). After
> that is successful, we can free up the pages, but we do that with no
> extra manipulation of the pages themselves: no page locks, just removal
> from the NFS private radix tree, and freeing up of the NFS private
> structures.
> 
> We only need to touch the pages again in the unlikely case that the
> COMMIT fails because the server has rebooted. In this case we have to
> resend the writes, and so the pages are marked as dirty, so we can go
> through the whole writepages() rigmarole again...
> 
> So, no. I don't see sending pages through pageout() as being at all
> helpful.

Well, the thing is, we throttle pageout in throttle_vm_writeout(). As it
stand we can deadlock there because it just waits for the numbers to
drop, and unstable pages don't automagically dissapear. Only
write_inodes() - normally called from balance_dirty_pages() will call
COMMIT.

So my thought was that calling pageout() on an unstable page would do
the COMMIT - we're low on memory, otherwise we would not be paging, so
getting rid of unstable pages seems to make sense to me.



--
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] remove throttle_vm_writeout()

[Trond Myklebust]

On Fri, 2007-10-05 at 20:32 +0200, Peter Zijlstra wrote:
> Well, the thing is, we throttle pageout in throttle_vm_writeout(). As it
> stand we can deadlock there because it just waits for the numbers to
> drop, and unstable pages don't automagically dissapear. Only
> write_inodes() - normally called from balance_dirty_pages() will call
> COMMIT.
> 
> So my thought was that calling pageout() on an unstable page would do
> the COMMIT - we're low on memory, otherwise we would not be paging, so
> getting rid of unstable pages seems to make sense to me.

Why not rather track which mappings have large numbers of outstanding
unstable writes at the VM level, and then add some form of callback to
allow it to notify the filesystem when it needs to flush them out?

Cheers
  Trond

--
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] remove throttle_vm_writeout()

[Trond Myklebust]

On Fri, 2007-10-05 at 15:20 -0400, Trond Myklebust wrote:
> On Fri, 2007-10-05 at 20:32 +0200, Peter Zijlstra wrote:
> > Well, the thing is, we throttle pageout in throttle_vm_writeout(). As it
> > stand we can deadlock there because it just waits for the numbers to
> > drop, and unstable pages don't automagically dissapear. Only
> > write_inodes() - normally called from balance_dirty_pages() will call
> > COMMIT.
> > 
> > So my thought was that calling pageout() on an unstable page would do
> > the COMMIT - we're low on memory, otherwise we would not be paging, so
> > getting rid of unstable pages seems to make sense to me.
> 
> Why not rather track which mappings have large numbers of outstanding
> unstable writes at the VM level, and then add some form of callback to
> allow it to notify the filesystem when it needs to flush them out?
> 
> Cheers
>   Trond

BTW: Please note that at least in the case of NFS, you will have to
allow for the fact that the filesystem may not be _able_ to cause the
numbers to drop. If the server is unavailable, then we're may be stuck
in unstable page limbo for quite some time.

Trond

--
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] remove throttle_vm_writeout()

[Peter Zijlstra]

On Fri, 2007-10-05 at 15:23 -0400, Trond Myklebust wrote: 
> On Fri, 2007-10-05 at 15:20 -0400, Trond Myklebust wrote:
> > On Fri, 2007-10-05 at 20:32 +0200, Peter Zijlstra wrote:
> > > Well, the thing is, we throttle pageout in throttle_vm_writeout(). As it
> > > stand we can deadlock there because it just waits for the numbers to
> > > drop, and unstable pages don't automagically dissapear. Only
> > > write_inodes() - normally called from balance_dirty_pages() will call
> > > COMMIT.
> > > 
> > > So my thought was that calling pageout() on an unstable page would do
> > > the COMMIT - we're low on memory, otherwise we would not be paging, so
> > > getting rid of unstable pages seems to make sense to me.
> > 
> > Why not rather track which mappings have large numbers of outstanding
> > unstable writes at the VM level, and then add some form of callback to
> > allow it to notify the filesystem when it needs to flush them out?

That would be nice, its just that the pageout throttling is not quite
that sophisticated. But we'll see what we can come up with.

> BTW: Please note that at least in the case of NFS, you will have to
> allow for the fact that the filesystem may not be _able_ to cause the
> numbers to drop. If the server is unavailable, then we're may be stuck
> in unstable page limbo for quite some time.

Agreed, it would be nice if that is handled is such a manner that it
does not take down all other paging.

The regular write path that only bothers with balance_dirty_pages()
already does this nicely.

--
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] remove throttle_vm_writeout()

[Fengguang Wu]

On Fri, Oct 05, 2007 at 08:32:19PM +0200, Peter Zijlstra wrote:
> 
> On Fri, 2007-10-05 at 13:50 -0400, Trond Myklebust wrote:
> > On Fri, 2007-10-05 at 12:57 +0200, Peter Zijlstra wrote:
> > > In this patch I totally ignored unstable, but I'm not sure that's the
> > > proper thing to do, I'd need to figure out what happens to an unstable
> > > page when passed into pageout() - or if its passed to pageout at all.
> > > 
> > > If unstable pages would be passed to pageout(), and it would properly
> > > convert them to writeback and clean them, then there is nothing wrong.
> > 
> > Why would we want to do that? That would be a hell of a lot of work
> > (locking pages, setting flags, unlocking pages, ...) for absolutely no
> > reason.
> > 
> > Unstable writes are writes which have been sent to the server, but which
> > haven't been written to disk on the server. A single RPC command is then
> > sent (COMMIT) which basically tells the server to call fsync(). After
> > that is successful, we can free up the pages, but we do that with no
> > extra manipulation of the pages themselves: no page locks, just removal
> > from the NFS private radix tree, and freeing up of the NFS private
> > structures.
> > 
> > We only need to touch the pages again in the unlikely case that the
> > COMMIT fails because the server has rebooted. In this case we have to
> > resend the writes, and so the pages are marked as dirty, so we can go
> > through the whole writepages() rigmarole again...
> > 
> > So, no. I don't see sending pages through pageout() as being at all
> > helpful.
> 
> Well, the thing is, we throttle pageout in throttle_vm_writeout(). As it
> stand we can deadlock there because it just waits for the numbers to
> drop, and unstable pages don't automagically dissapear. Only
> write_inodes() - normally called from balance_dirty_pages() will call
> COMMIT.

I wonder whether
        if (!bdi_nr_writeback)
                break;
or something like that could avoid the deadlock?

> So my thought was that calling pageout() on an unstable page would do
> the COMMIT - we're low on memory, otherwise we would not be paging, so
> getting rid of unstable pages seems to make sense to me.

I guess "many unstable pages" would be better if we are taking this way.

--
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] remove throttle_vm_writeout()

[Peter Zijlstra]

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

On Thu, 2007-10-04 at 16:09 -0700, Andrew Morton wrote:
> On Fri, 05 Oct 2007 00:39:16 +0200
> Miklos Szeredi <miklos@szeredi.hu> wrote:
> 
> > > throttle_vm_writeout() should be a per-zone thing, I guess.  Perhaps fixing
> > > that would fix your deadlock.  That's doubtful, but I don't know anything
> > > about your deadlock so I cannot say.
> > 
> > No, doing the throttling per-zone won't in itself fix the deadlock.
> > 
> > Here's a deadlock example:
> > 
> > Total memory = 32M
> > /proc/sys/vm/dirty_ratio = 10
> > dirty_threshold = 3M
> > ratelimit_pages = 1M
> > 
> > Some program dirties 4M (dirty_threshold + ratelimit_pages) of mmap on
> > a fuse fs.  Page balancing is called which turns all these into
> > writeback pages.
> > 
> > Then userspace filesystem gets a write request, and tries to allocate
> > memory needed to complete the writeout.
> > 
> > That will possibly trigger direct reclaim, and throttle_vm_writeout()
> > will be called.  That will block until nr_writeback goes below 3.3M
> > (dirty_threshold + 10%).  But since all 4M of writeback is from the
> > fuse fs, that will never happen.
> > 
> > Does that explain it better?
> > 
> 
> yup, thanks.
> 
> This is a somewhat general problem: a userspace process is in the IO path. 
> Userspace block drivers, for example - pretty much anything which involves
> kernel->userspace upcalls for storage applications.
> 
> I solved it once in the past by marking the userspace process as
> PF_MEMALLOC and I beleive that others have implemented the same hack.
> 
> I suspect that what we need is a general solution, and that the solution
> will involve explicitly telling the kernel that this process is one which
> actually cleans memory and needs special treatment.
> 
> Because I bet there will be other corner-cases where such a process needs
> kernel help, and there might be optimisation opportunities as well.
> 
> Problem is, any such mark-me-as-special syscall would need to be
> privileged, and FUSE servers presently don't require special perms (do
> they?)

I think just adding nr_cpus * ratelimit_pages to the dirth_thresh in
throttle_vm_writeout() will also solve the problem

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

Re: [PATCH] remove throttle_vm_writeout()

[Rik van Riel]

On Fri, 05 Oct 2007 09:32:57 +0200
Peter Zijlstra <a.p.zijlstra@chello.nl> wrote:

> I think just adding nr_cpus * ratelimit_pages to the dirth_thresh in
> throttle_vm_writeout() will also solve the problem

Agreed, that should fix the main latency issues.
 
-- 
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>