Re: Change in default vm_dirty_ratio

Linus Torvalds
2007-06-24T16:42:18Z

On Sat, 23 Jun 2007, Peter Zijlstra wrote:

> On Thu, 2007-06-21 at 16:08 -0700, Linus Torvalds wrote:
> >
> > The vm_dirty_ratio thing is a global value, and I think we need that
> > regardless (for the independent issue of memory deadlocks etc), but if we
> > *additionally* had a per-device throttle that was based on some kind of
> > adaptive thing, we could probably raise the global (hard) vm_dirty_ratio a
> > lot.
>
> I just did quite a bit of that:
>
> http://lkml.org/lkml/2007/6/14/437

Ok, that does look interesting.

A few comments:

- Cosmetic: please please *please* don't do this:

- if (atomic_long_dec_return(&nfss->writeback) < NFS_CONGESTION_OFF_THRESH) {
+ if (atomic_long_dec_return(&nfss->writeback) <
+ NFS_CONGESTION_OFF_THRESH)

we had a discussion about this not that long ago, and it drives me wild
to see people split lines like that. It used to be readable. Now it's
not.

If it's the checkpatch.pl thing that caused you to split it like that,
I think we should just change the value where we start complaining.
Maybe set it at 95 characters per line or something.

- I appreciate the extensive comments on floating proportions, and the
code looks really quite clean (apart from the cosmetic thing about)
from a quick look-through.

HOWEVER. It does seem to be a bit of an overkill. Do we really need to
be quite that clever, and do we really need to do 64-bit calculations
for this? The 64-bit ops in particular seem quite iffy: if we ever
actually pass in an amount that doesn't fit in 32 bits, we'll turn
those per-cpu counters into totally synchronous global counters, which
seems to defeat the whole point of them. So I'm a bit taken aback by
that whole "mod64" thing

(I also hate the naming. I don't think "..._mod()" was a good name to
begin with: "mod" means "modulus" to me, not "modify". Making it be
called "mod64" just makes it even *worse*, since it's now _obviously_
about modulus - but isn't)

So I'd appreciate some more explanations, but I'd also appreciate some
renaming of those functions. What used to be pretty bad naming just turned
*really* bad, imnsho.

Linus