* [PATCH 1/3] fs: Use get_max_files() instead of files_stat.max_files in alloc_empty_file()
@ 2020-06-06 6:32 Tiezhu Yang
2020-06-06 6:32 ` [PATCH 2/3] fs: Introduce cmdline argument exceed_file_max_panic Tiezhu Yang
2020-06-06 6:32 ` [PATCH 3/3] docs: admin-guide: Explain cmdline argument exceed_file_max_panic in fs.rst Tiezhu Yang
0 siblings, 2 replies; 5+ messages in thread
From: Tiezhu Yang @ 2020-06-06 6:32 UTC (permalink / raw)
To: Alexander Viro, Jonathan Corbet
Cc: linux-fsdevel, linux-doc, linux-kernel, Xuefeng Li
It is better to use get_max_files() instead of files_stat.max_files
to improve readability.
Signed-off-by: Tiezhu Yang <yangtiezhu@loongson.cn>
---
fs/file_table.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/fs/file_table.c b/fs/file_table.c
index 656647f..26516d0 100644
--- a/fs/file_table.c
+++ b/fs/file_table.c
@@ -139,12 +139,12 @@ struct file *alloc_empty_file(int flags, const struct cred *cred)
/*
* Privileged users can go above max_files
*/
- if (get_nr_files() >= files_stat.max_files && !capable(CAP_SYS_ADMIN)) {
+ if (get_nr_files() >= get_max_files() && !capable(CAP_SYS_ADMIN)) {
/*
* percpu_counters are inaccurate. Do an expensive check before
* we go and fail.
*/
- if (percpu_counter_sum_positive(&nr_files) >= files_stat.max_files)
+ if (percpu_counter_sum_positive(&nr_files) >= get_max_files())
goto over;
}
--
2.1.0
^ permalink raw reply related [flat|nested] 5+ messages in thread
* [PATCH 2/3] fs: Introduce cmdline argument exceed_file_max_panic
2020-06-06 6:32 [PATCH 1/3] fs: Use get_max_files() instead of files_stat.max_files in alloc_empty_file() Tiezhu Yang
@ 2020-06-06 6:32 ` Tiezhu Yang
2020-06-06 14:13 ` Matthew Wilcox
2020-06-06 14:28 ` Al Viro
2020-06-06 6:32 ` [PATCH 3/3] docs: admin-guide: Explain cmdline argument exceed_file_max_panic in fs.rst Tiezhu Yang
1 sibling, 2 replies; 5+ messages in thread
From: Tiezhu Yang @ 2020-06-06 6:32 UTC (permalink / raw)
To: Alexander Viro, Jonathan Corbet
Cc: linux-fsdevel, linux-doc, linux-kernel, Xuefeng Li
It is important to ensure that files that are opened always get closed.
Failing to close files can result in file descriptor leaks. One common
answer to this problem is to just raise the limit of open file handles
and then restart the server every day or every few hours, this is not
a good idea for long-lived servers if there is no leaks.
If there exists file descriptor leaks, when file-max limit reached, we
can see that the system can not work well and at worst the user can do
nothing, it is even impossible to execute reboot command due to too many
open files in system. In order to reboot automatically to recover to the
normal status, introduce a new cmdline argument exceed_file_max_panic for
user to control whether to call panic in this case.
We can reproduce this problem used with the following simple test:
[yangtiezhu@linux ~]$ cat exceed_file_max_test.c
#include <stdio.h>
#include <sys/types.h>
#include <sys/stat.h>
#include <fcntl.h>
int main()
{
int fd;
while (1) {
fd = open("/usr/include/stdio.h", 0444);
if (fd == -1)
fprintf(stderr, "%s\n", "open failed");
}
return 0;
}
[yangtiezhu@linux ~]$ cat exceed_file_max_test.sh
#!/bin/bash
gcc exceed_file_max_test.c -o exceed_file_max_test.bin -Wall
while true
do
./exceed_file_max_test.bin >/dev/null 2>&1 &
done
[yangtiezhu@linux ~]$ sh exceed_file_max_test.sh &
[yangtiezhu@linux ~]$ reboot
bash: start pipeline: pgrp pipe: Too many open files in system
bash: /usr/sbin/reboot: Too many open files in system
Signed-off-by: Tiezhu Yang <yangtiezhu@loongson.cn>
---
fs/file_table.c | 14 ++++++++++++++
1 file changed, 14 insertions(+)
diff --git a/fs/file_table.c b/fs/file_table.c
index 26516d0..6943945 100644
--- a/fs/file_table.c
+++ b/fs/file_table.c
@@ -121,6 +121,17 @@ static struct file *__alloc_file(int flags, const struct cred *cred)
return f;
}
+static bool exceed_file_max_panic;
+static int __init exceed_file_max_panic_setup(char *str)
+{
+ pr_info("Call panic when exceed file-max limit\n");
+ exceed_file_max_panic = true;
+
+ return 1;
+}
+
+__setup("exceed_file_max_panic", exceed_file_max_panic_setup);
+
/* Find an unused file structure and return a pointer to it.
* Returns an error pointer if some error happend e.g. we over file
* structures limit, run out of memory or operation is not permitted.
@@ -159,6 +170,9 @@ struct file *alloc_empty_file(int flags, const struct cred *cred)
if (get_nr_files() > old_max) {
pr_info("VFS: file-max limit %lu reached\n", get_max_files());
old_max = get_nr_files();
+
+ if (exceed_file_max_panic)
+ panic("VFS: Too many open files in system\n");
}
return ERR_PTR(-ENFILE);
}
--
2.1.0
^ permalink raw reply related [flat|nested] 5+ messages in thread
* [PATCH 3/3] docs: admin-guide: Explain cmdline argument exceed_file_max_panic in fs.rst
2020-06-06 6:32 [PATCH 1/3] fs: Use get_max_files() instead of files_stat.max_files in alloc_empty_file() Tiezhu Yang
2020-06-06 6:32 ` [PATCH 2/3] fs: Introduce cmdline argument exceed_file_max_panic Tiezhu Yang
@ 2020-06-06 6:32 ` Tiezhu Yang
1 sibling, 0 replies; 5+ messages in thread
From: Tiezhu Yang @ 2020-06-06 6:32 UTC (permalink / raw)
To: Alexander Viro, Jonathan Corbet
Cc: linux-fsdevel, linux-doc, linux-kernel, Xuefeng Li
Explain the cmdline argument exceed_file_max_panic in the file
Documentation/admin-guide/sysctl/fs.rst
Signed-off-by: Tiezhu Yang <yangtiezhu@loongson.cn>
---
Documentation/admin-guide/sysctl/fs.rst | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/Documentation/admin-guide/sysctl/fs.rst b/Documentation/admin-guide/sysctl/fs.rst
index 2a45119..0cfc5c4 100644
--- a/Documentation/admin-guide/sysctl/fs.rst
+++ b/Documentation/admin-guide/sysctl/fs.rst
@@ -126,6 +126,13 @@ Attempts to allocate more file descriptors than file-max are
reported with printk, look for "VFS: file-max limit <number>
reached".
+If there exists file descriptor leaks, when file-max limit reached,
+we can see that the system can not work well and at worst the user
+can do nothing, it is even impossible to execute reboot command due
+to too many open files in system. In order to reboot automatically
+to recover to the normal status, we can use the cmdline argument
+exceed_file_max_panic to control whether to call panic in this case.
+
nr_open
-------
--
2.1.0
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH 2/3] fs: Introduce cmdline argument exceed_file_max_panic
2020-06-06 6:32 ` [PATCH 2/3] fs: Introduce cmdline argument exceed_file_max_panic Tiezhu Yang
@ 2020-06-06 14:13 ` Matthew Wilcox
2020-06-06 14:28 ` Al Viro
1 sibling, 0 replies; 5+ messages in thread
From: Matthew Wilcox @ 2020-06-06 14:13 UTC (permalink / raw)
To: Tiezhu Yang
Cc: Alexander Viro, Jonathan Corbet, linux-fsdevel, linux-doc,
linux-kernel, Xuefeng Li
On Sat, Jun 06, 2020 at 02:32:19PM +0800, Tiezhu Yang wrote:
> It is important to ensure that files that are opened always get closed.
> Failing to close files can result in file descriptor leaks. One common
> answer to this problem is to just raise the limit of open file handles
> and then restart the server every day or every few hours, this is not
> a good idea for long-lived servers if there is no leaks.
>
> If there exists file descriptor leaks, when file-max limit reached, we
> can see that the system can not work well and at worst the user can do
> nothing, it is even impossible to execute reboot command due to too many
> open files in system. In order to reboot automatically to recover to the
> normal status, introduce a new cmdline argument exceed_file_max_panic for
> user to control whether to call panic in this case.
ulimit -n is your friend.
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH 2/3] fs: Introduce cmdline argument exceed_file_max_panic
2020-06-06 6:32 ` [PATCH 2/3] fs: Introduce cmdline argument exceed_file_max_panic Tiezhu Yang
2020-06-06 14:13 ` Matthew Wilcox
@ 2020-06-06 14:28 ` Al Viro
1 sibling, 0 replies; 5+ messages in thread
From: Al Viro @ 2020-06-06 14:28 UTC (permalink / raw)
To: Tiezhu Yang
Cc: Jonathan Corbet, linux-fsdevel, linux-doc, linux-kernel, Xuefeng Li
On Sat, Jun 06, 2020 at 02:32:19PM +0800, Tiezhu Yang wrote:
> It is important to ensure that files that are opened always get closed.
> Failing to close files can result in file descriptor leaks. One common
> answer to this problem is to just raise the limit of open file handles
> and then restart the server every day or every few hours, this is not
> a good idea for long-lived servers if there is no leaks.
>
> If there exists file descriptor leaks, when file-max limit reached, we
> can see that the system can not work well and at worst the user can do
> nothing, it is even impossible to execute reboot command due to too many
> open files in system. In order to reboot automatically to recover to the
> normal status, introduce a new cmdline argument exceed_file_max_panic for
> user to control whether to call panic in this case.
What the hell? You are modifying the path for !CAP_SYS_ADMIN. IOW,
you've just handed an ability to panic the box to any non-priveleged
process.
NAK. That makes no sense whatsoever. Note that root is *NOT* affected
by any of that, so you can bloody well have a userland process running
as root and checking the number of files once in a while. And doing
whatever it wants to do, up to and including reboot/writing to
/proc/sys/sysrq-trigger, etc. Or just looking at the leaky processes
and killing them, with a nastygram along the lines of "$program appears
to be leaking descriptors; LART the authors of that FPOS if they can
be located" sent into log/over mail/etc.
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2020-06-06 14:29 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-06-06 6:32 [PATCH 1/3] fs: Use get_max_files() instead of files_stat.max_files in alloc_empty_file() Tiezhu Yang
2020-06-06 6:32 ` [PATCH 2/3] fs: Introduce cmdline argument exceed_file_max_panic Tiezhu Yang
2020-06-06 14:13 ` Matthew Wilcox
2020-06-06 14:28 ` Al Viro
2020-06-06 6:32 ` [PATCH 3/3] docs: admin-guide: Explain cmdline argument exceed_file_max_panic in fs.rst Tiezhu Yang
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).