[PATCH] XFS: Kill filestreams cache flush

[PATCH] XFS: Kill filestreams cache flush

[Dave Chinner]

The filestreams cache flush is not needed in the sync code as it
does not affect data writeback, and it is now not used by the
growfs code, either, so kill it.

Signed-off-by: Dave Chinner <david@fromorbit.com>
---
 fs/xfs/linux-2.6/xfs_sync.c |    3 ---
 fs/xfs/xfs_filestream.c     |   14 --------------
 fs/xfs/xfs_filestream.h     |    1 -
 fs/xfs/xfs_mru_cache.c      |    2 +-
 fs/xfs/xfs_mru_cache.h      |    1 -
 5 files changed, 1 insertions(+), 20 deletions(-)

diff --git a/fs/xfs/linux-2.6/xfs_sync.c b/fs/xfs/linux-2.6/xfs_sync.c
index 0e17683..c980d68 100644
--- a/fs/xfs/linux-2.6/xfs_sync.c
+++ b/fs/xfs/linux-2.6/xfs_sync.c
@@ -444,9 +444,6 @@ xfs_quiesce_data(
 	xfs_sync_data(mp, SYNC_WAIT);
 	xfs_qm_sync(mp, SYNC_WAIT);
 
-	/* drop inode references pinned by filestreams */
-	xfs_filestream_flush(mp);
-
 	/* write superblock and hoover up shutdown errors */
 	error = xfs_sync_fsdata(mp, SYNC_WAIT);
 
diff --git a/fs/xfs/xfs_filestream.c b/fs/xfs/xfs_filestream.c
index 914d00d..390850e 100644
--- a/fs/xfs/xfs_filestream.c
+++ b/fs/xfs/xfs_filestream.c
@@ -455,20 +455,6 @@ xfs_filestream_unmount(
 }
 
 /*
- * If the mount point's m_perag tree is going to be modified, all
- * outstanding cache entries must be flushed to avoid accessing reference count
- * addresses that have been freed.  The call to xfs_filestream_flush() must be
- * made inside the block that holds the m_perag_lock in write mode to do the
- * reallocation.
- */
-void
-xfs_filestream_flush(
-	xfs_mount_t	*mp)
-{
-	xfs_mru_cache_flush(mp->m_filestream);
-}
-
-/*
  * Return the AG of the filestream the file or directory belongs to, or
  * NULLAGNUMBER otherwise.
  */
diff --git a/fs/xfs/xfs_filestream.h b/fs/xfs/xfs_filestream.h
index 58378b2..260f757 100644
--- a/fs/xfs/xfs_filestream.h
+++ b/fs/xfs/xfs_filestream.h
@@ -135,7 +135,6 @@ int xfs_filestream_init(void);
 void xfs_filestream_uninit(void);
 int xfs_filestream_mount(struct xfs_mount *mp);
 void xfs_filestream_unmount(struct xfs_mount *mp);
-void xfs_filestream_flush(struct xfs_mount *mp);
 xfs_agnumber_t xfs_filestream_lookup_ag(struct xfs_inode *ip);
 int xfs_filestream_associate(struct xfs_inode *dip, struct xfs_inode *ip);
 void xfs_filestream_deassociate(struct xfs_inode *ip);
diff --git a/fs/xfs/xfs_mru_cache.c b/fs/xfs/xfs_mru_cache.c
index 4b0613d..45ce15d 100644
--- a/fs/xfs/xfs_mru_cache.c
+++ b/fs/xfs/xfs_mru_cache.c
@@ -398,7 +398,7 @@ exit:
  * guaranteed that all the free functions for all the elements have finished
  * executing and the reaper is not running.
  */
-void
+static void
 xfs_mru_cache_flush(
 	xfs_mru_cache_t		*mru)
 {
diff --git a/fs/xfs/xfs_mru_cache.h b/fs/xfs/xfs_mru_cache.h
index 5d439f3..36dd3ec 100644
--- a/fs/xfs/xfs_mru_cache.h
+++ b/fs/xfs/xfs_mru_cache.h
@@ -42,7 +42,6 @@ void xfs_mru_cache_uninit(void);
 int xfs_mru_cache_create(struct xfs_mru_cache **mrup, unsigned int lifetime_ms,
 			     unsigned int grp_count,
 			     xfs_mru_cache_free_func_t free_func);
-void xfs_mru_cache_flush(xfs_mru_cache_t *mru);
 void xfs_mru_cache_destroy(struct xfs_mru_cache *mru);
 int xfs_mru_cache_insert(struct xfs_mru_cache *mru, unsigned long key,
 				void *value);
-- 
1.6.5

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

Re: [PATCH] XFS: Kill filestreams cache flush

[Christoph Hellwig]

On Sat, Jan 02, 2010 at 01:38:20PM +1100, Dave Chinner wrote:
> The filestreams cache flush is not needed in the sync code as it
> does not affect data writeback, and it is now not used by the
> growfs code, either, so kill it.

I have to admit I never understood the purpose of this call.

What xfs_fstrm_free_func does is:

 - clear the filesystems flag on the inode
 - decrement pagf_fstrms
 - drop references on the inode (and parent if known)
 - free the filestream item

Nothing of that should matter for sync, although dropping inode
references during sync might generally be a good idea.

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

Re: [PATCH] XFS: Kill filestreams cache flush

[Dave Chinner]

On Sat, Jan 02, 2010 at 06:53:38AM -0500, Christoph Hellwig wrote:
> On Sat, Jan 02, 2010 at 01:38:20PM +1100, Dave Chinner wrote:
> > The filestreams cache flush is not needed in the sync code as it
> > does not affect data writeback, and it is now not used by the
> > growfs code, either, so kill it.
> 
> I have to admit I never understood the purpose of this call.

I can't remember why it was put there, either. I may have even put
it there when I was getting filestreams ready for merge but I can't remember
at all...

> What xfs_fstrm_free_func does is:
> 
>  - clear the filesystems flag on the inode
>  - decrement pagf_fstrms
>  - drop references on the inode (and parent if known)
>  - free the filestream item
> 
> Nothing of that should matter for sync, although dropping inode
> references during sync might generally be a good idea.

Which means that sync destroys active filestreams associations and
that seems more broken to me than anything an active reference will
cause. I think just letting them time out is better than having them
destroyed by external operations. I'm happy to leave it there if you
want....

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

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

Re: [PATCH] XFS: Kill filestreams cache flush

[Christoph Hellwig]

On Sat, Jan 02, 2010 at 11:12:03PM +1100, Dave Chinner wrote:
> Which means that sync destroys active filestreams associations and
> that seems more broken to me than anything an active reference will
> cause. I think just letting them time out is better than having them
> destroyed by external operations. I'm happy to leave it there if you
> want....

Just trying to understand what's going on here.  If you think the new
behaviour is better and it doesn't cause any regressions in QA I'm fine
with it.  Consider it:


Reviewed-by: Christoph Hellwig <hch@lst.de>

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