diff mbox

[v2,06/20] fsnotify: remove redundant arguments to handle_event()

Message ID 1522934301-6520-7-git-send-email-amir73il@gmail.com (mailing list archive)
State New, archived
Headers show

Commit Message

Amir Goldstein April 5, 2018, 1:18 p.m. UTC
inode_mark and vfsmount_mark arguments are passed to handle_event()
operation as function arguments as well as on iter_info struct.
The difference is that iter_info struct may contain marks that should
not be handled and are represented as NULL arguments to inode_mark or
vfsmount_mark.

Instead of passing the inode_mark and vfsmount_mark arguments, add
a type_mask member to iter_info struct to indicate which marks should
be handled, versus marks that should only be kept alive during user
wait.

This change is going to be used for passing more mark types
with handle_event() (i.e. super block marks).

Signed-off-by: Amir Goldstein <amir73il@gmail.com>
---
 fs/notify/dnotify/dnotify.c          |  6 ++---
 fs/notify/fanotify/fanotify.c        | 14 +++++-----
 fs/notify/fsnotify.c                 | 51 ++++++++++++++++++------------------
 fs/notify/fsnotify.h                 |  6 -----
 fs/notify/inotify/inotify.h          |  2 --
 fs/notify/inotify/inotify_fsnotify.c |  6 ++---
 fs/notify/inotify/inotify_user.c     |  6 +++--
 include/linux/fsnotify_backend.h     | 31 ++++++++++++++++++++--
 kernel/audit_fsnotify.c              |  3 +--
 kernel/audit_tree.c                  |  2 --
 kernel/audit_watch.c                 |  3 +--
 11 files changed, 73 insertions(+), 57 deletions(-)

Comments

Jan Kara April 17, 2018, 4:33 p.m. UTC | #1
On Thu 05-04-18 16:18:07, Amir Goldstein wrote:
> inode_mark and vfsmount_mark arguments are passed to handle_event()
> operation as function arguments as well as on iter_info struct.
> The difference is that iter_info struct may contain marks that should
> not be handled and are represented as NULL arguments to inode_mark or
> vfsmount_mark.
> 
> Instead of passing the inode_mark and vfsmount_mark arguments, add
> a type_mask member to iter_info struct to indicate which marks should
> be handled, versus marks that should only be kept alive during user
> wait.
> 
> This change is going to be used for passing more mark types
> with handle_event() (i.e. super block marks).
> 
> Signed-off-by: Amir Goldstein <amir73il@gmail.com>

Again only nit here:

> @@ -212,6 +210,35 @@ enum fsnotify_obj_type {
>  #define FSNOTIFY_OBJ_TYPE_VFSMOUNT_FL	(1U << FSNOTIFY_OBJ_TYPE_VFSMOUNT)
>  #define FSNOTIFY_OBJ_ALL_TYPES_MASK	((1U << FSNOTIFY_OBJ_TYPE_MAX) - 1)
>  
> +struct fsnotify_iter_info {
> +	struct fsnotify_mark *inode_mark;
> +	struct fsnotify_mark *vfsmount_mark;
> +	unsigned int type_mask;
> +	int srcu_idx;
> +};

I'd rename 'type_mask' to something more expressing the fact that these are
marks we currently report to - 'report_mask'?

								Honza
Amir Goldstein April 17, 2018, 5:59 p.m. UTC | #2
On Tue, Apr 17, 2018 at 7:33 PM, Jan Kara <jack@suse.cz> wrote:
> On Thu 05-04-18 16:18:07, Amir Goldstein wrote:
>> inode_mark and vfsmount_mark arguments are passed to handle_event()
>> operation as function arguments as well as on iter_info struct.
>> The difference is that iter_info struct may contain marks that should
>> not be handled and are represented as NULL arguments to inode_mark or
>> vfsmount_mark.
>>
>> Instead of passing the inode_mark and vfsmount_mark arguments, add
>> a type_mask member to iter_info struct to indicate which marks should
>> be handled, versus marks that should only be kept alive during user
>> wait.
>>
>> This change is going to be used for passing more mark types
>> with handle_event() (i.e. super block marks).
>>
>> Signed-off-by: Amir Goldstein <amir73il@gmail.com>
>
> Again only nit here:
>
>> @@ -212,6 +210,35 @@ enum fsnotify_obj_type {
>>  #define FSNOTIFY_OBJ_TYPE_VFSMOUNT_FL        (1U << FSNOTIFY_OBJ_TYPE_VFSMOUNT)
>>  #define FSNOTIFY_OBJ_ALL_TYPES_MASK  ((1U << FSNOTIFY_OBJ_TYPE_MAX) - 1)
>>
>> +struct fsnotify_iter_info {
>> +     struct fsnotify_mark *inode_mark;
>> +     struct fsnotify_mark *vfsmount_mark;
>> +     unsigned int type_mask;
>> +     int srcu_idx;
>> +};
>
> I'd rename 'type_mask' to something more expressing the fact that these are
> marks we currently report to - 'report_mask'?
>

OK. For lack of a better name.

Thanks,
Amir.
diff mbox

Patch

diff --git a/fs/notify/dnotify/dnotify.c b/fs/notify/dnotify/dnotify.c
index 63a1ca4b9dee..b4e685b63f33 100644
--- a/fs/notify/dnotify/dnotify.c
+++ b/fs/notify/dnotify/dnotify.c
@@ -79,12 +79,11 @@  static void dnotify_recalc_inode_mask(struct fsnotify_mark *fsn_mark)
  */
 static int dnotify_handle_event(struct fsnotify_group *group,
 				struct inode *inode,
-				struct fsnotify_mark *inode_mark,
-				struct fsnotify_mark *vfsmount_mark,
 				u32 mask, const void *data, int data_type,
 				const unsigned char *file_name, u32 cookie,
 				struct fsnotify_iter_info *iter_info)
 {
+	struct fsnotify_mark *inode_mark = fsnotify_iter_inode_mark(iter_info);
 	struct dnotify_mark *dn_mark;
 	struct dnotify_struct *dn;
 	struct dnotify_struct **prev;
@@ -95,7 +94,8 @@  static int dnotify_handle_event(struct fsnotify_group *group,
 	if (!S_ISDIR(inode->i_mode))
 		return 0;
 
-	BUG_ON(vfsmount_mark);
+	if (WARN_ON(fsnotify_iter_vfsmount_mark(iter_info)))
+		return 0;
 
 	dn_mark = container_of(inode_mark, struct dnotify_mark, fsn_mark);
 
diff --git a/fs/notify/fanotify/fanotify.c b/fs/notify/fanotify/fanotify.c
index e0e6a9d627df..86ccf79c0671 100644
--- a/fs/notify/fanotify/fanotify.c
+++ b/fs/notify/fanotify/fanotify.c
@@ -87,11 +87,12 @@  static int fanotify_get_response(struct fsnotify_group *group,
 	return ret;
 }
 
-static bool fanotify_should_send_event(struct fsnotify_mark *inode_mark,
-				       struct fsnotify_mark *vfsmnt_mark,
-				       u32 event_mask,
-				       const void *data, int data_type)
+static bool fanotify_should_send_event(struct fsnotify_iter_info *iter_info,
+				       u32 event_mask, const void *data,
+				       int data_type)
 {
+	struct fsnotify_mark *inode_mark = fsnotify_iter_inode_mark(iter_info);
+	struct fsnotify_mark *vfsmnt_mark = fsnotify_iter_vfsmount_mark(iter_info);
 	__u32 marks_mask = 0, marks_ignored_mask = 0;
 	const struct path *path = data;
 
@@ -169,8 +170,6 @@  init: __maybe_unused
 
 static int fanotify_handle_event(struct fsnotify_group *group,
 				 struct inode *inode,
-				 struct fsnotify_mark *inode_mark,
-				 struct fsnotify_mark *fanotify_mark,
 				 u32 mask, const void *data, int data_type,
 				 const unsigned char *file_name, u32 cookie,
 				 struct fsnotify_iter_info *iter_info)
@@ -190,8 +189,7 @@  static int fanotify_handle_event(struct fsnotify_group *group,
 	BUILD_BUG_ON(FAN_ACCESS_PERM != FS_ACCESS_PERM);
 	BUILD_BUG_ON(FAN_ONDIR != FS_ISDIR);
 
-	if (!fanotify_should_send_event(inode_mark, fanotify_mark, mask, data,
-					data_type))
+	if (!fanotify_should_send_event(iter_info, mask, data, data_type))
 		return 0;
 
 	pr_debug("%s: group=%p inode=%p mask=%x\n", __func__, group, inode,
diff --git a/fs/notify/fsnotify.c b/fs/notify/fsnotify.c
index 613ec7e5a465..cb0afa664057 100644
--- a/fs/notify/fsnotify.c
+++ b/fs/notify/fsnotify.c
@@ -184,22 +184,20 @@  int __fsnotify_parent(const struct path *path, struct dentry *dentry, __u32 mask
 EXPORT_SYMBOL_GPL(__fsnotify_parent);
 
 static int send_to_group(struct inode *to_tell,
-			 struct fsnotify_mark *inode_mark,
-			 struct fsnotify_mark *vfsmount_mark,
 			 __u32 mask, const void *data,
 			 int data_is, u32 cookie,
 			 const unsigned char *file_name,
 			 struct fsnotify_iter_info *iter_info)
 {
+	struct fsnotify_mark *inode_mark = fsnotify_iter_inode_mark(iter_info);
+	struct fsnotify_mark *vfsmount_mark = fsnotify_iter_vfsmount_mark(iter_info);
 	struct fsnotify_group *group = NULL;
 	__u32 test_mask = (mask & ~FS_EVENT_ON_CHILD);
 	__u32 marks_mask = 0;
 	__u32 marks_ignored_mask = 0;
 
-	if (unlikely(!inode_mark && !vfsmount_mark)) {
-		BUG();
+	if (WARN_ON(!iter_info->type_mask))
 		return 0;
-	}
 
 	/* clear ignored on inode modification */
 	if (mask & FS_MODIFY) {
@@ -235,8 +233,7 @@  static int send_to_group(struct inode *to_tell,
 	if (!(test_mask & marks_mask & ~marks_ignored_mask))
 		return 0;
 
-	return group->ops->handle_event(group, to_tell, inode_mark,
-					vfsmount_mark, mask, data, data_is,
+	return group->ops->handle_event(group, to_tell, mask, data, data_is,
 					file_name, cookie, iter_info);
 }
 
@@ -307,16 +304,16 @@  int fsnotify(struct inode *to_tell, __u32 mask, const void *data, int data_is,
 
 	if ((mask & FS_MODIFY) ||
 	    (test_mask & to_tell->i_fsnotify_mask)) {
-		iter_info.inode_mark =
-			fsnotify_first_mark(&to_tell->i_fsnotify_marks);
+		fsnotify_iter_set_inode_mark(&iter_info,
+			fsnotify_first_mark(&to_tell->i_fsnotify_marks));
 	}
 
 	if (mnt && ((mask & FS_MODIFY) ||
 		    (test_mask & mnt->mnt_fsnotify_mask))) {
-		iter_info.inode_mark =
-			fsnotify_first_mark(&to_tell->i_fsnotify_marks);
-		iter_info.vfsmount_mark =
-			fsnotify_first_mark(&mnt->mnt_fsnotify_marks);
+		fsnotify_iter_set_inode_mark(&iter_info,
+			fsnotify_first_mark(&to_tell->i_fsnotify_marks));
+		fsnotify_iter_set_vfsmount_mark(&iter_info,
+			fsnotify_first_mark(&mnt->mnt_fsnotify_marks));
 	}
 
 	/*
@@ -324,7 +321,7 @@  int fsnotify(struct inode *to_tell, __u32 mask, const void *data, int data_is,
 	 * ignore masks are properly reflected for mount mark notifications.
 	 * That's why this traversal is so complicated...
 	 */
-	while (iter_info.inode_mark || iter_info.vfsmount_mark) {
+	while (iter_info.type_mask) {
 		struct fsnotify_mark *inode_mark = iter_info.inode_mark;
 		struct fsnotify_mark *vfsmount_mark = iter_info.vfsmount_mark;
 
@@ -332,24 +329,28 @@  int fsnotify(struct inode *to_tell, __u32 mask, const void *data, int data_is,
 			int cmp = fsnotify_compare_groups(inode_mark->group,
 							  vfsmount_mark->group);
 			if (cmp > 0)
-				inode_mark = NULL;
+				iter_info.type_mask &= ~FSNOTIFY_OBJ_TYPE_INODE_FL;
 			else if (cmp < 0)
-				vfsmount_mark = NULL;
+				iter_info.type_mask &= ~FSNOTIFY_OBJ_TYPE_VFSMOUNT_FL;
 		}
 
-		ret = send_to_group(to_tell, inode_mark, vfsmount_mark, mask,
-				    data, data_is, cookie, file_name,
-				    &iter_info);
+		ret = send_to_group(to_tell, mask, data, data_is, cookie,
+				    file_name, &iter_info);
 
 		if (ret && (mask & ALL_FSNOTIFY_PERM_EVENTS))
 			goto out;
 
-		if (inode_mark)
-			iter_info.inode_mark =
-				fsnotify_next_mark(iter_info.inode_mark);
-		if (vfsmount_mark)
-			iter_info.vfsmount_mark =
-				fsnotify_next_mark(iter_info.vfsmount_mark);
+		if (iter_info.type_mask & FSNOTIFY_OBJ_TYPE_INODE_FL)
+			fsnotify_iter_set_inode_mark(&iter_info,
+				fsnotify_next_mark(iter_info.inode_mark));
+		else if (iter_info.inode_mark)
+			iter_info.type_mask |= FSNOTIFY_OBJ_TYPE_INODE_FL;
+
+		if (iter_info.type_mask & FSNOTIFY_OBJ_TYPE_VFSMOUNT_FL)
+			fsnotify_iter_set_vfsmount_mark(&iter_info,
+				fsnotify_next_mark(iter_info.vfsmount_mark));
+		else if (iter_info.vfsmount_mark)
+			iter_info.type_mask |= FSNOTIFY_OBJ_TYPE_VFSMOUNT_FL;
 	}
 	ret = 0;
 out:
diff --git a/fs/notify/fsnotify.h b/fs/notify/fsnotify.h
index 60f365dc1408..34515d2c4ba3 100644
--- a/fs/notify/fsnotify.h
+++ b/fs/notify/fsnotify.h
@@ -9,12 +9,6 @@ 
 
 #include "../mount.h"
 
-struct fsnotify_iter_info {
-	struct fsnotify_mark *inode_mark;
-	struct fsnotify_mark *vfsmount_mark;
-	int srcu_idx;
-};
-
 /* destroy all events sitting in this groups notification queue */
 extern void fsnotify_flush_notify(struct fsnotify_group *group);
 
diff --git a/fs/notify/inotify/inotify.h b/fs/notify/inotify/inotify.h
index c00d2caca894..7e4578d35b61 100644
--- a/fs/notify/inotify/inotify.h
+++ b/fs/notify/inotify/inotify.h
@@ -25,8 +25,6 @@  extern void inotify_ignored_and_remove_idr(struct fsnotify_mark *fsn_mark,
 					   struct fsnotify_group *group);
 extern int inotify_handle_event(struct fsnotify_group *group,
 				struct inode *inode,
-				struct fsnotify_mark *inode_mark,
-				struct fsnotify_mark *vfsmount_mark,
 				u32 mask, const void *data, int data_type,
 				const unsigned char *file_name, u32 cookie,
 				struct fsnotify_iter_info *iter_info);
diff --git a/fs/notify/inotify/inotify_fsnotify.c b/fs/notify/inotify/inotify_fsnotify.c
index 8b73332735ba..3193ef6c584d 100644
--- a/fs/notify/inotify/inotify_fsnotify.c
+++ b/fs/notify/inotify/inotify_fsnotify.c
@@ -65,12 +65,11 @@  static int inotify_merge(struct list_head *list,
 
 int inotify_handle_event(struct fsnotify_group *group,
 			 struct inode *inode,
-			 struct fsnotify_mark *inode_mark,
-			 struct fsnotify_mark *vfsmount_mark,
 			 u32 mask, const void *data, int data_type,
 			 const unsigned char *file_name, u32 cookie,
 			 struct fsnotify_iter_info *iter_info)
 {
+	struct fsnotify_mark *inode_mark = fsnotify_iter_inode_mark(iter_info);
 	struct inotify_inode_mark *i_mark;
 	struct inotify_event_info *event;
 	struct fsnotify_event *fsn_event;
@@ -78,7 +77,8 @@  int inotify_handle_event(struct fsnotify_group *group,
 	int len = 0;
 	int alloc_len = sizeof(struct inotify_event_info);
 
-	BUG_ON(vfsmount_mark);
+	if (WARN_ON(fsnotify_iter_vfsmount_mark(iter_info)))
+		return 0;
 
 	if ((inode_mark->mask & FS_EXCL_UNLINK) &&
 	    (data_type == FSNOTIFY_EVENT_PATH)) {
diff --git a/fs/notify/inotify/inotify_user.c b/fs/notify/inotify/inotify_user.c
index 2c908b31d6c9..0e44bf585a17 100644
--- a/fs/notify/inotify/inotify_user.c
+++ b/fs/notify/inotify/inotify_user.c
@@ -471,10 +471,12 @@  void inotify_ignored_and_remove_idr(struct fsnotify_mark *fsn_mark,
 				    struct fsnotify_group *group)
 {
 	struct inotify_inode_mark *i_mark;
+	struct fsnotify_iter_info iter_info = {};
 
 	/* Queue ignore event for the watch */
-	inotify_handle_event(group, NULL, fsn_mark, NULL, FS_IN_IGNORED,
-			     NULL, FSNOTIFY_EVENT_NONE, NULL, 0, NULL);
+	fsnotify_iter_set_inode_mark(&iter_info, fsn_mark);
+	inotify_handle_event(group, NULL, FS_IN_IGNORED, NULL,
+			     FSNOTIFY_EVENT_NONE, NULL, 0, &iter_info);
 
 	i_mark = container_of(fsn_mark, struct inotify_inode_mark, fsn_mark);
 	/* remove this mark from the idr */
diff --git a/include/linux/fsnotify_backend.h b/include/linux/fsnotify_backend.h
index 9c690eb692a2..62e05b8a9bc7 100644
--- a/include/linux/fsnotify_backend.h
+++ b/include/linux/fsnotify_backend.h
@@ -98,8 +98,6 @@  struct fsnotify_iter_info;
 struct fsnotify_ops {
 	int (*handle_event)(struct fsnotify_group *group,
 			    struct inode *inode,
-			    struct fsnotify_mark *inode_mark,
-			    struct fsnotify_mark *vfsmount_mark,
 			    u32 mask, const void *data, int data_type,
 			    const unsigned char *file_name, u32 cookie,
 			    struct fsnotify_iter_info *iter_info);
@@ -212,6 +210,35 @@  enum fsnotify_obj_type {
 #define FSNOTIFY_OBJ_TYPE_VFSMOUNT_FL	(1U << FSNOTIFY_OBJ_TYPE_VFSMOUNT)
 #define FSNOTIFY_OBJ_ALL_TYPES_MASK	((1U << FSNOTIFY_OBJ_TYPE_MAX) - 1)
 
+struct fsnotify_iter_info {
+	struct fsnotify_mark *inode_mark;
+	struct fsnotify_mark *vfsmount_mark;
+	unsigned int type_mask;
+	int srcu_idx;
+};
+
+#define FSNOTIFY_ITER_FUNCS(name, NAME) \
+static inline struct fsnotify_mark *fsnotify_iter_##name##_mark( \
+		struct fsnotify_iter_info *iter_info) \
+{ \
+	return (iter_info->type_mask & FSNOTIFY_OBJ_TYPE_##NAME##_FL) ? \
+		iter_info->name##_mark : NULL; \
+} \
+\
+static inline void fsnotify_iter_set_##name##_mark( \
+		struct fsnotify_iter_info *iter_info, \
+		struct fsnotify_mark *mark) \
+{ \
+	iter_info->name##_mark = mark; \
+	if (mark) \
+		iter_info->type_mask |= FSNOTIFY_OBJ_TYPE_##NAME##_FL; \
+	else \
+		iter_info->type_mask &= ~FSNOTIFY_OBJ_TYPE_##NAME##_FL; \
+}
+
+FSNOTIFY_ITER_FUNCS(inode, INODE)
+FSNOTIFY_ITER_FUNCS(vfsmount, VFSMOUNT)
+
 /*
  * Inode / vfsmount point to this structure which tracks all marks attached to
  * the inode / vfsmount. The reference to inode / vfsmount is held by this
diff --git a/kernel/audit_fsnotify.c b/kernel/audit_fsnotify.c
index 52f368b6561e..1b80ff8d6632 100644
--- a/kernel/audit_fsnotify.c
+++ b/kernel/audit_fsnotify.c
@@ -165,12 +165,11 @@  static void audit_autoremove_mark_rule(struct audit_fsnotify_mark *audit_mark)
 /* Update mark data in audit rules based on fsnotify events. */
 static int audit_mark_handle_event(struct fsnotify_group *group,
 				    struct inode *to_tell,
-				    struct fsnotify_mark *inode_mark,
-				    struct fsnotify_mark *vfsmount_mark,
 				    u32 mask, const void *data, int data_type,
 				    const unsigned char *dname, u32 cookie,
 				    struct fsnotify_iter_info *iter_info)
 {
+	struct fsnotify_mark *inode_mark = fsnotify_iter_inode_mark(iter_info);
 	struct audit_fsnotify_mark *audit_mark;
 	const struct inode *inode = NULL;
 
diff --git a/kernel/audit_tree.c b/kernel/audit_tree.c
index fd353120e0d9..f5b85c33f0cb 100644
--- a/kernel/audit_tree.c
+++ b/kernel/audit_tree.c
@@ -989,8 +989,6 @@  static void evict_chunk(struct audit_chunk *chunk)
 
 static int audit_tree_handle_event(struct fsnotify_group *group,
 				   struct inode *to_tell,
-				   struct fsnotify_mark *inode_mark,
-				   struct fsnotify_mark *vfsmount_mark,
 				   u32 mask, const void *data, int data_type,
 				   const unsigned char *file_name, u32 cookie,
 				   struct fsnotify_iter_info *iter_info)
diff --git a/kernel/audit_watch.c b/kernel/audit_watch.c
index 9eb8b3511636..43fcae4b0500 100644
--- a/kernel/audit_watch.c
+++ b/kernel/audit_watch.c
@@ -472,12 +472,11 @@  void audit_remove_watch_rule(struct audit_krule *krule)
 /* Update watch data in audit rules based on fsnotify events. */
 static int audit_watch_handle_event(struct fsnotify_group *group,
 				    struct inode *to_tell,
-				    struct fsnotify_mark *inode_mark,
-				    struct fsnotify_mark *vfsmount_mark,
 				    u32 mask, const void *data, int data_type,
 				    const unsigned char *dname, u32 cookie,
 				    struct fsnotify_iter_info *iter_info)
 {
+	struct fsnotify_mark *inode_mark = fsnotify_iter_inode_mark(iter_info);
 	const struct inode *inode;
 	struct audit_parent *parent;