Documentation/filesystems/proc.rst | 17 ++++++++++++ fs/proc/fd.c | 44 ++++++++++++++++++++++++++++++ 2 files changed, 61 insertions(+)
Many monitoring tools include open file count as a metric. Currently
the only way to get this number is to enumerate the files in /proc/pid/fd.
The problem with the current approach is that it does many things people
generally don't care about when they need one number for a metric.
In our tests for cadvisor, which reports open file counts per cgroup,
we observed that reading the number of open files is slow. Out of 35.23%
of CPU time spent in `proc_readfd_common`, we see 29.43% spent in
`proc_fill_cache`, which is responsible for filling dentry info.
Some of this extra time is spinlock contention, but it's a contention
for the lock we don't want to take to begin with.
We considered putting the number of open files in /proc/pid/status.
Unfortunately, counting the number of fds involves iterating the open_files
bitmap, which has a linear complexity in proportion with the number
of open files (bitmap slots really, but it's close). We don't want
to make /proc/pid/status any slower, so instead we put this info
in /proc/pid/fd as a size member of the stat syscall result.
Previously the reported number was zero, so there's very little
risk of breaking anything, while still providing a somewhat logical
way to count the open files with a fallback if it's zero.
RFC for this patch included iterating open fds under RCU. Thanks
to Frank Hofmann for the suggestion to use the bitmap instead.
Previously:
```
$ sudo stat /proc/1/fd | head -n2
File: /proc/1/fd
Size: 0 Blocks: 0 IO Block: 1024 directory
```
With this patch:
```
$ sudo stat /proc/1/fd | head -n2
File: /proc/1/fd
Size: 65 Blocks: 0 IO Block: 1024 directory
```
Correctness check:
```
$ sudo ls /proc/1/fd | wc -l
65
```
I added the docs for /proc/<pid>/fd while I'm at it.
Signed-off-by: Ivan Babrou <ivan@cloudflare.com>
---
v2: Added missing rcu_read_lock() / rcu_read_unlock(),
task_lock() / task_unlock() and put_task_struct().
---
Documentation/filesystems/proc.rst | 17 ++++++++++++
fs/proc/fd.c | 44 ++++++++++++++++++++++++++++++
2 files changed, 61 insertions(+)
diff --git a/Documentation/filesystems/proc.rst b/Documentation/filesystems/proc.rst
index e7aafc82be99..394548d26187 100644
--- a/Documentation/filesystems/proc.rst
+++ b/Documentation/filesystems/proc.rst
@@ -47,6 +47,7 @@ fixes/update part 1.1 Stefani Seibold <stefani@seibold.net> June 9 2009
3.10 /proc/<pid>/timerslack_ns - Task timerslack value
3.11 /proc/<pid>/patch_state - Livepatch patch operation state
3.12 /proc/<pid>/arch_status - Task architecture specific information
+ 3.13 /proc/<pid>/fd - List of symlinks to open files
4 Configuring procfs
4.1 Mount options
@@ -2145,6 +2146,22 @@ AVX512_elapsed_ms
the task is unlikely an AVX512 user, but depends on the workload and the
scheduling scenario, it also could be a false negative mentioned above.
+3.13 /proc/<pid>/fd - List of symlinks to open files
+-------------------------------------------------------
+This directory contains symbolic links which represent open files
+the process is maintaining. Example output::
+
+ lr-x------ 1 root root 64 Sep 20 17:53 0 -> /dev/null
+ l-wx------ 1 root root 64 Sep 20 17:53 1 -> /dev/null
+ lrwx------ 1 root root 64 Sep 20 17:53 10 -> 'socket:[12539]'
+ lrwx------ 1 root root 64 Sep 20 17:53 11 -> 'socket:[12540]'
+ lrwx------ 1 root root 64 Sep 20 17:53 12 -> 'socket:[12542]'
+
+The number of open files for the process is stored in 'size' member
+of stat() output for /proc/<pid>/fd for fast access.
+-------------------------------------------------------
+
+
Chapter 4: Configuring procfs
=============================
diff --git a/fs/proc/fd.c b/fs/proc/fd.c
index 913bef0d2a36..ff526dfc5faa 100644
--- a/fs/proc/fd.c
+++ b/fs/proc/fd.c
@@ -279,6 +279,34 @@ static int proc_readfd_common(struct file *file, struct dir_context *ctx,
return 0;
}
+static int proc_readfd_count(struct inode *inode)
+{
+ struct task_struct *p = get_proc_task(inode);
+ struct fdtable *fdt;
+ unsigned int i, size, open_fds = 0;
+
+ if (!p)
+ return -ENOENT;
+
+ task_lock(p);
+ if (p->files) {
+ rcu_read_lock();
+
+ fdt = files_fdtable(p->files);
+ size = fdt->max_fds;
+
+ for (i = size / BITS_PER_LONG; i > 0;)
+ open_fds += hweight64(fdt->open_fds[--i]);
+
+ rcu_read_unlock();
+ }
+ task_unlock(p);
+
+ put_task_struct(p);
+
+ return open_fds;
+}
+
static int proc_readfd(struct file *file, struct dir_context *ctx)
{
return proc_readfd_common(file, ctx, proc_fd_instantiate);
@@ -319,9 +347,25 @@ int proc_fd_permission(struct user_namespace *mnt_userns,
return rv;
}
+static int proc_fd_getattr(struct user_namespace *mnt_userns,
+ const struct path *path, struct kstat *stat,
+ u32 request_mask, unsigned int query_flags)
+{
+ struct inode *inode = d_inode(path->dentry);
+
+ generic_fillattr(&init_user_ns, inode, stat);
+
+ /* If it's a directory, put the number of open fds there */
+ if (S_ISDIR(inode->i_mode))
+ stat->size = proc_readfd_count(inode);
+
+ return 0;
+}
+
const struct inode_operations proc_fd_inode_operations = {
.lookup = proc_lookupfd,
.permission = proc_fd_permission,
+ .getattr = proc_fd_getattr,
.setattr = proc_setattr,
};
--
2.37.2On Thu, 22 Sep 2022 15:40:26 -0700 Ivan Babrou <ivan@cloudflare.com> wrote:
> Many monitoring tools include open file count as a metric. Currently
> the only way to get this number is to enumerate the files in /proc/pid/fd.
>
> The problem with the current approach is that it does many things people
> generally don't care about when they need one number for a metric.
> In our tests for cadvisor, which reports open file counts per cgroup,
> we observed that reading the number of open files is slow. Out of 35.23%
> of CPU time spent in `proc_readfd_common`, we see 29.43% spent in
> `proc_fill_cache`, which is responsible for filling dentry info.
> Some of this extra time is spinlock contention, but it's a contention
> for the lock we don't want to take to begin with.
>
> We considered putting the number of open files in /proc/pid/status.
> Unfortunately, counting the number of fds involves iterating the open_files
> bitmap, which has a linear complexity in proportion with the number
> of open files (bitmap slots really, but it's close). We don't want
> to make /proc/pid/status any slower, so instead we put this info
> in /proc/pid/fd as a size member of the stat syscall result.
That sounds awfully logical.
> Previously the reported number was zero, so there's very little
> risk of breaking anything, while still providing a somewhat logical
> way to count the open files with a fallback if it's zero.
>
> RFC for this patch included iterating open fds under RCU. Thanks
> to Frank Hofmann for the suggestion to use the bitmap instead.
>
> Previously:
>
> ```
> $ sudo stat /proc/1/fd | head -n2
> File: /proc/1/fd
> Size: 0 Blocks: 0 IO Block: 1024 directory
> ```
>
> With this patch:
>
> ```
> $ sudo stat /proc/1/fd | head -n2
> File: /proc/1/fd
> Size: 65 Blocks: 0 IO Block: 1024 directory
> ```
>
> Correctness check:
>
> ```
> $ sudo ls /proc/1/fd | wc -l
> 65
> ```
>
> I added the docs for /proc/<pid>/fd while I'm at it.
>
Dang that's a good changelog.
> index e7aafc82be99..394548d26187 100644
> --- a/Documentation/filesystems/proc.rst
> +++ b/Documentation/filesystems/proc.rst
> @@ -47,6 +47,7 @@ fixes/update part 1.1 Stefani Seibold <stefani@seibold.net> June 9 2009
> 3.10 /proc/<pid>/timerslack_ns - Task timerslack value
> 3.11 /proc/<pid>/patch_state - Livepatch patch operation state
> 3.12 /proc/<pid>/arch_status - Task architecture specific information
> + 3.13 /proc/<pid>/fd - List of symlinks to open files
>
> 4 Configuring procfs
> 4.1 Mount options
> @@ -2145,6 +2146,22 @@ AVX512_elapsed_ms
> the task is unlikely an AVX512 user, but depends on the workload and the
> scheduling scenario, it also could be a false negative mentioned above.
>
> +3.13 /proc/<pid>/fd - List of symlinks to open files
> +-------------------------------------------------------
> +This directory contains symbolic links which represent open files
> +the process is maintaining. Example output::
> +
> + lr-x------ 1 root root 64 Sep 20 17:53 0 -> /dev/null
> + l-wx------ 1 root root 64 Sep 20 17:53 1 -> /dev/null
> + lrwx------ 1 root root 64 Sep 20 17:53 10 -> 'socket:[12539]'
> + lrwx------ 1 root root 64 Sep 20 17:53 11 -> 'socket:[12540]'
> + lrwx------ 1 root root 64 Sep 20 17:53 12 -> 'socket:[12542]'
> +
> +The number of open files for the process is stored in 'size' member
> +of stat() output for /proc/<pid>/fd for fast access.
> +-------------------------------------------------------
> +
> +
> Chapter 4: Configuring procfs
> =============================
>
> diff --git a/fs/proc/fd.c b/fs/proc/fd.c
> index 913bef0d2a36..ff526dfc5faa 100644
> --- a/fs/proc/fd.c
> +++ b/fs/proc/fd.c
> @@ -279,6 +279,34 @@ static int proc_readfd_common(struct file *file, struct dir_context *ctx,
> return 0;
> }
>
> +static int proc_readfd_count(struct inode *inode)
> +{
> + struct task_struct *p = get_proc_task(inode);
> + struct fdtable *fdt;
> + unsigned int i, size, open_fds = 0;
> +
> + if (!p)
> + return -ENOENT;
> +
> + task_lock(p);
> + if (p->files) {
> + rcu_read_lock();
> +
> + fdt = files_fdtable(p->files);
> + size = fdt->max_fds;
> +
> + for (i = size / BITS_PER_LONG; i > 0;)
> + open_fds += hweight64(fdt->open_fds[--i]);
Could BITMAP_WEIGHT() or __bitmap_weight() or bitmap_weight() be used here?
> + rcu_read_unlock();
> + }
> + task_unlock(p);
> +
> + put_task_struct(p);
> +
> + return open_fds;
> +}
> +
> static int proc_readfd(struct file *file, struct dir_context *ctx)
> {
> return proc_readfd_common(file, ctx, proc_fd_instantiate);
> @@ -319,9 +347,25 @@ int proc_fd_permission(struct user_namespace *mnt_userns,
> return rv;
> }
>
> +static int proc_fd_getattr(struct user_namespace *mnt_userns,
> + const struct path *path, struct kstat *stat,
> + u32 request_mask, unsigned int query_flags)
> +{
> + struct inode *inode = d_inode(path->dentry);
> +
> + generic_fillattr(&init_user_ns, inode, stat);
> +
> + /* If it's a directory, put the number of open fds there */
> + if (S_ISDIR(inode->i_mode))
> + stat->size = proc_readfd_count(inode);
> +
> + return 0;
> +}
> +
> const struct inode_operations proc_fd_inode_operations = {
> .lookup = proc_lookupfd,
> .permission = proc_fd_permission,
> + .getattr = proc_fd_getattr,
> .setattr = proc_setattr,
> };
>
> --
> 2.37.2
On Mon, Oct 17, 2022 at 6:47 PM Andrew Morton <akpm@linux-foundation.org> wrote: > > + > > + fdt = files_fdtable(p->files); > > + size = fdt->max_fds; > > + > > + for (i = size / BITS_PER_LONG; i > 0;) > > + open_fds += hweight64(fdt->open_fds[--i]); > > Could BITMAP_WEIGHT() or __bitmap_weight() or bitmap_weight() be used here? That's a great suggestion. I tested it with bitmap_weight() and it looks much cleaner while providing the same result. I just sent the v3 with this suggestion applied.
On Tue, Oct 18, 2022 at 6:02 AM Ivan Babrou <ivan@cloudflare.com> wrote: > > On Mon, Oct 17, 2022 at 6:47 PM Andrew Morton <akpm@linux-foundation.org> wrote: > > > + > > > + fdt = files_fdtable(p->files); > > > + size = fdt->max_fds; > > > + > > > + for (i = size / BITS_PER_LONG; i > 0;) > > > + open_fds += hweight64(fdt->open_fds[--i]); > > > > Could BITMAP_WEIGHT() or __bitmap_weight() or bitmap_weight() be used here? > > That's a great suggestion. I tested it with bitmap_weight() and it > looks much cleaner while providing the same result. > > I just sent the v3 with this suggestion applied. +1 from me on using bitmap_weight() - good spotting that. FrankH.
From: Frank Hofmann > Sent: 18 October 2022 09:13 > > On Tue, Oct 18, 2022 at 6:02 AM Ivan Babrou <ivan@cloudflare.com> wrote: > > > > On Mon, Oct 17, 2022 at 6:47 PM Andrew Morton <akpm@linux-foundation.org> wrote: > > > > + > > > > + fdt = files_fdtable(p->files); > > > > + size = fdt->max_fds; > > > > + > > > > + for (i = size / BITS_PER_LONG; i > 0;) > > > > + open_fds += hweight64(fdt->open_fds[--i]); > > > > > > Could BITMAP_WEIGHT() or __bitmap_weight() or bitmap_weight() be used here? > > > > That's a great suggestion. I tested it with bitmap_weight() and it > > looks much cleaner while providing the same result. > > > > I just sent the v3 with this suggestion applied. > > +1 from me on using bitmap_weight() - good spotting that. Does that have the optimisations for the value being 0, ~0u or 2**n-1 all of which are likely for the fd table. (Especially if there is no 'popcnt' instruction.) David - Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK Registration No: 1397386 (Wales)
On Tue, Oct 18, 2022 at 9:25 AM David Laight <David.Laight@aculab.com> wrote: > > From: Frank Hofmann > > Sent: 18 October 2022 09:13 > > > > On Tue, Oct 18, 2022 at 6:02 AM Ivan Babrou <ivan@cloudflare.com> wrote: > > > > > > On Mon, Oct 17, 2022 at 6:47 PM Andrew Morton <akpm@linux-foundation.org> wrote: > > > > > + > > > > > + fdt = files_fdtable(p->files); > > > > > + size = fdt->max_fds; > > > > > + > > > > > + for (i = size / BITS_PER_LONG; i > 0;) > > > > > + open_fds += hweight64(fdt->open_fds[--i]); > > > > > > > > Could BITMAP_WEIGHT() or __bitmap_weight() or bitmap_weight() be used here? > > > > > > That's a great suggestion. I tested it with bitmap_weight() and it > > > looks much cleaner while providing the same result. > > > > > > I just sent the v3 with this suggestion applied. > > > > +1 from me on using bitmap_weight() - good spotting that. > > Does that have the optimisations for the value being 0, ~0u > or 2**n-1 all of which are likely for the fd table. > (Especially if there is no 'popcnt' instruction.) > > David bitmap_weight() uses hweight_*() under the hood, which then falls through to platform-specific popcnt where available. Re, lib/bitmap.c and arch/.../asm/bitops.h or arch/.../hweight.S, for the impl details. FrankH. > > - > Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK > Registration No: 1397386 (Wales)
On Thu, Sep 22, 2022 at 3:40 PM Ivan Babrou <ivan@cloudflare.com> wrote: > > Many monitoring tools include open file count as a metric. Currently > the only way to get this number is to enumerate the files in /proc/pid/fd. > > The problem with the current approach is that it does many things people > generally don't care about when they need one number for a metric. > In our tests for cadvisor, which reports open file counts per cgroup, > we observed that reading the number of open files is slow. Out of 35.23% > of CPU time spent in `proc_readfd_common`, we see 29.43% spent in > `proc_fill_cache`, which is responsible for filling dentry info. > Some of this extra time is spinlock contention, but it's a contention > for the lock we don't want to take to begin with. > > We considered putting the number of open files in /proc/pid/status. > Unfortunately, counting the number of fds involves iterating the open_files > bitmap, which has a linear complexity in proportion with the number > of open files (bitmap slots really, but it's close). We don't want > to make /proc/pid/status any slower, so instead we put this info > in /proc/pid/fd as a size member of the stat syscall result. > Previously the reported number was zero, so there's very little > risk of breaking anything, while still providing a somewhat logical > way to count the open files with a fallback if it's zero. > > RFC for this patch included iterating open fds under RCU. Thanks > to Frank Hofmann for the suggestion to use the bitmap instead. > > Previously: > > ``` > $ sudo stat /proc/1/fd | head -n2 > File: /proc/1/fd > Size: 0 Blocks: 0 IO Block: 1024 directory > ``` > > With this patch: > > ``` > $ sudo stat /proc/1/fd | head -n2 > File: /proc/1/fd > Size: 65 Blocks: 0 IO Block: 1024 directory > ``` > > Correctness check: > > ``` > $ sudo ls /proc/1/fd | wc -l > 65 > ``` > > I added the docs for /proc/<pid>/fd while I'm at it. > > Signed-off-by: Ivan Babrou <ivan@cloudflare.com> > > --- > v2: Added missing rcu_read_lock() / rcu_read_unlock(), > task_lock() / task_unlock() and put_task_struct(). > --- > Documentation/filesystems/proc.rst | 17 ++++++++++++ > fs/proc/fd.c | 44 ++++++++++++++++++++++++++++++ > 2 files changed, 61 insertions(+) Now that Linux 6.1-rc1 is out, should this patch be looked at for inclusion? I see that the net-next tree has opened, not sure if the same rules apply here. We've been running the v2 version of this patch in production successfully for some time now.
© 2016 - 2026 Red Hat, Inc.