public inbox for linux-xfs@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH] xfs: active inodes stat is broken
@ 2014-12-01 22:34 Dave Chinner
  2014-12-02 13:52 ` Mark Tinguely
  0 siblings, 1 reply; 5+ messages in thread
From: Dave Chinner @ 2014-12-01 22:34 UTC (permalink / raw)
  To: xfs

From: Dave Chinner <dchinner@redhat.com>

vn_active only ever gets decremented, so it has a very large
negative number.  Make it track the inode count we currently have
allocated properly so we can easily track the size of the inode
cache via tools like PCP.

Signed-off-by: Dave Chinner <dchinner@redhat.com>
---
 fs/xfs/xfs_icache.c | 3 +++
 fs/xfs/xfs_super.c  | 1 -
 2 files changed, 3 insertions(+), 1 deletion(-)

diff --git a/fs/xfs/xfs_icache.c b/fs/xfs/xfs_icache.c
index 92ca910..8bc3d78 100644
--- a/fs/xfs/xfs_icache.c
+++ b/fs/xfs/xfs_icache.c
@@ -128,6 +128,7 @@ xfs_inode_free(
 	/* asserts to verify all state is correct here */
 	ASSERT(atomic_read(&ip->i_pincount) == 0);
 	ASSERT(!xfs_isiflocked(ip));
+	XFS_STATS_DEC(vn_active);
 
 	call_rcu(&VFS_I(ip)->i_rcu, xfs_inode_free_callback);
 }
@@ -287,6 +288,8 @@ xfs_iget_cache_miss(
 	if (!ip)
 		return -ENOMEM;
 
+	XFS_STATS_INC(vn_active);
+
 	error = xfs_iread(mp, tp, ip, flags);
 	if (error)
 		goto out_destroy;
diff --git a/fs/xfs/xfs_super.c b/fs/xfs/xfs_super.c
index fee11c8..6b95851 100644
--- a/fs/xfs/xfs_super.c
+++ b/fs/xfs/xfs_super.c
@@ -1005,7 +1005,6 @@ xfs_fs_evict_inode(
 	clear_inode(inode);
 	XFS_STATS_INC(vn_rele);
 	XFS_STATS_INC(vn_remove);
-	XFS_STATS_DEC(vn_active);
 
 	xfs_inactive(ip);
 }
-- 
2.0.0

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

^ permalink raw reply related	[flat|nested] 5+ messages in thread
* Re: [PATCH] xfs: active inodes stat is broken
  2014-12-01 22:34 [PATCH] xfs: active inodes stat is broken Dave Chinner
@ 2014-12-02 13:52 ` Mark Tinguely
  2014-12-02 21:02   ` Dave Chinner
  0 siblings, 1 reply; 5+ messages in thread
From: Mark Tinguely @ 2014-12-02 13:52 UTC (permalink / raw)
  To: Dave Chinner; +Cc: xfs

On 12/01/14 16:34, Dave Chinner wrote:
> From: Dave Chinner<dchinner@redhat.com>
>
> vn_active only ever gets decremented, so it has a very large
> negative number.  Make it track the inode count we currently have
> allocated properly so we can easily track the size of the inode
> cache via tools like PCP.
>
> Signed-off-by: Dave Chinner<dchinner@redhat.com>
> ---
>   fs/xfs/xfs_icache.c | 3 +++
>   fs/xfs/xfs_super.c  | 1 -
>   2 files changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/fs/xfs/xfs_icache.c b/fs/xfs/xfs_icache.c
> index 92ca910..8bc3d78 100644
> --- a/fs/xfs/xfs_icache.c
> +++ b/fs/xfs/xfs_icache.c
> @@ -128,6 +128,7 @@ xfs_inode_free(
>   	/* asserts to verify all state is correct here */
>   	ASSERT(atomic_read(&ip->i_pincount) == 0);
>   	ASSERT(!xfs_isiflocked(ip));
> +	XFS_STATS_DEC(vn_active);
>
>   	call_rcu(&VFS_I(ip)->i_rcu, xfs_inode_free_callback);
>   }

Would the inode allocated and freed in xfs_recover_inode_owner_change()
cause this the count to go negative?

--Mark.

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

^ permalink raw reply	[flat|nested] 5+ messages in thread
* Re: [PATCH] xfs: active inodes stat is broken
  2014-12-02 13:52 ` Mark Tinguely
@ 2014-12-02 21:02   ` Dave Chinner
  2014-12-02 22:34     ` [PATCH v2] " Dave Chinner
  0 siblings, 1 reply; 5+ messages in thread
From: Dave Chinner @ 2014-12-02 21:02 UTC (permalink / raw)
  To: Mark Tinguely; +Cc: xfs

On Tue, Dec 02, 2014 at 07:52:17AM -0600, Mark Tinguely wrote:
> On 12/01/14 16:34, Dave Chinner wrote:
> >From: Dave Chinner<dchinner@redhat.com>
> >
> >vn_active only ever gets decremented, so it has a very large
> >negative number.  Make it track the inode count we currently have
> >allocated properly so we can easily track the size of the inode
> >cache via tools like PCP.
> >
> >Signed-off-by: Dave Chinner<dchinner@redhat.com>
> >---
> >  fs/xfs/xfs_icache.c | 3 +++
> >  fs/xfs/xfs_super.c  | 1 -
> >  2 files changed, 3 insertions(+), 1 deletion(-)
> >
> >diff --git a/fs/xfs/xfs_icache.c b/fs/xfs/xfs_icache.c
> >index 92ca910..8bc3d78 100644
> >--- a/fs/xfs/xfs_icache.c
> >+++ b/fs/xfs/xfs_icache.c
> >@@ -128,6 +128,7 @@ xfs_inode_free(
> >  	/* asserts to verify all state is correct here */
> >  	ASSERT(atomic_read(&ip->i_pincount) == 0);
> >  	ASSERT(!xfs_isiflocked(ip));
> >+	XFS_STATS_DEC(vn_active);
> >
> >  	call_rcu(&VFS_I(ip)->i_rcu, xfs_inode_free_callback);
> >  }
> 
> Would the inode allocated and freed in xfs_recover_inode_owner_change()
> cause this the count to go negative?

Good catch - it will. Recovering swap extent operations is pretty
rare, so I wouldn't have noticed an off-by-one-or-two on a normal
system where there are several thousand allocated and cached inodes
even at idle. I'll fix it.

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

^ permalink raw reply	[flat|nested] 5+ messages in thread
* [PATCH v2] xfs: active inodes stat is broken
  2014-12-02 21:02   ` Dave Chinner
@ 2014-12-02 22:34     ` Dave Chinner
  2014-12-02 22:45       ` Mark Tinguely
  0 siblings, 1 reply; 5+ messages in thread
From: Dave Chinner @ 2014-12-02 22:34 UTC (permalink / raw)
  To: Mark Tinguely; +Cc: xfs


From: Dave Chinner <dchinner@redhat.com>

vn_active only ever gets decremented, so it has a very large
negative number.  Make it track the inode count we currently have
allocated properly so we can easily track the size of the inode
cache via tools like PCP.

Signed-off-by: Dave Chinner <dchinner@redhat.com>
---
Version 2:
- move stat increment to inode allocation code rather than cache
  miss code so that inc/dec are evenly balanced no matter who calls
  them.

 fs/xfs/xfs_icache.c | 2 ++
 fs/xfs/xfs_super.c  | 1 -
 2 files changed, 2 insertions(+), 1 deletion(-)

diff --git a/fs/xfs/xfs_icache.c b/fs/xfs/xfs_icache.c
index 92ca910..9771b7e 100644
--- a/fs/xfs/xfs_icache.c
+++ b/fs/xfs/xfs_icache.c
@@ -63,6 +63,7 @@ xfs_inode_alloc(
 		return NULL;
 	}
 
+	XFS_STATS_INC(vn_active);
 	ASSERT(atomic_read(&ip->i_pincount) == 0);
 	ASSERT(!spin_is_locked(&ip->i_flags_lock));
 	ASSERT(!xfs_isiflocked(ip));
@@ -128,6 +129,7 @@ xfs_inode_free(
 	/* asserts to verify all state is correct here */
 	ASSERT(atomic_read(&ip->i_pincount) == 0);
 	ASSERT(!xfs_isiflocked(ip));
+	XFS_STATS_DEC(vn_active);
 
 	call_rcu(&VFS_I(ip)->i_rcu, xfs_inode_free_callback);
 }
diff --git a/fs/xfs/xfs_super.c b/fs/xfs/xfs_super.c
index fee11c8..6b95851 100644
--- a/fs/xfs/xfs_super.c
+++ b/fs/xfs/xfs_super.c
@@ -1005,7 +1005,6 @@ xfs_fs_evict_inode(
 	clear_inode(inode);
 	XFS_STATS_INC(vn_rele);
 	XFS_STATS_INC(vn_remove);
-	XFS_STATS_DEC(vn_active);
 
 	xfs_inactive(ip);
 }

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

^ permalink raw reply related	[flat|nested] 5+ messages in thread
* Re: [PATCH v2] xfs: active inodes stat is broken
  2014-12-02 22:34     ` [PATCH v2] " Dave Chinner
@ 2014-12-02 22:45       ` Mark Tinguely
  0 siblings, 0 replies; 5+ messages in thread
From: Mark Tinguely @ 2014-12-02 22:45 UTC (permalink / raw)
  To: Dave Chinner; +Cc: xfs

On 12/02/14 16:34, Dave Chinner wrote:
>
> From: Dave Chinner <dchinner@redhat.com>
>
> vn_active only ever gets decremented, so it has a very large
> negative number.  Make it track the inode count we currently have
> allocated properly so we can easily track the size of the inode
> cache via tools like PCP.
>
> Signed-off-by: Dave Chinner <dchinner@redhat.com>
> ---

Nice to have that working.

Reviewed-by: Mark Tinguely <tinguely@sgi.com>

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

^ permalink raw reply	[flat|nested] 5+ messages in thread
end of thread, other threads:[~2014-12-02 22:45 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-12-01 22:34 [PATCH] xfs: active inodes stat is broken Dave Chinner
2014-12-02 13:52 ` Mark Tinguely
2014-12-02 21:02   ` Dave Chinner
2014-12-02 22:34     ` [PATCH v2] " Dave Chinner
2014-12-02 22:45       ` Mark Tinguely

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox