Re: [PATCH] mm: moving dirty pages balancing to pdfludh entirely

[Nikita Danilov <nikita@clusterfs.com>]

Ananiev, Leonid I writes:
 > Nikita Danilov writes:
 > >> by introducing MAX_PDFLUSH_THREADS=8. Why just 8?
 > > Sorry, I don't understand. pdflush.c appeared in 2.5.8 and
 > Thanks for explanation. Why just 8? Answer: it was introduced in 2.5.8.

No, no, I was commenting on your (incorrect) statement that kernel "was
patched up by introducing MAX_PDFLUSH_THREADS=8". It never was: this
limit is part of the original design, not some "patch".

 > Why the constant MAX_PDFLUSH_THREADS is needed? Is it because current
 > kernel may create huge number of pdflush threads and overload the
 > system? Why we can not set MAX_PDFLUSH_THREADS=512? 1000?

Because this is not needed for current pdflush usage patterns. pdflush
is used for light background write-out, and small number of pdflush
threads is able to achieve necessary IO concurrency level, because they
are mostly non-blocking, thanks to the current_is_pdflush() checks. If
asynchronous write-out cannot cope with the rate of dirtying,
synchronous write-out starts.

You, on the other hand, are trying to use pdflush for the goal it wasn't
designed for. No wonder it doesn't work.

 > 
 > >> Why 48?
 > > This is explained in comment just above sync_writeback_pages()
 > > definition. Basically, ratelimit_pages pages might be dirtied between
 > > calls to balance_dirty_pages(), and the latter tries to write out
 > *more*
 > > pages to keep number of dirty pages under control: negative feedback
 > > control loop, of sorts.
 > 
 > I had asked "why 48?" is hard coded for any configuration. I do not see
 > "48" in your explanation.

You didn't look carefully enough. :-)

/*
 * When balance_dirty_pages decides that the caller needs to perform some
 * non-background writeback, this is how many pages it will attempt to write.
 * It should be somewhat larger than RATELIMIT_PAGES to ensure that reasonably
 * large amounts of I/O are submitted.
 */
static inline long sync_writeback_pages(void)
{
	return ratelimit_pages + ratelimit_pages / 2;
}

ratelimit_pages = 32, hence, sync_writeback_pages() returns 48. It could
be different, of course, if ratelimit_pages were.

 > 
 > You do not recommend to use hard coded constants but
 > MAX_PDFLUSH_THREADS=8 and write_chunk=48 are sacred according you mind. 

No, I think the fewer hard-coded constants are here---the better. I'd
happily eliminate MAX_PDFLUSH_THREADS, but just dropping this constraint
is a non-solution in search of a problem, obviously.

But this discussion is degenerating, so I shall participate in it no
longer.

 > 
 > Leonid

Nikita.