Re: [PATCH] xfs_repair: join realtime inodes to transaction only once

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Thu, Feb 27, 2020 at 12:50:54PM -0800, Eric Sandeen wrote:
> fill_rbmino() and fill_rsumino() can join the inode to the transactions
> multiple times before committing, which is not permitted.
> 
> This leads to cache purge errors when running repair:
> 
>   "cache_purge: shake on cache 0x92f5c0 left 129 nodes!?"
> 
> Move the libxfs_trans_ijoin out of the while loop to avoid this.
> 
> Fixes: e2dd0e1cc ("libxfs: remove libxfs_trans_iget")
> Signed-off-by: Eric Sandeen <sandeen@xxxxxxxxxx>

Looks reasonable, insofar as I have a patchset to port a few more kernel
APIs to xfsprogs so thta we can replace all the opencoded file setting
code in mkfs/repair to use that.

OH, heh, I never sent that.  Sigh....

This code takes advantage of behavioral differences between xfsprogs
transactions and kernel transactions to wrap up all the bmapi_write
calls in a single transaction.  This whole loop thing looks *weird* but
this does fix the ijoin usage to be correct WRT userspace transactions,
so...

Reviewed-by: Darrick J. Wong <darrick.wong@xxxxxxxxxx>

--D

> ---
> 
> 
> diff --git a/repair/phase6.c b/repair/phase6.c
> index 70135694..7bbc6da2 100644
> --- a/repair/phase6.c
> +++ b/repair/phase6.c
> @@ -645,7 +645,6 @@ fill_rbmino(xfs_mount_t *mp)
>  		/*
>  		 * fill the file one block at a time
>  		 */
> -		libxfs_trans_ijoin(tp, ip, 0);
>  		nmap = 1;
>  		error = -libxfs_bmapi_write(tp, ip, bno, 1, 0, 1, &map, &nmap);
>  		if (error || nmap != 1) {
> @@ -676,6 +675,7 @@ _("can't access block %" PRIu64 " (fsbno %" PRIu64 ") of realtime bitmap inode %
>  		bno++;
>  	}
>  
> +	libxfs_trans_ijoin(tp, ip, 0);
>  	error = -libxfs_trans_commit(tp);
>  	if (error)
>  		do_error(_("%s: commit failed, error %d\n"), __func__, error);
> @@ -716,7 +716,6 @@ fill_rsumino(xfs_mount_t *mp)
>  		/*
>  		 * fill the file one block at a time
>  		 */
> -		libxfs_trans_ijoin(tp, ip, 0);
>  		nmap = 1;
>  		error = -libxfs_bmapi_write(tp, ip, bno, 1, 0, 1, &map, &nmap);
>  		if (error || nmap != 1) {
> @@ -748,6 +747,7 @@ _("can't access block %" PRIu64 " (fsbno %" PRIu64 ") of realtime summary inode
>  		bno++;
>  	}
>  
> +	libxfs_trans_ijoin(tp, ip, 0);
>  	error = -libxfs_trans_commit(tp);
>  	if (error)
>  		do_error(_("%s: commit failed, error %d\n"), __func__, error);
> 



[Index of Archives]     [XFS Filesystem Development (older mail)]     [Linux Filesystem Development]     [Linux Audio Users]     [Yosemite Trails]     [Linux Kernel]     [Linux RAID]     [Linux SCSI]


  Powered by Linux