Re: [PATCH] bump up nr_to_write in xfs_vm_writepage

KOSAKI Motohiro
2009-07-10T06:49:40Z

> On Thu, Jul 09, 2009 at 11:04:32AM +0900, KOSAKI Motohiro wrote:
> > > On Tue, Jul 07, 2009 at 07:33:04PM +0900, KOSAKI Motohiro wrote:
> > > > At least, I agree with Olaf. if you got someone's NAK in past thread,
> > > > Could you please tell me its url?
> > >
> > > The previous thread was simply dead-ended and nothing happened.
> > >
> >
> > Can you remember this thread subject? sorry, I haven't remember it.
>
> This is the original thread, it did lead to a few different patches
> going in, but the nr_to_write change wasn't one of them.
>
> http://kerneltrap.org/mailarchive/linux-kernel/2008/10/1/3472704/thread

Thanks good pointer. This thread have multiple interesting discussion.

1. making ext4_write_cache_pages() or modifying write_cache_pages()

I think this is Christoph's homework. he said

> I agree. But I'm still not quite sure if that requirement is unique to
> ext4 anyway. Give me some time to dive into the writeback code again,
> haven't been there for quite a while.

if he says modifying write_cache_pages() is necessary, I'd like to review it.

2. Current mapping->writeback_index updating is not proper?

I'm not sure which solution is better. but I think your first proposal is
enough acceptable.

3. Current wbc->nr_to_write value is not proper?

Current writeback_set_ratelimit() doesn't permit that ratelimit_pages exceed
4M byte. but it is too low restriction for nowadays.
(that's my understand. right?)

=======================================================
void writeback_set_ratelimit(void)
{
ratelimit_pages = vm_total_pages / (num_online_cpus() * 32);
if (ratelimit_pages < 16)
ratelimit_pages = 16;
if (ratelimit_pages * PAGE_CACHE_SIZE > 4096 * 1024)
ratelimit_pages = (4096 * 1024) / PAGE_CACHE_SIZE;
}
=======================================================

Yes, 4M bytes are pretty magical constant. We have three choice
A. Remove magical 4M constant simple (a bit danger)
B. Decide high border from IO capability
C. Introduce new /proc knob (as Olaf proposed)

In my personal prefer, B & C are better.