Re: [PATCH] remove throttle_vm_writeout()

2007-10-07 Thread 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

Re: [PATCH] remove throttle_vm_writeout()

2007-10-07 Thread 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

Re: [PATCH] remove throttle_vm_writeout()

2007-10-05 Thread 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 <[EMAIL PROTECTED]> wrote: > > > > commit c4e2d7ddde9693a4c05da7afd485db02c27a7a09 > > > Author: akpm > > > Date: Sun Dec 22 01:07:33 2002 + > > > > > > [PATCH] Give kswapd

Re: [PATCH] remove throttle_vm_writeout()

2007-10-05 Thread 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

Re: [PATCH] remove throttle_vm_writeout()

2007-10-05 Thread 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 becau

Re: [PATCH] remove throttle_vm_writeout()

2007-10-05 Thread Rik van Riel
On Fri, 05 Oct 2007 09:32:57 +0200 Peter Zijlstra <[EMAIL PROTECTED]> 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 from

Re: [PATCH] remove throttle_vm_writeout()

2007-10-05 Thread 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

Re: [PATCH] remove throttle_vm_writeout()

2007-10-05 Thread 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 calle

Re: [PATCH] remove throttle_vm_writeout()

2007-10-05 Thread 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 pageou

Re: [PATCH] remove throttle_vm_writeout()

2007-10-05 Thread 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 wo

Re: [PATCH] remove throttle_vm_writeout()

2007-10-05 Thread Andrew Morton
On Fri, 5 Oct 2007 20:30:28 +0800 Fengguang Wu <[EMAIL PROTECTED]> wrote: > > commit c4e2d7ddde9693a4c05da7afd485db02c27a7a09 > > Author: akpm > > Date: Sun Dec 22 01:07:33 2002 + > > > > [PATCH] Give kswapd writeback higher priority than pdflush > > > > The `low latency page

Re: [PATCH] remove throttle_vm_writeout()

2007-10-05 Thread 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

Re: [PATCH] remove throttle_vm_writeout()

2007-10-05 Thread 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 <[EMAIL PROTECTED]> 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

Re: [PATCH] remove throttle_vm_writeout()

2007-10-05 Thread 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 zo

Re: [PATCH] remove throttle_vm_writeout()

2007-10-05 Thread Peter Zijlstra
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 ino

Re: [PATCH] remove throttle_vm_writeout()

2007-10-05 Thread 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 nev

Re: [PATCH] remove throttle_vm_writeout()

2007-10-05 Thread 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_

Re: [PATCH] remove throttle_vm_writeout()

2007-10-05 Thread Peter Zijlstra
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

Re: [PATCH] remove throttle_vm_writeout()

2007-10-05 Thread 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 fr

Re: [PATCH] remove throttle_vm_writeout()

2007-10-05 Thread Peter Zijlstra
On Thu, 2007-10-04 at 17:48 -0700, Andrew Morton wrote: > On Fri, 05 Oct 2007 02:12:30 +0200 Miklos Szeredi <[EMAIL PROTECTED]> 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? > > > >

Re: [PATCH] remove throttle_vm_writeout()

2007-10-05 Thread Peter Zijlstra
On Thu, 2007-10-04 at 16:09 -0700, Andrew Morton wrote: > On Fri, 05 Oct 2007 00:39:16 +0200 > Miklos Szeredi <[EMAIL PROTECTED]> 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

Re: [PATCH] remove throttle_vm_writeout()

2007-10-04 Thread Andrew Morton
On Fri, 05 Oct 2007 02:12:30 +0200 Miklos Szeredi <[EMAIL PROTECTED]> 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

Re: [PATCH] remove throttle_vm_writeout()

2007-10-04 Thread 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 proc

Re: [PATCH] remove throttle_vm_writeout()

2007-10-04 Thread Andrew Morton
On Fri, 05 Oct 2007 01:26:12 +0200 Miklos Szeredi <[EMAIL PROTECTED]> 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. > > >

Re: [PATCH] remove throttle_vm_writeout()

2007-10-04 Thread 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

Re: [PATCH] remove throttle_vm_writeout()

2007-10-04 Thread Andrew Morton
On Fri, 05 Oct 2007 00:39:16 +0200 Miklos Szeredi <[EMAIL PROTECTED]> 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 thr

Re: [PATCH] remove throttle_vm_writeout()

2007-10-04 Thread 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. >

Re: [PATCH] remove throttle_vm_writeout()

2007-10-04 Thread Andrew Morton
On Thu, 04 Oct 2007 14:25:22 +0200 Miklos Szeredi <[EMAIL PROTECTED]> wrote: > From: Miklos Szeredi <[EMAIL PROTECTED]> > > 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

Re: [PATCH] remove throttle_vm_writeout()

2007-10-04 Thread 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

Re: [PATCH] remove throttle_vm_writeout()

2007-10-04 Thread Andrew Morton
On Thu, 04 Oct 2007 20:10:10 +0200 Peter Zijlstra <[EMAIL PROTECTED]> wrote: > > On Thu, 2007-10-04 at 10:46 -0700, Andrew Morton wrote: > > On Thu, 04 Oct 2007 18:47:07 +0200 Peter Zijlstra <[EMAIL PROTECTED]> wrote: > > > > static int may_write_to_queue(struct backing_dev_info *bdi) > > > { >

Re: [PATCH] remove throttle_vm_writeout()

2007-10-04 Thread 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 <[EMAIL PROTECTED]> wrote: > > static int may_write_to_queue(struct backing_dev_info *bdi) > > { > > if (current->flags & PF_SWAPWRITE) > > return 1; > > if (!bdi_write_

Re: [PATCH] remove throttle_vm_writeout()

2007-10-04 Thread Andrew Morton
On Thu, 04 Oct 2007 18:47:07 +0200 Peter Zijlstra <[EMAIL PROTECTED]> 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

Re: [PATCH] remove throttle_vm_writeout()

2007-10-04 Thread 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. Si

Re: [PATCH] remove throttle_vm_writeout()

2007-10-04 Thread 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 p

Re: [PATCH] remove throttle_vm_writeout()

2007-10-04 Thread Peter Zijlstra
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 swa

Re: [PATCH] remove throttle_vm_writeout()

2007-10-04 Thread 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

Re: [PATCH] remove throttle_vm_writeout()

2007-10-04 Thread Peter Zijlstra
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 i