XFS: remove i_gen from incore inode
PATCH 1/1 XFS: remove i_gen from incore inode
i_gen is incremented in directory operations when the directory is changed. It is never read or otherwise used so it should be removed to help reduce the size of the struct xfs_inode.
Signed-off-by Dave Chinner <[email protected]>
--- fs/xfs/xfs_inode.h | 1 - fs/xfs/xfs_rename.c | 12 ++---------- fs/xfs/xfs_vnodeops.c | 29 ++--------------------------- 3 files changed, 4 insertions(+), 38 deletions(-)
fs/xfs/xfs_inode.h +0 -1
@@ -223,7 +223,6 @@ typedef struct xfs_inode {unsigned short i_flags;/* see defined flags below */ unsigned char i_update_core;/* timestamps/size is dirty */ unsigned char i_update_size;/* di_size field is dirty */ -unsigned int i_gen;/* generation count */ unsigned int i_delayed_blks;/* count of delay alloc blks */ xfs_icdinode_t i_d;/* most of ondisk inode */
fs/xfs/xfs_rename.c +2 -10
@@ -367,19 +367,11 @@ xfs_rename( &first_block, &free_list, spaceres);if (error) goto abort_return; -xfs_ichgtime (src_dp,XFS_ICHGTIME_MOD |XFS_ICHGTIME_CHG ); -/* -* Update the generation counts on all the directory inodes -* that we're modifying. -*/ - src_dp->i_gen++; +xfs_ichgtime (src_dp,XFS_ICHGTIME_MOD |XFS_ICHGTIME_CHG );xfs_trans_log_inode (tp, src_dp,XFS_ILOG_CORE ); - -if (new_parent) { - target_dp->i_gen++; +if (new_parent)xfs_trans_log_inode (tp, target_dp,XFS_ILOG_CORE ); - }/* * If this is a synchronous mount, make sure that the
fs/xfs/xfs_vnodeops.c +2 -27
@@ -1619,8 +1619,6 @@ xfs_create(xfs_trans_set_sync (tp); } - dp->i_gen++; -/* * Attach the dquot(s) to the inodes and modify them incore. * These ids of the inode couldn't have changed since the new
@@ -2050,13 +2048,6 @@ xfs_remove( }xfs_ichgtime (dp,XFS_ICHGTIME_MOD |XFS_ICHGTIME_CHG ); -/* -* Bump the in memory generation count on the parent -* directory so that other can know that it has changed. -*/ - dp->i_gen++; -xfs_trans_log_inode (tp, dp,XFS_ILOG_CORE ); -if (is_dir) {/* * Drop the link from ip's "..".
@@ -2074,8 +2065,8 @@ xfs_remove( }else {/* * When removing a non-directory we need to log the parent -* inode here for the i_gen update. For a directory this is -* done implicitly by the xfs_droplink call for the ".." entry. +* inode here. For a directory this is done implicitly +* by the xfs_droplink call for the ".." entry. */ xfs_trans_log_inode (tp, dp,XFS_ILOG_CORE ); }
@@ -2244,7 +2235,6 @@ xfs_link(if (error) goto abort_return;xfs_ichgtime (tdp,XFS_ICHGTIME_MOD |XFS_ICHGTIME_CHG ); - tdp->i_gen++;xfs_trans_log_inode (tp, tdp,XFS_ILOG_CORE ); error =xfs_bumplink (tp, sip);
@@ -2421,18 +2411,10 @@ xfs_mkdir( }xfs_ichgtime (dp,XFS_ICHGTIME_MOD |XFS_ICHGTIME_CHG ); -/* -* Bump the in memory version number of the parent directory -* so that other processes accessing it will recognize that -* the directory has changed. -*/ - dp->i_gen++; - error =xfs_dir_init (tp, cdp, dp);if (error) goto error2; - cdp->i_gen =1 ; error =xfs_bumplink (tp, dp);if (error) goto error2;
@@ -2719,13 +2701,6 @@ xfs_symlink(xfs_trans_log_inode (tp, dp,XFS_ILOG_CORE );/* -* Bump the in memory version number of the parent directory -* so that other processes accessing it will recognize that -* the directory has changed. -*/ - dp->i_gen++; - -/* * If this is a synchronous mount, make sure that the * symlink transaction goes to disk before returning to * the user.
Signature · 1 line
1.5.6
Quoted 4 lines · Dave Chinner
i_genis incremented in directory operations when the directory is changed. It is never read or otherwise used so it should be removed to help reduce the size of the structxfs_inode.
It also removes a xfs_trans_log_inode in xfs_remove, which looks fine to me from a quick glance, but this should be verified and documented.
Otherwise looks good.
Quoted 9 lines · Christoph Hellwig
Quoted 4 lines · Dave Chinner
i_genis incremented in directory operations when the directory is changed. It is never read or otherwise used so it should be removed to help reduce the size of the structxfs_inode.It also removes a
xfs_trans_log_inodeinxfs_remove, which looks fine to me from a quick glance, but this should be verified and documented.
If you note in the very next if/else statement the directory inode gets logged - either by dropping the link or directly. IOWs, we were logging the directory inode twice with the same value (XFS_ILOG_CORE). So I don't think thereśs any problem here.
Cheers,
Dave.
Signature · 2 lines
Dave Chinner [email protected]
Quoted 7 lines · Dave Chinner
Quoted 6 lines · Christoph Hellwig
Quoted 4 lines · Dave Chinner
i_genis incremented in directory operations when the directory is changed. It is never read or otherwise used so it should be removed to help reduce the size of the structxfs_inode.
Quoted 8 lines
Quoted 3 lines
It also removes a
xfs_trans_log_inodeinxfs_remove, which looks fine to me from a quick glance, but this should be verified and documented.If you note in the very next if/else statement the directory inode gets logged - either by dropping the link or directly. IOWs, we were logging the directory inode twice with the same value (
XFS_ILOG_CORE). So I don't think there??s any problem here.
Indeed. The patch looks good but a little description like the one above in the commit message would be nice.
Quoted 12 lines · Christoph Hellwig
Quoted 11 lines · Dave Chinner
Quoted 9 lines · Christoph Hellwig
Quoted 4 lines · Dave Chinner
i_genis incremented in directory operations when the directory is changed. It is never read or otherwise used so it should be removed to help reduce the size of the structxfs_inode.It also removes a
xfs_trans_log_inodeinxfs_remove, which looks fine to me from a quick glance, but this should be verified and documented.
Quoted 7 lines
Quoted 4 lines
If you note in the very next if/else statement the directory inode gets logged - either by dropping the link or directly. IOWs, we were logging the directory inode twice with the same value (
XFS_ILOG_CORE). So I don't think there??s any problem here.Indeed. The patch looks good but a little description like the one above in the commit message would be nice.
Updated patch below.
Cheers,
Dave.
Signature · 21 lines
Dave Chinner [email protected] XFS: remove i_gen from incore inode i_gen is incremented in directory operations when the directory is changed. It is never read or otherwise used so it should be removed to help reduce the size of the struct xfs_inode. The patch also removes a duplicate logging of the directory inode core. We only need to do this once per transaction so kill the one associated with the i_gen increment. Signed-off-by: Dave Chinner <[email protected]> --- fs/xfs/xfs_inode.h | 1 - fs/xfs/xfs_rename.c | 12 ++---------- fs/xfs/xfs_vnodeops.c | 29 ++--------------------------- 3 files changed, 4 insertions(+), 38 deletions(-)
fs/xfs/xfs_inode.h +0 -1
@@ -223,7 +223,6 @@ typedef struct xfs_inode {unsigned short i_flags;/* see defined flags below */ unsigned char i_update_core;/* timestamps/size is dirty */ unsigned char i_update_size;/* di_size field is dirty */ -unsigned int i_gen;/* generation count */ unsigned int i_delayed_blks;/* count of delay alloc blks */ xfs_icdinode_t i_d;/* most of ondisk inode */
fs/xfs/xfs_rename.c +2 -10
@@ -367,19 +367,11 @@ xfs_rename( &first_block, &free_list, spaceres);if (error) goto abort_return; -xfs_ichgtime (src_dp,XFS_ICHGTIME_MOD |XFS_ICHGTIME_CHG ); -/* -* Update the generation counts on all the directory inodes -* that we're modifying. -*/ - src_dp->i_gen++; +xfs_ichgtime (src_dp,XFS_ICHGTIME_MOD |XFS_ICHGTIME_CHG );xfs_trans_log_inode (tp, src_dp,XFS_ILOG_CORE ); - -if (new_parent) { - target_dp->i_gen++; +if (new_parent)xfs_trans_log_inode (tp, target_dp,XFS_ILOG_CORE ); - }/* * If this is a synchronous mount, make sure that the
fs/xfs/xfs_vnodeops.c +2 -27
@@ -1798,8 +1798,6 @@ xfs_create(xfs_trans_set_sync (tp); } - dp->i_gen++; -/* * Attach the dquot(s) to the inodes and modify them incore. * These ids of the inode couldn't have changed since the new
@@ -2229,13 +2227,6 @@ xfs_remove( }xfs_ichgtime (dp,XFS_ICHGTIME_MOD |XFS_ICHGTIME_CHG ); -/* -* Bump the in memory generation count on the parent -* directory so that other can know that it has changed. -*/ - dp->i_gen++; -xfs_trans_log_inode (tp, dp,XFS_ILOG_CORE ); -if (is_dir) {/* * Drop the link from ip's "..".
@@ -2253,8 +2244,8 @@ xfs_remove( }else {/* * When removing a non-directory we need to log the parent -* inode here for the i_gen update. For a directory this is -* done implicitly by the xfs_droplink call for the ".." entry. +* inode here. For a directory this is done implicitly +* by the xfs_droplink call for the ".." entry. */ xfs_trans_log_inode (tp, dp,XFS_ILOG_CORE ); }
@@ -2423,7 +2414,6 @@ xfs_link(if (error) goto abort_return;xfs_ichgtime (tdp,XFS_ICHGTIME_MOD |XFS_ICHGTIME_CHG ); - tdp->i_gen++;xfs_trans_log_inode (tp, tdp,XFS_ILOG_CORE ); error =xfs_bumplink (tp, sip);
@@ -2600,18 +2590,10 @@ xfs_mkdir( }xfs_ichgtime (dp,XFS_ICHGTIME_MOD |XFS_ICHGTIME_CHG ); -/* -* Bump the in memory version number of the parent directory -* so that other processes accessing it will recognize that -* the directory has changed. -*/ - dp->i_gen++; - error =xfs_dir_init (tp, cdp, dp);if (error) goto error2; - cdp->i_gen =1 ; error =xfs_bumplink (tp, dp);if (error) goto error2;
@@ -2898,13 +2880,6 @@ xfs_symlink(xfs_trans_log_inode (tp, dp,XFS_ILOG_CORE );/* -* Bump the in memory version number of the parent directory -* so that other processes accessing it will recognize that -* the directory has changed. -*/ - dp->i_gen++; - -/* * If this is a synchronous mount, make sure that the * symlink transaction goes to disk before returning to * the user.
Looks good.
Quoted 4 lines · Dave Chinner
Quoted 2 lines
Indeed. The patch looks good but a little description like the one above in the commit message would be nice.
Updated patch below.
Missing the needed xfsidbg changes. Updated version below:
fs/xfs/xfs_inode.h +0 -1
@@ -230,7 +230,6 @@ typedef struct xfs_inode {unsigned short i_flags;/* see defined flags below */ unsigned char i_update_core;/* timestamps/size is dirty */ unsigned char i_update_size;/* di_size field is dirty */ -unsigned int i_gen;/* generation count */ unsigned int i_delayed_blks;/* count of delay alloc blks */ xfs_icdinode_t i_d;/* most of ondisk inode */
fs/xfs/xfs_rename.c +2 -10
@@ -367,19 +367,11 @@ xfs_rename( &first_block, &free_list, spaceres);if (error) goto abort_return; -xfs_ichgtime (src_dp,XFS_ICHGTIME_MOD |XFS_ICHGTIME_CHG ); -/* -* Update the generation counts on all the directory inodes -* that we're modifying. -*/ - src_dp->i_gen++; +xfs_ichgtime (src_dp,XFS_ICHGTIME_MOD |XFS_ICHGTIME_CHG );xfs_trans_log_inode (tp, src_dp,XFS_ILOG_CORE ); - -if (new_parent) { - target_dp->i_gen++; +if (new_parent)xfs_trans_log_inode (tp, target_dp,XFS_ILOG_CORE ); - }/* * If this is a synchronous mount, make sure that the
fs/xfs/xfs_vnodeops.c +2 -27
@@ -1625,8 +1625,6 @@ xfs_create(xfs_trans_set_sync (tp); } - dp->i_gen++; -/* * Attach the dquot(s) to the inodes and modify them incore. * These ids of the inode couldn't have changed since the new
@@ -1985,13 +1983,6 @@ xfs_remove( }xfs_ichgtime (dp,XFS_ICHGTIME_MOD |XFS_ICHGTIME_CHG ); -/* -* Bump the in memory generation count on the parent -* directory so that other can know that it has changed. -*/ - dp->i_gen++; -xfs_trans_log_inode (tp, dp,XFS_ILOG_CORE ); -if (is_dir) {/* * Drop the link from ip's "..".
@@ -2009,8 +2000,8 @@ xfs_remove( }else {/* * When removing a non-directory we need to log the parent -* inode here for the i_gen update. For a directory this is -* done implicitly by the xfs_droplink call for the ".." entry. +* inode here. For a directory this is done implicitly +* by the xfs_droplink call for the ".." entry. */ xfs_trans_log_inode (tp, dp,XFS_ILOG_CORE ); }
@@ -2170,7 +2161,6 @@ xfs_link(if (error) goto abort_return;xfs_ichgtime (tdp,XFS_ICHGTIME_MOD |XFS_ICHGTIME_CHG ); - tdp->i_gen++;xfs_trans_log_inode (tp, tdp,XFS_ILOG_CORE ); error =xfs_bumplink (tp, sip);
@@ -2347,18 +2337,10 @@ xfs_mkdir( }xfs_ichgtime (dp,XFS_ICHGTIME_MOD |XFS_ICHGTIME_CHG ); -/* -* Bump the in memory version number of the parent directory -* so that other processes accessing it will recognize that -* the directory has changed. -*/ - dp->i_gen++; - error =xfs_dir_init (tp, cdp, dp);if (error) goto error2; - cdp->i_gen =1 ; error =xfs_bumplink (tp, dp);if (error) goto error2;
@@ -2645,13 +2627,6 @@ xfs_symlink(xfs_trans_log_inode (tp, dp,XFS_ILOG_CORE );/* -* Bump the in memory version number of the parent directory -* so that other processes accessing it will recognize that -* the directory has changed. -*/ - dp->i_gen++; - -/* * If this is a synchronous mount, make sure that the * symlink transaction goes to disk before returning to * the user.
fs/xfs/xfsidbg.c +2 -6
@@ -3633,7 +3633,6 @@ xfs_prdinode_incore(xfs_icdinode_t *dip)xfs_fmtformat ((xfs_dinode_fmt_t )dip->di_aformat), dip->di_dmevmask, dip->di_dmstate);printflags (dip->di_flags, diflags,"flags" ); -kdb_printf ("gen 0x%x\n" , dip->di_gen); }#ifdef XFS_RW_TRACE
@@ -6385,11 +6384,8 @@ xfsidbg_xnode(xfs_inode_t *ip)kdb_printf ("\n" );kdb_printf ("update_core %d update size %d\n" , (int )(ip->i_update_core), (int ) ip->i_update_size); -kdb_printf ("gen 0x%x delayed blks %d" , - ip->i_gen, - ip->i_delayed_blks); -kdb_printf ("size %lld\n" , - ip->i_size); +kdb_printf ("delayed blks %d size %lld\n" , + ip->i_delayed_blks, ip->i_size);#ifdef XFS_INODE_TRACE qprintf (" trace 0x%p\n" , ip->i_trace);
Any chance we can get this patch in? It's been out for exactly 4 month in it's final reviewed form, and I still have patches that I rebased on top of it waiting..
Quoted 127 lines · Dave Chinner
XFS: remove
i_genfrom incore inode
i_genis incremented in directory operations when the directory is changed. It is never read or otherwise used so it should be removed to help reduce the size of the structxfs_inode.The patch also removes a duplicate logging of the directory inode core. We only need to do this once per transaction so kill the one associated with the
i_genincrement.Signed-off-by Dave Chinner <[email protected]>--- fs/xfs/xfs_inode.h | 1 - fs/xfs/xfs_rename.c | 12 ++---------- fs/xfs/xfs_vnodeops.c | 29 ++--------------------------- 3 files changed, 4 insertions(+), 38 deletions(-)fs/xfs/xfs_inode.h +0
-1@@ -223,7 +223,6 @@ typedef struct xfs_inode {unsigned short i_flags;/* see defined flags below */ unsigned char i_update_core;/* timestamps/size is dirty */ unsigned char i_update_size;/* di_size field is dirty */ -unsigned int i_gen;/* generation count */ unsigned int i_delayed_blks;/* count of delay alloc blks */ xfs_icdinode_t i_d;/* most of ondisk inode */ fs/xfs/xfs_rename.c +2
-10@@ -367,19 +367,11 @@ xfs_rename( &first_block, &free_list, spaceres);if (error) goto abort_return; -xfs_ichgtime (src_dp,XFS_ICHGTIME_MOD |XFS_ICHGTIME_CHG ); -/* -* Update the generation counts on all the directory inodes -* that we're modifying. -*/ - src_dp->i_gen++; +xfs_ichgtime (src_dp,XFS_ICHGTIME_MOD |XFS_ICHGTIME_CHG );xfs_trans_log_inode (tp, src_dp,XFS_ILOG_CORE ); - -if (new_parent) { - target_dp->i_gen++; +if (new_parent)xfs_trans_log_inode (tp, target_dp,XFS_ILOG_CORE ); - }/* * If this is a synchronous mount, make sure that the fs/xfs/xfs_vnodeops.c +2
-20@@ -1798,8 +1798,6 @@ xfs_create(xfs_trans_set_sync (tp); } - dp->i_gen++; -/* * Attach the dquot(s) to the inodes and modify them incore. * These ids of the inode couldn't have changed since the new @@ -2229,13 +2227,6 @@ xfs_remove( }xfs_ichgtime (dp,XFS_ICHGTIME_MOD |XFS_ICHGTIME_CHG ); -/* -* Bump the in memory generation count on the parent -* directory so that other can know that it has changed. -*/ - dp->i_gen++; -xfs_trans_log_inode (tp, dp,XFS_ILOG_CORE ); -if (is_dir) {/* * Drop the link from ip's "..". @@ -2253,8 +2244,8 @@ xfs_remove( }else {/* * When removing a non-directory we need to log the parent -* inode here for the i_gen update. For a directory this is -* done implicitly by the xfs_droplink call for the ".." entry. +* inode here. For a directory this is done implicitly +* by the xfs_droplink call for the ".." entry. */ xfs_trans_log_inode (tp, dp,XFS_ILOG_CORE ); }@@ -2423,7 +2414,6 @@ xfs_link(if (error) goto abort_return;xfs_ichgtime (tdp,XFS_ICHGTIME_MOD |XFS_ICHGTIME_CHG ); - tdp->i_gen++;xfs_trans_log_inode (tp, tdp,XFS_ILOG_CORE ); error =xfs_bumplink (tp, sip);@@ -2600,18 +2590,10 @@ xfs_mkdir( }xfs_ichgtime (dp,XFS_ICHGTIME_MOD |XFS_ICHGTIME_CHG ); -/* -* Bump the in memory version number of the parent directory -* so that other processes accessing it will recognize that -* the directory has changed. -*/ - dp->i_gen++; - error =xfs_dir_init (tp, cdp, dp);if (error) goto error2; - cdp->i_gen =1 ; error =xfs_bumplink (tp, dp);if (error) goto error2;@@ -2898,13 +2880,6 @@ xfs_symlink(xfs_trans_log_inode (tp, dp,XFS_ILOG_CORE );/*
fs/xfs/xfs_vnodeops.c xfs_symlink( · line 2885
Quoted 10 lines
fs/xfs/xfs_vnodeops.c +0
-7-* Bump the in memory version number of the parent directory -* so that other processes accessing it will recognize that -* the directory has changed. -*/ - dp->i_gen++; - -/* * If this is a synchronous mount, make sure that the * symlink transaction goes to disk before returning to * the user.
---end quoted text---
_______________________________________________
xfs mailing list
[email protected]
http://oss.sgi.com/mailman/listinfo/xfs