On Thu, Mar 08, 2018 at 04:40:17PM -0500, Waiman Long wrote: > On 03/08/2018 03:49 PM, Luis R. Rodriguez wrote: > > On Thu, Mar 08, 2018 at 02:57:09PM -0500, Waiman Long wrote: > >> On 03/08/2018 01:31 PM, Luis R. Rodriguez wrote: > >>> On Thu, Mar 01, 2018 at 12:43:38PM -0500, Waiman Long wrote: > >>>> Even with clamped sysctl parameters, it is still not that straight > >>>> forward to figure out the exact range of those parameters. One may > >>>> try to write extreme parameter values to see if they get clamped. > >>>> To make it easier, a warning with the expected range will now be > >>>> printed in the kernel ring buffer when a clamped sysctl parameter > >>>> receives an out of range value. > >>>> > >>>> Signed-off-by: Waiman Long <[email protected]> > >>>> --- > >>>> include/linux/sysctl.h | 3 +++ > >>>> kernel/sysctl.c | 52 > >>>> ++++++++++++++++++++++++++++++++++++++++++-------- > >>>> 2 files changed, 47 insertions(+), 8 deletions(-) > >>>> > >>>> diff --git a/include/linux/sysctl.h b/include/linux/sysctl.h > >>>> index 448aa72..3db57af 100644 > >>>> --- a/include/linux/sysctl.h > >>>> +++ b/include/linux/sysctl.h > >>>> @@ -130,11 +130,14 @@ struct ctl_table > >>>> * @CTL_FLAGS_CLAMP_RANGE: Set to indicate that the entry should be > >>>> * flexibly clamped to min/max range in case the user provided > >>>> * an incorrect value. > >>>> + * @CTL_FLAGS_OOR_WARNED: Set to indicate that an out of range warning > >>>> + * had been issued for that entry. > >>>> * > >>>> * At most 16 different flags will be allowed. > >>>> */ > >>>> enum ctl_table_flags { > >>>> CTL_FLAGS_CLAMP_RANGE = BIT(0), > >>>> + CTL_FLAGS_OOR_WARNED = BIT(1), > >>>> }; > >>> Ugh, no. Now I see why you had to set this flag later. > >>> > >>> You are not using this flag to "warn" but rather for an internal > >>> status checker if you have warned or not. Internal flags should > >>> not be something the user sets. If we want a flag for warning > >>> that's one thing. If we need a flag to keep tabs if we have > >>> warned or not that needs to be kept separately and internally, > >>> nothing the user has to do set or reset. > >>> > >>> Luis > >> What I want to do is a printk_once for each sysctl parameter. So the > >> flag is used as a marker that a warning has been printed. > >> > >> I do understand that it gets somewhat ugly in the case of msgmni and > >> shmmni because of the copying back of the flag. Another alternative that > >> had been suggested by Kees is to use prink_ratelimited. That we don't > >> need that flag at all. > > However it is done, a user flag should not be used also for internal > > flag settings. That's just gross. Internal state machine stuff should > > remain far from what the user is able to modify. > > > > Also, why can't it just use pr_warn_once() and be done with it? > > Different sysctl parameters can use the same minmax proc_handler. Using > pr_warn_once() means mistake in one will prevent mistakes in other > parameters from showing up.
OK then use a separate internal set of flags for internal book keeping. Its nothing the user should ever have to set. Luis
