Re: [RFC 05/18] limits: track and present RLIMIT_NOFILE actual max

[Topi Miettinen]

On 06/13/16 21:16, Andy Lutomirski wrote:
> On Mon, Jun 13, 2016 at 2:13 PM, Topi Miettinen <toiwoton@xxxxxxxxx> wrote:
>> On 06/13/16 20:40, Andy Lutomirski wrote:
>>> On 06/13/2016 12:44 PM, Topi Miettinen wrote:
>>>> Track maximum number of files for the process, present current maximum
>>>> in /proc/self/limits.
>>>
>>> The core part should be its own patch.
>>>
>>> Also, you have this weirdly named (and racy!) function bump_rlimit.
>>
>> I can change the name if you have better suggestions. rlimit_track_max?
>>
>> The max value is written often but read seldom, if ever. What kind of
>> locking should I use then?
> 
> Possibly none, but WRITE_ONCE would be good as would a comment
> indicating that your code in intentionally racy.  Or you could use
> atomic_cmpxchg if that won't kill performance.
> 
> rlimit_track_max sounds like a better name to me.
> 
>>
>>> Wouldn't this be nicer if you taught the rlimit code to track the
>>> *current* usage generically and to derive the max usage from that?
>>
>> Current rlimit code performs checks against current limits. These are
>> typically done early in the calling function and further checks could
>> also fail. Thus max should not be updated until much later. Maybe these
>> could be combined, but not easily if at all.
> 
> I mean:  why not actually show the current value in /proc/pid/limits
> and track the max via whatever teaches proc about the current value?
> 

That could be interesting data too. In other comments, a new file was
proposed and then your model would be good choice.

>>
>>>
>>>> diff --git a/fs/proc/base.c b/fs/proc/base.c
>>>> index a11eb71..227997b 100644
>>>> --- a/fs/proc/base.c
>>>> +++ b/fs/proc/base.c
>>>> @@ -630,8 +630,8 @@ static int proc_pid_limits(struct seq_file *m,
>>>> struct pid_namespace *ns,
>>>>      /*
>>>>       * print the file header
>>>>       */
>>>> -       seq_printf(m, "%-25s %-20s %-20s %-10s\n",
>>>> -          "Limit", "Soft Limit", "Hard Limit", "Units");
>>>> +    seq_printf(m, "%-25s %-20s %-20s %-10s %-20s\n",
>>>> +           "Limit", "Soft Limit", "Hard Limit", "Units", "Max");
>>>
>>> What existing programs, if any, does this break?
>>
>> Using Debian codesearch for /limits" string, I'd check pam_limits and
>> rtkit. The max values could be put into a new file if you prefer.
> 
> If it actually breaks them, then you need to change the patch so you
> don't break them.
>