Re: [PATCH] mm: prevent divide error for small values of vm_dirty_bytes

[Andrew Morton <akpm@linux-foundation.org>]

On Fri, 1 May 2009 16:56:40 +0200
Andrea Righi <righi.andrea@gmail.com> wrote:

> On Wed, Apr 29, 2009 at 02:46:55PM -0700, Andrew Morton wrote:
> > On Wed, 29 Apr 2009 11:34:51 +0200
> > Andrea Righi <righi.andrea@gmail.com> wrote:
> > 
> > > --- a/Documentation/sysctl/vm.txt
> > > +++ b/Documentation/sysctl/vm.txt
> > > @@ -90,6 +90,10 @@ will itself start writeback.
> > >  If dirty_bytes is written, dirty_ratio becomes a function of its value
> > >  (dirty_bytes / the amount of dirtyable system memory).
> > >  
> > > +Note: the minimum value allowed for dirty_bytes is two pages (in bytes); any
> > > +value lower than this limit will be ignored and the old configuration will be
> > > +retained.
> > 
> > Well.  This implies that the write to the procfs file would appear to
> > succeed.  One hopes that the write would in fact return -EINVAL or
> > such?
> 
> I definitely agree. Just tested the following patch and it looks much
> better with the error code.
> 
> -Andrea
> 
> ---
> sysctl: return error code if values are not within a valid range
> 
> Currently __do_proc_doulongvec_minmax(), as well as
> __do_proc_dointvec(), simply skip the invalid values instead of return
> -EINVAL.

Oh geeze, I didn't know that.

> A more correct behaviour is to report to the userspace that some values
> were invalid and they couldn't be written instead of silently drop
> them.
> 
> For example (vm_dirty_bytes must be greater or equal than 2*PAGE_SIZE):
> - before:
>   # cat /proc/sys/vm/dirty_bytes
>   0
>   # /bin/echo 1 > /proc/sys/vm/dirty_bytes
>   # cat /proc/sys/vm/dirty_bytes
>   0
>   # /bin/echo 8192 > /proc/sys/vm/dirty_bytes
>   # cat /proc/sys/vm/dirty_bytes
>   8192
> 
> - after:
>   # cat /proc/sys/vm/dirty_bytes
>   0
>   # /bin/echo 1 > /proc/sys/vm/dirty_bytes
>   /bin/echo: write error: Invalid argument
>   # cat /proc/sys/vm/dirty_bytes
>   0
>   # /bin/echo 8192 > /proc/sys/vm/dirty_bytes
>   # cat /proc/sys/vm/dirty_bytes
>   8192
> 

Unfortunately the potential here for breaking existing userspace is
huge.  I think it's too late for us to fix this :(