Re: [RFC][PATCH 1/2] fanotify: configurable limits via sysfs
From: Amir Goldstein <amir73il@gmail.com>
Date: 2021-02-16 18:03:44
Also in:
linux-api
On Tue, Feb 16, 2021 at 6:27 PM Jan Kara [off-list ref] wrote:
Hi Amir! I'm sorry that I've got to this only now. On Sun 24-01-21 20:42:03, Amir Goldstein wrote:quoted
fanotify has some hardcoded limits. The only APIs to escape those limits are FAN_UNLIMITED_QUEUE and FAN_UNLIMITED_MARKS. Allow finer grained tuning of the system limits via sysfs tunables under /proc/sys/fs/fanotify/, similar to tunables under /proc/sys/fs/inotify, with some minor differences. - max_queued_events - global system tunable for group queue size limit. Like the inotify tunable with the same name, it defaults to 16384 and applies on initialization of a new group. - max_listener_marks - global system tunable of marks limit per group. Defaults to 8192. inotify has no per group marks limit. - max_user_marks - user ns tunable for marks limit per user. Like the inotify tunable named max_user_watches, it defaults to 1048576 and is accounted for every containing user ns. - max_user_listeners - user ns tunable for number of listeners per user. Like the inotify tunable named max_user_instances, it defaults to 128 and is accounted for every containing user ns.I think term 'group' is used in the manpages even more and in the code as well. 'listener' more generally tends to refer to the application listening to the events. So I'd rather call the limits 'max_group_marks' and 'max_user_groups'.
ok.
quoted
The slightly different tunable names are derived from the "listener" and "mark" terminology used in the fanotify man pages. max_listener_marks was kept for compatibility with legacy fanotify behavior. Given that inotify max_user_instances was increased from 8192 to 1048576 in kernel v5.10, we may want to consider changing also the default for max_listener_marks or remove it completely, leaving only the per user marks limit.Yes, probably I'd just drop 'max_group_marks' completely and leave just per-user marks limit. You can always tune it in init_user_ns if you wish. Can't you?
Yes, I suppose.
Also as a small style nit, please try to stay within 80 columns. Otherwise the patch looks OK to me.
Ever since discussions that led to: bdc48fa11e46 checkpatch/coding-style: deprecate 80-column warning I've tuned my editor to warn on 100 columns. I still try to refrain from long lines, but breaking a ~82 columns line in an ugly way is something that I try to avoid. I'll try harder to stay below 80 when it does not create ugly code, unless you absolutely want the patches to fit in 80 columns. Thanks, Amir.