linux-fsdevel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/6] Prepare for fanotify name events
@ 2020-01-14 15:16 Amir Goldstein
  2020-01-14 15:16 ` [PATCH 1/6] fsnotify: tidy up FS_ and FAN_ constants Amir Goldstein
                   ` (5 more replies)
  0 siblings, 6 replies; 7+ messages in thread
From: Amir Goldstein @ 2020-01-14 15:16 UTC (permalink / raw)
  To: Jan Kara; +Cc: linux-fsdevel

Jan,

Fanotify name event patches are ready at [1].
I am still updating the tests and the demo.

In the mean while, I wanted to post these few prep patches including
two bug fixes, so perhaps you will be able to process them in time for
v5.6-rc1.

Patches 1-4 are just cleanup and minor re-factoring in prep for the
name event patches.

Patches 5-6 are fixes for minor bug that I found during the work.
I improved ltp tests fanotify09 and fanotify15 to cover these bugs [2].

I did not mark those patches for stable, because backporting is not
trivial and the bugs are really minor.  For the same reason, I did
not bother to provide bug fix patches that are not dependent on the
cleanup patches, although that might have been possible to do.

Thanks,
Amir.

[1] https://github.com/amir73il/linux/commits/fanotify_name
[2] https://github.com/amir73il/ltp/commits/fsnotify-fixes

Amir Goldstein (6):
  fsnotify: tidy up FS_ and FAN_ constants
  fsnotify: pass dentry instead of inode for events possible on child
  fsnotify: simplify arguments passing to fsnotify_parent()
  fsnotify: replace inode pointer with tag
  fanotify: merge duplicate events on parent and child
  fanotify: fix merging marks masks with FAN_ONDIR

 fs/notify/fanotify/fanotify.c        | 36 ++++++++++++++++++++--------
 fs/notify/fsnotify.c                 | 21 ++++++++--------
 fs/notify/inotify/inotify_fsnotify.c |  2 +-
 include/linux/fsnotify.h             | 26 ++++++++------------
 include/linux/fsnotify_backend.h     | 34 +++++++++++++-------------
 include/uapi/linux/fanotify.h        |  4 ++--
 kernel/audit_fsnotify.c              |  5 +++-
 kernel/audit_watch.c                 |  5 +++-
 8 files changed, 73 insertions(+), 60 deletions(-)

-- 
2.17.1


^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PATCH 1/6] fsnotify: tidy up FS_ and FAN_ constants
  2020-01-14 15:16 [PATCH 0/6] Prepare for fanotify name events Amir Goldstein
@ 2020-01-14 15:16 ` Amir Goldstein
  2020-01-14 15:16 ` [PATCH 2/6] fsnotify: pass dentry instead of inode for events possible on child Amir Goldstein
                   ` (4 subsequent siblings)
  5 siblings, 0 replies; 7+ messages in thread
From: Amir Goldstein @ 2020-01-14 15:16 UTC (permalink / raw)
  To: Jan Kara; +Cc: linux-fsdevel

Order by value, so the free value ranges are easier to find.

Signed-off-by: Amir Goldstein <amir73il@gmail.com>
---
 include/linux/fsnotify_backend.h | 11 +++++------
 include/uapi/linux/fanotify.h    |  4 ++--
 2 files changed, 7 insertions(+), 8 deletions(-)

diff --git a/include/linux/fsnotify_backend.h b/include/linux/fsnotify_backend.h
index 1915bdba2fad..db3cabb4600e 100644
--- a/include/linux/fsnotify_backend.h
+++ b/include/linux/fsnotify_backend.h
@@ -49,16 +49,15 @@
 #define FS_OPEN_EXEC_PERM	0x00040000	/* open/exec event in a permission hook */
 
 #define FS_EXCL_UNLINK		0x04000000	/* do not send events if object is unlinked */
-#define FS_ISDIR		0x40000000	/* event occurred against dir */
-#define FS_IN_ONESHOT		0x80000000	/* only send event once */
-
-#define FS_DN_RENAME		0x10000000	/* file renamed */
-#define FS_DN_MULTISHOT		0x20000000	/* dnotify multishot */
-
 /* This inode cares about things that happen to its children.  Always set for
  * dnotify and inotify. */
 #define FS_EVENT_ON_CHILD	0x08000000
 
+#define FS_DN_RENAME		0x10000000	/* file renamed */
+#define FS_DN_MULTISHOT		0x20000000	/* dnotify multishot */
+#define FS_ISDIR		0x40000000	/* event occurred against dir */
+#define FS_IN_ONESHOT		0x80000000	/* only send event once */
+
 #define FS_MOVE			(FS_MOVED_FROM | FS_MOVED_TO)
 
 /*
diff --git a/include/uapi/linux/fanotify.h b/include/uapi/linux/fanotify.h
index b9effa6f8503..2a1844edda47 100644
--- a/include/uapi/linux/fanotify.h
+++ b/include/uapi/linux/fanotify.h
@@ -25,9 +25,9 @@
 #define FAN_ACCESS_PERM		0x00020000	/* File accessed in perm check */
 #define FAN_OPEN_EXEC_PERM	0x00040000	/* File open/exec in perm check */
 
-#define FAN_ONDIR		0x40000000	/* event occurred against dir */
+#define FAN_EVENT_ON_CHILD	0x08000000	/* Interested in child events */
 
-#define FAN_EVENT_ON_CHILD	0x08000000	/* interested in child events */
+#define FAN_ONDIR		0x40000000	/* Event occurred against dir */
 
 /* helper events */
 #define FAN_CLOSE		(FAN_CLOSE_WRITE | FAN_CLOSE_NOWRITE) /* close */
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH 2/6] fsnotify: pass dentry instead of inode for events possible on child
  2020-01-14 15:16 [PATCH 0/6] Prepare for fanotify name events Amir Goldstein
  2020-01-14 15:16 ` [PATCH 1/6] fsnotify: tidy up FS_ and FAN_ constants Amir Goldstein
@ 2020-01-14 15:16 ` Amir Goldstein
  2020-01-14 15:16 ` [PATCH 3/6] fsnotify: simplify arguments passing to fsnotify_parent() Amir Goldstein
                   ` (3 subsequent siblings)
  5 siblings, 0 replies; 7+ messages in thread
From: Amir Goldstein @ 2020-01-14 15:16 UTC (permalink / raw)
  To: Jan Kara; +Cc: linux-fsdevel

Most events that can be reported to watching parent pass
FSNOTIFY_EVENT_PATH as event data, except for FS_ARRTIB and FS_MODIFY
as a result of truncate.

Define a new data type to pass for event - FSNOTIFY_EVENT_DENTRY
and use it to pass the dentry instead of it's ->d_inode for those events.

Add a helper fsnotify_dentry(), similar to fsnotify_path() to report
those events to child and parent.

Soon, we are going to use the dentry data type to report events
with name info in fanotify backend.

Signed-off-by: Amir Goldstein <amir73il@gmail.com>
---
 fs/notify/fanotify/fanotify.c    |  2 ++
 fs/notify/fsnotify.c             |  4 ++--
 include/linux/fsnotify.h         | 16 ++++++++++------
 include/linux/fsnotify_backend.h |  3 ++-
 kernel/audit_fsnotify.c          |  5 ++++-
 kernel/audit_watch.c             |  5 ++++-
 6 files changed, 24 insertions(+), 11 deletions(-)

diff --git a/fs/notify/fanotify/fanotify.c b/fs/notify/fanotify/fanotify.c
index 5778d1347b35..b4cd90afece1 100644
--- a/fs/notify/fanotify/fanotify.c
+++ b/fs/notify/fanotify/fanotify.c
@@ -271,6 +271,8 @@ static struct inode *fanotify_fid_inode(struct inode *to_tell, u32 event_mask,
 		return to_tell;
 	else if (data_type == FSNOTIFY_EVENT_INODE)
 		return (struct inode *)data;
+	else if (data_type == FSNOTIFY_EVENT_DENTRY)
+		return d_inode(data);
 	else if (data_type == FSNOTIFY_EVENT_PATH)
 		return d_inode(((struct path *)data)->dentry);
 	return NULL;
diff --git a/fs/notify/fsnotify.c b/fs/notify/fsnotify.c
index 46f225580009..13578372aee8 100644
--- a/fs/notify/fsnotify.c
+++ b/fs/notify/fsnotify.c
@@ -172,8 +172,8 @@ int __fsnotify_parent(const struct path *path, struct dentry *dentry, __u32 mask
 			ret = fsnotify(p_inode, mask, path, FSNOTIFY_EVENT_PATH,
 				       &name.name, 0);
 		else
-			ret = fsnotify(p_inode, mask, dentry->d_inode, FSNOTIFY_EVENT_INODE,
-				       &name.name, 0);
+			ret = fsnotify(p_inode, mask, dentry,
+				       FSNOTIFY_EVENT_DENTRY, &name.name, 0);
 		release_dentry_name_snapshot(&name);
 	}
 
diff --git a/include/linux/fsnotify.h b/include/linux/fsnotify.h
index a2d5d175d3c1..5746420bb121 100644
--- a/include/linux/fsnotify.h
+++ b/include/linux/fsnotify.h
@@ -41,9 +41,15 @@ static inline int fsnotify_parent(const struct path *path,
 }
 
 /*
- * Simple wrapper to consolidate calls fsnotify_parent()/fsnotify() when
- * an event is on a path.
+ * Simple wrappers to consolidate calls fsnotify_parent()/fsnotify() when
+ * an event is on a path/dentry.
  */
+static inline void fsnotify_dentry(struct dentry *dentry, __u32 mask)
+{
+	fsnotify_parent(NULL, dentry, mask);
+	fsnotify(d_inode(dentry), mask, dentry, FSNOTIFY_EVENT_DENTRY, NULL, 0);
+}
+
 static inline int fsnotify_path(struct inode *inode, const struct path *path,
 				__u32 mask)
 {
@@ -301,8 +307,7 @@ static inline void fsnotify_xattr(struct dentry *dentry)
 	if (S_ISDIR(inode->i_mode))
 		mask |= FS_ISDIR;
 
-	fsnotify_parent(NULL, dentry, mask);
-	fsnotify(inode, mask, inode, FSNOTIFY_EVENT_INODE, NULL, 0);
+	fsnotify_dentry(dentry, mask);
 }
 
 /*
@@ -336,8 +341,7 @@ static inline void fsnotify_change(struct dentry *dentry, unsigned int ia_valid)
 		if (S_ISDIR(inode->i_mode))
 			mask |= FS_ISDIR;
 
-		fsnotify_parent(NULL, dentry, mask);
-		fsnotify(inode, mask, inode, FSNOTIFY_EVENT_INODE, NULL, 0);
+		fsnotify_dentry(dentry, mask);
 	}
 }
 
diff --git a/include/linux/fsnotify_backend.h b/include/linux/fsnotify_backend.h
index db3cabb4600e..cb47759b1ce9 100644
--- a/include/linux/fsnotify_backend.h
+++ b/include/linux/fsnotify_backend.h
@@ -212,10 +212,11 @@ struct fsnotify_group {
 	};
 };
 
-/* when calling fsnotify tell it if the data is a path or inode */
+/* when calling fsnotify tell it if the data is a path or inode or dentry */
 #define FSNOTIFY_EVENT_NONE	0
 #define FSNOTIFY_EVENT_PATH	1
 #define FSNOTIFY_EVENT_INODE	2
+#define FSNOTIFY_EVENT_DENTRY	3
 
 enum fsnotify_obj_type {
 	FSNOTIFY_OBJ_TYPE_INODE,
diff --git a/kernel/audit_fsnotify.c b/kernel/audit_fsnotify.c
index f0d243318452..ec6d00fd11b4 100644
--- a/kernel/audit_fsnotify.c
+++ b/kernel/audit_fsnotify.c
@@ -168,7 +168,10 @@ static int audit_mark_handle_event(struct fsnotify_group *group,
 
 	switch (data_type) {
 	case (FSNOTIFY_EVENT_PATH):
-		inode = ((const struct path *)data)->dentry->d_inode;
+		inode = d_inode(((const struct path *)data)->dentry);
+		break;
+	case (FSNOTIFY_EVENT_DENTRY):
+		inode = d_inode(data);
 		break;
 	case (FSNOTIFY_EVENT_INODE):
 		inode = (const struct inode *)data;
diff --git a/kernel/audit_watch.c b/kernel/audit_watch.c
index 4508d5e0cf69..85e007184677 100644
--- a/kernel/audit_watch.c
+++ b/kernel/audit_watch.c
@@ -482,7 +482,10 @@ static int audit_watch_handle_event(struct fsnotify_group *group,
 
 	switch (data_type) {
 	case (FSNOTIFY_EVENT_PATH):
-		inode = d_backing_inode(((const struct path *)data)->dentry);
+		inode = d_inode(((const struct path *)data)->dentry);
+		break;
+	case (FSNOTIFY_EVENT_DENTRY):
+		inode = d_inode(data);
 		break;
 	case (FSNOTIFY_EVENT_INODE):
 		inode = (const struct inode *)data;
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH 3/6] fsnotify: simplify arguments passing to fsnotify_parent()
  2020-01-14 15:16 [PATCH 0/6] Prepare for fanotify name events Amir Goldstein
  2020-01-14 15:16 ` [PATCH 1/6] fsnotify: tidy up FS_ and FAN_ constants Amir Goldstein
  2020-01-14 15:16 ` [PATCH 2/6] fsnotify: pass dentry instead of inode for events possible on child Amir Goldstein
@ 2020-01-14 15:16 ` Amir Goldstein
  2020-01-14 15:16 ` [PATCH 4/6] fsnotify: replace inode pointer with tag Amir Goldstein
                   ` (2 subsequent siblings)
  5 siblings, 0 replies; 7+ messages in thread
From: Amir Goldstein @ 2020-01-14 15:16 UTC (permalink / raw)
  To: Jan Kara; +Cc: linux-fsdevel

Instead of passing both dentry and path and having to figure out which
one to use, use the data/data_type convention to simplify the code.

Signed-off-by: Amir Goldstein <amir73il@gmail.com>
---
 fs/notify/fsnotify.c             | 21 ++++++++++-----------
 include/linux/fsnotify.h         | 14 ++------------
 include/linux/fsnotify_backend.h | 12 ++++++------
 3 files changed, 18 insertions(+), 29 deletions(-)

diff --git a/fs/notify/fsnotify.c b/fs/notify/fsnotify.c
index 13578372aee8..a8b281569bbf 100644
--- a/fs/notify/fsnotify.c
+++ b/fs/notify/fsnotify.c
@@ -143,14 +143,18 @@ void __fsnotify_update_child_dentry_flags(struct inode *inode)
 }
 
 /* Notify this dentry's parent about a child's events. */
-int __fsnotify_parent(const struct path *path, struct dentry *dentry, __u32 mask)
+int fsnotify_parent(__u32 mask, const void *data, int data_type)
 {
-	struct dentry *parent;
+	struct dentry *parent, *dentry;
 	struct inode *p_inode;
 	int ret = 0;
 
-	if (!dentry)
-		dentry = path->dentry;
+	if (data_type == FSNOTIFY_EVENT_DENTRY)
+		dentry = (struct dentry *)data;
+	else if (data_type == FSNOTIFY_EVENT_PATH)
+		dentry = ((struct path *)data)->dentry;
+	else
+		return 0;
 
 	if (!(dentry->d_flags & DCACHE_FSNOTIFY_PARENT_WATCHED))
 		return 0;
@@ -168,12 +172,7 @@ int __fsnotify_parent(const struct path *path, struct dentry *dentry, __u32 mask
 		mask |= FS_EVENT_ON_CHILD;
 
 		take_dentry_name_snapshot(&name, dentry);
-		if (path)
-			ret = fsnotify(p_inode, mask, path, FSNOTIFY_EVENT_PATH,
-				       &name.name, 0);
-		else
-			ret = fsnotify(p_inode, mask, dentry,
-				       FSNOTIFY_EVENT_DENTRY, &name.name, 0);
+		ret = fsnotify(p_inode, mask, data, data_type, &name.name, 0);
 		release_dentry_name_snapshot(&name);
 	}
 
@@ -181,7 +180,7 @@ int __fsnotify_parent(const struct path *path, struct dentry *dentry, __u32 mask
 
 	return ret;
 }
-EXPORT_SYMBOL_GPL(__fsnotify_parent);
+EXPORT_SYMBOL_GPL(fsnotify_parent);
 
 static int send_to_group(struct inode *to_tell,
 			 __u32 mask, const void *data,
diff --git a/include/linux/fsnotify.h b/include/linux/fsnotify.h
index 5746420bb121..dfdc8a1a3c38 100644
--- a/include/linux/fsnotify.h
+++ b/include/linux/fsnotify.h
@@ -30,30 +30,20 @@ static inline int fsnotify_dirent(struct inode *dir, struct dentry *dentry,
 			&dentry->d_name, 0);
 }
 
-/* Notify this dentry's parent about a child's events. */
-static inline int fsnotify_parent(const struct path *path,
-				  struct dentry *dentry, __u32 mask)
-{
-	if (!dentry)
-		dentry = path->dentry;
-
-	return __fsnotify_parent(path, dentry, mask);
-}
-
 /*
  * Simple wrappers to consolidate calls fsnotify_parent()/fsnotify() when
  * an event is on a path/dentry.
  */
 static inline void fsnotify_dentry(struct dentry *dentry, __u32 mask)
 {
-	fsnotify_parent(NULL, dentry, mask);
+	fsnotify_parent(mask, dentry, FSNOTIFY_EVENT_DENTRY);
 	fsnotify(d_inode(dentry), mask, dentry, FSNOTIFY_EVENT_DENTRY, NULL, 0);
 }
 
 static inline int fsnotify_path(struct inode *inode, const struct path *path,
 				__u32 mask)
 {
-	int ret = fsnotify_parent(path, NULL, mask);
+	int ret = fsnotify_parent(mask, path, FSNOTIFY_EVENT_PATH);
 
 	if (ret)
 		return ret;
diff --git a/include/linux/fsnotify_backend.h b/include/linux/fsnotify_backend.h
index cb47759b1ce9..77edd866926f 100644
--- a/include/linux/fsnotify_backend.h
+++ b/include/linux/fsnotify_backend.h
@@ -351,9 +351,9 @@ struct fsnotify_mark {
 /* called from the vfs helpers */
 
 /* main fsnotify call to send events */
-extern int fsnotify(struct inode *to_tell, __u32 mask, const void *data, int data_is,
-		    const struct qstr *name, u32 cookie);
-extern int __fsnotify_parent(const struct path *path, struct dentry *dentry, __u32 mask);
+extern int fsnotify(struct inode *to_tell, __u32 mask, const void *data,
+		    int data_type, const struct qstr *name, u32 cookie);
+extern int fsnotify_parent(__u32 mask, const void *data, int data_type);
 extern void __fsnotify_inode_delete(struct inode *inode);
 extern void __fsnotify_vfsmount_delete(struct vfsmount *mnt);
 extern void fsnotify_sb_delete(struct super_block *sb);
@@ -508,13 +508,13 @@ static inline void fsnotify_init_event(struct fsnotify_event *event,
 
 #else
 
-static inline int fsnotify(struct inode *to_tell, __u32 mask, const void *data, int data_is,
-			   const struct qstr *name, u32 cookie)
+static inline int fsnotify(struct inode *to_tell, __u32 mask, const void *data,
+			   int data_type, const struct qstr *name, u32 cookie)
 {
 	return 0;
 }
 
-static inline int __fsnotify_parent(const struct path *path, struct dentry *dentry, __u32 mask)
+static inline int fsnotify_parent(__u32 mask, const void *data, int data_type)
 {
 	return 0;
 }
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH 4/6] fsnotify: replace inode pointer with tag
  2020-01-14 15:16 [PATCH 0/6] Prepare for fanotify name events Amir Goldstein
                   ` (2 preceding siblings ...)
  2020-01-14 15:16 ` [PATCH 3/6] fsnotify: simplify arguments passing to fsnotify_parent() Amir Goldstein
@ 2020-01-14 15:16 ` Amir Goldstein
  2020-01-14 15:16 ` [PATCH 5/6] fanotify: merge duplicate events on parent and child Amir Goldstein
  2020-01-14 15:16 ` [PATCH 6/6] fanotify: fix merging marks masks with FAN_ONDIR Amir Goldstein
  5 siblings, 0 replies; 7+ messages in thread
From: Amir Goldstein @ 2020-01-14 15:16 UTC (permalink / raw)
  To: Jan Kara; +Cc: linux-fsdevel

The event inode field is used only for comparison in queue merges and
cannot be dereferenced after handle_event(), because it does not hold a
refcount on the inode.

Replace it with an abstract tag do to the same thing. We are going to
set this tag for values other than inode pointer in fanotify.

Signed-off-by: Amir Goldstein <amir73il@gmail.com>
---
 fs/notify/fanotify/fanotify.c        | 2 +-
 fs/notify/inotify/inotify_fsnotify.c | 2 +-
 include/linux/fsnotify_backend.h     | 8 +++-----
 3 files changed, 5 insertions(+), 7 deletions(-)

diff --git a/fs/notify/fanotify/fanotify.c b/fs/notify/fanotify/fanotify.c
index b4cd90afece1..34454390e4b6 100644
--- a/fs/notify/fanotify/fanotify.c
+++ b/fs/notify/fanotify/fanotify.c
@@ -26,7 +26,7 @@ static bool should_merge(struct fsnotify_event *old_fsn,
 	old = FANOTIFY_E(old_fsn);
 	new = FANOTIFY_E(new_fsn);
 
-	if (old_fsn->inode != new_fsn->inode || old->pid != new->pid ||
+	if (old_fsn->tag != new_fsn->tag || old->pid != new->pid ||
 	    old->fh_type != new->fh_type || old->fh_len != new->fh_len)
 		return false;
 
diff --git a/fs/notify/inotify/inotify_fsnotify.c b/fs/notify/inotify/inotify_fsnotify.c
index d510223d302c..cbaaec234fcd 100644
--- a/fs/notify/inotify/inotify_fsnotify.c
+++ b/fs/notify/inotify/inotify_fsnotify.c
@@ -39,7 +39,7 @@ static bool event_compare(struct fsnotify_event *old_fsn,
 	if (old->mask & FS_IN_IGNORED)
 		return false;
 	if ((old->mask == new->mask) &&
-	    (old_fsn->inode == new_fsn->inode) &&
+	    (old_fsn->tag == new_fsn->tag) &&
 	    (old->name_len == new->name_len) &&
 	    (!old->name_len || !strcmp(old->name, new->name)))
 		return true;
diff --git a/include/linux/fsnotify_backend.h b/include/linux/fsnotify_backend.h
index 77edd866926f..caf8bbc1be08 100644
--- a/include/linux/fsnotify_backend.h
+++ b/include/linux/fsnotify_backend.h
@@ -132,8 +132,7 @@ struct fsnotify_ops {
  */
 struct fsnotify_event {
 	struct list_head list;
-	/* inode may ONLY be dereferenced during handle_event(). */
-	struct inode *inode;	/* either the inode the event happened to or its parent */
+	unsigned long tag;	/* identifier for queue merges */
 };
 
 /*
@@ -499,11 +498,10 @@ extern void fsnotify_put_mark(struct fsnotify_mark *mark);
 extern void fsnotify_finish_user_wait(struct fsnotify_iter_info *iter_info);
 extern bool fsnotify_prepare_user_wait(struct fsnotify_iter_info *iter_info);
 
-static inline void fsnotify_init_event(struct fsnotify_event *event,
-				       struct inode *inode)
+static inline void fsnotify_init_event(struct fsnotify_event *event, void *tag)
 {
 	INIT_LIST_HEAD(&event->list);
-	event->inode = inode;
+	event->tag = (unsigned long)tag;
 }
 
 #else
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH 5/6] fanotify: merge duplicate events on parent and child
  2020-01-14 15:16 [PATCH 0/6] Prepare for fanotify name events Amir Goldstein
                   ` (3 preceding siblings ...)
  2020-01-14 15:16 ` [PATCH 4/6] fsnotify: replace inode pointer with tag Amir Goldstein
@ 2020-01-14 15:16 ` Amir Goldstein
  2020-01-14 15:16 ` [PATCH 6/6] fanotify: fix merging marks masks with FAN_ONDIR Amir Goldstein
  5 siblings, 0 replies; 7+ messages in thread
From: Amir Goldstein @ 2020-01-14 15:16 UTC (permalink / raw)
  To: Jan Kara; +Cc: linux-fsdevel

With inotify, when a watch is set on a directory and on its child, an
event on the child is reported twice, once with wd of the parent watch
and once with wd of the child watch without the filename.

With fanotify, when a watch is set on a directory and on its child, an
event on the child is reported twice, but it has the exact same
information - either an open file descriptor of the child or an encoded
fid of the child.

The reason that the two identical events are not merged is because the
tag used for merging events in the queue is the child inode in one event
and parent inode in the other.

For events with path or dentry data, use the dentry instead of inode as
the tag for event merging, so that the event reported on parent will be
merged with the event reported on the child.

Signed-off-by: Amir Goldstein <amir73il@gmail.com>
---
 fs/notify/fanotify/fanotify.c | 23 +++++++++++++++++------
 1 file changed, 17 insertions(+), 6 deletions(-)

diff --git a/fs/notify/fanotify/fanotify.c b/fs/notify/fanotify/fanotify.c
index 34454390e4b6..2ae82040f26f 100644
--- a/fs/notify/fanotify/fanotify.c
+++ b/fs/notify/fanotify/fanotify.c
@@ -265,17 +265,21 @@ static int fanotify_encode_fid(struct fanotify_event *event,
  * FS_CREATE reports the modified dir inode and not the created inode.
  */
 static struct inode *fanotify_fid_inode(struct inode *to_tell, u32 event_mask,
-					const void *data, int data_type)
+					const void *data, int data_type,
+					struct dentry **pdentry)
 {
 	if (event_mask & ALL_FSNOTIFY_DIRENT_EVENTS)
 		return to_tell;
 	else if (data_type == FSNOTIFY_EVENT_INODE)
 		return (struct inode *)data;
 	else if (data_type == FSNOTIFY_EVENT_DENTRY)
-		return d_inode(data);
+		*pdentry = (struct dentry *)data;
 	else if (data_type == FSNOTIFY_EVENT_PATH)
-		return d_inode(((struct path *)data)->dentry);
-	return NULL;
+		*pdentry = ((struct path *)data)->dentry;
+	else
+		return NULL;
+
+	return d_inode(*pdentry);
 }
 
 struct fanotify_event *fanotify_alloc_event(struct fsnotify_group *group,
@@ -285,7 +289,9 @@ struct fanotify_event *fanotify_alloc_event(struct fsnotify_group *group,
 {
 	struct fanotify_event *event = NULL;
 	gfp_t gfp = GFP_KERNEL_ACCOUNT;
-	struct inode *id = fanotify_fid_inode(inode, mask, data, data_type);
+	struct dentry *dentry = NULL;
+	struct inode *id = fanotify_fid_inode(inode, mask, data, data_type,
+					      &dentry);
 
 	/*
 	 * For queues with unlimited length lost events are not expected and
@@ -316,7 +322,12 @@ struct fanotify_event *fanotify_alloc_event(struct fsnotify_group *group,
 	if (!event)
 		goto out;
 init: __maybe_unused
-	fsnotify_init_event(&event->fse, inode);
+	/*
+	 * Use the dentry instead of inode as tag for event queue, so event
+	 * reported on parent is merged with event reported on child when both
+	 * directory and child watches exist.
+	 */
+	fsnotify_init_event(&event->fse, (void *)dentry ?: inode);
 	event->mask = mask;
 	if (FAN_GROUP_FLAG(group, FAN_REPORT_TID))
 		event->pid = get_pid(task_pid(current));
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH 6/6] fanotify: fix merging marks masks with FAN_ONDIR
  2020-01-14 15:16 [PATCH 0/6] Prepare for fanotify name events Amir Goldstein
                   ` (4 preceding siblings ...)
  2020-01-14 15:16 ` [PATCH 5/6] fanotify: merge duplicate events on parent and child Amir Goldstein
@ 2020-01-14 15:16 ` Amir Goldstein
  5 siblings, 0 replies; 7+ messages in thread
From: Amir Goldstein @ 2020-01-14 15:16 UTC (permalink / raw)
  To: Jan Kara; +Cc: linux-fsdevel

Change the logic of FAN_ONDIR in two ways that are similar to the logic
of FAN_EVENT_ON_CHILD, that was fixed in commit 54a307ba8d3c ("fanotify:
fix logic of events on child"):

1. The flag is meaningless in ignore mask
2. The flag refers only to events in the mask of the mark where it is set

This is what the fanotify_mark.2 man page says about FAN_ONDIR:
"Without this flag, only events for files are created."  It doesn't
say anything about setting this flag in ignore mask to stop getting
events on directories nor can I think of any setup where this capability
would be useful.

Currently, when marks masks are merged, the FAN_ONDIR flag set in one
mark affects the events that are set in another mark's mask and this
behavior causes unexpected results.  For example, a user adds a mark on a
directory with mask FAN_ATTRIB | FAN_ONDIR and a mount mark with mask
FAN_OPEN (without FAN_ONDIR).  An opendir() of that directory (which is
inside that mount) generates a FAN_OPEN event even though neither of the
marks requested to get open events on directories.

Signed-off-by: Amir Goldstein <amir73il@gmail.com>
---
 fs/notify/fanotify/fanotify.c | 11 +++++++----
 1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/fs/notify/fanotify/fanotify.c b/fs/notify/fanotify/fanotify.c
index 2ae82040f26f..a98ee4340eaa 100644
--- a/fs/notify/fanotify/fanotify.c
+++ b/fs/notify/fanotify/fanotify.c
@@ -171,6 +171,13 @@ static u32 fanotify_group_event_mask(struct fsnotify_group *group,
 		if (!fsnotify_iter_should_report_type(iter_info, type))
 			continue;
 		mark = iter_info->marks[type];
+		/*
+		 * If the event is on dir and this mark doesn't care about
+		 * events on dir, don't send it!
+		 */
+		if (event_mask & FS_ISDIR && !(mark->mask & FS_ISDIR))
+			continue;
+
 		/*
 		 * If the event is for a child and this mark doesn't care about
 		 * events on a child, don't send it!
@@ -203,10 +210,6 @@ static u32 fanotify_group_event_mask(struct fsnotify_group *group,
 		user_mask &= ~FAN_ONDIR;
 	}
 
-	if (event_mask & FS_ISDIR &&
-	    !(marks_mask & FS_ISDIR & ~marks_ignored_mask))
-		return 0;
-
 	return test_mask & user_mask;
 }
 
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2020-01-14 15:17 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-01-14 15:16 [PATCH 0/6] Prepare for fanotify name events Amir Goldstein
2020-01-14 15:16 ` [PATCH 1/6] fsnotify: tidy up FS_ and FAN_ constants Amir Goldstein
2020-01-14 15:16 ` [PATCH 2/6] fsnotify: pass dentry instead of inode for events possible on child Amir Goldstein
2020-01-14 15:16 ` [PATCH 3/6] fsnotify: simplify arguments passing to fsnotify_parent() Amir Goldstein
2020-01-14 15:16 ` [PATCH 4/6] fsnotify: replace inode pointer with tag Amir Goldstein
2020-01-14 15:16 ` [PATCH 5/6] fanotify: merge duplicate events on parent and child Amir Goldstein
2020-01-14 15:16 ` [PATCH 6/6] fanotify: fix merging marks masks with FAN_ONDIR Amir Goldstein

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).