fsnotify: use hash table for faster events merge
authorAmir Goldstein <amir73il@gmail.com>
Thu, 4 Mar 2021 10:48:25 +0000 (12:48 +0200)
committerJan Kara <jack@suse.cz>
Tue, 16 Mar 2021 15:37:51 +0000 (16:37 +0100)
In order to improve event merge performance, hash events in a 128 size
hash table by the event merge key.

The fanotify_event size grows by two pointers, but we just reduced its
size by removing the objectid member, so overall its size is increased
by one pointer.

Permission events and overflow event are not merged so they are also
not hashed.

Link: https://lore.kernel.org/r/20210304104826.3993892-5-amir73il@gmail.com
Signed-off-by: Amir Goldstein <amir73il@gmail.com>
Signed-off-by: Jan Kara <jack@suse.cz>
fs/notify/fanotify/fanotify.c
fs/notify/fanotify/fanotify.h
fs/notify/fanotify/fanotify_user.c
fs/notify/inotify/inotify_fsnotify.c
fs/notify/notification.c
include/linux/fsnotify_backend.h

index 43a606f..50b3abc 100644 (file)
@@ -149,12 +149,15 @@ static bool fanotify_should_merge(struct fanotify_event *old,
 }
 
 /* and the list better be locked by something too! */
 }
 
 /* and the list better be locked by something too! */
-static int fanotify_merge(struct list_head *list, struct fsnotify_event *event)
+static int fanotify_merge(struct fsnotify_group *group,
+                         struct fsnotify_event *event)
 {
 {
-       struct fsnotify_event *test_event;
        struct fanotify_event *old, *new = FANOTIFY_E(event);
        struct fanotify_event *old, *new = FANOTIFY_E(event);
+       unsigned int bucket = fanotify_event_hash_bucket(group, new);
+       struct hlist_head *hlist = &group->fanotify_data.merge_hash[bucket];
 
 
-       pr_debug("%s: list=%p event=%p\n", __func__, list, event);
+       pr_debug("%s: group=%p event=%p bucket=%u\n", __func__,
+                group, event, bucket);
 
        /*
         * Don't merge a permission event with any other event so that we know
 
        /*
         * Don't merge a permission event with any other event so that we know
@@ -164,8 +167,7 @@ static int fanotify_merge(struct list_head *list, struct fsnotify_event *event)
        if (fanotify_is_perm_event(new->mask))
                return 0;
 
        if (fanotify_is_perm_event(new->mask))
                return 0;
 
-       list_for_each_entry_reverse(test_event, list, list) {
-               old = FANOTIFY_E(test_event);
+       hlist_for_each_entry(old, hlist, merge_list) {
                if (fanotify_should_merge(old, new)) {
                        old->mask |= new->mask;
                        return 1;
                if (fanotify_should_merge(old, new)) {
                        old->mask |= new->mask;
                        return 1;
@@ -203,8 +205,11 @@ static int fanotify_get_response(struct fsnotify_group *group,
                        return ret;
                }
                /* Event not yet reported? Just remove it. */
                        return ret;
                }
                /* Event not yet reported? Just remove it. */
-               if (event->state == FAN_EVENT_INIT)
+               if (event->state == FAN_EVENT_INIT) {
                        fsnotify_remove_queued_event(group, &event->fae.fse);
                        fsnotify_remove_queued_event(group, &event->fae.fse);
+                       /* Permission events are not supposed to be hashed */
+                       WARN_ON_ONCE(!hlist_unhashed(&event->fae.merge_list));
+               }
                /*
                 * Event may be also answered in case signal delivery raced
                 * with wakeup. In that case we have nothing to do besides
                /*
                 * Event may be also answered in case signal delivery raced
                 * with wakeup. In that case we have nothing to do besides
@@ -679,6 +684,24 @@ static __kernel_fsid_t fanotify_get_fsid(struct fsnotify_iter_info *iter_info)
        return fsid;
 }
 
        return fsid;
 }
 
+/*
+ * Add an event to hash table for faster merge.
+ */
+static void fanotify_insert_event(struct fsnotify_group *group,
+                                 struct fsnotify_event *fsn_event)
+{
+       struct fanotify_event *event = FANOTIFY_E(fsn_event);
+       unsigned int bucket = fanotify_event_hash_bucket(group, event);
+       struct hlist_head *hlist = &group->fanotify_data.merge_hash[bucket];
+
+       assert_spin_locked(&group->notification_lock);
+
+       pr_debug("%s: group=%p event=%p bucket=%u\n", __func__,
+                group, event, bucket);
+
+       hlist_add_head(&event->merge_list, hlist);
+}
+
 static int fanotify_handle_event(struct fsnotify_group *group, u32 mask,
                                 const void *data, int data_type,
                                 struct inode *dir,
 static int fanotify_handle_event(struct fsnotify_group *group, u32 mask,
                                 const void *data, int data_type,
                                 struct inode *dir,
@@ -749,7 +772,9 @@ static int fanotify_handle_event(struct fsnotify_group *group, u32 mask,
        }
 
        fsn_event = &event->fse;
        }
 
        fsn_event = &event->fse;
-       ret = fsnotify_add_event(group, fsn_event, fanotify_merge);
+       ret = fsnotify_add_event(group, fsn_event, fanotify_merge,
+                                fanotify_is_hashed_event(mask) ?
+                                fanotify_insert_event : NULL);
        if (ret) {
                /* Permission events shouldn't be merged */
                BUG_ON(ret == 1 && mask & FANOTIFY_PERM_EVENTS);
        if (ret) {
                /* Permission events shouldn't be merged */
                BUG_ON(ret == 1 && mask & FANOTIFY_PERM_EVENTS);
@@ -772,6 +797,7 @@ static void fanotify_free_group_priv(struct fsnotify_group *group)
 {
        struct user_struct *user;
 
 {
        struct user_struct *user;
 
+       kfree(group->fanotify_data.merge_hash);
        user = group->fanotify_data.user;
        atomic_dec(&user->fanotify_listeners);
        free_uid(user);
        user = group->fanotify_data.user;
        atomic_dec(&user->fanotify_listeners);
        free_uid(user);
index 9871f76..4a5e555 100644 (file)
@@ -3,6 +3,7 @@
 #include <linux/path.h>
 #include <linux/slab.h>
 #include <linux/exportfs.h>
 #include <linux/path.h>
 #include <linux/slab.h>
 #include <linux/exportfs.h>
+#include <linux/hashtable.h>
 
 extern struct kmem_cache *fanotify_mark_cache;
 extern struct kmem_cache *fanotify_fid_event_cachep;
 
 extern struct kmem_cache *fanotify_mark_cache;
 extern struct kmem_cache *fanotify_fid_event_cachep;
@@ -150,6 +151,7 @@ enum fanotify_event_type {
 
 struct fanotify_event {
        struct fsnotify_event fse;
 
 struct fanotify_event {
        struct fsnotify_event fse;
+       struct hlist_node merge_list;   /* List for hashed merge */
        u32 mask;
        struct {
                unsigned int type : FANOTIFY_EVENT_TYPE_BITS;
        u32 mask;
        struct {
                unsigned int type : FANOTIFY_EVENT_TYPE_BITS;
@@ -162,6 +164,7 @@ static inline void fanotify_init_event(struct fanotify_event *event,
                                       unsigned int hash, u32 mask)
 {
        fsnotify_init_event(&event->fse);
                                       unsigned int hash, u32 mask)
 {
        fsnotify_init_event(&event->fse);
+       INIT_HLIST_NODE(&event->merge_list);
        event->hash = hash;
        event->mask = mask;
        event->pid = NULL;
        event->hash = hash;
        event->mask = mask;
        event->pid = NULL;
@@ -299,3 +302,25 @@ static inline struct path *fanotify_event_path(struct fanotify_event *event)
        else
                return NULL;
 }
        else
                return NULL;
 }
+
+/*
+ * Use 128 size hash table to speed up events merge.
+ */
+#define FANOTIFY_HTABLE_BITS   (7)
+#define FANOTIFY_HTABLE_SIZE   (1 << FANOTIFY_HTABLE_BITS)
+#define FANOTIFY_HTABLE_MASK   (FANOTIFY_HTABLE_SIZE - 1)
+
+/*
+ * Permission events and overflow event do not get merged - don't hash them.
+ */
+static inline bool fanotify_is_hashed_event(u32 mask)
+{
+       return !fanotify_is_perm_event(mask) && !(mask & FS_Q_OVERFLOW);
+}
+
+static inline unsigned int fanotify_event_hash_bucket(
+                                               struct fsnotify_group *group,
+                                               struct fanotify_event *event)
+{
+       return event->hash & FANOTIFY_HTABLE_MASK;
+}
index 1616220..b89f332 100644 (file)
@@ -89,6 +89,23 @@ static int fanotify_event_info_len(unsigned int fid_mode,
        return info_len;
 }
 
        return info_len;
 }
 
+/*
+ * Remove an hashed event from merge hash table.
+ */
+static void fanotify_unhash_event(struct fsnotify_group *group,
+                                 struct fanotify_event *event)
+{
+       assert_spin_locked(&group->notification_lock);
+
+       pr_debug("%s: group=%p event=%p bucket=%u\n", __func__,
+                group, event, fanotify_event_hash_bucket(group, event));
+
+       if (WARN_ON_ONCE(hlist_unhashed(&event->merge_list)))
+               return;
+
+       hlist_del_init(&event->merge_list);
+}
+
 /*
  * Get an fanotify notification event if one exists and is small
  * enough to fit in "count". Return an error pointer if the count
 /*
  * Get an fanotify notification event if one exists and is small
  * enough to fit in "count". Return an error pointer if the count
@@ -126,6 +143,8 @@ static struct fanotify_event *get_one_event(struct fsnotify_group *group,
        fsnotify_remove_first_event(group);
        if (fanotify_is_perm_event(event->mask))
                FANOTIFY_PERM(event)->state = FAN_EVENT_REPORTED;
        fsnotify_remove_first_event(group);
        if (fanotify_is_perm_event(event->mask))
                FANOTIFY_PERM(event)->state = FAN_EVENT_REPORTED;
+       if (fanotify_is_hashed_event(event->mask))
+               fanotify_unhash_event(group, event);
 out:
        spin_unlock(&group->notification_lock);
        return event;
 out:
        spin_unlock(&group->notification_lock);
        return event;
@@ -925,6 +944,20 @@ static struct fsnotify_event *fanotify_alloc_overflow_event(void)
        return &oevent->fse;
 }
 
        return &oevent->fse;
 }
 
+static struct hlist_head *fanotify_alloc_merge_hash(void)
+{
+       struct hlist_head *hash;
+
+       hash = kmalloc(sizeof(struct hlist_head) << FANOTIFY_HTABLE_BITS,
+                      GFP_KERNEL_ACCOUNT);
+       if (!hash)
+               return NULL;
+
+       __hash_init(hash, FANOTIFY_HTABLE_SIZE);
+
+       return hash;
+}
+
 /* fanotify syscalls */
 SYSCALL_DEFINE2(fanotify_init, unsigned int, flags, unsigned int, event_f_flags)
 {
 /* fanotify syscalls */
 SYSCALL_DEFINE2(fanotify_init, unsigned int, flags, unsigned int, event_f_flags)
 {
@@ -993,6 +1026,12 @@ SYSCALL_DEFINE2(fanotify_init, unsigned int, flags, unsigned int, event_f_flags)
        atomic_inc(&user->fanotify_listeners);
        group->memcg = get_mem_cgroup_from_mm(current->mm);
 
        atomic_inc(&user->fanotify_listeners);
        group->memcg = get_mem_cgroup_from_mm(current->mm);
 
+       group->fanotify_data.merge_hash = fanotify_alloc_merge_hash();
+       if (!group->fanotify_data.merge_hash) {
+               fd = -ENOMEM;
+               goto out_destroy_group;
+       }
+
        group->overflow_event = fanotify_alloc_overflow_event();
        if (unlikely(!group->overflow_event)) {
                fd = -ENOMEM;
        group->overflow_event = fanotify_alloc_overflow_event();
        if (unlikely(!group->overflow_event)) {
                fd = -ENOMEM;
index 0533bac..d1a64da 100644 (file)
@@ -46,9 +46,10 @@ static bool event_compare(struct fsnotify_event *old_fsn,
        return false;
 }
 
        return false;
 }
 
-static int inotify_merge(struct list_head *list,
-                         struct fsnotify_event *event)
+static int inotify_merge(struct fsnotify_group *group,
+                        struct fsnotify_event *event)
 {
 {
+       struct list_head *list = &group->notification_list;
        struct fsnotify_event *last_event;
 
        last_event = list_entry(list->prev, struct fsnotify_event, list);
        struct fsnotify_event *last_event;
 
        last_event = list_entry(list->prev, struct fsnotify_event, list);
@@ -115,7 +116,7 @@ int inotify_handle_inode_event(struct fsnotify_mark *inode_mark, u32 mask,
        if (len)
                strcpy(event->name, name->name);
 
        if (len)
                strcpy(event->name, name->name);
 
-       ret = fsnotify_add_event(group, fsn_event, inotify_merge);
+       ret = fsnotify_add_event(group, fsn_event, inotify_merge, NULL);
        if (ret) {
                /* Our event wasn't used in the end. Free it. */
                fsnotify_destroy_event(group, fsn_event);
        if (ret) {
                /* Our event wasn't used in the end. Free it. */
                fsnotify_destroy_event(group, fsn_event);
index 001cfe7..32f4554 100644 (file)
@@ -68,16 +68,22 @@ void fsnotify_destroy_event(struct fsnotify_group *group,
 }
 
 /*
 }
 
 /*
- * Add an event to the group notification queue.  The group can later pull this
- * event off the queue to deal with.  The function returns 0 if the event was
- * added to the queue, 1 if the event was merged with some other queued event,
+ * Try to add an event to the notification queue.
+ * The group can later pull this event off the queue to deal with.
+ * The group can use the @merge hook to merge the event with a queued event.
+ * The group can use the @insert hook to insert the event into hash table.
+ * The function returns:
+ * 0 if the event was added to a queue
+ * 1 if the event was merged with some other queued event
  * 2 if the event was not queued - either the queue of events has overflown
  * 2 if the event was not queued - either the queue of events has overflown
- * or the group is shutting down.
+ *   or the group is shutting down.
  */
 int fsnotify_add_event(struct fsnotify_group *group,
                       struct fsnotify_event *event,
  */
 int fsnotify_add_event(struct fsnotify_group *group,
                       struct fsnotify_event *event,
-                      int (*merge)(struct list_head *,
-                                   struct fsnotify_event *))
+                      int (*merge)(struct fsnotify_group *,
+                                   struct fsnotify_event *),
+                      void (*insert)(struct fsnotify_group *,
+                                     struct fsnotify_event *))
 {
        int ret = 0;
        struct list_head *list = &group->notification_list;
 {
        int ret = 0;
        struct list_head *list = &group->notification_list;
@@ -104,7 +110,7 @@ int fsnotify_add_event(struct fsnotify_group *group,
        }
 
        if (!list_empty(list) && merge) {
        }
 
        if (!list_empty(list) && merge) {
-               ret = merge(list, event);
+               ret = merge(group, event);
                if (ret) {
                        spin_unlock(&group->notification_lock);
                        return ret;
                if (ret) {
                        spin_unlock(&group->notification_lock);
                        return ret;
@@ -114,6 +120,8 @@ int fsnotify_add_event(struct fsnotify_group *group,
 queue:
        group->q_len++;
        list_add_tail(&event->list, list);
 queue:
        group->q_len++;
        list_add_tail(&event->list, list);
+       if (insert)
+               insert(group, event);
        spin_unlock(&group->notification_lock);
 
        wake_up(&group->notification_waitq);
        spin_unlock(&group->notification_lock);
 
        wake_up(&group->notification_waitq);
index fc98f9f..63fb766 100644 (file)
@@ -233,6 +233,8 @@ struct fsnotify_group {
 #endif
 #ifdef CONFIG_FANOTIFY
                struct fanotify_group_private_data {
 #endif
 #ifdef CONFIG_FANOTIFY
                struct fanotify_group_private_data {
+                       /* Hash table of events for merge */
+                       struct hlist_head *merge_hash;
                        /* allows a group to block waiting for a userspace response */
                        struct list_head access_list;
                        wait_queue_head_t access_waitq;
                        /* allows a group to block waiting for a userspace response */
                        struct list_head access_list;
                        wait_queue_head_t access_waitq;
@@ -486,12 +488,14 @@ extern void fsnotify_destroy_event(struct fsnotify_group *group,
 /* attach the event to the group notification queue */
 extern int fsnotify_add_event(struct fsnotify_group *group,
                              struct fsnotify_event *event,
 /* attach the event to the group notification queue */
 extern int fsnotify_add_event(struct fsnotify_group *group,
                              struct fsnotify_event *event,
-                             int (*merge)(struct list_head *,
-                                          struct fsnotify_event *));
+                             int (*merge)(struct fsnotify_group *,
+                                          struct fsnotify_event *),
+                             void (*insert)(struct fsnotify_group *,
+                                            struct fsnotify_event *));
 /* Queue overflow event to a notification group */
 static inline void fsnotify_queue_overflow(struct fsnotify_group *group)
 {
 /* Queue overflow event to a notification group */
 static inline void fsnotify_queue_overflow(struct fsnotify_group *group)
 {
-       fsnotify_add_event(group, group->overflow_event, NULL);
+       fsnotify_add_event(group, group->overflow_event, NULL, NULL);
 }
 
 static inline bool fsnotify_notify_queue_is_empty(struct fsnotify_group *group)
 }
 
 static inline bool fsnotify_notify_queue_is_empty(struct fsnotify_group *group)