Re: [PATCH v2 01/14] fsnotify: Don't call insert hook for overflow events
From: Amir Goldstein <amir73il@gmail.com>
Date: 2021-06-16 07:49:24
Also in:
linux-fsdevel
On Wed, Jun 16, 2021 at 2:56 AM Gabriel Krisman Bertazi [off-list ref] wrote:
quoted hunk ↗ jump to hunk
Overflow events are not mergeable, so they are not hashed_events. But, when failing inside fsnotify_add_event, for lack of space, fsnotify_add_event() still calls the insert hook, which adds the overflow event to the merge list. Avoid calling the insert hook when adding an overflow event. Fixes: 94e00d28a680 ("fsnotify: use hash table for faster events merge") Signed-off-by: Gabriel Krisman Bertazi <redacted> --- fs/notify/notification.c | 5 +++++ 1 file changed, 5 insertions(+)diff --git a/fs/notify/notification.c b/fs/notify/notification.c index 32f45543b9c6..033294669e07 100644 --- a/fs/notify/notification.c +++ b/fs/notify/notification.c@@ -106,6 +106,11 @@ int fsnotify_add_event(struct fsnotify_group *group, return ret; } event = group->overflow_event; + /* + * Since overflow events are not mergeable, don't insert + * them in the merge hash. + */ + insert = NULL; goto queue; }
Hmm, the fix looks correct, but a bit fragile. While it makes sense that @insert is the counterpart of @merge there is nothing in the API that mandates it. Therefore, it would be more robust IMO to add a check fanotify_is_hashed_event(mask) in fanotify_insert_event() to match fanotify_is_hashed_event(mask) in get_one_event(). If we do that, we can also simplify:
--- a/fs/notify/fanotify/fanotify.c
+++ b/fs/notify/fanotify/fanotify.c@@ -779,8 +779,7 @@ static int fanotify_handle_event(structfsnotify_group *group, u32 mask,
fsn_event = &event->fse;
ret = fsnotify_add_event(group, fsn_event, fanotify_merge,
- fanotify_is_hashed_event(mask) ?
- fanotify_insert_event : NULL);
+ fanotify_insert_event);
Thanks,
Amir.