All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2 0/4] Performance optimization for no fsnotify marks
@ 2021-08-10 15:12 Amir Goldstein
  2021-08-10 15:12 ` [PATCH v2 1/4] fsnotify: replace igrab() with ihold() on attach connector Amir Goldstein
                   ` (4 more replies)
  0 siblings, 5 replies; 9+ messages in thread
From: Amir Goldstein @ 2021-08-10 15:12 UTC (permalink / raw)
  To: Jan Kara; +Cc: Matthew Bobrowski, linux-fsdevel

Hi Jan,

Following v2 addresses review comments from v1 [1]

[1] https://lore.kernel.org/linux-fsdevel/20210803180344.2398374-1-amir73il@gmail.com/

Changes since v1:
- Rebase on 5.14-rc5 + pidfd patches
- Added RVB
- Helper to get connector's sb (Matthew)
- Fix deadlock bug on umount (Jan)

Amir Goldstein (4):
  fsnotify: replace igrab() with ihold() on attach connector
  fsnotify: count s_fsnotify_inode_refs for attached connectors
  fsnotify: count all objects with attached connectors
  fsnotify: optimize the case of no marks of any type

 fs/notify/fsnotify.c     |  6 ++---
 fs/notify/fsnotify.h     | 15 ++++++++++++
 fs/notify/mark.c         | 52 ++++++++++++++++++++++++++++++----------
 include/linux/fs.h       |  4 ++--
 include/linux/fsnotify.h |  9 +++++++
 5 files changed, 69 insertions(+), 17 deletions(-)

-- 
2.32.0


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

* [PATCH v2 1/4] fsnotify: replace igrab() with ihold() on attach connector
  2021-08-10 15:12 [PATCH v2 0/4] Performance optimization for no fsnotify marks Amir Goldstein
@ 2021-08-10 15:12 ` Amir Goldstein
  2021-08-10 15:12 ` [PATCH v2 2/4] fsnotify: count s_fsnotify_inode_refs for attached connectors Amir Goldstein
                   ` (3 subsequent siblings)
  4 siblings, 0 replies; 9+ messages in thread
From: Amir Goldstein @ 2021-08-10 15:12 UTC (permalink / raw)
  To: Jan Kara; +Cc: Matthew Bobrowski, linux-fsdevel, Matthew Bobrowski

We must have a reference on inode, so ihold is cheaper.

Reviewed-by: Matthew Bobrowski <repnop@google.com>
Signed-off-by: Amir Goldstein <amir73il@gmail.com>
---
 fs/notify/mark.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/fs/notify/mark.c b/fs/notify/mark.c
index d32ab349db74..80459db58f63 100644
--- a/fs/notify/mark.c
+++ b/fs/notify/mark.c
@@ -493,8 +493,11 @@ static int fsnotify_attach_connector_to_object(fsnotify_connp_t *connp,
 		conn->fsid.val[0] = conn->fsid.val[1] = 0;
 		conn->flags = 0;
 	}
-	if (conn->type == FSNOTIFY_OBJ_TYPE_INODE)
-		inode = igrab(fsnotify_conn_inode(conn));
+	if (conn->type == FSNOTIFY_OBJ_TYPE_INODE) {
+		inode = fsnotify_conn_inode(conn);
+		ihold(inode);
+	}
+
 	/*
 	 * cmpxchg() provides the barrier so that readers of *connp can see
 	 * only initialized structure
-- 
2.32.0


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

* [PATCH v2 2/4] fsnotify: count s_fsnotify_inode_refs for attached connectors
  2021-08-10 15:12 [PATCH v2 0/4] Performance optimization for no fsnotify marks Amir Goldstein
  2021-08-10 15:12 ` [PATCH v2 1/4] fsnotify: replace igrab() with ihold() on attach connector Amir Goldstein
@ 2021-08-10 15:12 ` Amir Goldstein
  2021-08-10 15:12 ` [PATCH v2 3/4] fsnotify: count all objects with " Amir Goldstein
                   ` (2 subsequent siblings)
  4 siblings, 0 replies; 9+ messages in thread
From: Amir Goldstein @ 2021-08-10 15:12 UTC (permalink / raw)
  To: Jan Kara; +Cc: Matthew Bobrowski, linux-fsdevel, Matthew Bobrowski

Instead of incrementing s_fsnotify_inode_refs when detaching connector
from inode, increment it earlier when attaching connector to inode.
Next patch is going to use s_fsnotify_inode_refs to count all objects
with attached connectors.

Reviewed-by: Matthew Bobrowski <repnop@google.com>
Signed-off-by: Amir Goldstein <amir73il@gmail.com>
---
 fs/notify/mark.c | 29 ++++++++++++++++++-----------
 1 file changed, 18 insertions(+), 11 deletions(-)

diff --git a/fs/notify/mark.c b/fs/notify/mark.c
index 80459db58f63..2d8c46e1167d 100644
--- a/fs/notify/mark.c
+++ b/fs/notify/mark.c
@@ -169,6 +169,21 @@ static void fsnotify_connector_destroy_workfn(struct work_struct *work)
 	}
 }
 
+static void fsnotify_get_inode_ref(struct inode *inode)
+{
+	ihold(inode);
+	atomic_long_inc(&inode->i_sb->s_fsnotify_inode_refs);
+}
+
+static void fsnotify_put_inode_ref(struct inode *inode)
+{
+	struct super_block *sb = inode->i_sb;
+
+	iput(inode);
+	if (atomic_long_dec_and_test(&sb->s_fsnotify_inode_refs))
+		wake_up_var(&sb->s_fsnotify_inode_refs);
+}
+
 static void *fsnotify_detach_connector_from_object(
 					struct fsnotify_mark_connector *conn,
 					unsigned int *type)
@@ -182,7 +197,6 @@ static void *fsnotify_detach_connector_from_object(
 	if (conn->type == FSNOTIFY_OBJ_TYPE_INODE) {
 		inode = fsnotify_conn_inode(conn);
 		inode->i_fsnotify_mask = 0;
-		atomic_long_inc(&inode->i_sb->s_fsnotify_inode_refs);
 	} else if (conn->type == FSNOTIFY_OBJ_TYPE_VFSMOUNT) {
 		fsnotify_conn_mount(conn)->mnt_fsnotify_mask = 0;
 	} else if (conn->type == FSNOTIFY_OBJ_TYPE_SB) {
@@ -209,19 +223,12 @@ static void fsnotify_final_mark_destroy(struct fsnotify_mark *mark)
 /* Drop object reference originally held by a connector */
 static void fsnotify_drop_object(unsigned int type, void *objp)
 {
-	struct inode *inode;
-	struct super_block *sb;
-
 	if (!objp)
 		return;
 	/* Currently only inode references are passed to be dropped */
 	if (WARN_ON_ONCE(type != FSNOTIFY_OBJ_TYPE_INODE))
 		return;
-	inode = objp;
-	sb = inode->i_sb;
-	iput(inode);
-	if (atomic_long_dec_and_test(&sb->s_fsnotify_inode_refs))
-		wake_up_var(&sb->s_fsnotify_inode_refs);
+	fsnotify_put_inode_ref(objp);
 }
 
 void fsnotify_put_mark(struct fsnotify_mark *mark)
@@ -495,7 +502,7 @@ static int fsnotify_attach_connector_to_object(fsnotify_connp_t *connp,
 	}
 	if (conn->type == FSNOTIFY_OBJ_TYPE_INODE) {
 		inode = fsnotify_conn_inode(conn);
-		ihold(inode);
+		fsnotify_get_inode_ref(inode);
 	}
 
 	/*
@@ -505,7 +512,7 @@ static int fsnotify_attach_connector_to_object(fsnotify_connp_t *connp,
 	if (cmpxchg(connp, NULL, conn)) {
 		/* Someone else created list structure for us */
 		if (inode)
-			iput(inode);
+			fsnotify_put_inode_ref(inode);
 		kmem_cache_free(fsnotify_mark_connector_cachep, conn);
 	}
 
-- 
2.32.0


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

* [PATCH v2 3/4] fsnotify: count all objects with attached connectors
  2021-08-10 15:12 [PATCH v2 0/4] Performance optimization for no fsnotify marks Amir Goldstein
  2021-08-10 15:12 ` [PATCH v2 1/4] fsnotify: replace igrab() with ihold() on attach connector Amir Goldstein
  2021-08-10 15:12 ` [PATCH v2 2/4] fsnotify: count s_fsnotify_inode_refs for attached connectors Amir Goldstein
@ 2021-08-10 15:12 ` Amir Goldstein
  2021-08-10 21:49   ` Matthew Bobrowski
  2021-08-11 11:37   ` Jan Kara
  2021-08-10 15:12 ` [PATCH v2 4/4] fsnotify: optimize the case of no marks of any type Amir Goldstein
  2021-08-11 12:02 ` [PATCH v2 0/4] Performance optimization for no fsnotify marks Jan Kara
  4 siblings, 2 replies; 9+ messages in thread
From: Amir Goldstein @ 2021-08-10 15:12 UTC (permalink / raw)
  To: Jan Kara; +Cc: Matthew Bobrowski, linux-fsdevel

Rename s_fsnotify_inode_refs to s_fsnotify_connectors and count all
objects with attached connectors, not only inodes with attached
connectors.

This will be used to optimize fsnotify() calls on sb without any
type of marks.

Signed-off-by: Amir Goldstein <amir73il@gmail.com>
---
 fs/notify/fsnotify.c |  6 +++---
 fs/notify/fsnotify.h | 15 +++++++++++++++
 fs/notify/mark.c     | 24 +++++++++++++++++++++---
 include/linux/fs.h   |  4 ++--
 4 files changed, 41 insertions(+), 8 deletions(-)

diff --git a/fs/notify/fsnotify.c b/fs/notify/fsnotify.c
index 30d422b8c0fc..963e6ce75b96 100644
--- a/fs/notify/fsnotify.c
+++ b/fs/notify/fsnotify.c
@@ -87,15 +87,15 @@ static void fsnotify_unmount_inodes(struct super_block *sb)
 
 	if (iput_inode)
 		iput(iput_inode);
-	/* Wait for outstanding inode references from connectors */
-	wait_var_event(&sb->s_fsnotify_inode_refs,
-		       !atomic_long_read(&sb->s_fsnotify_inode_refs));
 }
 
 void fsnotify_sb_delete(struct super_block *sb)
 {
 	fsnotify_unmount_inodes(sb);
 	fsnotify_clear_marks_by_sb(sb);
+	/* Wait for outstanding object references from connectors */
+	wait_var_event(&sb->s_fsnotify_connectors,
+		       !atomic_long_read(&sb->s_fsnotify_connectors));
 }
 
 /*
diff --git a/fs/notify/fsnotify.h b/fs/notify/fsnotify.h
index ff2063ec6b0f..87d8a50ee803 100644
--- a/fs/notify/fsnotify.h
+++ b/fs/notify/fsnotify.h
@@ -27,6 +27,21 @@ static inline struct super_block *fsnotify_conn_sb(
 	return container_of(conn->obj, struct super_block, s_fsnotify_marks);
 }
 
+static inline struct super_block *fsnotify_connector_sb(
+				struct fsnotify_mark_connector *conn)
+{
+	switch (conn->type) {
+	case FSNOTIFY_OBJ_TYPE_INODE:
+		return fsnotify_conn_inode(conn)->i_sb;
+	case FSNOTIFY_OBJ_TYPE_VFSMOUNT:
+		return fsnotify_conn_mount(conn)->mnt.mnt_sb;
+	case FSNOTIFY_OBJ_TYPE_SB:
+		return fsnotify_conn_sb(conn);
+	default:
+		return NULL;
+	}
+}
+
 /* destroy all events sitting in this groups notification queue */
 extern void fsnotify_flush_notify(struct fsnotify_group *group);
 
diff --git a/fs/notify/mark.c b/fs/notify/mark.c
index 2d8c46e1167d..95006d1d29ab 100644
--- a/fs/notify/mark.c
+++ b/fs/notify/mark.c
@@ -172,7 +172,7 @@ static void fsnotify_connector_destroy_workfn(struct work_struct *work)
 static void fsnotify_get_inode_ref(struct inode *inode)
 {
 	ihold(inode);
-	atomic_long_inc(&inode->i_sb->s_fsnotify_inode_refs);
+	atomic_long_inc(&inode->i_sb->s_fsnotify_connectors);
 }
 
 static void fsnotify_put_inode_ref(struct inode *inode)
@@ -180,8 +180,24 @@ static void fsnotify_put_inode_ref(struct inode *inode)
 	struct super_block *sb = inode->i_sb;
 
 	iput(inode);
-	if (atomic_long_dec_and_test(&sb->s_fsnotify_inode_refs))
-		wake_up_var(&sb->s_fsnotify_inode_refs);
+	if (atomic_long_dec_and_test(&sb->s_fsnotify_connectors))
+		wake_up_var(&sb->s_fsnotify_connectors);
+}
+
+static void fsnotify_get_sb_connectors(struct fsnotify_mark_connector *conn)
+{
+	struct super_block *sb = fsnotify_connector_sb(conn);
+
+	if (sb)
+		atomic_long_inc(&sb->s_fsnotify_connectors);
+}
+
+static void fsnotify_put_sb_connectors(struct fsnotify_mark_connector *conn)
+{
+	struct super_block *sb = fsnotify_connector_sb(conn);
+
+	if (sb && atomic_long_dec_and_test(&sb->s_fsnotify_connectors))
+		wake_up_var(&sb->s_fsnotify_connectors);
 }
 
 static void *fsnotify_detach_connector_from_object(
@@ -203,6 +219,7 @@ static void *fsnotify_detach_connector_from_object(
 		fsnotify_conn_sb(conn)->s_fsnotify_mask = 0;
 	}
 
+	fsnotify_put_sb_connectors(conn);
 	rcu_assign_pointer(*(conn->obj), NULL);
 	conn->obj = NULL;
 	conn->type = FSNOTIFY_OBJ_TYPE_DETACHED;
@@ -504,6 +521,7 @@ static int fsnotify_attach_connector_to_object(fsnotify_connp_t *connp,
 		inode = fsnotify_conn_inode(conn);
 		fsnotify_get_inode_ref(inode);
 	}
+	fsnotify_get_sb_connectors(conn);
 
 	/*
 	 * cmpxchg() provides the barrier so that readers of *connp can see
diff --git a/include/linux/fs.h b/include/linux/fs.h
index 640574294216..d48d2018dfa4 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -1507,8 +1507,8 @@ struct super_block {
 	/* Number of inodes with nlink == 0 but still referenced */
 	atomic_long_t s_remove_count;
 
-	/* Pending fsnotify inode refs */
-	atomic_long_t s_fsnotify_inode_refs;
+	/* Number of inode/mount/sb objects that are being watched */
+	atomic_long_t s_fsnotify_connectors;
 
 	/* Being remounted read-only */
 	int s_readonly_remount;
-- 
2.32.0


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

* [PATCH v2 4/4] fsnotify: optimize the case of no marks of any type
  2021-08-10 15:12 [PATCH v2 0/4] Performance optimization for no fsnotify marks Amir Goldstein
                   ` (2 preceding siblings ...)
  2021-08-10 15:12 ` [PATCH v2 3/4] fsnotify: count all objects with " Amir Goldstein
@ 2021-08-10 15:12 ` Amir Goldstein
  2021-08-11 12:02 ` [PATCH v2 0/4] Performance optimization for no fsnotify marks Jan Kara
  4 siblings, 0 replies; 9+ messages in thread
From: Amir Goldstein @ 2021-08-10 15:12 UTC (permalink / raw)
  To: Jan Kara; +Cc: Matthew Bobrowski, linux-fsdevel, Matthew Bobrowski

Add a simple check in the inline helpers to avoid calling fsnotify()
and __fsnotify_parent() in case there are no marks of any type
(inode/sb/mount) for an inode's sb, so there can be no objects
of any type interested in the event.

Reviewed-by: Matthew Bobrowski <repnop@google.com>
Signed-off-by: Amir Goldstein <amir73il@gmail.com>
---
 include/linux/fsnotify.h | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/include/linux/fsnotify.h b/include/linux/fsnotify.h
index f8acddcf54fb..12d3a7d308ab 100644
--- a/include/linux/fsnotify.h
+++ b/include/linux/fsnotify.h
@@ -30,6 +30,9 @@ static inline void fsnotify_name(struct inode *dir, __u32 mask,
 				 struct inode *child,
 				 const struct qstr *name, u32 cookie)
 {
+	if (atomic_long_read(&dir->i_sb->s_fsnotify_connectors) == 0)
+		return;
+
 	fsnotify(mask, child, FSNOTIFY_EVENT_INODE, dir, name, NULL, cookie);
 }
 
@@ -41,6 +44,9 @@ static inline void fsnotify_dirent(struct inode *dir, struct dentry *dentry,
 
 static inline void fsnotify_inode(struct inode *inode, __u32 mask)
 {
+	if (atomic_long_read(&inode->i_sb->s_fsnotify_connectors) == 0)
+		return;
+
 	if (S_ISDIR(inode->i_mode))
 		mask |= FS_ISDIR;
 
@@ -53,6 +59,9 @@ static inline int fsnotify_parent(struct dentry *dentry, __u32 mask,
 {
 	struct inode *inode = d_inode(dentry);
 
+	if (atomic_long_read(&inode->i_sb->s_fsnotify_connectors) == 0)
+		return 0;
+
 	if (S_ISDIR(inode->i_mode)) {
 		mask |= FS_ISDIR;
 
-- 
2.32.0


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

* Re: [PATCH v2 3/4] fsnotify: count all objects with attached connectors
  2021-08-10 15:12 ` [PATCH v2 3/4] fsnotify: count all objects with " Amir Goldstein
@ 2021-08-10 21:49   ` Matthew Bobrowski
  2021-08-11 11:37   ` Jan Kara
  1 sibling, 0 replies; 9+ messages in thread
From: Matthew Bobrowski @ 2021-08-10 21:49 UTC (permalink / raw)
  To: Amir Goldstein; +Cc: Jan Kara, Matthew Bobrowski, linux-fsdevel

On Tue, Aug 10, 2021 at 06:12:19PM +0300, Amir Goldstein wrote:
> Rename s_fsnotify_inode_refs to s_fsnotify_connectors and count all
> objects with attached connectors, not only inodes with attached
> connectors.
> 
> This will be used to optimize fsnotify() calls on sb without any
> type of marks.
> 
> Signed-off-by: Amir Goldstein <amir73il@gmail.com>

LGTM.

Reviewed-by: Matthew Bobrowski <repnop@google.com>

> ---
>  fs/notify/fsnotify.c |  6 +++---
>  fs/notify/fsnotify.h | 15 +++++++++++++++
>  fs/notify/mark.c     | 24 +++++++++++++++++++++---
>  include/linux/fs.h   |  4 ++--
>  4 files changed, 41 insertions(+), 8 deletions(-)
> 
> diff --git a/fs/notify/fsnotify.c b/fs/notify/fsnotify.c
> index 30d422b8c0fc..963e6ce75b96 100644
> --- a/fs/notify/fsnotify.c
> +++ b/fs/notify/fsnotify.c
> @@ -87,15 +87,15 @@ static void fsnotify_unmount_inodes(struct super_block *sb)
>  
>  	if (iput_inode)
>  		iput(iput_inode);
> -	/* Wait for outstanding inode references from connectors */
> -	wait_var_event(&sb->s_fsnotify_inode_refs,
> -		       !atomic_long_read(&sb->s_fsnotify_inode_refs));
>  }
>  
>  void fsnotify_sb_delete(struct super_block *sb)
>  {
>  	fsnotify_unmount_inodes(sb);
>  	fsnotify_clear_marks_by_sb(sb);
> +	/* Wait for outstanding object references from connectors */
> +	wait_var_event(&sb->s_fsnotify_connectors,
> +		       !atomic_long_read(&sb->s_fsnotify_connectors));
>  }
>  
>  /*
> diff --git a/fs/notify/fsnotify.h b/fs/notify/fsnotify.h
> index ff2063ec6b0f..87d8a50ee803 100644
> --- a/fs/notify/fsnotify.h
> +++ b/fs/notify/fsnotify.h
> @@ -27,6 +27,21 @@ static inline struct super_block *fsnotify_conn_sb(
>  	return container_of(conn->obj, struct super_block, s_fsnotify_marks);
>  }
>  
> +static inline struct super_block *fsnotify_connector_sb(
> +				struct fsnotify_mark_connector *conn)
> +{
> +	switch (conn->type) {
> +	case FSNOTIFY_OBJ_TYPE_INODE:
> +		return fsnotify_conn_inode(conn)->i_sb;
> +	case FSNOTIFY_OBJ_TYPE_VFSMOUNT:
> +		return fsnotify_conn_mount(conn)->mnt.mnt_sb;
> +	case FSNOTIFY_OBJ_TYPE_SB:
> +		return fsnotify_conn_sb(conn);
> +	default:
> +		return NULL;
> +	}
> +}
> +
>  /* destroy all events sitting in this groups notification queue */
>  extern void fsnotify_flush_notify(struct fsnotify_group *group);
>  
> diff --git a/fs/notify/mark.c b/fs/notify/mark.c
> index 2d8c46e1167d..95006d1d29ab 100644
> --- a/fs/notify/mark.c
> +++ b/fs/notify/mark.c
> @@ -172,7 +172,7 @@ static void fsnotify_connector_destroy_workfn(struct work_struct *work)
>  static void fsnotify_get_inode_ref(struct inode *inode)
>  {
>  	ihold(inode);
> -	atomic_long_inc(&inode->i_sb->s_fsnotify_inode_refs);
> +	atomic_long_inc(&inode->i_sb->s_fsnotify_connectors);
>  }
>  
>  static void fsnotify_put_inode_ref(struct inode *inode)
> @@ -180,8 +180,24 @@ static void fsnotify_put_inode_ref(struct inode *inode)
>  	struct super_block *sb = inode->i_sb;
>  
>  	iput(inode);
> -	if (atomic_long_dec_and_test(&sb->s_fsnotify_inode_refs))
> -		wake_up_var(&sb->s_fsnotify_inode_refs);
> +	if (atomic_long_dec_and_test(&sb->s_fsnotify_connectors))
> +		wake_up_var(&sb->s_fsnotify_connectors);
> +}
> +
> +static void fsnotify_get_sb_connectors(struct fsnotify_mark_connector *conn)
> +{
> +	struct super_block *sb = fsnotify_connector_sb(conn);
> +
> +	if (sb)
> +		atomic_long_inc(&sb->s_fsnotify_connectors);
> +}
> +
> +static void fsnotify_put_sb_connectors(struct fsnotify_mark_connector *conn)
> +{
> +	struct super_block *sb = fsnotify_connector_sb(conn);
> +
> +	if (sb && atomic_long_dec_and_test(&sb->s_fsnotify_connectors))
> +		wake_up_var(&sb->s_fsnotify_connectors);
>  }
>  
>  static void *fsnotify_detach_connector_from_object(
> @@ -203,6 +219,7 @@ static void *fsnotify_detach_connector_from_object(
>  		fsnotify_conn_sb(conn)->s_fsnotify_mask = 0;
>  	}
>  
> +	fsnotify_put_sb_connectors(conn);
>  	rcu_assign_pointer(*(conn->obj), NULL);
>  	conn->obj = NULL;
>  	conn->type = FSNOTIFY_OBJ_TYPE_DETACHED;
> @@ -504,6 +521,7 @@ static int fsnotify_attach_connector_to_object(fsnotify_connp_t *connp,
>  		inode = fsnotify_conn_inode(conn);
>  		fsnotify_get_inode_ref(inode);
>  	}
> +	fsnotify_get_sb_connectors(conn);
>  
>  	/*
>  	 * cmpxchg() provides the barrier so that readers of *connp can see
> diff --git a/include/linux/fs.h b/include/linux/fs.h
> index 640574294216..d48d2018dfa4 100644
> --- a/include/linux/fs.h
> +++ b/include/linux/fs.h
> @@ -1507,8 +1507,8 @@ struct super_block {
>  	/* Number of inodes with nlink == 0 but still referenced */
>  	atomic_long_t s_remove_count;
>  
> -	/* Pending fsnotify inode refs */
> -	atomic_long_t s_fsnotify_inode_refs;
> +	/* Number of inode/mount/sb objects that are being watched */
> +	atomic_long_t s_fsnotify_connectors;
>  
>  	/* Being remounted read-only */
>  	int s_readonly_remount;
> -- 
> 2.32.0
> 
/M

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

* Re: [PATCH v2 3/4] fsnotify: count all objects with attached connectors
  2021-08-10 15:12 ` [PATCH v2 3/4] fsnotify: count all objects with " Amir Goldstein
  2021-08-10 21:49   ` Matthew Bobrowski
@ 2021-08-11 11:37   ` Jan Kara
  2021-08-11 13:59     ` Amir Goldstein
  1 sibling, 1 reply; 9+ messages in thread
From: Jan Kara @ 2021-08-11 11:37 UTC (permalink / raw)
  To: Amir Goldstein; +Cc: Jan Kara, Matthew Bobrowski, linux-fsdevel

On Tue 10-08-21 18:12:19, Amir Goldstein wrote:
> Rename s_fsnotify_inode_refs to s_fsnotify_connectors and count all
> objects with attached connectors, not only inodes with attached
> connectors.
> 
> This will be used to optimize fsnotify() calls on sb without any
> type of marks.
> 
> Signed-off-by: Amir Goldstein <amir73il@gmail.com>

... just a minor nit below ...

> diff --git a/include/linux/fs.h b/include/linux/fs.h
> index 640574294216..d48d2018dfa4 100644
> --- a/include/linux/fs.h
> +++ b/include/linux/fs.h
> @@ -1507,8 +1507,8 @@ struct super_block {
>  	/* Number of inodes with nlink == 0 but still referenced */
>  	atomic_long_t s_remove_count;
>  
> -	/* Pending fsnotify inode refs */
> -	atomic_long_t s_fsnotify_inode_refs;
> +	/* Number of inode/mount/sb objects that are being watched */
> +	atomic_long_t s_fsnotify_connectors;

I've realized inode watches will be double-accounted because we increment
s_fsnotify_connectors both when attaching a connector and when grabbing
inode reference. It doesn't really matter and avoiding this would require
special treatment of inode connectors (we need to decrement
s_fsnotify_connectors only after dropping inode reference). So I'll just
reflect this in the comment here so that we don't forget.

								Honza
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR

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

* Re: [PATCH v2 0/4] Performance optimization for no fsnotify marks
  2021-08-10 15:12 [PATCH v2 0/4] Performance optimization for no fsnotify marks Amir Goldstein
                   ` (3 preceding siblings ...)
  2021-08-10 15:12 ` [PATCH v2 4/4] fsnotify: optimize the case of no marks of any type Amir Goldstein
@ 2021-08-11 12:02 ` Jan Kara
  4 siblings, 0 replies; 9+ messages in thread
From: Jan Kara @ 2021-08-11 12:02 UTC (permalink / raw)
  To: Amir Goldstein; +Cc: Jan Kara, Matthew Bobrowski, linux-fsdevel

On Tue 10-08-21 18:12:16, Amir Goldstein wrote:
> Hi Jan,
> 
> Following v2 addresses review comments from v1 [1]
> 
> [1] https://lore.kernel.org/linux-fsdevel/20210803180344.2398374-1-amir73il@gmail.com/

Thanks for the patches and for the review to Matthew. I've picked up the
patches to my tree.

								Honza

> 
> Changes since v1:
> - Rebase on 5.14-rc5 + pidfd patches
> - Added RVB
> - Helper to get connector's sb (Matthew)
> - Fix deadlock bug on umount (Jan)
> 
> Amir Goldstein (4):
>   fsnotify: replace igrab() with ihold() on attach connector
>   fsnotify: count s_fsnotify_inode_refs for attached connectors
>   fsnotify: count all objects with attached connectors
>   fsnotify: optimize the case of no marks of any type
> 
>  fs/notify/fsnotify.c     |  6 ++---
>  fs/notify/fsnotify.h     | 15 ++++++++++++
>  fs/notify/mark.c         | 52 ++++++++++++++++++++++++++++++----------
>  include/linux/fs.h       |  4 ++--
>  include/linux/fsnotify.h |  9 +++++++
>  5 files changed, 69 insertions(+), 17 deletions(-)
> 
> -- 
> 2.32.0
> 
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR

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

* Re: [PATCH v2 3/4] fsnotify: count all objects with attached connectors
  2021-08-11 11:37   ` Jan Kara
@ 2021-08-11 13:59     ` Amir Goldstein
  0 siblings, 0 replies; 9+ messages in thread
From: Amir Goldstein @ 2021-08-11 13:59 UTC (permalink / raw)
  To: Jan Kara; +Cc: Matthew Bobrowski, linux-fsdevel

On Wed, Aug 11, 2021 at 2:37 PM Jan Kara <jack@suse.cz> wrote:
>
> On Tue 10-08-21 18:12:19, Amir Goldstein wrote:
> > Rename s_fsnotify_inode_refs to s_fsnotify_connectors and count all
> > objects with attached connectors, not only inodes with attached
> > connectors.
> >
> > This will be used to optimize fsnotify() calls on sb without any
> > type of marks.
> >
> > Signed-off-by: Amir Goldstein <amir73il@gmail.com>
>
> ... just a minor nit below ...
>
> > diff --git a/include/linux/fs.h b/include/linux/fs.h
> > index 640574294216..d48d2018dfa4 100644
> > --- a/include/linux/fs.h
> > +++ b/include/linux/fs.h
> > @@ -1507,8 +1507,8 @@ struct super_block {
> >       /* Number of inodes with nlink == 0 but still referenced */
> >       atomic_long_t s_remove_count;
> >
> > -     /* Pending fsnotify inode refs */
> > -     atomic_long_t s_fsnotify_inode_refs;
> > +     /* Number of inode/mount/sb objects that are being watched */
> > +     atomic_long_t s_fsnotify_connectors;
>
> I've realized inode watches will be double-accounted because we increment
> s_fsnotify_connectors both when attaching a connector and when grabbing
> inode reference. It doesn't really matter and avoiding this would require
> special treatment of inode connectors (we need to decrement
> s_fsnotify_connectors only after dropping inode reference). So I'll just
> reflect this in the comment here so that we don't forget.

Great!
I actually have a patch to make the inode reference optional
per backend/mark [1], so keeping double account makes things
a lot easier for that code as well.

Thanks,
Amir.

[1] https://github.com/amir73il/linux/commits/fsnotify-volatile

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

end of thread, other threads:[~2021-08-11 14:00 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-08-10 15:12 [PATCH v2 0/4] Performance optimization for no fsnotify marks Amir Goldstein
2021-08-10 15:12 ` [PATCH v2 1/4] fsnotify: replace igrab() with ihold() on attach connector Amir Goldstein
2021-08-10 15:12 ` [PATCH v2 2/4] fsnotify: count s_fsnotify_inode_refs for attached connectors Amir Goldstein
2021-08-10 15:12 ` [PATCH v2 3/4] fsnotify: count all objects with " Amir Goldstein
2021-08-10 21:49   ` Matthew Bobrowski
2021-08-11 11:37   ` Jan Kara
2021-08-11 13:59     ` Amir Goldstein
2021-08-10 15:12 ` [PATCH v2 4/4] fsnotify: optimize the case of no marks of any type Amir Goldstein
2021-08-11 12:02 ` [PATCH v2 0/4] Performance optimization for no fsnotify marks Jan Kara

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.