From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1424114AbcFMVNU (ORCPT ); Mon, 13 Jun 2016 17:13:20 -0400 Received: from mail-wm0-f67.google.com ([74.125.82.67]:34908 "EHLO mail-wm0-f67.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1423644AbcFMVNQ (ORCPT ); Mon, 13 Jun 2016 17:13:16 -0400 Subject: Re: [RFC 05/18] limits: track and present RLIMIT_NOFILE actual max To: Andy Lutomirski , linux-kernel@vger.kernel.org References: <1465847065-3577-1-git-send-email-toiwoton@gmail.com> <1465847065-3577-6-git-send-email-toiwoton@gmail.com> <887b928a-87f3-46aa-cfd3-d962fe40b85f@kernel.org> Cc: Alexander Viro , Ingo Molnar , Peter Zijlstra , Andrew Morton , Kees Cook , Cyrill Gorcunov , Alexey Dobriyan , John Stultz , Janis Danisevskis , Calvin Owens , Jann Horn , "open list:FILESYSTEMS (VFS and infrastructure)" From: Topi Miettinen Openpgp: id=A0F2EB0D8452DA908BEC8E911CF9ADDBD610E936 Message-ID: Date: Mon, 13 Jun 2016 21:13:12 +0000 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Icedove/45.1.0 MIME-Version: 1.0 In-Reply-To: <887b928a-87f3-46aa-cfd3-d962fe40b85f@kernel.org> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 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? > 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. > >> 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. > >> >> for (i = 0; i < RLIM_NLIMITS; i++) { >> if (rlim[i].rlim_cur == RLIM_INFINITY) >> @@ -647,9 +647,11 @@ static int proc_pid_limits(struct seq_file *m, >> struct pid_namespace *ns, >> seq_printf(m, "%-20lu ", rlim[i].rlim_max); >> >> if (lnames[i].unit) >> - seq_printf(m, "%-10s\n", lnames[i].unit); >> + seq_printf(m, "%-10s", lnames[i].unit); >> else >> - seq_putc(m, '\n'); >> + seq_printf(m, "%-10s", ""); >> + seq_printf(m, "%-20lu\n", >> + task->signal->rlim_curmax[i]); >> } >> >> return 0; >> diff --git a/include/linux/sched.h b/include/linux/sched.h >> index 9c48a08..0150380 100644 >> --- a/include/linux/sched.h >> +++ b/include/linux/sched.h >> @@ -782,6 +782,7 @@ struct signal_struct { >> * have no need to disable irqs. >> */ >> struct rlimit rlim[RLIM_NLIMITS]; >> + unsigned long rlim_curmax[RLIM_NLIMITS]; >> >> #ifdef CONFIG_BSD_PROCESS_ACCT >> struct pacct_struct pacct; /* per-process accounting >> information */ >> @@ -3376,6 +3377,12 @@ static inline unsigned long rlimit_max(unsigned >> int limit) >> return task_rlimit_max(current, limit); >> } >> >> +static inline void bump_rlimit(unsigned int limit, unsigned long r) >> +{ >> + if (READ_ONCE(current->signal->rlim_curmax[limit]) < r) >> + current->signal->rlim_curmax[limit] = r; >> +} >> + >> #ifdef CONFIG_CPU_FREQ >> struct update_util_data { >> void (*func)(struct update_util_data *data, >> >