Messages in this thread | | | Date | Fri, 1 May 2009 12:53:17 -0700 | From | Andrew Morton <> | Subject | Re: [PATCH] mm: prevent divide error for small values of vm_dirty_bytes |
| |
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 :(
| |