Re: [PATCH] fix task dirty balancing

[Nick Piggin <nickpiggin@yahoo.com.au>]

On Saturday 05 July 2008 19:30, Peter Zijlstra wrote:
> On Sat, 2008-07-05 at 15:04 +0900, KAMEZAWA Hiroyuki wrote:

> > And task-dirty-limit don't have to take care of following 2 case ?
> >   - __set_page_dirty_nobuffers(struct page *page) (increment
> > BDI_RECRAIMABLE) - test_set_page_writeback() (increment BDI_RECLAIMABLE)
>
> Gah - what a mess...

It's not so bad once you get past the funny names and conventions :)


> It's in set_page_dirty() so it wouldn't have to be in all the
> a_ops->set_page_dirty() functions...

At some point, they have to actually set the page dirty though, in
which case they would normally call __set_page_dirty_nobuffers or
similar (ie. rather than SetPageDirty, unless they really know what
they're doing).


> But now it turns out people don't use set_page_dirty() to dirty
> pages :-(

They do, but they also use other things :) Filesystems of course are
in complete control of the aops, so they can definitely bypass it.


> For the purpose of task_dirty_inc() I guess we might as well pair it
> with task_io_account_write() for each PAGE_CACHE_SIZE (and ignore the
> DIO bit, since that doesn't care about the dirty limit anyway).

Yes, the dirty increment should go in the same places where we increment
all the dirty statistics... it's the right spot to do it AFAIKS.


> Might be my ignorance, but _why_ do we have __set_page_dirty_nobuffers()
> reimplemented in fs/buffers.c:__set_page_dirty() ? - those two functions
> look suspiciously similar.

Probably no really good reason. The buffers.c code should probably be
merged / unified in page-writeback.c.


> Also, why was the EXPORT added anyway - fs/buffers.o never ends up in
> modules?

Definitely it would. Almost every filesystem can be build modularly. Or
did you mean some other symbol?