[1/8] xfs: clean up iunlink functions
diff mbox series

Message ID 154897667679.26065.17995098675615600714.stgit@magnolia
State New
Headers show
Series
  • xfs: incore unlinked list
Related show

Commit Message

Darrick J. Wong Jan. 31, 2019, 11:17 p.m. UTC
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(-)

Comments

Christoph Hellwig Feb. 1, 2019, 8:01 a.m. UTC | #1
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.
Darrick J. Wong Feb. 2, 2019, 7:15 p.m. UTC | #2
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

Patch
diff mbox series

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,