Re: [PATCH 0/5] sysctl: Enable easy addition of range showing sysctl parameters

2018-03-08 Thread Waiman Long
On 03/07/2018 05:48 PM, Andrew Morton wrote:
> On Wed,  7 Mar 2018 15:34:24 -0500 Waiman Long  wrote:
>
>> This patch series is to be applied on top of the patch
>> series "ipc: Clamp *mni to the real IPCMNI limit"
>> (https://lkml.org/lkml/2018/3/1/716).
>>
>> The purpose of this patch series is to enable easy addition of new
>> auxillary sysctl parameters that can be read to display ranges of
>> other sysctl parameters they are associated with.
>>
>> With this patch series applied, a developer can add a range-showing
>> auxillary sysctl entries by just setting the CTL_FLAGS_SHOW_RANGE
>> flag in the ctl_table entry that has a range to show and add one more
>> to the number of reserved range entries in the CTL_RESERVE_RANGES()
>> macro before the terminating null entry. The new auxillary sysctl
>> parameters will have the "_range" suffix added.
>>
>> The number of ctl_table entries that have the CTL_FLAGS_SHOW_RANGE flag
>> should match the number of reserved entries in the CTL_RESERVE_RANGES()
>> macro. Otherwise, warning or error like below will be logged in the
>> kernel ring buffer.
>>
>>   Warning: Too many reserved ctl_table range entries ("shmmax")!
>>
>>   Error: Insufficient reserved ctl_table range entries ("shmmax")!
>>
>> The IPC sysctl parameters msgmni and shmmni are extended to have
>> those auxillary sysctl entries. As a result, one can now find out
>> the range of allowable value by looking at the corresponding
>> auxillary _range sysctl parameters. For example,
>>
>>   % cat msgmni_range
>>   [0, 32768]
>>
>> That means the msgmni sysctl parameter has to be within the 0 - 32768
>> range inclusively.
>>
>> ...
>>
>>  fs/proc/proc_sysctl.c  | 99 
>> +++---
>>  include/linux/sysctl.h | 52 +-
>>  ipc/ipc_sysctl.c   |  5 ++-
>>  kernel/sysctl.c| 91 ++
>>  4 files changed, 238 insertions(+), 9 deletions(-)
> That quite a bunch of new code and it's not clear to me that we get a
> lot of value from it all.  Perhaps I'm missing the point.
>
> A worked example would help, along the lines of:
>
>
> a) Here's how we do X at present and here's the output
>
> b) Here's how we X after these patches and here's the new output
>
> c) b) is better than a) for 

We do have a big customer asking for ways to find out what are the
actual ranges of sysctl parameters that they are using other than
digging into the source code to figure it out. This patch series and
previous one are part of the effort to fulfill this customer need.

Please ignore this patch series. I will focus on getting the first one
right and upstream first before working on a followup.

Thanks,
Longman




Re: [PATCH 0/5] sysctl: Enable easy addition of range showing sysctl parameters

2018-03-08 Thread Waiman Long
On 03/07/2018 05:48 PM, Andrew Morton wrote:
> On Wed,  7 Mar 2018 15:34:24 -0500 Waiman Long  wrote:
>
>> This patch series is to be applied on top of the patch
>> series "ipc: Clamp *mni to the real IPCMNI limit"
>> (https://lkml.org/lkml/2018/3/1/716).
>>
>> The purpose of this patch series is to enable easy addition of new
>> auxillary sysctl parameters that can be read to display ranges of
>> other sysctl parameters they are associated with.
>>
>> With this patch series applied, a developer can add a range-showing
>> auxillary sysctl entries by just setting the CTL_FLAGS_SHOW_RANGE
>> flag in the ctl_table entry that has a range to show and add one more
>> to the number of reserved range entries in the CTL_RESERVE_RANGES()
>> macro before the terminating null entry. The new auxillary sysctl
>> parameters will have the "_range" suffix added.
>>
>> The number of ctl_table entries that have the CTL_FLAGS_SHOW_RANGE flag
>> should match the number of reserved entries in the CTL_RESERVE_RANGES()
>> macro. Otherwise, warning or error like below will be logged in the
>> kernel ring buffer.
>>
>>   Warning: Too many reserved ctl_table range entries ("shmmax")!
>>
>>   Error: Insufficient reserved ctl_table range entries ("shmmax")!
>>
>> The IPC sysctl parameters msgmni and shmmni are extended to have
>> those auxillary sysctl entries. As a result, one can now find out
>> the range of allowable value by looking at the corresponding
>> auxillary _range sysctl parameters. For example,
>>
>>   % cat msgmni_range
>>   [0, 32768]
>>
>> That means the msgmni sysctl parameter has to be within the 0 - 32768
>> range inclusively.
>>
>> ...
>>
>>  fs/proc/proc_sysctl.c  | 99 
>> +++---
>>  include/linux/sysctl.h | 52 +-
>>  ipc/ipc_sysctl.c   |  5 ++-
>>  kernel/sysctl.c| 91 ++
>>  4 files changed, 238 insertions(+), 9 deletions(-)
> That quite a bunch of new code and it's not clear to me that we get a
> lot of value from it all.  Perhaps I'm missing the point.
>
> A worked example would help, along the lines of:
>
>
> a) Here's how we do X at present and here's the output
>
> b) Here's how we X after these patches and here's the new output
>
> c) b) is better than a) for 

We do have a big customer asking for ways to find out what are the
actual ranges of sysctl parameters that they are using other than
digging into the source code to figure it out. This patch series and
previous one are part of the effort to fulfill this customer need.

Please ignore this patch series. I will focus on getting the first one
right and upstream first before working on a followup.

Thanks,
Longman




Re: [PATCH 0/5] sysctl: Enable easy addition of range showing sysctl parameters

2018-03-08 Thread Waiman Long
On 03/08/2018 01:34 PM, Luis R. Rodriguez wrote:
> On Wed, Mar 07, 2018 at 03:34:24PM -0500, Waiman Long wrote:
>>   % cat msgmni_range
>>   [0, 32768]
> All that sounds promising but I think you are jumping a few
> steps ahead of what needs to get done. Let's first sort out the
> first series well.
>
>   Luis

Yes, you are right. I will focus on the getting the first series right
first, before considering that.

In fact, I have a slight change of mind on how to do it. Instead of
adding a new _range parameter to show just the range of a particular
sysctl parameter, I am thinking of a generic sysctl_ranges parameter
that iterates all the ctl_table entries and list out the ranges of all
the parameters that are flagged to have a min/max range. That will
eliminate the interface clutter due to a bunch of new *_range /proc/sys
files.

-Longman




Re: [PATCH 0/5] sysctl: Enable easy addition of range showing sysctl parameters

2018-03-08 Thread Waiman Long
On 03/08/2018 01:34 PM, Luis R. Rodriguez wrote:
> On Wed, Mar 07, 2018 at 03:34:24PM -0500, Waiman Long wrote:
>>   % cat msgmni_range
>>   [0, 32768]
> All that sounds promising but I think you are jumping a few
> steps ahead of what needs to get done. Let's first sort out the
> first series well.
>
>   Luis

Yes, you are right. I will focus on the getting the first series right
first, before considering that.

In fact, I have a slight change of mind on how to do it. Instead of
adding a new _range parameter to show just the range of a particular
sysctl parameter, I am thinking of a generic sysctl_ranges parameter
that iterates all the ctl_table entries and list out the ranges of all
the parameters that are flagged to have a min/max range. That will
eliminate the interface clutter due to a bunch of new *_range /proc/sys
files.

-Longman




Re: [PATCH 0/5] sysctl: Enable easy addition of range showing sysctl parameters

2018-03-08 Thread Luis R. Rodriguez
On Wed, Mar 07, 2018 at 03:34:24PM -0500, Waiman Long wrote:
> 
>   % cat msgmni_range
>   [0, 32768]

All that sounds promising but I think you are jumping a few
steps ahead of what needs to get done. Let's first sort out the
first series well.

  Luis


Re: [PATCH 0/5] sysctl: Enable easy addition of range showing sysctl parameters

2018-03-08 Thread Luis R. Rodriguez
On Wed, Mar 07, 2018 at 03:34:24PM -0500, Waiman Long wrote:
> 
>   % cat msgmni_range
>   [0, 32768]

All that sounds promising but I think you are jumping a few
steps ahead of what needs to get done. Let's first sort out the
first series well.

  Luis


Re: [PATCH 0/5] sysctl: Enable easy addition of range showing sysctl parameters

2018-03-07 Thread Andrew Morton
On Wed,  7 Mar 2018 15:34:24 -0500 Waiman Long  wrote:

> This patch series is to be applied on top of the patch
> series "ipc: Clamp *mni to the real IPCMNI limit"
> (https://lkml.org/lkml/2018/3/1/716).
> 
> The purpose of this patch series is to enable easy addition of new
> auxillary sysctl parameters that can be read to display ranges of
> other sysctl parameters they are associated with.
> 
> With this patch series applied, a developer can add a range-showing
> auxillary sysctl entries by just setting the CTL_FLAGS_SHOW_RANGE
> flag in the ctl_table entry that has a range to show and add one more
> to the number of reserved range entries in the CTL_RESERVE_RANGES()
> macro before the terminating null entry. The new auxillary sysctl
> parameters will have the "_range" suffix added.
> 
> The number of ctl_table entries that have the CTL_FLAGS_SHOW_RANGE flag
> should match the number of reserved entries in the CTL_RESERVE_RANGES()
> macro. Otherwise, warning or error like below will be logged in the
> kernel ring buffer.
> 
>   Warning: Too many reserved ctl_table range entries ("shmmax")!
> 
>   Error: Insufficient reserved ctl_table range entries ("shmmax")!
> 
> The IPC sysctl parameters msgmni and shmmni are extended to have
> those auxillary sysctl entries. As a result, one can now find out
> the range of allowable value by looking at the corresponding
> auxillary _range sysctl parameters. For example,
> 
>   % cat msgmni_range
>   [0, 32768]
> 
> That means the msgmni sysctl parameter has to be within the 0 - 32768
> range inclusively.
> 
> ...
> 
>  fs/proc/proc_sysctl.c  | 99 
> +++---
>  include/linux/sysctl.h | 52 +-
>  ipc/ipc_sysctl.c   |  5 ++-
>  kernel/sysctl.c| 91 ++
>  4 files changed, 238 insertions(+), 9 deletions(-)

That quite a bunch of new code and it's not clear to me that we get a
lot of value from it all.  Perhaps I'm missing the point.

A worked example would help, along the lines of:


a) Here's how we do X at present and here's the output

b) Here's how we X after these patches and here's the new output

c) b) is better than a) for 


Re: [PATCH 0/5] sysctl: Enable easy addition of range showing sysctl parameters

2018-03-07 Thread Andrew Morton
On Wed,  7 Mar 2018 15:34:24 -0500 Waiman Long  wrote:

> This patch series is to be applied on top of the patch
> series "ipc: Clamp *mni to the real IPCMNI limit"
> (https://lkml.org/lkml/2018/3/1/716).
> 
> The purpose of this patch series is to enable easy addition of new
> auxillary sysctl parameters that can be read to display ranges of
> other sysctl parameters they are associated with.
> 
> With this patch series applied, a developer can add a range-showing
> auxillary sysctl entries by just setting the CTL_FLAGS_SHOW_RANGE
> flag in the ctl_table entry that has a range to show and add one more
> to the number of reserved range entries in the CTL_RESERVE_RANGES()
> macro before the terminating null entry. The new auxillary sysctl
> parameters will have the "_range" suffix added.
> 
> The number of ctl_table entries that have the CTL_FLAGS_SHOW_RANGE flag
> should match the number of reserved entries in the CTL_RESERVE_RANGES()
> macro. Otherwise, warning or error like below will be logged in the
> kernel ring buffer.
> 
>   Warning: Too many reserved ctl_table range entries ("shmmax")!
> 
>   Error: Insufficient reserved ctl_table range entries ("shmmax")!
> 
> The IPC sysctl parameters msgmni and shmmni are extended to have
> those auxillary sysctl entries. As a result, one can now find out
> the range of allowable value by looking at the corresponding
> auxillary _range sysctl parameters. For example,
> 
>   % cat msgmni_range
>   [0, 32768]
> 
> That means the msgmni sysctl parameter has to be within the 0 - 32768
> range inclusively.
> 
> ...
> 
>  fs/proc/proc_sysctl.c  | 99 
> +++---
>  include/linux/sysctl.h | 52 +-
>  ipc/ipc_sysctl.c   |  5 ++-
>  kernel/sysctl.c| 91 ++
>  4 files changed, 238 insertions(+), 9 deletions(-)

That quite a bunch of new code and it's not clear to me that we get a
lot of value from it all.  Perhaps I'm missing the point.

A worked example would help, along the lines of:


a) Here's how we do X at present and here's the output

b) Here's how we X after these patches and here's the new output

c) b) is better than a) for