diff mbox series

[v3,08/15] fsnotify: Support passing argument to insert callback on add_event

Message ID 20210629191035.681913-9-krisman@collabora.com (mailing list archive)
State New, archived
Headers show
Series File system wide monitoring | expand

Commit Message

Gabriel Krisman Bertazi June 29, 2021, 7:10 p.m. UTC
FAN_FS_ERROR requires some initialization to happen from inside the
insert hook.  This allows a user of fanotify_add_event to pass an
argument to be sent to the insert callback.

Signed-off-by: Gabriel Krisman Bertazi <krisman@collabora.com>
---
 fs/notify/fanotify/fanotify.c        | 5 +++--
 fs/notify/inotify/inotify_fsnotify.c | 2 +-
 fs/notify/notification.c             | 6 ++++--
 include/linux/fsnotify_backend.h     | 7 +++++--
 4 files changed, 13 insertions(+), 7 deletions(-)

Comments

Amir Goldstein June 30, 2021, 3:40 a.m. UTC | #1
On Tue, Jun 29, 2021 at 10:12 PM Gabriel Krisman Bertazi
<krisman@collabora.com> wrote:
>
> FAN_FS_ERROR requires some initialization to happen from inside the
> insert hook.  This allows a user of fanotify_add_event to pass an
> argument to be sent to the insert callback.
>
> Signed-off-by: Gabriel Krisman Bertazi <krisman@collabora.com>

Reviewed-by: Amir Goldstein <amir73il@gmail.com>

with optional suggestion for improvement...

> ---
>  fs/notify/fanotify/fanotify.c        | 5 +++--
>  fs/notify/inotify/inotify_fsnotify.c | 2 +-
>  fs/notify/notification.c             | 6 ++++--
>  include/linux/fsnotify_backend.h     | 7 +++++--
>  4 files changed, 13 insertions(+), 7 deletions(-)
>
> diff --git a/fs/notify/fanotify/fanotify.c b/fs/notify/fanotify/fanotify.c
> index 4f2febb15e94..aba06b84da91 100644
> --- a/fs/notify/fanotify/fanotify.c
> +++ b/fs/notify/fanotify/fanotify.c
> @@ -695,7 +695,8 @@ static __kernel_fsid_t fanotify_get_fsid(struct fsnotify_iter_info *iter_info)
>   * Add an event to hash table for faster merge.
>   */
>  static void fanotify_insert_event(struct fsnotify_group *group,
> -                                 struct fsnotify_event *fsn_event)
> +                                 struct fsnotify_event *fsn_event,
> +                                 const void *data)
>  {
>         struct fanotify_event *event = FANOTIFY_E(fsn_event);
>         unsigned int bucket = fanotify_event_hash_bucket(group, event);
> @@ -779,7 +780,7 @@ static int fanotify_handle_event(struct fsnotify_group *group, u32 mask,
>
>         fsn_event = &event->fse;
>         ret = fsnotify_add_event(group, fsn_event, fanotify_merge,
> -                                fanotify_insert_event);
> +                                fanotify_insert_event, NULL);
>         if (ret) {
>                 /* Permission events shouldn't be merged */
>                 BUG_ON(ret == 1 && mask & FANOTIFY_PERM_EVENTS);
> diff --git a/fs/notify/inotify/inotify_fsnotify.c b/fs/notify/inotify/inotify_fsnotify.c
> index d1a64daa0171..a003a64ff8ee 100644
> --- a/fs/notify/inotify/inotify_fsnotify.c
> +++ b/fs/notify/inotify/inotify_fsnotify.c
> @@ -116,7 +116,7 @@ int inotify_handle_inode_event(struct fsnotify_mark *inode_mark, u32 mask,
>         if (len)
>                 strcpy(event->name, name->name);
>
> -       ret = fsnotify_add_event(group, fsn_event, inotify_merge, NULL);
> +       ret = fsnotify_add_event(group, fsn_event, inotify_merge, NULL, NULL);
>         if (ret) {
>                 /* Our event wasn't used in the end. Free it. */
>                 fsnotify_destroy_event(group, fsn_event);
> diff --git a/fs/notify/notification.c b/fs/notify/notification.c
> index 32f45543b9c6..0d9ba592d725 100644
> --- a/fs/notify/notification.c
> +++ b/fs/notify/notification.c
> @@ -83,7 +83,9 @@ int fsnotify_add_event(struct fsnotify_group *group,
>                        int (*merge)(struct fsnotify_group *,
>                                     struct fsnotify_event *),
>                        void (*insert)(struct fsnotify_group *,
> -                                     struct fsnotify_event *))
> +                                     struct fsnotify_event *,
> +                                     const void *),
> +                      const void *insert_data)
>  {
>         int ret = 0;
>         struct list_head *list = &group->notification_list;
> @@ -121,7 +123,7 @@ int fsnotify_add_event(struct fsnotify_group *group,
>         group->q_len++;
>         list_add_tail(&event->list, list);
>         if (insert)
> -               insert(group, event);
> +               insert(group, event, insert_data);
>         spin_unlock(&group->notification_lock);
>
>         wake_up(&group->notification_waitq);
> diff --git a/include/linux/fsnotify_backend.h b/include/linux/fsnotify_backend.h
> index b1590f654ade..8222fe12a6c9 100644
> --- a/include/linux/fsnotify_backend.h
> +++ b/include/linux/fsnotify_backend.h
> @@ -526,11 +526,14 @@ extern int fsnotify_add_event(struct fsnotify_group *group,
>                               int (*merge)(struct fsnotify_group *,
>                                            struct fsnotify_event *),
>                               void (*insert)(struct fsnotify_group *,
> -                                            struct fsnotify_event *));
> +                                            struct fsnotify_event *,
> +                                            const void *),
> +                             const void *insert_data);
> +
>  /* 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, NULL);
> +       fsnotify_add_event(group, group->overflow_event, NULL, NULL, NULL);
>  }
>

Looking back, when I added the insert() callback, it might have been wiser to
rename fsnotify_add_event() to fsnotify_insert_event() add a wrapper:

static inline int fsnotify_add_event(group, event, merge) {
    return fsnotify_insert_event(group, event, merge, NULL);
}

But the two call sites did not seem to justify it.
Perhaps with the growing list of NULL arguments it is time to
reconsider this small tidy up?
Not critical though.

Thanks,
Amir.
Jan Kara July 8, 2021, 10:48 a.m. UTC | #2
On Tue 29-06-21 15:10:28, Gabriel Krisman Bertazi wrote:
> FAN_FS_ERROR requires some initialization to happen from inside the
> insert hook.  This allows a user of fanotify_add_event to pass an
> argument to be sent to the insert callback.
> 
> Signed-off-by: Gabriel Krisman Bertazi <krisman@collabora.com>

Looks good. Feel free to add:

Reviewed-by: Jan Kara <jack@suse.cz>

								Honza

> ---
>  fs/notify/fanotify/fanotify.c        | 5 +++--
>  fs/notify/inotify/inotify_fsnotify.c | 2 +-
>  fs/notify/notification.c             | 6 ++++--
>  include/linux/fsnotify_backend.h     | 7 +++++--
>  4 files changed, 13 insertions(+), 7 deletions(-)
> 
> diff --git a/fs/notify/fanotify/fanotify.c b/fs/notify/fanotify/fanotify.c
> index 4f2febb15e94..aba06b84da91 100644
> --- a/fs/notify/fanotify/fanotify.c
> +++ b/fs/notify/fanotify/fanotify.c
> @@ -695,7 +695,8 @@ static __kernel_fsid_t fanotify_get_fsid(struct fsnotify_iter_info *iter_info)
>   * Add an event to hash table for faster merge.
>   */
>  static void fanotify_insert_event(struct fsnotify_group *group,
> -				  struct fsnotify_event *fsn_event)
> +				  struct fsnotify_event *fsn_event,
> +				  const void *data)
>  {
>  	struct fanotify_event *event = FANOTIFY_E(fsn_event);
>  	unsigned int bucket = fanotify_event_hash_bucket(group, event);
> @@ -779,7 +780,7 @@ static int fanotify_handle_event(struct fsnotify_group *group, u32 mask,
>  
>  	fsn_event = &event->fse;
>  	ret = fsnotify_add_event(group, fsn_event, fanotify_merge,
> -				 fanotify_insert_event);
> +				 fanotify_insert_event, NULL);
>  	if (ret) {
>  		/* Permission events shouldn't be merged */
>  		BUG_ON(ret == 1 && mask & FANOTIFY_PERM_EVENTS);
> diff --git a/fs/notify/inotify/inotify_fsnotify.c b/fs/notify/inotify/inotify_fsnotify.c
> index d1a64daa0171..a003a64ff8ee 100644
> --- a/fs/notify/inotify/inotify_fsnotify.c
> +++ b/fs/notify/inotify/inotify_fsnotify.c
> @@ -116,7 +116,7 @@ int inotify_handle_inode_event(struct fsnotify_mark *inode_mark, u32 mask,
>  	if (len)
>  		strcpy(event->name, name->name);
>  
> -	ret = fsnotify_add_event(group, fsn_event, inotify_merge, NULL);
> +	ret = fsnotify_add_event(group, fsn_event, inotify_merge, NULL, NULL);
>  	if (ret) {
>  		/* Our event wasn't used in the end. Free it. */
>  		fsnotify_destroy_event(group, fsn_event);
> diff --git a/fs/notify/notification.c b/fs/notify/notification.c
> index 32f45543b9c6..0d9ba592d725 100644
> --- a/fs/notify/notification.c
> +++ b/fs/notify/notification.c
> @@ -83,7 +83,9 @@ int fsnotify_add_event(struct fsnotify_group *group,
>  		       int (*merge)(struct fsnotify_group *,
>  				    struct fsnotify_event *),
>  		       void (*insert)(struct fsnotify_group *,
> -				      struct fsnotify_event *))
> +				      struct fsnotify_event *,
> +				      const void *),
> +		       const void *insert_data)
>  {
>  	int ret = 0;
>  	struct list_head *list = &group->notification_list;
> @@ -121,7 +123,7 @@ int fsnotify_add_event(struct fsnotify_group *group,
>  	group->q_len++;
>  	list_add_tail(&event->list, list);
>  	if (insert)
> -		insert(group, event);
> +		insert(group, event, insert_data);
>  	spin_unlock(&group->notification_lock);
>  
>  	wake_up(&group->notification_waitq);
> diff --git a/include/linux/fsnotify_backend.h b/include/linux/fsnotify_backend.h
> index b1590f654ade..8222fe12a6c9 100644
> --- a/include/linux/fsnotify_backend.h
> +++ b/include/linux/fsnotify_backend.h
> @@ -526,11 +526,14 @@ extern int fsnotify_add_event(struct fsnotify_group *group,
>  			      int (*merge)(struct fsnotify_group *,
>  					   struct fsnotify_event *),
>  			      void (*insert)(struct fsnotify_group *,
> -					     struct fsnotify_event *));
> +					     struct fsnotify_event *,
> +					     const void *),
> +			      const void *insert_data);
> +
>  /* 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, NULL);
> +	fsnotify_add_event(group, group->overflow_event, NULL, NULL, NULL);
>  }
>  
>  static inline bool fsnotify_is_overflow_event(u32 mask)
> -- 
> 2.32.0
>
diff mbox series

Patch

diff --git a/fs/notify/fanotify/fanotify.c b/fs/notify/fanotify/fanotify.c
index 4f2febb15e94..aba06b84da91 100644
--- a/fs/notify/fanotify/fanotify.c
+++ b/fs/notify/fanotify/fanotify.c
@@ -695,7 +695,8 @@  static __kernel_fsid_t fanotify_get_fsid(struct fsnotify_iter_info *iter_info)
  * Add an event to hash table for faster merge.
  */
 static void fanotify_insert_event(struct fsnotify_group *group,
-				  struct fsnotify_event *fsn_event)
+				  struct fsnotify_event *fsn_event,
+				  const void *data)
 {
 	struct fanotify_event *event = FANOTIFY_E(fsn_event);
 	unsigned int bucket = fanotify_event_hash_bucket(group, event);
@@ -779,7 +780,7 @@  static int fanotify_handle_event(struct fsnotify_group *group, u32 mask,
 
 	fsn_event = &event->fse;
 	ret = fsnotify_add_event(group, fsn_event, fanotify_merge,
-				 fanotify_insert_event);
+				 fanotify_insert_event, NULL);
 	if (ret) {
 		/* Permission events shouldn't be merged */
 		BUG_ON(ret == 1 && mask & FANOTIFY_PERM_EVENTS);
diff --git a/fs/notify/inotify/inotify_fsnotify.c b/fs/notify/inotify/inotify_fsnotify.c
index d1a64daa0171..a003a64ff8ee 100644
--- a/fs/notify/inotify/inotify_fsnotify.c
+++ b/fs/notify/inotify/inotify_fsnotify.c
@@ -116,7 +116,7 @@  int inotify_handle_inode_event(struct fsnotify_mark *inode_mark, u32 mask,
 	if (len)
 		strcpy(event->name, name->name);
 
-	ret = fsnotify_add_event(group, fsn_event, inotify_merge, NULL);
+	ret = fsnotify_add_event(group, fsn_event, inotify_merge, NULL, NULL);
 	if (ret) {
 		/* Our event wasn't used in the end. Free it. */
 		fsnotify_destroy_event(group, fsn_event);
diff --git a/fs/notify/notification.c b/fs/notify/notification.c
index 32f45543b9c6..0d9ba592d725 100644
--- a/fs/notify/notification.c
+++ b/fs/notify/notification.c
@@ -83,7 +83,9 @@  int fsnotify_add_event(struct fsnotify_group *group,
 		       int (*merge)(struct fsnotify_group *,
 				    struct fsnotify_event *),
 		       void (*insert)(struct fsnotify_group *,
-				      struct fsnotify_event *))
+				      struct fsnotify_event *,
+				      const void *),
+		       const void *insert_data)
 {
 	int ret = 0;
 	struct list_head *list = &group->notification_list;
@@ -121,7 +123,7 @@  int fsnotify_add_event(struct fsnotify_group *group,
 	group->q_len++;
 	list_add_tail(&event->list, list);
 	if (insert)
-		insert(group, event);
+		insert(group, event, insert_data);
 	spin_unlock(&group->notification_lock);
 
 	wake_up(&group->notification_waitq);
diff --git a/include/linux/fsnotify_backend.h b/include/linux/fsnotify_backend.h
index b1590f654ade..8222fe12a6c9 100644
--- a/include/linux/fsnotify_backend.h
+++ b/include/linux/fsnotify_backend.h
@@ -526,11 +526,14 @@  extern int fsnotify_add_event(struct fsnotify_group *group,
 			      int (*merge)(struct fsnotify_group *,
 					   struct fsnotify_event *),
 			      void (*insert)(struct fsnotify_group *,
-					     struct fsnotify_event *));
+					     struct fsnotify_event *,
+					     const void *),
+			      const void *insert_data);
+
 /* 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, NULL);
+	fsnotify_add_event(group, group->overflow_event, NULL, NULL, NULL);
 }
 
 static inline bool fsnotify_is_overflow_event(u32 mask)