Re: [PATCH] fix task dirty balancing

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

On Thursday 10 July 2008 13:10, YAMAMOTO Takashi wrote:
> hi,
>
> thanks for the review.
>
> > On Wednesday 09 July 2008 09:38, YAMAMOTO Takashi wrote:
> > > hi,
> > >
> > > > Please beat me to cleaning up this stuff - otherwise I'll have to
> > > > look at it when I get back from holidays.
> > >
> > > how about the following?
> >
> > Quite good, however I would like to keep the buffers warning if it isn't
> > too difficult (it has already caught one or two real bugs).
>
> isn't the WARN_ON_ONCE in __set_page_dirty_nobuffers enough?

No, because it will skip warning if the page has buffers (which is a
very common case for fs/buffer.c :)).


> > Also, we should
> > split out the bugfix from the cleanup. But yes overall I think the result
> > looks quite nice.
>
> honestly, i don't think it makes much sense to separate the fix and
> the cleanup in this particular case.  trying to keep the bug while
> the code cleanup naturally fixes it, or vice versa, would be a waste
> of time.

It always makes sense to separate fix and cleanup IMO. The most important
reason is that it makes it clearer to review the fix. Secondarily, it
makes it easier to ensure no unwanted changes in the cleanup part.