xfs: fix reloading entire unlinked bucket lists

During review of the patcheset that provided reloading of the incore
iunlink list, Dave made a few suggestions, and I updated the copy in my
dev tree.  Unfortunately, I then got distracted by ... who even knows
what ... and forgot to backport those changes from my dev tree to my
release candidate branch.  I then sent multiple pull requests with stale
patches, and that's what was merged into -rc3.

So.

This patch re-adds the use of an unlocked iunlink list check to
determine if we want to allocate the resources to recreate the incore
list.  Since lost iunlinked inodes are supposed to be rare, this change
helps us avoid paying the transaction and AGF locking costs every time
we open any inode.

This also re-adds the shutdowns on failure, and re-applies the
restructuring of the inner loop in xfs_inode_reload_unlinked_bucket, and
re-adds a requested comment about the quotachecking code.

Retain the original RVB tag from Dave since there's no code change from
the last submission.

Fixes: 68b957f64f ("xfs: load uncached unlinked inodes into memory on demand")
Signed-off-by: Darrick J. Wong <djwong@kernel.org>
Reviewed-by: Dave Chinner <dchinner@redhat.com>
This commit is contained in:
Darrick J. Wong 2023-09-24 08:35:53 -07:00
parent 6465e260f4
commit 537c013b14
4 changed files with 61 additions and 20 deletions

View File

@ -146,10 +146,18 @@ xfs_nfs_get_inode(
return ERR_PTR(error);
}
error = xfs_inode_reload_unlinked(ip);
if (error) {
xfs_irele(ip);
return ERR_PTR(error);
/*
* Reload the incore unlinked list to avoid failure in inodegc.
* Use an unlocked check here because unrecovered unlinked inodes
* should be somewhat rare.
*/
if (xfs_inode_unlinked_incomplete(ip)) {
error = xfs_inode_reload_unlinked(ip);
if (error) {
xfs_force_shutdown(mp, SHUTDOWN_CORRUPT_INCORE);
xfs_irele(ip);
return ERR_PTR(error);
}
}
if (VFS_I(ip)->i_generation != generation) {

View File

@ -1743,6 +1743,14 @@ xfs_inactive(
truncate = 1;
if (xfs_iflags_test(ip, XFS_IQUOTAUNCHECKED)) {
/*
* If this inode is being inactivated during a quotacheck and
* has not yet been scanned by quotacheck, we /must/ remove
* the dquots from the inode before inactivation changes the
* block and inode counts. Most probably this is a result of
* reloading the incore iunlinked list to purge unrecovered
* unlinked inodes.
*/
xfs_qm_dqdetach(ip);
} else {
error = xfs_qm_dqattach(ip);
@ -3641,6 +3649,16 @@ xfs_inode_reload_unlinked_bucket(
if (error)
return error;
/*
* We've taken ILOCK_SHARED and the AGI buffer lock to stabilize the
* incore unlinked list pointers for this inode. Check once more to
* see if we raced with anyone else to reload the unlinked list.
*/
if (!xfs_inode_unlinked_incomplete(ip)) {
foundit = true;
goto out_agibp;
}
bucket = agino % XFS_AGI_UNLINKED_BUCKETS;
agi = agibp->b_addr;
@ -3655,25 +3673,27 @@ xfs_inode_reload_unlinked_bucket(
while (next_agino != NULLAGINO) {
struct xfs_inode *next_ip = NULL;
/* Found this caller's inode, set its backlink. */
if (next_agino == agino) {
/* Found this inode, set its backlink. */
next_ip = ip;
next_ip->i_prev_unlinked = prev_agino;
foundit = true;
goto next_inode;
}
if (!next_ip) {
/* Inode already in memory. */
next_ip = xfs_iunlink_lookup(pag, next_agino);
}
if (!next_ip) {
/* Inode not in memory, reload. */
error = xfs_iunlink_reload_next(tp, agibp, prev_agino,
next_agino);
if (error)
break;
next_ip = xfs_iunlink_lookup(pag, next_agino);
}
/* Try in-memory lookup first. */
next_ip = xfs_iunlink_lookup(pag, next_agino);
if (next_ip)
goto next_inode;
/* Inode not in memory, try reloading it. */
error = xfs_iunlink_reload_next(tp, agibp, prev_agino,
next_agino);
if (error)
break;
/* Grab the reloaded inode. */
next_ip = xfs_iunlink_lookup(pag, next_agino);
if (!next_ip) {
/* No incore inode at all? We reloaded it... */
ASSERT(next_ip != NULL);
@ -3681,10 +3701,12 @@ xfs_inode_reload_unlinked_bucket(
break;
}
next_inode:
prev_agino = next_agino;
next_agino = next_ip->i_next_unlinked;
}
out_agibp:
xfs_trans_brelse(tp, agibp);
/* Should have found this inode somewhere in the iunlinked bucket. */
if (!error && !foundit)

View File

@ -80,10 +80,12 @@ xfs_bulkstat_one_int(
if (error)
goto out;
/* Reload the incore unlinked list to avoid failure in inodegc. */
if (xfs_inode_unlinked_incomplete(ip)) {
error = xfs_inode_reload_unlinked_bucket(tp, ip);
if (error) {
xfs_iunlock(ip, XFS_ILOCK_SHARED);
xfs_force_shutdown(mp, SHUTDOWN_CORRUPT_INCORE);
xfs_irele(ip);
return error;
}

View File

@ -1160,9 +1160,18 @@ xfs_qm_dqusage_adjust(
if (error)
return error;
error = xfs_inode_reload_unlinked(ip);
if (error)
goto error0;
/*
* Reload the incore unlinked list to avoid failure in inodegc.
* Use an unlocked check here because unrecovered unlinked inodes
* should be somewhat rare.
*/
if (xfs_inode_unlinked_incomplete(ip)) {
error = xfs_inode_reload_unlinked(ip);
if (error) {
xfs_force_shutdown(mp, SHUTDOWN_CORRUPT_INCORE);
goto error0;
}
}
ASSERT(ip->i_delayed_blks == 0);