| Message ID | 154897667679.26065.17995098675615600714.stgit@magnolia (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | xfs: incore unlinked list | expand |
On Thu, Jan 31, 2019 at 03:17:56PM -0800, Darrick J. Wong wrote: > From: Darrick J. Wong <darrick.wong@oracle.com> > > Fix some indentation issues with the iunlink functions and reorganize > the tops of the functions to be identical. No functional changes. > > Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com> > --- > fs/xfs/xfs_inode.c | 77 +++++++++++++++++++++++++++------------------------- > 1 file changed, 40 insertions(+), 37 deletions(-) > > > diff --git a/fs/xfs/xfs_inode.c b/fs/xfs/xfs_inode.c > index c8bf02be0003..d18354517320 100644 > --- a/fs/xfs/xfs_inode.c > +++ b/fs/xfs/xfs_inode.c > @@ -1892,26 +1892,32 @@ xfs_inactive( > */ > STATIC int > xfs_iunlink( > - struct xfs_trans *tp, > - struct xfs_inode *ip) > + struct xfs_trans *tp, > + struct xfs_inode *ip) > { > - xfs_mount_t *mp = tp->t_mountp; > - xfs_agi_t *agi; > - xfs_dinode_t *dip; > - xfs_buf_t *agibp; > - xfs_buf_t *ibp; > - xfs_agino_t agino; > - short bucket_index; > - int offset; > - int error; > + struct xfs_mount *mp = tp->t_mountp; > + struct xfs_agi *agi; > + struct xfs_dinode *dip; > + struct xfs_buf *agibp; > + struct xfs_buf *ibp; > + xfs_agnumber_t agno; > + xfs_agino_t agino; > + short bucket_index; > + int offset; > + int error; > > ASSERT(VFS_I(ip)->i_mode != 0); > + ASSERT(xfs_verify_ino(mp, ip->i_ino)); This verify_ino calls seems odd given that we pass in an initialized inode struct.. > + > + agno = XFS_INO_TO_AGNO(mp, ip->i_ino); > + agino = XFS_INO_TO_AGINO(mp, ip->i_ino); And then we could move these initializations up int othe lines with the variable declarations. > + bucket_index = agino % XFS_AGI_UNLINKED_BUCKETS; > > /* > * Get the agi buffer first. It ensures lock ordering > * on the list. > */ If you are touching this anyway, the whole comment could be changed into a single line.. > + if (!xfs_verify_ino(mp, ip->i_ino)) > + return -EFSCORRUPTED; > > - mp = tp->t_mountp; > agno = XFS_INO_TO_AGNO(mp, ip->i_ino); > + agino = XFS_INO_TO_AGINO(mp, ip->i_ino); Same here.
On Fri, Feb 01, 2019 at 12:01:36AM -0800, Christoph Hellwig wrote: > On Thu, Jan 31, 2019 at 03:17:56PM -0800, Darrick J. Wong wrote: > > From: Darrick J. Wong <darrick.wong@oracle.com> > > > > Fix some indentation issues with the iunlink functions and reorganize > > the tops of the functions to be identical. No functional changes. > > > > Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com> > > --- > > fs/xfs/xfs_inode.c | 77 +++++++++++++++++++++++++++------------------------- > > 1 file changed, 40 insertions(+), 37 deletions(-) > > > > > > diff --git a/fs/xfs/xfs_inode.c b/fs/xfs/xfs_inode.c > > index c8bf02be0003..d18354517320 100644 > > --- a/fs/xfs/xfs_inode.c > > +++ b/fs/xfs/xfs_inode.c > > @@ -1892,26 +1892,32 @@ xfs_inactive( > > */ > > STATIC int > > xfs_iunlink( > > - struct xfs_trans *tp, > > - struct xfs_inode *ip) > > + struct xfs_trans *tp, > > + struct xfs_inode *ip) > > { > > - xfs_mount_t *mp = tp->t_mountp; > > - xfs_agi_t *agi; > > - xfs_dinode_t *dip; > > - xfs_buf_t *agibp; > > - xfs_buf_t *ibp; > > - xfs_agino_t agino; > > - short bucket_index; > > - int offset; > > - int error; > > + struct xfs_mount *mp = tp->t_mountp; > > + struct xfs_agi *agi; > > + struct xfs_dinode *dip; > > + struct xfs_buf *agibp; > > + struct xfs_buf *ibp; > > + xfs_agnumber_t agno; > > + xfs_agino_t agino; > > + short bucket_index; > > + int offset; > > + int error; > > > > ASSERT(VFS_I(ip)->i_mode != 0); > > + ASSERT(xfs_verify_ino(mp, ip->i_ino)); > > This verify_ino calls seems odd given that we pass in an initialized > inode struct.. > > > + > > + agno = XFS_INO_TO_AGNO(mp, ip->i_ino); > > + agino = XFS_INO_TO_AGINO(mp, ip->i_ino); > > And then we could move these initializations up int othe lines with > the variable declarations. > > > + bucket_index = agino % XFS_AGI_UNLINKED_BUCKETS; > > > > /* > > * Get the agi buffer first. It ensures lock ordering > > * on the list. > > */ > > If you are touching this anyway, the whole comment could be changed > into a single line.. > > > + if (!xfs_verify_ino(mp, ip->i_ino)) > > + return -EFSCORRUPTED; > > > > - mp = tp->t_mountp; > > agno = XFS_INO_TO_AGNO(mp, ip->i_ino); > > + agino = XFS_INO_TO_AGINO(mp, ip->i_ino); > > Same here. Fixed all of these. --D
diff --git a/fs/xfs/xfs_inode.c b/fs/xfs/xfs_inode.c index c8bf02be0003..d18354517320 100644 --- a/fs/xfs/xfs_inode.c +++ b/fs/xfs/xfs_inode.c @@ -1892,26 +1892,32 @@ xfs_inactive( */ STATIC int xfs_iunlink( - struct xfs_trans *tp, - struct xfs_inode *ip) + struct xfs_trans *tp, + struct xfs_inode *ip) { - xfs_mount_t *mp = tp->t_mountp; - xfs_agi_t *agi; - xfs_dinode_t *dip; - xfs_buf_t *agibp; - xfs_buf_t *ibp; - xfs_agino_t agino; - short bucket_index; - int offset; - int error; + struct xfs_mount *mp = tp->t_mountp; + struct xfs_agi *agi; + struct xfs_dinode *dip; + struct xfs_buf *agibp; + struct xfs_buf *ibp; + xfs_agnumber_t agno; + xfs_agino_t agino; + short bucket_index; + int offset; + int error; ASSERT(VFS_I(ip)->i_mode != 0); + ASSERT(xfs_verify_ino(mp, ip->i_ino)); + + agno = XFS_INO_TO_AGNO(mp, ip->i_ino); + agino = XFS_INO_TO_AGINO(mp, ip->i_ino); + bucket_index = agino % XFS_AGI_UNLINKED_BUCKETS; /* * Get the agi buffer first. It ensures lock ordering * on the list. */ - error = xfs_read_agi(mp, tp, XFS_INO_TO_AGNO(mp, ip->i_ino), &agibp); + error = xfs_read_agi(mp, tp, agno, &agibp); if (error) return error; agi = XFS_BUF_TO_AGI(agibp); @@ -1920,9 +1926,6 @@ xfs_iunlink( * Get the index into the agi hash table for the * list this inode will go on. */ - agino = XFS_INO_TO_AGINO(mp, ip->i_ino); - ASSERT(agino != 0); - bucket_index = agino % XFS_AGI_UNLINKED_BUCKETS; ASSERT(agi->agi_unlinked[bucket_index]); ASSERT(be32_to_cpu(agi->agi_unlinked[bucket_index]) != agino); @@ -1969,26 +1972,31 @@ xfs_iunlink( */ STATIC int xfs_iunlink_remove( - xfs_trans_t *tp, - xfs_inode_t *ip) + struct xfs_trans *tp, + struct xfs_inode *ip) { - xfs_ino_t next_ino; - xfs_mount_t *mp; - xfs_agi_t *agi; - xfs_dinode_t *dip; - xfs_buf_t *agibp; - xfs_buf_t *ibp; - xfs_agnumber_t agno; - xfs_agino_t agino; - xfs_agino_t next_agino; - xfs_buf_t *last_ibp; - xfs_dinode_t *last_dip = NULL; - short bucket_index; - int offset, last_offset = 0; - int error; + struct xfs_mount *mp = tp->t_mountp; + struct xfs_agi *agi; + struct xfs_dinode *dip; + struct xfs_buf *agibp; + struct xfs_buf *ibp; + struct xfs_buf *last_ibp; + struct xfs_dinode *last_dip = NULL; + xfs_ino_t next_ino; + xfs_agnumber_t agno; + xfs_agino_t agino; + xfs_agino_t next_agino; + short bucket_index; + int offset; + int last_offset = 0; + int error; + + if (!xfs_verify_ino(mp, ip->i_ino)) + return -EFSCORRUPTED; - mp = tp->t_mountp; agno = XFS_INO_TO_AGNO(mp, ip->i_ino); + agino = XFS_INO_TO_AGINO(mp, ip->i_ino); + bucket_index = agino % XFS_AGI_UNLINKED_BUCKETS; /* * Get the agi buffer first. It ensures lock ordering @@ -1997,17 +2005,12 @@ xfs_iunlink_remove( error = xfs_read_agi(mp, tp, agno, &agibp); if (error) return error; - agi = XFS_BUF_TO_AGI(agibp); /* * Get the index into the agi hash table for the * list this inode will go on. */ - agino = XFS_INO_TO_AGINO(mp, ip->i_ino); - if (!xfs_verify_agino(mp, agno, agino)) - return -EFSCORRUPTED; - bucket_index = agino % XFS_AGI_UNLINKED_BUCKETS; if (!xfs_verify_agino(mp, agno, be32_to_cpu(agi->agi_unlinked[bucket_index]))) { XFS_CORRUPTION_ERROR(__func__, XFS_ERRLEVEL_LOW, mp,