| Message ID | 1449626509-3736-1-git-send-email-wangyufen@huawei.com |
|---|---|
| State | Changes Requested, archived |
| Delegated to: | David Miller |
| Headers | show |
Hello. On 12/9/2015 5:01 AM, Wang Yufen wrote: > From: Yufen Wang <wangyufen@huawei.com> > > I tried on linux-4.1: > linux:~# cat /proc/sys/net/ipv4/tcp_mem > 8388608 12582912 16777216 > linux:~# echo 1234 >/proc/sys/net/ipv4/tcp_mem > -bash: echo: write error: Invalid argument > linux:~# cat /proc/sys/net/ipv4/tcp_mem > 1234 12582912 16777216 > > the echo operation got error, but value already written to tcp_mem. > If a write() returns an error like EINVAL, we expect no change occurred. > This patch fix the confusing corner and makes __do_proc_doulongvec_minmax > works the same as __do_proc_dointvec > > Signed-off-by: Yufen Wang <wangyufen@huawei.com> > Cc: Eric Dumazet <eric.dumazet@gmail.com> > Cc: "Eric W. Biederman" <ebiederm@xmission.com> > --- > kernel/sysctl.c | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/kernel/sysctl.c b/kernel/sysctl.c > index c3eee4c..e3ee4be 100644 > --- a/kernel/sysctl.c > +++ b/kernel/sysctl.c > @@ -2318,6 +2318,8 @@ static int __do_proc_doulongvec_minmax(void *data, struct ctl_table *table, int > bool neg; > > left -= proc_skip_spaces(&kbuf); > + if (!left) > + break; Please use tabs for indentation, not spaces. [...] MBR, Sergei -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Wang Yufen <wangyufen@huawei.com> writes: > From: Yufen Wang <wangyufen@huawei.com> > > I tried on linux-4.1: > linux:~# cat /proc/sys/net/ipv4/tcp_mem > 8388608 12582912 16777216 > linux:~# echo 1234 >/proc/sys/net/ipv4/tcp_mem > -bash: echo: write error: Invalid argument > linux:~# cat /proc/sys/net/ipv4/tcp_mem > 1234 12582912 16777216 > > the echo operation got error, but value already written to tcp_mem. > If a write() returns an error like EINVAL, we expect no change occurred. > This patch fix the confusing corner and makes __do_proc_doulongvec_minmax > works the same as __do_proc_dointvec Nacked-by: "Eric W. Biederman" <ebiederm@xmission.com> Except for possibly breaking your muscle memory this does not explain why this is a problem. Further you are changing a whole lot more than tcp_mem, without a word of justification in your description. I do not think changing every integer use of sysctl for some unknown reason. Is justified in this case. Eric > Signed-off-by: Yufen Wang <wangyufen@huawei.com> > Cc: Eric Dumazet <eric.dumazet@gmail.com> > Cc: "Eric W. Biederman" <ebiederm@xmission.com> > --- > kernel/sysctl.c | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/kernel/sysctl.c b/kernel/sysctl.c > index c3eee4c..e3ee4be 100644 > --- a/kernel/sysctl.c > +++ b/kernel/sysctl.c > @@ -2318,6 +2318,8 @@ static int __do_proc_doulongvec_minmax(void *data, struct ctl_table *table, int > bool neg; > > left -= proc_skip_spaces(&kbuf); > + if (!left) > + break; > > err = proc_get_long(&kbuf, &left, &val, &neg, > proc_wspace_sep, -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Eric, On 2015/12/10 0:38, Eric W. Biederman wrote: > Wang Yufen <wangyufen@huawei.com> writes: > >> From: Yufen Wang <wangyufen@huawei.com> >> >> I tried on linux-4.1: >> linux:~# cat /proc/sys/net/ipv4/tcp_mem >> 8388608 12582912 16777216 >> linux:~# echo 1234 >/proc/sys/net/ipv4/tcp_mem >> -bash: echo: write error: Invalid argument >> linux:~# cat /proc/sys/net/ipv4/tcp_mem >> 1234 12582912 16777216 >> >> the echo operation got error, but value already written to tcp_mem. >> If a write() returns an error like EINVAL, we expect no change occurred. >> This patch fix the confusing corner and makes __do_proc_doulongvec_minmax >> works the same as __do_proc_dointvec > Nacked-by: "Eric W. Biederman" <ebiederm@xmission.com> > > Except for possibly breaking your muscle memory this does not explain > why this is a problem. I think it's really confusing that we got write error but actually the value echoed is written properly, at least we need to fix it and make the result match the behavior, right? > > Further you are changing a whole lot more than tcp_mem, without a word > of justification in your description. I agree, we can update the change log. Thanks Hanjun -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/kernel/sysctl.c b/kernel/sysctl.c index c3eee4c..e3ee4be 100644 --- a/kernel/sysctl.c +++ b/kernel/sysctl.c @@ -2318,6 +2318,8 @@ static int __do_proc_doulongvec_minmax(void *data, struct ctl_table *table, int bool neg; left -= proc_skip_spaces(&kbuf); + if (!left) + break; err = proc_get_long(&kbuf, &left, &val, &neg, proc_wspace_sep,