Amir Goldstein [off-list ref] writes:
On Fri, Oct 15, 2021 at 12:39 AM Gabriel Krisman Bertazi
[off-list ref] wrote:
quoted
Since FAN_FS_ERROR memory must be pre-allocated, limit a single group
from watching too many file systems at once. The current scheme
guarantees 1 slot per filesystem, so limit the number of marks with
FAN_FS_ERROR per group.
Signed-off-by: Gabriel Krisman Bertazi <redacted>
---
fs/notify/fanotify/fanotify_user.c | 10 ++++++++++
include/linux/fsnotify_backend.h | 1 +
2 files changed, 11 insertions(+)
diff --git a/fs/notify/fanotify/fanotify_user.c b/fs/notify/fanotify/fanotify_user.c
index f1cf863d6f9f..5324890500fc 100644
--- a/fs/notify/fanotify/fanotify_user.c
+++ b/fs/notify/fanotify/fanotify_user.c
@@ -959,6 +959,10 @@ static int fanotify_remove_mark(struct fsnotify_group *group,
removed = fanotify_mark_remove_from_mask(fsn_mark, mask, flags,
umask, &destroy_mark);
+
+ if (removed & FAN_FS_ERROR)
+ group->fanotify_data.error_event_marks--;
+
if (removed & fsnotify_conn_mask(fsn_mark->connector))
fsnotify_recalc_mask(fsn_mark->connector);
if (destroy_mark)@@ -1057,6 +1061,9 @@ static struct fsnotify_mark *fanotify_add_new_mark(struct fsnotify_group *group,
static int fanotify_group_init_error_pool(struct fsnotify_group *group)
{
+ if (group->fanotify_data.error_event_marks >= FANOTIFY_DEFAULT_FEE_POOL)
+ return -ENOMEM;
Why not try to mempool_resize()?
Jan suggested we might not need to bother with it, but I can do that for
the next version.
Also, I did not read the rest of the patches yet, but don't we need two
slots per mark? one for alloc-pre-enqueue and one for free-post-dequeue?
I don't understand what you mean by two slots for alloc-pre-enqueue and
free-post-dequeue. I suspect it is no longer necessary now that
FAN_FS_ERROR is handled like any other event on enqueue/dequeue, but can
you confirm or clarify?
Thanks,
--
Gabriel Krisman Bertazi
On Fri, Oct 15, 2021 at 7:53 PM Gabriel Krisman Bertazi
[off-list ref] wrote:
Amir Goldstein [off-list ref] writes:
quoted
On Fri, Oct 15, 2021 at 12:39 AM Gabriel Krisman Bertazi
[off-list ref] wrote:
quoted
Since FAN_FS_ERROR memory must be pre-allocated, limit a single group
from watching too many file systems at once. The current scheme
guarantees 1 slot per filesystem, so limit the number of marks with
FAN_FS_ERROR per group.
Signed-off-by: Gabriel Krisman Bertazi <redacted>
---
fs/notify/fanotify/fanotify_user.c | 10 ++++++++++
include/linux/fsnotify_backend.h | 1 +
2 files changed, 11 insertions(+)
diff --git a/fs/notify/fanotify/fanotify_user.c b/fs/notify/fanotify/fanotify_user.c
index f1cf863d6f9f..5324890500fc 100644
--- a/fs/notify/fanotify/fanotify_user.c
+++ b/fs/notify/fanotify/fanotify_user.c
@@ -959,6 +959,10 @@ static int fanotify_remove_mark(struct fsnotify_group *group,
removed = fanotify_mark_remove_from_mask(fsn_mark, mask, flags,
umask, &destroy_mark);
+
+ if (removed & FAN_FS_ERROR)
+ group->fanotify_data.error_event_marks--;
+
if (removed & fsnotify_conn_mask(fsn_mark->connector))
fsnotify_recalc_mask(fsn_mark->connector);
if (destroy_mark)@@ -1057,6 +1061,9 @@ static struct fsnotify_mark *fanotify_add_new_mark(struct fsnotify_group *group,
static int fanotify_group_init_error_pool(struct fsnotify_group *group)
{
+ if (group->fanotify_data.error_event_marks >= FANOTIFY_DEFAULT_FEE_POOL)
+ return -ENOMEM;
Why not try to mempool_resize()?
Jan suggested we might not need to bother with it, but I can do that for
the next version.
quoted
Also, I did not read the rest of the patches yet, but don't we need two
slots per mark? one for alloc-pre-enqueue and one for free-post-dequeue?
I don't understand what you mean by two slots for alloc-pre-enqueue and
free-post-dequeue. I suspect it is no longer necessary now that
FAN_FS_ERROR is handled like any other event on enqueue/dequeue, but can
you confirm or clarify?
What I meant was, your code is counting error_event_marks.
Every mark accounts for either 1 or 0 queued events, because more
errors from the same fs would merge with the single queued event.
I thought we could have one more event per fs being copied to user
but really we could potentially have many events allocated, before
being "merged", so my calculation was wrong.
Anyway, as Jan explained, the limited size of mempool does not doom
allocations to fail, so we probably have nothing to worry about and
there is no need to mempool_resize().
Thanks,
Amir.