| Message ID | 20200713202151.64750-1-bfoster@redhat.com (mailing list archive) |
|---|---|
| State | Accepted, archived |
| Headers | show |
| Series | xfs: finish dfops on every insert range shift iteration | expand |
On Mon, Jul 13, 2020 at 04:21:51PM -0400, Brian Foster wrote: > The recent change to make insert range an atomic operation used the > incorrect transaction rolling mechanism. The explicit transaction > roll does not finish deferred operations. This means that intents > for rmapbt updates caused by extent shifts are not logged until the > final transaction commits. Thus if a crash occurs during an insert > range, log recovery might leave the rmapbt in an inconsistent state. > This was discovered by repeated runs of generic/455. > > Update insert range to finish dfops on every shift iteration. This > is similar to collapse range and ensures that intents are logged > with the transactions that make associated changes. > > Fixes: dd87f87d87fa ("xfs: rework insert range into an atomic operation") > Signed-off-by: Brian Foster <bfoster@redhat.com> > --- Ping? > fs/xfs/xfs_bmap_util.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/fs/xfs/xfs_bmap_util.c b/fs/xfs/xfs_bmap_util.c > index afdc7f8e0e70..feb277874a1f 100644 > --- a/fs/xfs/xfs_bmap_util.c > +++ b/fs/xfs/xfs_bmap_util.c > @@ -1165,7 +1165,7 @@ xfs_insert_file_space( > goto out_trans_cancel; > > do { > - error = xfs_trans_roll_inode(&tp, ip); > + error = xfs_defer_finish(&tp); > if (error) > goto out_trans_cancel; > > -- > 2.21.3 >
On Mon, Jul 13, 2020 at 04:21:51PM -0400, Brian Foster wrote: > The recent change to make insert range an atomic operation used the > incorrect transaction rolling mechanism. The explicit transaction > roll does not finish deferred operations. This means that intents > for rmapbt updates caused by extent shifts are not logged until the > final transaction commits. Thus if a crash occurs during an insert > range, log recovery might leave the rmapbt in an inconsistent state. > This was discovered by repeated runs of generic/455. > > Update insert range to finish dfops on every shift iteration. This > is similar to collapse range and ensures that intents are logged > with the transactions that make associated changes. > > Fixes: dd87f87d87fa ("xfs: rework insert range into an atomic operation") > Signed-off-by: Brian Foster <bfoster@redhat.com> Seems reasonable to me, sorry for dropping this by accident. :/ Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com> --D > --- > fs/xfs/xfs_bmap_util.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/fs/xfs/xfs_bmap_util.c b/fs/xfs/xfs_bmap_util.c > index afdc7f8e0e70..feb277874a1f 100644 > --- a/fs/xfs/xfs_bmap_util.c > +++ b/fs/xfs/xfs_bmap_util.c > @@ -1165,7 +1165,7 @@ xfs_insert_file_space( > goto out_trans_cancel; > > do { > - error = xfs_trans_roll_inode(&tp, ip); > + error = xfs_defer_finish(&tp); > if (error) > goto out_trans_cancel; > > -- > 2.21.3 >
diff --git a/fs/xfs/xfs_bmap_util.c b/fs/xfs/xfs_bmap_util.c index afdc7f8e0e70..feb277874a1f 100644 --- a/fs/xfs/xfs_bmap_util.c +++ b/fs/xfs/xfs_bmap_util.c @@ -1165,7 +1165,7 @@ xfs_insert_file_space( goto out_trans_cancel; do { - error = xfs_trans_roll_inode(&tp, ip); + error = xfs_defer_finish(&tp); if (error) goto out_trans_cancel;
The recent change to make insert range an atomic operation used the incorrect transaction rolling mechanism. The explicit transaction roll does not finish deferred operations. This means that intents for rmapbt updates caused by extent shifts are not logged until the final transaction commits. Thus if a crash occurs during an insert range, log recovery might leave the rmapbt in an inconsistent state. This was discovered by repeated runs of generic/455. Update insert range to finish dfops on every shift iteration. This is similar to collapse range and ensures that intents are logged with the transactions that make associated changes. Fixes: dd87f87d87fa ("xfs: rework insert range into an atomic operation") Signed-off-by: Brian Foster <bfoster@redhat.com> --- fs/xfs/xfs_bmap_util.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)