From f2fc16a3d7c12224d4c19055fef40ca6379b2045 Mon Sep 17 00:00:00 2001 From: "Darrick J. Wong" Date: Thu, 7 Feb 2019 10:37:15 -0800 Subject: [PATCH] xfs: refactor inode unlinked pointer update functions Hoist the functions that update an inode's unlinked pointer updates into a helper. No functional changes. Signed-off-by: Darrick J. Wong Reviewed-by: Christoph Hellwig Reviewed-by: Brian Foster --- fs/xfs/xfs_inode.c | 191 +++++++++++++++++++++++---------------------- fs/xfs/xfs_trace.h | 26 ++++++ 2 files changed, 125 insertions(+), 92 deletions(-) diff --git a/fs/xfs/xfs_inode.c b/fs/xfs/xfs_inode.c index ba19cfd52b5e..eb51fa33f91a 100644 --- a/fs/xfs/xfs_inode.c +++ b/fs/xfs/xfs_inode.c @@ -1943,6 +1943,85 @@ xfs_iunlink_update_bucket( return 0; } +/* Set an on-disk inode's next_unlinked pointer. */ +STATIC void +xfs_iunlink_update_dinode( + struct xfs_trans *tp, + xfs_agnumber_t agno, + xfs_agino_t agino, + struct xfs_buf *ibp, + struct xfs_dinode *dip, + struct xfs_imap *imap, + xfs_agino_t next_agino) +{ + struct xfs_mount *mp = tp->t_mountp; + int offset; + + ASSERT(xfs_verify_agino_or_null(mp, agno, next_agino)); + + trace_xfs_iunlink_update_dinode(mp, agno, agino, + be32_to_cpu(dip->di_next_unlinked), next_agino); + + dip->di_next_unlinked = cpu_to_be32(next_agino); + offset = imap->im_boffset + + offsetof(struct xfs_dinode, di_next_unlinked); + + /* need to recalc the inode CRC if appropriate */ + xfs_dinode_calc_crc(mp, dip); + xfs_trans_inode_buf(tp, ibp); + xfs_trans_log_buf(tp, ibp, offset, offset + sizeof(xfs_agino_t) - 1); + xfs_inobp_check(mp, ibp); +} + +/* Set an in-core inode's unlinked pointer and return the old value. */ +STATIC int +xfs_iunlink_update_inode( + struct xfs_trans *tp, + struct xfs_inode *ip, + xfs_agnumber_t agno, + xfs_agino_t next_agino, + xfs_agino_t *old_next_agino) +{ + struct xfs_mount *mp = tp->t_mountp; + struct xfs_dinode *dip; + struct xfs_buf *ibp; + xfs_agino_t old_value; + int error; + + ASSERT(xfs_verify_agino_or_null(mp, agno, next_agino)); + + error = xfs_imap_to_bp(mp, tp, &ip->i_imap, &dip, &ibp, 0, 0); + if (error) + return error; + + /* Make sure the old pointer isn't garbage. */ + old_value = be32_to_cpu(dip->di_next_unlinked); + if (!xfs_verify_agino_or_null(mp, agno, old_value)) { + error = -EFSCORRUPTED; + goto out; + } + + /* + * Since we're updating a linked list, we should never find that the + * current pointer is the same as the new value, unless we're + * terminating the list. + */ + *old_next_agino = old_value; + if (old_value == next_agino) { + if (next_agino != NULLAGINO) + error = -EFSCORRUPTED; + goto out; + } + + /* Ok, update the new pointer. */ + xfs_iunlink_update_dinode(tp, agno, XFS_INO_TO_AGINO(mp, ip->i_ino), + ibp, dip, &ip->i_imap, next_agino); + return 0; +out: + xfs_trans_brelse(tp, ibp); + return error; +} + /* * This is called when the inode's link count goes to 0 or we are creating a * tmpfile via O_TMPFILE. In the case of a tmpfile, @ignore_linkcount will be @@ -1960,14 +2039,11 @@ xfs_iunlink( { struct xfs_mount *mp = tp->t_mountp; struct xfs_agi *agi; - struct xfs_dinode *dip; struct xfs_buf *agibp; - struct xfs_buf *ibp; xfs_agino_t next_agino; xfs_agnumber_t agno = XFS_INO_TO_AGNO(mp, ip->i_ino); xfs_agino_t agino = XFS_INO_TO_AGINO(mp, ip->i_ino); short bucket_index = agino % XFS_AGI_UNLINKED_BUCKETS; - int offset; int error; ASSERT(VFS_I(ip)->i_mode != 0); @@ -1989,29 +2065,17 @@ xfs_iunlink( return -EFSCORRUPTED; if (next_agino != NULLAGINO) { + xfs_agino_t old_agino; + /* - * There is already another inode in the bucket we need - * to add ourselves to. Add us at the front of the list. - * Here we put the head pointer into our next pointer, - * and then we fall through to point the head at us. + * There is already another inode in the bucket, so point this + * inode to the current head of the list. */ - error = xfs_imap_to_bp(mp, tp, &ip->i_imap, &dip, &ibp, - 0, 0); + error = xfs_iunlink_update_inode(tp, ip, agno, next_agino, + &old_agino); if (error) return error; - - ASSERT(dip->di_next_unlinked == cpu_to_be32(NULLAGINO)); - dip->di_next_unlinked = agi->agi_unlinked[bucket_index]; - offset = ip->i_imap.im_boffset + - offsetof(xfs_dinode_t, di_next_unlinked); - - /* need to recalc the inode CRC if appropriate */ - xfs_dinode_calc_crc(mp, dip); - - xfs_trans_inode_buf(tp, ibp); - xfs_trans_log_buf(tp, ibp, offset, - (offset + sizeof(xfs_agino_t) - 1)); - xfs_inobp_check(mp, ibp); + ASSERT(old_agino == NULLAGINO); } /* Point the head of the list to point to this inode. */ @@ -2028,9 +2092,7 @@ xfs_iunlink_remove( { 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; @@ -2038,8 +2100,6 @@ xfs_iunlink_remove( xfs_agino_t agino = XFS_INO_TO_AGINO(mp, ip->i_ino); xfs_agino_t next_agino; short bucket_index = agino % XFS_AGI_UNLINKED_BUCKETS; - int offset; - int last_offset = 0; int error; /* Get the agi buffer first. It ensures lock ordering on the list. */ @@ -2063,34 +2123,11 @@ xfs_iunlink_remove( /* * We're at the head of the list. Get the inode's on-disk * buffer to see if there is anyone after us on the list. - * Only modify our next pointer if it is not already NULLAGINO. - * This saves us the overhead of dealing with the buffer when - * there is no need to change it. */ - error = xfs_imap_to_bp(mp, tp, &ip->i_imap, &dip, &ibp, - 0, 0); - if (error) { - xfs_warn(mp, "%s: xfs_imap_to_bp returned error %d.", - __func__, error); + error = xfs_iunlink_update_inode(tp, ip, agno, NULLAGINO, + &next_agino); + if (error) return error; - } - next_agino = be32_to_cpu(dip->di_next_unlinked); - ASSERT(next_agino != 0); - if (next_agino != NULLAGINO) { - dip->di_next_unlinked = cpu_to_be32(NULLAGINO); - offset = ip->i_imap.im_boffset + - offsetof(xfs_dinode_t, di_next_unlinked); - - /* need to recalc the inode CRC if appropriate */ - xfs_dinode_calc_crc(mp, dip); - - xfs_trans_inode_buf(tp, ibp); - xfs_trans_log_buf(tp, ibp, offset, - (offset + sizeof(xfs_agino_t) - 1)); - xfs_inobp_check(mp, ibp); - } else { - xfs_trans_brelse(tp, ibp); - } /* Point the head of the list to the next unlinked inode. */ error = xfs_iunlink_update_bucket(tp, agno, agibp, bucket_index, @@ -2098,13 +2135,14 @@ xfs_iunlink_remove( if (error) return error; } else { + struct xfs_imap imap; + xfs_agino_t prev_agino; + /* * We need to search the list for the inode being freed. */ last_ibp = NULL; while (next_agino != agino) { - struct xfs_imap imap; - if (last_ibp) xfs_trans_brelse(tp, last_ibp); @@ -2128,7 +2166,7 @@ xfs_iunlink_remove( return error; } - last_offset = imap.im_boffset; + prev_agino = next_agino; next_agino = be32_to_cpu(last_dip->di_next_unlinked); if (!xfs_verify_agino(mp, agno, next_agino)) { XFS_CORRUPTION_ERROR(__func__, @@ -2142,45 +2180,14 @@ xfs_iunlink_remove( * Now last_ibp points to the buffer previous to us on the * unlinked list. Pull us from the list. */ - error = xfs_imap_to_bp(mp, tp, &ip->i_imap, &dip, &ibp, - 0, 0); - if (error) { - xfs_warn(mp, "%s: xfs_imap_to_bp(2) returned error %d.", - __func__, error); + error = xfs_iunlink_update_inode(tp, ip, agno, NULLAGINO, + &next_agino); + if (error) return error; - } - next_agino = be32_to_cpu(dip->di_next_unlinked); - ASSERT(next_agino != 0); - ASSERT(next_agino != agino); - if (next_agino != NULLAGINO) { - dip->di_next_unlinked = cpu_to_be32(NULLAGINO); - offset = ip->i_imap.im_boffset + - offsetof(xfs_dinode_t, di_next_unlinked); - - /* need to recalc the inode CRC if appropriate */ - xfs_dinode_calc_crc(mp, dip); - - xfs_trans_inode_buf(tp, ibp); - xfs_trans_log_buf(tp, ibp, offset, - (offset + sizeof(xfs_agino_t) - 1)); - xfs_inobp_check(mp, ibp); - } else { - xfs_trans_brelse(tp, ibp); - } - /* - * Point the previous inode on the list to the next inode. - */ - last_dip->di_next_unlinked = cpu_to_be32(next_agino); - ASSERT(next_agino != 0); - offset = last_offset + offsetof(xfs_dinode_t, di_next_unlinked); - - /* need to recalc the inode CRC if appropriate */ - xfs_dinode_calc_crc(mp, last_dip); - xfs_trans_inode_buf(tp, last_ibp); - xfs_trans_log_buf(tp, last_ibp, offset, - (offset + sizeof(xfs_agino_t) - 1)); - xfs_inobp_check(mp, last_ibp); + /* Point the previous inode on the list to the next inode. */ + xfs_iunlink_update_dinode(tp, agno, prev_agino, last_ibp, + last_dip, &imap, next_agino); } return 0; } diff --git a/fs/xfs/xfs_trace.h b/fs/xfs/xfs_trace.h index c10478e7e49a..fbec8f0e1a9a 100644 --- a/fs/xfs/xfs_trace.h +++ b/fs/xfs/xfs_trace.h @@ -3397,6 +3397,32 @@ TRACE_EVENT(xfs_iunlink_update_bucket, __entry->new_ptr) ); +TRACE_EVENT(xfs_iunlink_update_dinode, + TP_PROTO(struct xfs_mount *mp, xfs_agnumber_t agno, xfs_agino_t agino, + xfs_agino_t old_ptr, xfs_agino_t new_ptr), + TP_ARGS(mp, agno, agino, old_ptr, new_ptr), + TP_STRUCT__entry( + __field(dev_t, dev) + __field(xfs_agnumber_t, agno) + __field(xfs_agino_t, agino) + __field(xfs_agino_t, old_ptr) + __field(xfs_agino_t, new_ptr) + ), + TP_fast_assign( + __entry->dev = mp->m_super->s_dev; + __entry->agno = agno; + __entry->agino = agino; + __entry->old_ptr = old_ptr; + __entry->new_ptr = new_ptr; + ), + TP_printk("dev %d:%d agno %u agino 0x%x old 0x%x new 0x%x", + MAJOR(__entry->dev), MINOR(__entry->dev), + __entry->agno, + __entry->agino, + __entry->old_ptr, + __entry->new_ptr) +); + #endif /* _TRACE_XFS_H */ #undef TRACE_INCLUDE_PATH -- 2.20.1