| Message ID | 85aaa9e9-8aa4-301d-741a-94d4ef2291d6@redhat.com (mailing list archive) |
|---|---|
| State | Accepted |
| Headers | show |
| Series | xfs_repair: join realtime inodes to transaction only once | expand |
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@redhat.com> 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@oracle.com> --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); >
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);
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@redhat.com> ---