diff options
| author | Dave Chinner <dchinner@redhat.com> | 2010-07-20 03:53:25 -0400 |
|---|---|---|
| committer | Alex Elder <aelder@sgi.com> | 2010-07-26 14:16:48 -0400 |
| commit | 2f11feabb19748c0ffa2eb82d438e8a91b9f6ea0 (patch) | |
| tree | 02564de802ac344b7c2048ed10438443356bfce1 | |
| parent | ec53d1dbb3ca960e7b552397613358ba1dbd12bd (diff) | |
xfs: simplify and remove xfs_ireclaim
xfs_ireclaim has to get and put te pag structure because it is only
called with the inode to reclaim. The one caller of this function
already has a reference on the pag and a pointer to is, so move the
radix tree delete to the caller and remove xfs_ireclaim completely.
This avoids a xfs_perag_get/put on every inode being reclaimed.
The overhead was noticed in a bug report at:
https://bugzilla.kernel.org/show_bug.cgi?id=16348
Signed-off-by: Dave Chinner <dchinner@redhat.com>
Reviewed-by: Christoph Hellwig <hch@lst.de>
Reviewed-by: Alex Elder <aelder@sgi.com>
Signed-off-by: Dave Chinner <david@fromorbit.com>
| -rw-r--r-- | fs/xfs/linux-2.6/xfs_sync.c | 31 | ||||
| -rw-r--r-- | fs/xfs/xfs_iget.c | 53 | ||||
| -rw-r--r-- | fs/xfs/xfs_inode.h | 2 |
3 files changed, 32 insertions, 54 deletions
diff --git a/fs/xfs/linux-2.6/xfs_sync.c b/fs/xfs/linux-2.6/xfs_sync.c index 66cefb274385..dfcbd98d1599 100644 --- a/fs/xfs/linux-2.6/xfs_sync.c +++ b/fs/xfs/linux-2.6/xfs_sync.c | |||
| @@ -855,7 +855,36 @@ out: | |||
| 855 | reclaim: | 855 | reclaim: |
| 856 | xfs_ifunlock(ip); | 856 | xfs_ifunlock(ip); |
| 857 | xfs_iunlock(ip, XFS_ILOCK_EXCL); | 857 | xfs_iunlock(ip, XFS_ILOCK_EXCL); |
| 858 | xfs_ireclaim(ip); | 858 | |
| 859 | XFS_STATS_INC(xs_ig_reclaims); | ||
| 860 | /* | ||
| 861 | * Remove the inode from the per-AG radix tree. | ||
| 862 | * | ||
| 863 | * Because radix_tree_delete won't complain even if the item was never | ||
| 864 | * added to the tree assert that it's been there before to catch | ||
| 865 | * problems with the inode life time early on. | ||
| 866 | */ | ||
| 867 | write_lock(&pag->pag_ici_lock); | ||
| 868 | if (!radix_tree_delete(&pag->pag_ici_root, | ||
| 869 | XFS_INO_TO_AGINO(ip->i_mount, ip->i_ino))) | ||
| 870 | ASSERT(0); | ||
| 871 | write_unlock(&pag->pag_ici_lock); | ||
| 872 | |||
| 873 | /* | ||
| 874 | * Here we do an (almost) spurious inode lock in order to coordinate | ||
| 875 | * with inode cache radix tree lookups. This is because the lookup | ||
| 876 | * can reference the inodes in the cache without taking references. | ||
| 877 | * | ||
| 878 | * We make that OK here by ensuring that we wait until the inode is | ||
| 879 | * unlocked after the lookup before we go ahead and free it. We get | ||
| 880 | * both the ilock and the iolock because the code may need to drop the | ||
| 881 | * ilock one but will still hold the iolock. | ||
| 882 | */ | ||
| 883 | xfs_ilock(ip, XFS_ILOCK_EXCL | XFS_IOLOCK_EXCL); | ||
| 884 | xfs_qm_dqdetach(ip); | ||
| 885 | xfs_iunlock(ip, XFS_ILOCK_EXCL | XFS_IOLOCK_EXCL); | ||
| 886 | |||
| 887 | xfs_inode_free(ip); | ||
| 859 | return error; | 888 | return error; |
| 860 | 889 | ||
| 861 | } | 890 | } |
diff --git a/fs/xfs/xfs_iget.c b/fs/xfs/xfs_iget.c index 9e86f2116aa8..eba5ae61d362 100644 --- a/fs/xfs/xfs_iget.c +++ b/fs/xfs/xfs_iget.c | |||
| @@ -91,7 +91,7 @@ xfs_inode_alloc( | |||
| 91 | return ip; | 91 | return ip; |
| 92 | } | 92 | } |
| 93 | 93 | ||
| 94 | STATIC void | 94 | void |
| 95 | xfs_inode_free( | 95 | xfs_inode_free( |
| 96 | struct xfs_inode *ip) | 96 | struct xfs_inode *ip) |
| 97 | { | 97 | { |
| @@ -418,57 +418,6 @@ out_error_or_again: | |||
| 418 | } | 418 | } |
| 419 | 419 | ||
| 420 | /* | 420 | /* |
| 421 | * This is called free all the memory associated with an inode. | ||
| 422 | * It must free the inode itself and any buffers allocated for | ||
| 423 | * if_extents/if_data and if_broot. It must also free the lock | ||
| 424 | * associated with the inode. | ||
| 425 | * | ||
| 426 | * Note: because we don't initialise everything on reallocation out | ||
| 427 | * of the zone, we must ensure we nullify everything correctly before | ||
| 428 | * freeing the structure. | ||
| 429 | */ | ||
| 430 | void | ||
| 431 | xfs_ireclaim( | ||
| 432 | struct xfs_inode *ip) | ||
| 433 | { | ||
| 434 | struct xfs_mount *mp = ip->i_mount; | ||
| 435 | struct xfs_perag *pag; | ||
| 436 | xfs_agino_t agino = XFS_INO_TO_AGINO(mp, ip->i_ino); | ||
| 437 | |||
| 438 | XFS_STATS_INC(xs_ig_reclaims); | ||
| 439 | |||
| 440 | /* | ||
| 441 | * Remove the inode from the per-AG radix tree. | ||
| 442 | * | ||
| 443 | * Because radix_tree_delete won't complain even if the item was never | ||
| 444 | * added to the tree assert that it's been there before to catch | ||
| 445 | * problems with the inode life time early on. | ||
| 446 | */ | ||
| 447 | pag = xfs_perag_get(mp, XFS_INO_TO_AGNO(mp, ip->i_ino)); | ||
| 448 | write_lock(&pag->pag_ici_lock); | ||
| 449 | if (!radix_tree_delete(&pag->pag_ici_root, agino)) | ||
| 450 | ASSERT(0); | ||
| 451 | write_unlock(&pag->pag_ici_lock); | ||
| 452 | xfs_perag_put(pag); | ||
| 453 | |||
| 454 | /* | ||
| 455 | * Here we do an (almost) spurious inode lock in order to coordinate | ||
| 456 | * with inode cache radix tree lookups. This is because the lookup | ||
| 457 | * can reference the inodes in the cache without taking references. | ||
| 458 | * | ||
| 459 | * We make that OK here by ensuring that we wait until the inode is | ||
| 460 | * unlocked after the lookup before we go ahead and free it. We get | ||
| 461 | * both the ilock and the iolock because the code may need to drop the | ||
| 462 | * ilock one but will still hold the iolock. | ||
| 463 | */ | ||
| 464 | xfs_ilock(ip, XFS_ILOCK_EXCL | XFS_IOLOCK_EXCL); | ||
| 465 | xfs_qm_dqdetach(ip); | ||
| 466 | xfs_iunlock(ip, XFS_ILOCK_EXCL | XFS_IOLOCK_EXCL); | ||
| 467 | |||
| 468 | xfs_inode_free(ip); | ||
| 469 | } | ||
| 470 | |||
| 471 | /* | ||
| 472 | * This is a wrapper routine around the xfs_ilock() routine | 421 | * This is a wrapper routine around the xfs_ilock() routine |
| 473 | * used to centralize some grungy code. It is used in places | 422 | * used to centralize some grungy code. It is used in places |
| 474 | * that wish to lock the inode solely for reading the extents. | 423 | * that wish to lock the inode solely for reading the extents. |
diff --git a/fs/xfs/xfs_inode.h b/fs/xfs/xfs_inode.h index eb41559ea8cd..0898c5417d12 100644 --- a/fs/xfs/xfs_inode.h +++ b/fs/xfs/xfs_inode.h | |||
| @@ -450,7 +450,7 @@ void xfs_ilock_demote(xfs_inode_t *, uint); | |||
| 450 | int xfs_isilocked(xfs_inode_t *, uint); | 450 | int xfs_isilocked(xfs_inode_t *, uint); |
| 451 | uint xfs_ilock_map_shared(xfs_inode_t *); | 451 | uint xfs_ilock_map_shared(xfs_inode_t *); |
| 452 | void xfs_iunlock_map_shared(xfs_inode_t *, uint); | 452 | void xfs_iunlock_map_shared(xfs_inode_t *, uint); |
| 453 | void xfs_ireclaim(xfs_inode_t *); | 453 | void xfs_inode_free(struct xfs_inode *ip); |
| 454 | 454 | ||
| 455 | /* | 455 | /* |
| 456 | * xfs_inode.c prototypes. | 456 | * xfs_inode.c prototypes. |
