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 <longman@xxxxxxxxxx>
> >>>> ---
> >>>> 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
