On Tuesday 5 May 2020 6:42:35 AM IST Darrick J. Wong wrote:
> From: Darrick J. Wong <darrick.wong@xxxxxxxxxx>
>
> Hoist the unlinked inode processing logic out of the AG loop and into
> its own function. No functional changes.
>
> Signed-off-by: Darrick J. Wong <darrick.wong@xxxxxxxxxx>
> ---
> fs/xfs/xfs_unlink_recover.c | 91 +++++++++++++++++++++++++------------------
> 1 file changed, 52 insertions(+), 39 deletions(-)
>
>
> diff --git a/fs/xfs/xfs_unlink_recover.c b/fs/xfs/xfs_unlink_recover.c
> index 2a19d096e88d..413b34085640 100644
> --- a/fs/xfs/xfs_unlink_recover.c
> +++ b/fs/xfs/xfs_unlink_recover.c
> @@ -145,54 +145,67 @@ xlog_recover_process_one_iunlink(
> * scheduled on this CPU to ensure other scheduled work can run without undue
> * latency.
> */
> -void
> -xlog_recover_process_unlinked(
> - struct xlog *log)
> +STATIC int
> +xlog_recover_process_iunlinked(
> + struct xfs_mount *mp,
> + xfs_agnumber_t agno)
> {
> - struct xfs_mount *mp;
> struct xfs_agi *agi;
> struct xfs_buf *agibp;
> - xfs_agnumber_t agno;
> xfs_agino_t agino;
> int bucket;
> int error;
>
> - mp = log->l_mp;
> -
> - for (agno = 0; agno < mp->m_sb.sb_agcount; agno++) {
> - /*
> - * Find the agi for this ag.
> - */
> - error = xfs_read_agi(mp, NULL, agno, &agibp);
> - if (error) {
> - /*
> - * AGI is b0rked. Don't process it.
> - *
> - * We should probably mark the filesystem as corrupt
> - * after we've recovered all the ag's we can....
> - */
> - continue;
> - }
> + /*
> + * Find the agi for this ag.
> + */
> + error = xfs_read_agi(mp, NULL, agno, &agibp);
> + if (error) {
> /*
> - * Unlock the buffer so that it can be acquired in the normal
> - * course of the transaction to truncate and free each inode.
> - * Because we are not racing with anyone else here for the AGI
> - * buffer, we don't even need to hold it locked to read the
> - * initial unlinked bucket entries out of the buffer. We keep
> - * buffer reference though, so that it stays pinned in memory
> - * while we need the buffer.
> + * AGI is b0rked. Don't process it.
> + *
> + * We should probably mark the filesystem as corrupt
> + * after we've recovered all the ag's we can....
> */
> - agi = agibp->b_addr;
> - xfs_buf_unlock(agibp);
> -
> - for (bucket = 0; bucket < XFS_AGI_UNLINKED_BUCKETS; bucket++) {
> - agino = be32_to_cpu(agi->agi_unlinked[bucket]);
> - while (agino != NULLAGINO) {
> - agino = xlog_recover_process_one_iunlink(mp,
> - agno, agino, bucket);
> - cond_resched();
> - }
> + return error;
This causes a change in behaviour i.e. an error return from here would cause
xlog_recover_process_unlinked() to break "loop on all AGs". Before this
change, XFS would continue to process all the remaining AGs as described by
the above comment.
> + }
> +
> + /*
> + * Unlock the buffer so that it can be acquired in the normal
> + * course of the transaction to truncate and free each inode.
> + * Because we are not racing with anyone else here for the AGI
> + * buffer, we don't even need to hold it locked to read the
> + * initial unlinked bucket entries out of the buffer. We keep
> + * buffer reference though, so that it stays pinned in memory
> + * while we need the buffer.
> + */
> + agi = agibp->b_addr;
> + xfs_buf_unlock(agibp);
> +
> + for (bucket = 0; bucket < XFS_AGI_UNLINKED_BUCKETS; bucket++) {
> + agino = be32_to_cpu(agi->agi_unlinked[bucket]);
> + while (agino != NULLAGINO) {
> + agino = xlog_recover_process_one_iunlink(mp,
> + agno, agino, bucket);
> + cond_resched();
> }
> - xfs_buf_rele(agibp);
> + }
> + xfs_buf_rele(agibp);
> +
> + return 0;
> +}
> +
> +void
> +xlog_recover_process_unlinked(
> + struct xlog *log)
> +{
> + struct xfs_mount *mp = log->l_mp;
> + xfs_agnumber_t agno;
> + int error;
> +
> + for (agno = 0; agno < mp->m_sb.sb_agcount; agno++) {
> + error = xlog_recover_process_iunlinked(mp, agno);
> + if (error)
> + break;
> }
> }
>
>
--
chandan
