* [PATCH 6.6 1/4] xfs: recreate work items when recovering intent items
2025-03-22 14:34 [PATCH 6.6 0/4] fix kernel crash for xfs/235 test Fedor Pchelkin
@ 2025-03-22 14:34 ` Fedor Pchelkin
2025-03-22 14:34 ` [PATCH 6.6 2/4] xfs: dump the recovered xattri log item if corruption happens Fedor Pchelkin
` (3 subsequent siblings)
4 siblings, 0 replies; 7+ messages in thread
From: Fedor Pchelkin @ 2025-03-22 14:34 UTC (permalink / raw)
To: Leah Rumancik
Cc: Fedor Pchelkin, stable, xfs-stable, Darrick J. Wong,
Christoph Hellwig, Catherine Hoang, Greg Kroah-Hartman,
lvc-project
From: Darrick J. Wong <djwong@kernel.org>
commit e70fb328d5277297ea2d9169a3a046de6412d777 upstream.
Recreate work items for each xfs_defer_pending object when we are
recovering intent items.
Signed-off-by: Darrick J. Wong <djwong@kernel.org>
Reviewed-by: Christoph Hellwig <hch@lst.de>
[ fp: fix conflict in calling xfs_attr_namecheck() with 2 args instead
of 3 due to backported ea0b3e814741 ("xfs: enforce one namespace per
attribute") ]
Signed-off-by: Fedor Pchelkin <pchelkin@ispras.ru>
---
fs/xfs/libxfs/xfs_defer.c | 3 +-
fs/xfs/libxfs/xfs_defer.h | 9 +++
fs/xfs/xfs_attr_item.c | 92 +++++++++++++++++-------------
fs/xfs/xfs_bmap_item.c | 55 +++++++++++-------
fs/xfs/xfs_extfree_item.c | 49 +++++++++-------
fs/xfs/xfs_refcount_item.c | 60 ++++++++++----------
fs/xfs/xfs_rmap_item.c | 112 ++++++++++++++++++++-----------------
7 files changed, 216 insertions(+), 164 deletions(-)
diff --git a/fs/xfs/libxfs/xfs_defer.c b/fs/xfs/libxfs/xfs_defer.c
index 363da37a8e7f..8fb523e4f669 100644
--- a/fs/xfs/libxfs/xfs_defer.c
+++ b/fs/xfs/libxfs/xfs_defer.c
@@ -676,9 +676,8 @@ xfs_defer_add(
list_add_tail(&dfp->dfp_list, &tp->t_dfops);
}
- list_add_tail(li, &dfp->dfp_work);
+ xfs_defer_add_item(dfp, li);
trace_xfs_defer_add_item(tp->t_mountp, dfp, li);
- dfp->dfp_count++;
}
/*
diff --git a/fs/xfs/libxfs/xfs_defer.h b/fs/xfs/libxfs/xfs_defer.h
index 5dce938ba3d5..bef5823f61fb 100644
--- a/fs/xfs/libxfs/xfs_defer.h
+++ b/fs/xfs/libxfs/xfs_defer.h
@@ -130,6 +130,15 @@ void xfs_defer_start_recovery(struct xfs_log_item *lip,
void xfs_defer_cancel_recovery(struct xfs_mount *mp,
struct xfs_defer_pending *dfp);
+static inline void
+xfs_defer_add_item(
+ struct xfs_defer_pending *dfp,
+ struct list_head *work)
+{
+ list_add_tail(work, &dfp->dfp_work);
+ dfp->dfp_count++;
+}
+
int __init xfs_defer_init_item_caches(void);
void xfs_defer_destroy_item_caches(void);
diff --git a/fs/xfs/xfs_attr_item.c b/fs/xfs/xfs_attr_item.c
index df86c9c09720..2b73132fa607 100644
--- a/fs/xfs/xfs_attr_item.c
+++ b/fs/xfs/xfs_attr_item.c
@@ -546,43 +546,17 @@ xfs_attri_validate(
return xfs_verify_ino(mp, attrp->alfi_ino);
}
-/*
- * Process an attr intent item that was recovered from the log. We need to
- * delete the attr that it describes.
- */
-STATIC int
-xfs_attri_item_recover(
+static inline struct xfs_attr_intent *
+xfs_attri_recover_work(
+ struct xfs_mount *mp,
struct xfs_defer_pending *dfp,
- struct list_head *capture_list)
+ struct xfs_attri_log_format *attrp,
+ struct xfs_inode *ip,
+ struct xfs_attri_log_nameval *nv)
{
- struct xfs_log_item *lip = dfp->dfp_intent;
- struct xfs_attri_log_item *attrip = ATTRI_ITEM(lip);
struct xfs_attr_intent *attr;
- struct xfs_mount *mp = lip->li_log->l_mp;
- struct xfs_inode *ip;
struct xfs_da_args *args;
- struct xfs_trans *tp;
- struct xfs_trans_res resv;
- struct xfs_attri_log_format *attrp;
- struct xfs_attri_log_nameval *nv = attrip->attri_nameval;
- int error;
- int total;
int local;
- struct xfs_attrd_log_item *done_item = NULL;
-
- /*
- * First check the validity of the attr described by the ATTRI. If any
- * are bad, then assume that all are bad and just toss the ATTRI.
- */
- attrp = &attrip->attri_format;
- if (!xfs_attri_validate(mp, attrp) ||
- !xfs_attr_namecheck(attrp->alfi_attr_filter, nv->name.i_addr,
- nv->name.i_len))
- return -EFSCORRUPTED;
-
- error = xlog_recover_iget(mp, attrp->alfi_ino, &ip);
- if (error)
- return error;
attr = kmem_zalloc(sizeof(struct xfs_attr_intent) +
sizeof(struct xfs_da_args), KM_NOFS);
@@ -624,19 +598,59 @@ xfs_attri_item_recover(
case XFS_ATTRI_OP_FLAGS_REMOVE:
attr->xattri_dela_state = xfs_attr_init_remove_state(args);
break;
- default:
- ASSERT(0);
- error = -EFSCORRUPTED;
- goto out;
}
+ xfs_defer_add_item(dfp, &attr->xattri_list);
+ return attr;
+}
+
+/*
+ * Process an attr intent item that was recovered from the log. We need to
+ * delete the attr that it describes.
+ */
+STATIC int
+xfs_attri_item_recover(
+ struct xfs_defer_pending *dfp,
+ struct list_head *capture_list)
+{
+ struct xfs_log_item *lip = dfp->dfp_intent;
+ struct xfs_attri_log_item *attrip = ATTRI_ITEM(lip);
+ struct xfs_attr_intent *attr;
+ struct xfs_mount *mp = lip->li_log->l_mp;
+ struct xfs_inode *ip;
+ struct xfs_da_args *args;
+ struct xfs_trans *tp;
+ struct xfs_trans_res resv;
+ struct xfs_attri_log_format *attrp;
+ struct xfs_attri_log_nameval *nv = attrip->attri_nameval;
+ int error;
+ int total;
+ struct xfs_attrd_log_item *done_item = NULL;
+
+ /*
+ * First check the validity of the attr described by the ATTRI. If any
+ * are bad, then assume that all are bad and just toss the ATTRI.
+ */
+ attrp = &attrip->attri_format;
+ if (!xfs_attri_validate(mp, attrp) ||
+ !xfs_attr_namecheck(attrp->alfi_attr_filter, nv->name.i_addr,
+ nv->name.i_len))
+ return -EFSCORRUPTED;
+
+ error = xlog_recover_iget(mp, attrp->alfi_ino, &ip);
+ if (error)
+ return error;
+
+ attr = xfs_attri_recover_work(mp, dfp, attrp, ip, nv);
+ args = attr->xattri_da_args;
+
xfs_init_attr_trans(args, &resv, &total);
resv = xlog_recover_resv(&resv);
error = xfs_trans_alloc(mp, &resv, total, 0, XFS_TRANS_RESERVE, &tp);
if (error)
- goto out;
-
+ return error;
args->trans = tp;
+
done_item = xfs_trans_get_attrd(tp, attrip);
xlog_recover_transfer_intent(tp, dfp);
@@ -667,8 +681,6 @@ xfs_attri_item_recover(
out_unlock:
xfs_iunlock(ip, XFS_ILOCK_EXCL);
xfs_irele(ip);
-out:
- xfs_attr_free_item(attr);
return error;
}
diff --git a/fs/xfs/xfs_bmap_item.c b/fs/xfs/xfs_bmap_item.c
index b6d63b8bdad5..b65999bf0ea3 100644
--- a/fs/xfs/xfs_bmap_item.c
+++ b/fs/xfs/xfs_bmap_item.c
@@ -480,6 +480,28 @@ xfs_bui_validate(
return xfs_verify_fsbext(mp, map->me_startblock, map->me_len);
}
+static inline struct xfs_bmap_intent *
+xfs_bui_recover_work(
+ struct xfs_mount *mp,
+ struct xfs_defer_pending *dfp,
+ struct xfs_map_extent *map)
+{
+ struct xfs_bmap_intent *bi;
+
+ bi = kmem_cache_zalloc(xfs_bmap_intent_cache, GFP_NOFS | __GFP_NOFAIL);
+ bi->bi_whichfork = (map->me_flags & XFS_BMAP_EXTENT_ATTR_FORK) ?
+ XFS_ATTR_FORK : XFS_DATA_FORK;
+ bi->bi_type = map->me_flags & XFS_BMAP_EXTENT_TYPE_MASK;
+ bi->bi_bmap.br_startblock = map->me_startblock;
+ bi->bi_bmap.br_startoff = map->me_startoff;
+ bi->bi_bmap.br_blockcount = map->me_len;
+ bi->bi_bmap.br_state = (map->me_flags & XFS_BMAP_EXTENT_UNWRITTEN) ?
+ XFS_EXT_UNWRITTEN : XFS_EXT_NORM;
+
+ xfs_defer_add_item(dfp, &bi->bi_list);
+ return bi;
+}
+
/*
* Process a bmap update intent item that was recovered from the log.
* We need to update some inode's bmbt.
@@ -489,7 +511,6 @@ xfs_bui_item_recover(
struct xfs_defer_pending *dfp,
struct list_head *capture_list)
{
- struct xfs_bmap_intent fake = { };
struct xfs_trans_res resv;
struct xfs_log_item *lip = dfp->dfp_intent;
struct xfs_bui_log_item *buip = BUI_ITEM(lip);
@@ -498,6 +519,7 @@ xfs_bui_item_recover(
struct xfs_mount *mp = lip->li_log->l_mp;
struct xfs_map_extent *map;
struct xfs_bud_log_item *budp;
+ struct xfs_bmap_intent *fake;
int iext_delta;
int error = 0;
@@ -508,9 +530,7 @@ xfs_bui_item_recover(
}
map = &buip->bui_format.bui_extents[0];
- fake.bi_whichfork = (map->me_flags & XFS_BMAP_EXTENT_ATTR_FORK) ?
- XFS_ATTR_FORK : XFS_DATA_FORK;
- fake.bi_type = map->me_flags & XFS_BMAP_EXTENT_TYPE_MASK;
+ fake = xfs_bui_recover_work(mp, dfp, map);
error = xlog_recover_iget(mp, map->me_owner, &ip);
if (error)
@@ -529,36 +549,31 @@ xfs_bui_item_recover(
xfs_ilock(ip, XFS_ILOCK_EXCL);
xfs_trans_ijoin(tp, ip, 0);
- if (fake.bi_type == XFS_BMAP_MAP)
+ if (fake->bi_type == XFS_BMAP_MAP)
iext_delta = XFS_IEXT_ADD_NOSPLIT_CNT;
else
iext_delta = XFS_IEXT_PUNCH_HOLE_CNT;
- error = xfs_iext_count_may_overflow(ip, fake.bi_whichfork, iext_delta);
+ error = xfs_iext_count_may_overflow(ip, fake->bi_whichfork, iext_delta);
if (error == -EFBIG)
error = xfs_iext_count_upgrade(tp, ip, iext_delta);
if (error)
goto err_cancel;
- fake.bi_owner = ip;
- fake.bi_bmap.br_startblock = map->me_startblock;
- fake.bi_bmap.br_startoff = map->me_startoff;
- fake.bi_bmap.br_blockcount = map->me_len;
- fake.bi_bmap.br_state = (map->me_flags & XFS_BMAP_EXTENT_UNWRITTEN) ?
- XFS_EXT_UNWRITTEN : XFS_EXT_NORM;
+ fake->bi_owner = ip;
- xfs_bmap_update_get_group(mp, &fake);
- error = xfs_trans_log_finish_bmap_update(tp, budp, &fake);
+ xfs_bmap_update_get_group(mp, fake);
+ error = xfs_trans_log_finish_bmap_update(tp, budp, fake);
if (error == -EFSCORRUPTED)
- XFS_CORRUPTION_ERROR(__func__, XFS_ERRLEVEL_LOW, mp, map,
- sizeof(*map));
- xfs_bmap_update_put_group(&fake);
+ XFS_CORRUPTION_ERROR(__func__, XFS_ERRLEVEL_LOW, mp,
+ &buip->bui_format, sizeof(buip->bui_format));
+ xfs_bmap_update_put_group(fake);
if (error)
goto err_cancel;
- if (fake.bi_bmap.br_blockcount > 0) {
- ASSERT(fake.bi_type == XFS_BMAP_UNMAP);
- xfs_bmap_unmap_extent(tp, ip, &fake.bi_bmap);
+ if (fake->bi_bmap.br_blockcount > 0) {
+ ASSERT(fake->bi_type == XFS_BMAP_UNMAP);
+ xfs_bmap_unmap_extent(tp, ip, &fake->bi_bmap);
}
/*
diff --git a/fs/xfs/xfs_extfree_item.c b/fs/xfs/xfs_extfree_item.c
index c9908fb33765..41108a0b60c9 100644
--- a/fs/xfs/xfs_extfree_item.c
+++ b/fs/xfs/xfs_extfree_item.c
@@ -651,6 +651,24 @@ xfs_efi_validate_ext(
return xfs_verify_fsbext(mp, extp->ext_start, extp->ext_len);
}
+static inline void
+xfs_efi_recover_work(
+ struct xfs_mount *mp,
+ struct xfs_defer_pending *dfp,
+ struct xfs_extent *extp)
+{
+ struct xfs_extent_free_item *xefi;
+
+ xefi = kmem_cache_zalloc(xfs_extfree_item_cache,
+ GFP_KERNEL | __GFP_NOFAIL);
+ xefi->xefi_startblock = extp->ext_start;
+ xefi->xefi_blockcount = extp->ext_len;
+ xefi->xefi_agresv = XFS_AG_RESV_NONE;
+ xefi->xefi_owner = XFS_RMAP_OWN_UNKNOWN;
+
+ xfs_defer_add_item(dfp, &xefi->xefi_list);
+}
+
/*
* Process an extent free intent item that was recovered from
* the log. We need to free the extents that it describes.
@@ -666,6 +684,7 @@ xfs_efi_item_recover(
struct xfs_mount *mp = lip->li_log->l_mp;
struct xfs_efd_log_item *efdp;
struct xfs_trans *tp;
+ struct xfs_extent_free_item *fake;
int i;
int error = 0;
bool requeue_only = false;
@@ -683,6 +702,8 @@ xfs_efi_item_recover(
sizeof(efip->efi_format));
return -EFSCORRUPTED;
}
+
+ xfs_efi_recover_work(mp, dfp, &efip->efi_format.efi_extents[i]);
}
resv = xlog_recover_resv(&M_RES(mp)->tr_itruncate);
@@ -693,22 +714,11 @@ xfs_efi_item_recover(
efdp = xfs_trans_get_efd(tp, efip, efip->efi_format.efi_nextents);
xlog_recover_transfer_intent(tp, dfp);
- for (i = 0; i < efip->efi_format.efi_nextents; i++) {
- struct xfs_extent_free_item fake = {
- .xefi_owner = XFS_RMAP_OWN_UNKNOWN,
- .xefi_agresv = XFS_AG_RESV_NONE,
- };
- struct xfs_extent *extp;
-
- extp = &efip->efi_format.efi_extents[i];
-
- fake.xefi_startblock = extp->ext_start;
- fake.xefi_blockcount = extp->ext_len;
-
+ list_for_each_entry(fake, &dfp->dfp_work, xefi_list) {
if (!requeue_only) {
- xfs_extent_free_get_group(mp, &fake);
- error = xfs_trans_free_extent(tp, efdp, &fake);
- xfs_extent_free_put_group(&fake);
+ xfs_extent_free_get_group(mp, fake);
+ error = xfs_trans_free_extent(tp, efdp, fake);
+ xfs_extent_free_put_group(fake);
}
/*
@@ -717,10 +727,10 @@ xfs_efi_item_recover(
* run again later with a new transaction context.
*/
if (error == -EAGAIN || requeue_only) {
- error = xfs_free_extent_later(tp, fake.xefi_startblock,
- fake.xefi_blockcount,
+ error = xfs_free_extent_later(tp, fake->xefi_startblock,
+ fake->xefi_blockcount,
&XFS_RMAP_OINFO_ANY_OWNER,
- fake.xefi_agresv);
+ fake->xefi_agresv);
if (!error) {
requeue_only = true;
continue;
@@ -729,7 +739,8 @@ xfs_efi_item_recover(
if (error == -EFSCORRUPTED)
XFS_CORRUPTION_ERROR(__func__, XFS_ERRLEVEL_LOW, mp,
- extp, sizeof(*extp));
+ &efip->efi_format,
+ sizeof(efip->efi_format));
if (error)
goto abort_error;
diff --git a/fs/xfs/xfs_refcount_item.c b/fs/xfs/xfs_refcount_item.c
index f1b259223802..4ffc34e6f0a0 100644
--- a/fs/xfs/xfs_refcount_item.c
+++ b/fs/xfs/xfs_refcount_item.c
@@ -468,6 +468,23 @@ xfs_cui_validate_phys(
return xfs_verify_fsbext(mp, pmap->pe_startblock, pmap->pe_len);
}
+static inline void
+xfs_cui_recover_work(
+ struct xfs_mount *mp,
+ struct xfs_defer_pending *dfp,
+ struct xfs_phys_extent *pmap)
+{
+ struct xfs_refcount_intent *ri;
+
+ ri = kmem_cache_alloc(xfs_refcount_intent_cache,
+ GFP_NOFS | __GFP_NOFAIL);
+ ri->ri_type = pmap->pe_flags & XFS_REFCOUNT_EXTENT_TYPE_MASK;
+ ri->ri_startblock = pmap->pe_startblock;
+ ri->ri_blockcount = pmap->pe_len;
+
+ xfs_defer_add_item(dfp, &ri->ri_list);
+}
+
/*
* Process a refcount update intent item that was recovered from the log.
* We need to update the refcountbt.
@@ -484,7 +501,7 @@ xfs_cui_item_recover(
struct xfs_trans *tp;
struct xfs_btree_cur *rcur = NULL;
struct xfs_mount *mp = lip->li_log->l_mp;
- unsigned int refc_type;
+ struct xfs_refcount_intent *fake;
bool requeue_only = false;
int i;
int error = 0;
@@ -502,6 +519,8 @@ xfs_cui_item_recover(
sizeof(cuip->cui_format));
return -EFSCORRUPTED;
}
+
+ xfs_cui_recover_work(mp, dfp, &cuip->cui_format.cui_extents[i]);
}
/*
@@ -525,35 +544,12 @@ xfs_cui_item_recover(
cudp = xfs_trans_get_cud(tp, cuip);
xlog_recover_transfer_intent(tp, dfp);
- for (i = 0; i < cuip->cui_format.cui_nextents; i++) {
- struct xfs_refcount_intent fake = { };
- struct xfs_phys_extent *pmap;
-
- pmap = &cuip->cui_format.cui_extents[i];
- refc_type = pmap->pe_flags & XFS_REFCOUNT_EXTENT_TYPE_MASK;
- switch (refc_type) {
- case XFS_REFCOUNT_INCREASE:
- case XFS_REFCOUNT_DECREASE:
- case XFS_REFCOUNT_ALLOC_COW:
- case XFS_REFCOUNT_FREE_COW:
- fake.ri_type = refc_type;
- break;
- default:
- XFS_CORRUPTION_ERROR(__func__, XFS_ERRLEVEL_LOW, mp,
- &cuip->cui_format,
- sizeof(cuip->cui_format));
- error = -EFSCORRUPTED;
- goto abort_error;
- }
-
- fake.ri_startblock = pmap->pe_startblock;
- fake.ri_blockcount = pmap->pe_len;
-
+ list_for_each_entry(fake, &dfp->dfp_work, ri_list) {
if (!requeue_only) {
- xfs_refcount_update_get_group(mp, &fake);
+ xfs_refcount_update_get_group(mp, fake);
error = xfs_trans_log_finish_refcount_update(tp, cudp,
- &fake, &rcur);
- xfs_refcount_update_put_group(&fake);
+ fake, &rcur);
+ xfs_refcount_update_put_group(fake);
}
if (error == -EFSCORRUPTED)
XFS_CORRUPTION_ERROR(__func__, XFS_ERRLEVEL_LOW, mp,
@@ -563,13 +559,13 @@ xfs_cui_item_recover(
goto abort_error;
/* Requeue what we didn't finish. */
- if (fake.ri_blockcount > 0) {
+ if (fake->ri_blockcount > 0) {
struct xfs_bmbt_irec irec = {
- .br_startblock = fake.ri_startblock,
- .br_blockcount = fake.ri_blockcount,
+ .br_startblock = fake->ri_startblock,
+ .br_blockcount = fake->ri_blockcount,
};
- switch (fake.ri_type) {
+ switch (fake->ri_type) {
case XFS_REFCOUNT_INCREASE:
xfs_refcount_increase_extent(tp, &irec);
break;
diff --git a/fs/xfs/xfs_rmap_item.c b/fs/xfs/xfs_rmap_item.c
index 5e8a02d2b045..9fb3ae4bfd59 100644
--- a/fs/xfs/xfs_rmap_item.c
+++ b/fs/xfs/xfs_rmap_item.c
@@ -498,6 +498,58 @@ xfs_rui_validate_map(
return xfs_verify_fsbext(mp, map->me_startblock, map->me_len);
}
+static inline void
+xfs_rui_recover_work(
+ struct xfs_mount *mp,
+ struct xfs_defer_pending *dfp,
+ const struct xfs_map_extent *map)
+{
+ struct xfs_rmap_intent *ri;
+
+ ri = kmem_cache_alloc(xfs_rmap_intent_cache, GFP_NOFS | __GFP_NOFAIL);
+
+ switch (map->me_flags & XFS_RMAP_EXTENT_TYPE_MASK) {
+ case XFS_RMAP_EXTENT_MAP:
+ ri->ri_type = XFS_RMAP_MAP;
+ break;
+ case XFS_RMAP_EXTENT_MAP_SHARED:
+ ri->ri_type = XFS_RMAP_MAP_SHARED;
+ break;
+ case XFS_RMAP_EXTENT_UNMAP:
+ ri->ri_type = XFS_RMAP_UNMAP;
+ break;
+ case XFS_RMAP_EXTENT_UNMAP_SHARED:
+ ri->ri_type = XFS_RMAP_UNMAP_SHARED;
+ break;
+ case XFS_RMAP_EXTENT_CONVERT:
+ ri->ri_type = XFS_RMAP_CONVERT;
+ break;
+ case XFS_RMAP_EXTENT_CONVERT_SHARED:
+ ri->ri_type = XFS_RMAP_CONVERT_SHARED;
+ break;
+ case XFS_RMAP_EXTENT_ALLOC:
+ ri->ri_type = XFS_RMAP_ALLOC;
+ break;
+ case XFS_RMAP_EXTENT_FREE:
+ ri->ri_type = XFS_RMAP_FREE;
+ break;
+ default:
+ ASSERT(0);
+ return;
+ }
+
+ ri->ri_owner = map->me_owner;
+ ri->ri_whichfork = (map->me_flags & XFS_RMAP_EXTENT_ATTR_FORK) ?
+ XFS_ATTR_FORK : XFS_DATA_FORK;
+ ri->ri_bmap.br_startblock = map->me_startblock;
+ ri->ri_bmap.br_startoff = map->me_startoff;
+ ri->ri_bmap.br_blockcount = map->me_len;
+ ri->ri_bmap.br_state = (map->me_flags & XFS_RMAP_EXTENT_UNWRITTEN) ?
+ XFS_EXT_UNWRITTEN : XFS_EXT_NORM;
+
+ xfs_defer_add_item(dfp, &ri->ri_list);
+}
+
/*
* Process an rmap update intent item that was recovered from the log.
* We need to update the rmapbt.
@@ -514,6 +566,7 @@ xfs_rui_item_recover(
struct xfs_trans *tp;
struct xfs_btree_cur *rcur = NULL;
struct xfs_mount *mp = lip->li_log->l_mp;
+ struct xfs_rmap_intent *fake;
int i;
int error = 0;
@@ -530,6 +583,8 @@ xfs_rui_item_recover(
sizeof(ruip->rui_format));
return -EFSCORRUPTED;
}
+
+ xfs_rui_recover_work(mp, dfp, &ruip->rui_format.rui_extents[i]);
}
resv = xlog_recover_resv(&M_RES(mp)->tr_itruncate);
@@ -541,60 +596,15 @@ xfs_rui_item_recover(
rudp = xfs_trans_get_rud(tp, ruip);
xlog_recover_transfer_intent(tp, dfp);
- for (i = 0; i < ruip->rui_format.rui_nextents; i++) {
- struct xfs_rmap_intent fake = { };
- struct xfs_map_extent *map;
-
- map = &ruip->rui_format.rui_extents[i];
- switch (map->me_flags & XFS_RMAP_EXTENT_TYPE_MASK) {
- case XFS_RMAP_EXTENT_MAP:
- fake.ri_type = XFS_RMAP_MAP;
- break;
- case XFS_RMAP_EXTENT_MAP_SHARED:
- fake.ri_type = XFS_RMAP_MAP_SHARED;
- break;
- case XFS_RMAP_EXTENT_UNMAP:
- fake.ri_type = XFS_RMAP_UNMAP;
- break;
- case XFS_RMAP_EXTENT_UNMAP_SHARED:
- fake.ri_type = XFS_RMAP_UNMAP_SHARED;
- break;
- case XFS_RMAP_EXTENT_CONVERT:
- fake.ri_type = XFS_RMAP_CONVERT;
- break;
- case XFS_RMAP_EXTENT_CONVERT_SHARED:
- fake.ri_type = XFS_RMAP_CONVERT_SHARED;
- break;
- case XFS_RMAP_EXTENT_ALLOC:
- fake.ri_type = XFS_RMAP_ALLOC;
- break;
- case XFS_RMAP_EXTENT_FREE:
- fake.ri_type = XFS_RMAP_FREE;
- break;
- default:
- XFS_CORRUPTION_ERROR(__func__, XFS_ERRLEVEL_LOW, mp,
- &ruip->rui_format,
- sizeof(ruip->rui_format));
- error = -EFSCORRUPTED;
- goto abort_error;
- }
-
- fake.ri_owner = map->me_owner;
- fake.ri_whichfork = (map->me_flags & XFS_RMAP_EXTENT_ATTR_FORK) ?
- XFS_ATTR_FORK : XFS_DATA_FORK;
- fake.ri_bmap.br_startblock = map->me_startblock;
- fake.ri_bmap.br_startoff = map->me_startoff;
- fake.ri_bmap.br_blockcount = map->me_len;
- fake.ri_bmap.br_state = (map->me_flags & XFS_RMAP_EXTENT_UNWRITTEN) ?
- XFS_EXT_UNWRITTEN : XFS_EXT_NORM;
-
- xfs_rmap_update_get_group(mp, &fake);
- error = xfs_trans_log_finish_rmap_update(tp, rudp, &fake,
+ list_for_each_entry(fake, &dfp->dfp_work, ri_list) {
+ xfs_rmap_update_get_group(mp, fake);
+ error = xfs_trans_log_finish_rmap_update(tp, rudp, fake,
&rcur);
if (error == -EFSCORRUPTED)
XFS_CORRUPTION_ERROR(__func__, XFS_ERRLEVEL_LOW, mp,
- map, sizeof(*map));
- xfs_rmap_update_put_group(&fake);
+ &ruip->rui_format,
+ sizeof(ruip->rui_format));
+ xfs_rmap_update_put_group(fake);
if (error)
goto abort_error;
--
2.49.0
^ permalink raw reply related [flat|nested] 7+ messages in thread* [PATCH 6.6 2/4] xfs: dump the recovered xattri log item if corruption happens
2025-03-22 14:34 [PATCH 6.6 0/4] fix kernel crash for xfs/235 test Fedor Pchelkin
2025-03-22 14:34 ` [PATCH 6.6 1/4] xfs: recreate work items when recovering intent items Fedor Pchelkin
@ 2025-03-22 14:34 ` Fedor Pchelkin
2025-03-22 14:34 ` [PATCH 6.6 3/4] xfs: use xfs_defer_finish_one to finish recovered work items Fedor Pchelkin
` (2 subsequent siblings)
4 siblings, 0 replies; 7+ messages in thread
From: Fedor Pchelkin @ 2025-03-22 14:34 UTC (permalink / raw)
To: Leah Rumancik
Cc: Fedor Pchelkin, stable, xfs-stable, Darrick J. Wong,
Christoph Hellwig, Catherine Hoang, Greg Kroah-Hartman,
lvc-project
From: Darrick J. Wong <djwong@kernel.org>
commit a51489e140d302c7afae763eacf882a23513f7e4 upstream.
If xfs_attri_item_recover receives a corruption error when it tries to
finish a recovered log intent item, it should dump the log item for
debugging, just like all the other log intent items.
Signed-off-by: Darrick J. Wong <djwong@kernel.org>
Reviewed-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Fedor Pchelkin <pchelkin@ispras.ru>
---
fs/xfs/xfs_attr_item.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/fs/xfs/xfs_attr_item.c b/fs/xfs/xfs_attr_item.c
index 2b73132fa607..8ed840c189cb 100644
--- a/fs/xfs/xfs_attr_item.c
+++ b/fs/xfs/xfs_attr_item.c
@@ -672,6 +672,10 @@ xfs_attri_item_recover(
xfs_irele(ip);
return 0;
}
+ if (error == -EFSCORRUPTED)
+ XFS_CORRUPTION_ERROR(__func__, XFS_ERRLEVEL_LOW, mp,
+ &attrip->attri_format,
+ sizeof(attrip->attri_format));
if (error) {
xfs_trans_cancel(tp);
goto out_unlock;
--
2.49.0
^ permalink raw reply related [flat|nested] 7+ messages in thread* [PATCH 6.6 3/4] xfs: use xfs_defer_finish_one to finish recovered work items
2025-03-22 14:34 [PATCH 6.6 0/4] fix kernel crash for xfs/235 test Fedor Pchelkin
2025-03-22 14:34 ` [PATCH 6.6 1/4] xfs: recreate work items when recovering intent items Fedor Pchelkin
2025-03-22 14:34 ` [PATCH 6.6 2/4] xfs: dump the recovered xattri log item if corruption happens Fedor Pchelkin
@ 2025-03-22 14:34 ` Fedor Pchelkin
2025-03-22 14:34 ` [PATCH 6.6 4/4] xfs: move ->iop_recover to xfs_defer_op_type Fedor Pchelkin
2026-06-11 18:39 ` [PATCH 6.6 0/4] fix kernel crash for xfs/235 test Hamza Mahfooz
4 siblings, 0 replies; 7+ messages in thread
From: Fedor Pchelkin @ 2025-03-22 14:34 UTC (permalink / raw)
To: Leah Rumancik
Cc: Fedor Pchelkin, stable, xfs-stable, Darrick J. Wong,
Christoph Hellwig, Catherine Hoang, Greg Kroah-Hartman,
lvc-project
From: Darrick J. Wong <djwong@kernel.org>
commit e5f1a5146ec35f3ed5d7f5ac7807a10c0062b6b8 upstream.
Get rid of the open-coded calls to xfs_defer_finish_one. This also
means that the recovery transaction takes care of cleaning up the dfp,
and we have solved (I hope) all the ownership issues in recovery.
Signed-off-by: Darrick J. Wong <djwong@kernel.org>
Reviewed-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Fedor Pchelkin <pchelkin@ispras.ru>
---
fs/xfs/libxfs/xfs_defer.c | 2 +-
fs/xfs/libxfs/xfs_defer.h | 1 +
fs/xfs/libxfs/xfs_log_recover.h | 2 +-
fs/xfs/xfs_attr_item.c | 20 +----------
fs/xfs/xfs_bmap_item.c | 24 ++++---------
fs/xfs/xfs_extfree_item.c | 45 +++++-------------------
fs/xfs/xfs_log_recover.c | 22 +++++++-----
fs/xfs/xfs_refcount_item.c | 61 +++++----------------------------
fs/xfs/xfs_rmap_item.c | 29 +++++-----------
9 files changed, 49 insertions(+), 157 deletions(-)
diff --git a/fs/xfs/libxfs/xfs_defer.c b/fs/xfs/libxfs/xfs_defer.c
index 8fb523e4f669..eb262ea06122 100644
--- a/fs/xfs/libxfs/xfs_defer.c
+++ b/fs/xfs/libxfs/xfs_defer.c
@@ -484,7 +484,7 @@ xfs_defer_relog(
* Log an intent-done item for the first pending intent, and finish the work
* items.
*/
-static int
+int
xfs_defer_finish_one(
struct xfs_trans *tp,
struct xfs_defer_pending *dfp)
diff --git a/fs/xfs/libxfs/xfs_defer.h b/fs/xfs/libxfs/xfs_defer.h
index bef5823f61fb..c1a648e99174 100644
--- a/fs/xfs/libxfs/xfs_defer.h
+++ b/fs/xfs/libxfs/xfs_defer.h
@@ -41,6 +41,7 @@ void xfs_defer_add(struct xfs_trans *tp, enum xfs_defer_ops_type type,
struct list_head *h);
int xfs_defer_finish_noroll(struct xfs_trans **tp);
int xfs_defer_finish(struct xfs_trans **tp);
+int xfs_defer_finish_one(struct xfs_trans *tp, struct xfs_defer_pending *dfp);
void xfs_defer_cancel(struct xfs_trans *);
void xfs_defer_move(struct xfs_trans *dtp, struct xfs_trans *stp);
diff --git a/fs/xfs/libxfs/xfs_log_recover.h b/fs/xfs/libxfs/xfs_log_recover.h
index 13583df9f239..52162a17fc5e 100644
--- a/fs/xfs/libxfs/xfs_log_recover.h
+++ b/fs/xfs/libxfs/xfs_log_recover.h
@@ -155,7 +155,7 @@ xlog_recover_resv(const struct xfs_trans_res *r)
void xlog_recover_intent_item(struct xlog *log, struct xfs_log_item *lip,
xfs_lsn_t lsn, unsigned int dfp_type);
-void xlog_recover_transfer_intent(struct xfs_trans *tp,
+int xlog_recover_finish_intent(struct xfs_trans *tp,
struct xfs_defer_pending *dfp);
#endif /* __XFS_LOG_RECOVER_H__ */
diff --git a/fs/xfs/xfs_attr_item.c b/fs/xfs/xfs_attr_item.c
index 8ed840c189cb..33e31d42d214 100644
--- a/fs/xfs/xfs_attr_item.c
+++ b/fs/xfs/xfs_attr_item.c
@@ -625,7 +625,6 @@ xfs_attri_item_recover(
struct xfs_attri_log_nameval *nv = attrip->attri_nameval;
int error;
int total;
- struct xfs_attrd_log_item *done_item = NULL;
/*
* First check the validity of the attr described by the ATTRI. If any
@@ -651,27 +650,10 @@ xfs_attri_item_recover(
return error;
args->trans = tp;
- done_item = xfs_trans_get_attrd(tp, attrip);
- xlog_recover_transfer_intent(tp, dfp);
-
xfs_ilock(ip, XFS_ILOCK_EXCL);
xfs_trans_ijoin(tp, ip, 0);
- error = xfs_xattri_finish_update(attr, done_item);
- if (error == -EAGAIN) {
- /*
- * There's more work to do, so add the intent item to this
- * transaction so that we can continue it later.
- */
- xfs_defer_add(tp, XFS_DEFER_OPS_TYPE_ATTR, &attr->xattri_list);
- error = xfs_defer_ops_capture_and_commit(tp, capture_list);
- if (error)
- goto out_unlock;
-
- xfs_iunlock(ip, XFS_ILOCK_EXCL);
- xfs_irele(ip);
- return 0;
- }
+ error = xlog_recover_finish_intent(tp, dfp);
if (error == -EFSCORRUPTED)
XFS_CORRUPTION_ERROR(__func__, XFS_ERRLEVEL_LOW, mp,
&attrip->attri_format,
diff --git a/fs/xfs/xfs_bmap_item.c b/fs/xfs/xfs_bmap_item.c
index b65999bf0ea3..89f2d9e89607 100644
--- a/fs/xfs/xfs_bmap_item.c
+++ b/fs/xfs/xfs_bmap_item.c
@@ -497,6 +497,7 @@ xfs_bui_recover_work(
bi->bi_bmap.br_blockcount = map->me_len;
bi->bi_bmap.br_state = (map->me_flags & XFS_BMAP_EXTENT_UNWRITTEN) ?
XFS_EXT_UNWRITTEN : XFS_EXT_NORM;
+ xfs_bmap_update_get_group(mp, bi);
xfs_defer_add_item(dfp, &bi->bi_list);
return bi;
@@ -518,8 +519,7 @@ xfs_bui_item_recover(
struct xfs_inode *ip = NULL;
struct xfs_mount *mp = lip->li_log->l_mp;
struct xfs_map_extent *map;
- struct xfs_bud_log_item *budp;
- struct xfs_bmap_intent *fake;
+ struct xfs_bmap_intent *work;
int iext_delta;
int error = 0;
@@ -530,7 +530,7 @@ xfs_bui_item_recover(
}
map = &buip->bui_format.bui_extents[0];
- fake = xfs_bui_recover_work(mp, dfp, map);
+ work = xfs_bui_recover_work(mp, dfp, map);
error = xlog_recover_iget(mp, map->me_owner, &ip);
if (error)
@@ -543,39 +543,29 @@ xfs_bui_item_recover(
if (error)
goto err_rele;
- budp = xfs_trans_get_bud(tp, buip);
- xlog_recover_transfer_intent(tp, dfp);
-
xfs_ilock(ip, XFS_ILOCK_EXCL);
xfs_trans_ijoin(tp, ip, 0);
- if (fake->bi_type == XFS_BMAP_MAP)
+ if (work->bi_type == XFS_BMAP_MAP)
iext_delta = XFS_IEXT_ADD_NOSPLIT_CNT;
else
iext_delta = XFS_IEXT_PUNCH_HOLE_CNT;
- error = xfs_iext_count_may_overflow(ip, fake->bi_whichfork, iext_delta);
+ error = xfs_iext_count_may_overflow(ip, work->bi_whichfork, iext_delta);
if (error == -EFBIG)
error = xfs_iext_count_upgrade(tp, ip, iext_delta);
if (error)
goto err_cancel;
- fake->bi_owner = ip;
+ work->bi_owner = ip;
- xfs_bmap_update_get_group(mp, fake);
- error = xfs_trans_log_finish_bmap_update(tp, budp, fake);
+ error = xlog_recover_finish_intent(tp, dfp);
if (error == -EFSCORRUPTED)
XFS_CORRUPTION_ERROR(__func__, XFS_ERRLEVEL_LOW, mp,
&buip->bui_format, sizeof(buip->bui_format));
- xfs_bmap_update_put_group(fake);
if (error)
goto err_cancel;
- if (fake->bi_bmap.br_blockcount > 0) {
- ASSERT(fake->bi_type == XFS_BMAP_UNMAP);
- xfs_bmap_unmap_extent(tp, ip, &fake->bi_bmap);
- }
-
/*
* Commit transaction, which frees the transaction and saves the inode
* for later replay activities.
diff --git a/fs/xfs/xfs_extfree_item.c b/fs/xfs/xfs_extfree_item.c
index 41108a0b60c9..6a434ade486c 100644
--- a/fs/xfs/xfs_extfree_item.c
+++ b/fs/xfs/xfs_extfree_item.c
@@ -665,6 +665,7 @@ xfs_efi_recover_work(
xefi->xefi_blockcount = extp->ext_len;
xefi->xefi_agresv = XFS_AG_RESV_NONE;
xefi->xefi_owner = XFS_RMAP_OWN_UNKNOWN;
+ xfs_extent_free_get_group(mp, xefi);
xfs_defer_add_item(dfp, &xefi->xefi_list);
}
@@ -682,12 +683,9 @@ xfs_efi_item_recover(
struct xfs_log_item *lip = dfp->dfp_intent;
struct xfs_efi_log_item *efip = EFI_ITEM(lip);
struct xfs_mount *mp = lip->li_log->l_mp;
- struct xfs_efd_log_item *efdp;
struct xfs_trans *tp;
- struct xfs_extent_free_item *fake;
int i;
int error = 0;
- bool requeue_only = false;
/*
* First check the validity of the extents described by the
@@ -711,40 +709,13 @@ xfs_efi_item_recover(
if (error)
return error;
- efdp = xfs_trans_get_efd(tp, efip, efip->efi_format.efi_nextents);
- xlog_recover_transfer_intent(tp, dfp);
-
- list_for_each_entry(fake, &dfp->dfp_work, xefi_list) {
- if (!requeue_only) {
- xfs_extent_free_get_group(mp, fake);
- error = xfs_trans_free_extent(tp, efdp, fake);
- xfs_extent_free_put_group(fake);
- }
-
- /*
- * If we can't free the extent without potentially deadlocking,
- * requeue the rest of the extents to a new so that they get
- * run again later with a new transaction context.
- */
- if (error == -EAGAIN || requeue_only) {
- error = xfs_free_extent_later(tp, fake->xefi_startblock,
- fake->xefi_blockcount,
- &XFS_RMAP_OINFO_ANY_OWNER,
- fake->xefi_agresv);
- if (!error) {
- requeue_only = true;
- continue;
- }
- }
-
- if (error == -EFSCORRUPTED)
- XFS_CORRUPTION_ERROR(__func__, XFS_ERRLEVEL_LOW, mp,
- &efip->efi_format,
- sizeof(efip->efi_format));
- if (error)
- goto abort_error;
-
- }
+ error = xlog_recover_finish_intent(tp, dfp);
+ if (error == -EFSCORRUPTED)
+ XFS_CORRUPTION_ERROR(__func__, XFS_ERRLEVEL_LOW, mp,
+ &efip->efi_format,
+ sizeof(efip->efi_format));
+ if (error)
+ goto abort_error;
return xfs_defer_ops_capture_and_commit(tp, capture_list);
diff --git a/fs/xfs/xfs_log_recover.c b/fs/xfs/xfs_log_recover.c
index 60382eb49961..92bf97a6e108 100644
--- a/fs/xfs/xfs_log_recover.c
+++ b/fs/xfs/xfs_log_recover.c
@@ -2584,7 +2584,8 @@ xlog_recover_process_intents(
* replayed in the wrong order!
*
* The recovery function can free the log item, so we must not
- * access lip after it returns.
+ * access lip after it returns. It must dispose of @dfp if it
+ * returns 0.
*/
error = ops->iop_recover(dfp, &capture_list);
if (error) {
@@ -2592,8 +2593,6 @@ xlog_recover_process_intents(
ops->iop_recover);
break;
}
-
- xfs_defer_cancel_recovery(log->l_mp, dfp);
}
if (error)
goto err;
@@ -2627,15 +2626,22 @@ xlog_recover_cancel_intents(
}
/*
- * Transfer ownership of the recovered log intent item to the recovery
- * transaction.
+ * Transfer ownership of the recovered pending work to the recovery transaction
+ * and try to finish the work. If there is more work to be done, the dfp will
+ * remain attached to the transaction. If not, the dfp is freed.
*/
-void
-xlog_recover_transfer_intent(
+int
+xlog_recover_finish_intent(
struct xfs_trans *tp,
struct xfs_defer_pending *dfp)
{
- dfp->dfp_intent = NULL;
+ int error;
+
+ list_move(&dfp->dfp_list, &tp->t_dfops);
+ error = xfs_defer_finish_one(tp, dfp);
+ if (error == -EAGAIN)
+ return 0;
+ return error;
}
/*
diff --git a/fs/xfs/xfs_refcount_item.c b/fs/xfs/xfs_refcount_item.c
index 4ffc34e6f0a0..f561ca73c784 100644
--- a/fs/xfs/xfs_refcount_item.c
+++ b/fs/xfs/xfs_refcount_item.c
@@ -481,6 +481,7 @@ xfs_cui_recover_work(
ri->ri_type = pmap->pe_flags & XFS_REFCOUNT_EXTENT_TYPE_MASK;
ri->ri_startblock = pmap->pe_startblock;
ri->ri_blockcount = pmap->pe_len;
+ xfs_refcount_update_get_group(mp, ri);
xfs_defer_add_item(dfp, &ri->ri_list);
}
@@ -497,12 +498,8 @@ xfs_cui_item_recover(
struct xfs_trans_res resv;
struct xfs_log_item *lip = dfp->dfp_intent;
struct xfs_cui_log_item *cuip = CUI_ITEM(lip);
- struct xfs_cud_log_item *cudp;
struct xfs_trans *tp;
- struct xfs_btree_cur *rcur = NULL;
struct xfs_mount *mp = lip->li_log->l_mp;
- struct xfs_refcount_intent *fake;
- bool requeue_only = false;
int i;
int error = 0;
@@ -541,59 +538,17 @@ xfs_cui_item_recover(
if (error)
return error;
- cudp = xfs_trans_get_cud(tp, cuip);
- xlog_recover_transfer_intent(tp, dfp);
-
- list_for_each_entry(fake, &dfp->dfp_work, ri_list) {
- if (!requeue_only) {
- xfs_refcount_update_get_group(mp, fake);
- error = xfs_trans_log_finish_refcount_update(tp, cudp,
- fake, &rcur);
- xfs_refcount_update_put_group(fake);
- }
- if (error == -EFSCORRUPTED)
- XFS_CORRUPTION_ERROR(__func__, XFS_ERRLEVEL_LOW, mp,
- &cuip->cui_format,
- sizeof(cuip->cui_format));
- if (error)
- goto abort_error;
-
- /* Requeue what we didn't finish. */
- if (fake->ri_blockcount > 0) {
- struct xfs_bmbt_irec irec = {
- .br_startblock = fake->ri_startblock,
- .br_blockcount = fake->ri_blockcount,
- };
-
- switch (fake->ri_type) {
- case XFS_REFCOUNT_INCREASE:
- xfs_refcount_increase_extent(tp, &irec);
- break;
- case XFS_REFCOUNT_DECREASE:
- xfs_refcount_decrease_extent(tp, &irec);
- break;
- case XFS_REFCOUNT_ALLOC_COW:
- xfs_refcount_alloc_cow_extent(tp,
- irec.br_startblock,
- irec.br_blockcount);
- break;
- case XFS_REFCOUNT_FREE_COW:
- xfs_refcount_free_cow_extent(tp,
- irec.br_startblock,
- irec.br_blockcount);
- break;
- default:
- ASSERT(0);
- }
- requeue_only = true;
- }
- }
+ error = xlog_recover_finish_intent(tp, dfp);
+ if (error == -EFSCORRUPTED)
+ XFS_CORRUPTION_ERROR(__func__, XFS_ERRLEVEL_LOW, mp,
+ &cuip->cui_format,
+ sizeof(cuip->cui_format));
+ if (error)
+ goto abort_error;
- xfs_refcount_finish_one_cleanup(tp, rcur, error);
return xfs_defer_ops_capture_and_commit(tp, capture_list);
abort_error:
- xfs_refcount_finish_one_cleanup(tp, rcur, error);
xfs_trans_cancel(tp);
return error;
}
diff --git a/fs/xfs/xfs_rmap_item.c b/fs/xfs/xfs_rmap_item.c
index 9fb3ae4bfd59..23e736179894 100644
--- a/fs/xfs/xfs_rmap_item.c
+++ b/fs/xfs/xfs_rmap_item.c
@@ -546,6 +546,7 @@ xfs_rui_recover_work(
ri->ri_bmap.br_blockcount = map->me_len;
ri->ri_bmap.br_state = (map->me_flags & XFS_RMAP_EXTENT_UNWRITTEN) ?
XFS_EXT_UNWRITTEN : XFS_EXT_NORM;
+ xfs_rmap_update_get_group(mp, ri);
xfs_defer_add_item(dfp, &ri->ri_list);
}
@@ -562,11 +563,8 @@ xfs_rui_item_recover(
struct xfs_trans_res resv;
struct xfs_log_item *lip = dfp->dfp_intent;
struct xfs_rui_log_item *ruip = RUI_ITEM(lip);
- struct xfs_rud_log_item *rudp;
struct xfs_trans *tp;
- struct xfs_btree_cur *rcur = NULL;
struct xfs_mount *mp = lip->li_log->l_mp;
- struct xfs_rmap_intent *fake;
int i;
int error = 0;
@@ -593,28 +591,17 @@ xfs_rui_item_recover(
if (error)
return error;
- rudp = xfs_trans_get_rud(tp, ruip);
- xlog_recover_transfer_intent(tp, dfp);
-
- list_for_each_entry(fake, &dfp->dfp_work, ri_list) {
- xfs_rmap_update_get_group(mp, fake);
- error = xfs_trans_log_finish_rmap_update(tp, rudp, fake,
- &rcur);
- if (error == -EFSCORRUPTED)
- XFS_CORRUPTION_ERROR(__func__, XFS_ERRLEVEL_LOW, mp,
- &ruip->rui_format,
- sizeof(ruip->rui_format));
- xfs_rmap_update_put_group(fake);
- if (error)
- goto abort_error;
-
- }
+ error = xlog_recover_finish_intent(tp, dfp);
+ if (error == -EFSCORRUPTED)
+ XFS_CORRUPTION_ERROR(__func__, XFS_ERRLEVEL_LOW, mp,
+ &ruip->rui_format,
+ sizeof(ruip->rui_format));
+ if (error)
+ goto abort_error;
- xfs_rmap_finish_one_cleanup(tp, rcur, error);
return xfs_defer_ops_capture_and_commit(tp, capture_list);
abort_error:
- xfs_rmap_finish_one_cleanup(tp, rcur, error);
xfs_trans_cancel(tp);
return error;
}
--
2.49.0
^ permalink raw reply related [flat|nested] 7+ messages in thread* [PATCH 6.6 4/4] xfs: move ->iop_recover to xfs_defer_op_type
2025-03-22 14:34 [PATCH 6.6 0/4] fix kernel crash for xfs/235 test Fedor Pchelkin
` (2 preceding siblings ...)
2025-03-22 14:34 ` [PATCH 6.6 3/4] xfs: use xfs_defer_finish_one to finish recovered work items Fedor Pchelkin
@ 2025-03-22 14:34 ` Fedor Pchelkin
2026-06-11 18:39 ` [PATCH 6.6 0/4] fix kernel crash for xfs/235 test Hamza Mahfooz
4 siblings, 0 replies; 7+ messages in thread
From: Fedor Pchelkin @ 2025-03-22 14:34 UTC (permalink / raw)
To: Leah Rumancik
Cc: Fedor Pchelkin, stable, xfs-stable, Darrick J. Wong,
Christoph Hellwig, Catherine Hoang, Greg Kroah-Hartman,
lvc-project
From: Darrick J. Wong <djwong@kernel.org>
commit db7ccc0bac2add5a41b66578e376b49328fc99d0 upstream.
Finish off the series by moving the intent item recovery function
pointer to the xfs_defer_op_type struct, since this is really a deferred
work function now.
Signed-off-by: Darrick J. Wong <djwong@kernel.org>
Reviewed-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Fedor Pchelkin <pchelkin@ispras.ru>
---
fs/xfs/libxfs/xfs_defer.c | 17 +++++++++++++
fs/xfs/libxfs/xfs_defer.h | 4 +++
fs/xfs/libxfs/xfs_log_recover.h | 2 ++
fs/xfs/xfs_attr_item.c | 21 +++++++++-------
fs/xfs/xfs_bmap_item.c | 39 ++++++++++++++++--------------
fs/xfs/xfs_extfree_item.c | 43 +++++++++++++++++----------------
fs/xfs/xfs_log_recover.c | 19 ++++++---------
fs/xfs/xfs_refcount_item.c | 24 +++++++++---------
fs/xfs/xfs_rmap_item.c | 24 +++++++++---------
fs/xfs/xfs_trans.h | 4 ---
10 files changed, 109 insertions(+), 88 deletions(-)
diff --git a/fs/xfs/libxfs/xfs_defer.c b/fs/xfs/libxfs/xfs_defer.c
index eb262ea06122..dd565e4e3daf 100644
--- a/fs/xfs/libxfs/xfs_defer.c
+++ b/fs/xfs/libxfs/xfs_defer.c
@@ -713,6 +713,23 @@ xfs_defer_cancel_recovery(
xfs_defer_pending_cancel_work(mp, dfp);
}
+/* Replay the deferred work item created from a recovered log intent item. */
+int
+xfs_defer_finish_recovery(
+ struct xfs_mount *mp,
+ struct xfs_defer_pending *dfp,
+ struct list_head *capture_list)
+{
+ const struct xfs_defer_op_type *ops = defer_op_types[dfp->dfp_type];
+ int error;
+
+ error = ops->recover_work(dfp, capture_list);
+ if (error)
+ trace_xlog_intent_recovery_failed(mp, error,
+ ops->recover_work);
+ return error;
+}
+
/*
* Move deferred ops from one transaction to another and reset the source to
* initial state. This is primarily used to carry state forward across
diff --git a/fs/xfs/libxfs/xfs_defer.h b/fs/xfs/libxfs/xfs_defer.h
index c1a648e99174..ef86a7f9b059 100644
--- a/fs/xfs/libxfs/xfs_defer.h
+++ b/fs/xfs/libxfs/xfs_defer.h
@@ -57,6 +57,8 @@ struct xfs_defer_op_type {
void (*finish_cleanup)(struct xfs_trans *tp,
struct xfs_btree_cur *state, int error);
void (*cancel_item)(struct list_head *item);
+ int (*recover_work)(struct xfs_defer_pending *dfp,
+ struct list_head *capture_list);
unsigned int max_items;
};
@@ -130,6 +132,8 @@ void xfs_defer_start_recovery(struct xfs_log_item *lip,
enum xfs_defer_ops_type dfp_type, struct list_head *r_dfops);
void xfs_defer_cancel_recovery(struct xfs_mount *mp,
struct xfs_defer_pending *dfp);
+int xfs_defer_finish_recovery(struct xfs_mount *mp,
+ struct xfs_defer_pending *dfp, struct list_head *capture_list);
static inline void
xfs_defer_add_item(
diff --git a/fs/xfs/libxfs/xfs_log_recover.h b/fs/xfs/libxfs/xfs_log_recover.h
index 52162a17fc5e..c8e5d912895b 100644
--- a/fs/xfs/libxfs/xfs_log_recover.h
+++ b/fs/xfs/libxfs/xfs_log_recover.h
@@ -153,6 +153,8 @@ xlog_recover_resv(const struct xfs_trans_res *r)
return ret;
}
+struct xfs_defer_pending;
+
void xlog_recover_intent_item(struct xlog *log, struct xfs_log_item *lip,
xfs_lsn_t lsn, unsigned int dfp_type);
int xlog_recover_finish_intent(struct xfs_trans *tp,
diff --git a/fs/xfs/xfs_attr_item.c b/fs/xfs/xfs_attr_item.c
index 33e31d42d214..d539abdb54c2 100644
--- a/fs/xfs/xfs_attr_item.c
+++ b/fs/xfs/xfs_attr_item.c
@@ -551,12 +551,17 @@ xfs_attri_recover_work(
struct xfs_mount *mp,
struct xfs_defer_pending *dfp,
struct xfs_attri_log_format *attrp,
- struct xfs_inode *ip,
+ struct xfs_inode **ipp,
struct xfs_attri_log_nameval *nv)
{
struct xfs_attr_intent *attr;
struct xfs_da_args *args;
int local;
+ int error;
+
+ error = xlog_recover_iget(mp, attrp->alfi_ino, ipp);
+ if (error)
+ return ERR_PTR(error);
attr = kmem_zalloc(sizeof(struct xfs_attr_intent) +
sizeof(struct xfs_da_args), KM_NOFS);
@@ -574,7 +579,7 @@ xfs_attri_recover_work(
attr->xattri_nameval = xfs_attri_log_nameval_get(nv);
ASSERT(attr->xattri_nameval);
- args->dp = ip;
+ args->dp = *ipp;
args->geo = mp->m_attr_geo;
args->whichfork = XFS_ATTR_FORK;
args->name = nv->name.i_addr;
@@ -609,7 +614,7 @@ xfs_attri_recover_work(
* delete the attr that it describes.
*/
STATIC int
-xfs_attri_item_recover(
+xfs_attr_recover_work(
struct xfs_defer_pending *dfp,
struct list_head *capture_list)
{
@@ -636,11 +641,9 @@ xfs_attri_item_recover(
nv->name.i_len))
return -EFSCORRUPTED;
- error = xlog_recover_iget(mp, attrp->alfi_ino, &ip);
- if (error)
- return error;
-
- attr = xfs_attri_recover_work(mp, dfp, attrp, ip, nv);
+ attr = xfs_attri_recover_work(mp, dfp, attrp, &ip, nv);
+ if (IS_ERR(attr))
+ return PTR_ERR(attr);
args = attr->xattri_da_args;
xfs_init_attr_trans(args, &resv, &total);
@@ -890,6 +893,7 @@ const struct xfs_defer_op_type xfs_attr_defer_type = {
.create_done = xfs_attr_create_done,
.finish_item = xfs_attr_finish_item,
.cancel_item = xfs_attr_cancel_item,
+ .recover_work = xfs_attr_recover_work,
};
/*
@@ -926,7 +930,6 @@ static const struct xfs_item_ops xfs_attri_item_ops = {
.iop_format = xfs_attri_item_format,
.iop_unpin = xfs_attri_item_unpin,
.iop_release = xfs_attri_item_release,
- .iop_recover = xfs_attri_item_recover,
.iop_match = xfs_attri_item_match,
.iop_relog = xfs_attri_item_relog,
};
diff --git a/fs/xfs/xfs_bmap_item.c b/fs/xfs/xfs_bmap_item.c
index 89f2d9e89607..bd8f6fe22b40 100644
--- a/fs/xfs/xfs_bmap_item.c
+++ b/fs/xfs/xfs_bmap_item.c
@@ -437,15 +437,6 @@ xfs_bmap_update_cancel_item(
kmem_cache_free(xfs_bmap_intent_cache, bi);
}
-const struct xfs_defer_op_type xfs_bmap_update_defer_type = {
- .max_items = XFS_BUI_MAX_FAST_EXTENTS,
- .create_intent = xfs_bmap_update_create_intent,
- .abort_intent = xfs_bmap_update_abort_intent,
- .create_done = xfs_bmap_update_create_done,
- .finish_item = xfs_bmap_update_finish_item,
- .cancel_item = xfs_bmap_update_cancel_item,
-};
-
/* Is this recovered BUI ok? */
static inline bool
xfs_bui_validate(
@@ -484,9 +475,15 @@ static inline struct xfs_bmap_intent *
xfs_bui_recover_work(
struct xfs_mount *mp,
struct xfs_defer_pending *dfp,
+ struct xfs_inode **ipp,
struct xfs_map_extent *map)
{
struct xfs_bmap_intent *bi;
+ int error;
+
+ error = xlog_recover_iget(mp, map->me_owner, ipp);
+ if (error)
+ return ERR_PTR(error);
bi = kmem_cache_zalloc(xfs_bmap_intent_cache, GFP_NOFS | __GFP_NOFAIL);
bi->bi_whichfork = (map->me_flags & XFS_BMAP_EXTENT_ATTR_FORK) ?
@@ -497,6 +494,7 @@ xfs_bui_recover_work(
bi->bi_bmap.br_blockcount = map->me_len;
bi->bi_bmap.br_state = (map->me_flags & XFS_BMAP_EXTENT_UNWRITTEN) ?
XFS_EXT_UNWRITTEN : XFS_EXT_NORM;
+ bi->bi_owner = *ipp;
xfs_bmap_update_get_group(mp, bi);
xfs_defer_add_item(dfp, &bi->bi_list);
@@ -508,7 +506,7 @@ xfs_bui_recover_work(
* We need to update some inode's bmbt.
*/
STATIC int
-xfs_bui_item_recover(
+xfs_bmap_recover_work(
struct xfs_defer_pending *dfp,
struct list_head *capture_list)
{
@@ -530,11 +528,9 @@ xfs_bui_item_recover(
}
map = &buip->bui_format.bui_extents[0];
- work = xfs_bui_recover_work(mp, dfp, map);
-
- error = xlog_recover_iget(mp, map->me_owner, &ip);
- if (error)
- return error;
+ work = xfs_bui_recover_work(mp, dfp, &ip, map);
+ if (IS_ERR(work))
+ return PTR_ERR(work);
/* Allocate transaction and do the work. */
resv = xlog_recover_resv(&M_RES(mp)->tr_itruncate);
@@ -557,8 +553,6 @@ xfs_bui_item_recover(
if (error)
goto err_cancel;
- work->bi_owner = ip;
-
error = xlog_recover_finish_intent(tp, dfp);
if (error == -EFSCORRUPTED)
XFS_CORRUPTION_ERROR(__func__, XFS_ERRLEVEL_LOW, mp,
@@ -587,6 +581,16 @@ xfs_bui_item_recover(
return error;
}
+const struct xfs_defer_op_type xfs_bmap_update_defer_type = {
+ .max_items = XFS_BUI_MAX_FAST_EXTENTS,
+ .create_intent = xfs_bmap_update_create_intent,
+ .abort_intent = xfs_bmap_update_abort_intent,
+ .create_done = xfs_bmap_update_create_done,
+ .finish_item = xfs_bmap_update_finish_item,
+ .cancel_item = xfs_bmap_update_cancel_item,
+ .recover_work = xfs_bmap_recover_work,
+};
+
STATIC bool
xfs_bui_item_match(
struct xfs_log_item *lip,
@@ -627,7 +631,6 @@ static const struct xfs_item_ops xfs_bui_item_ops = {
.iop_format = xfs_bui_item_format,
.iop_unpin = xfs_bui_item_unpin,
.iop_release = xfs_bui_item_release,
- .iop_recover = xfs_bui_item_recover,
.iop_match = xfs_bui_item_match,
.iop_relog = xfs_bui_item_relog,
};
diff --git a/fs/xfs/xfs_extfree_item.c b/fs/xfs/xfs_extfree_item.c
index 6a434ade486c..49e96ffd64e0 100644
--- a/fs/xfs/xfs_extfree_item.c
+++ b/fs/xfs/xfs_extfree_item.c
@@ -567,15 +567,6 @@ xfs_extent_free_cancel_item(
kmem_cache_free(xfs_extfree_item_cache, xefi);
}
-const struct xfs_defer_op_type xfs_extent_free_defer_type = {
- .max_items = XFS_EFI_MAX_FAST_EXTENTS,
- .create_intent = xfs_extent_free_create_intent,
- .abort_intent = xfs_extent_free_abort_intent,
- .create_done = xfs_extent_free_create_done,
- .finish_item = xfs_extent_free_finish_item,
- .cancel_item = xfs_extent_free_cancel_item,
-};
-
/*
* AGFL blocks are accounted differently in the reserve pools and are not
* inserted into the busy extent list.
@@ -632,16 +623,6 @@ xfs_agfl_free_finish_item(
return error;
}
-/* sub-type with special handling for AGFL deferred frees */
-const struct xfs_defer_op_type xfs_agfl_free_defer_type = {
- .max_items = XFS_EFI_MAX_FAST_EXTENTS,
- .create_intent = xfs_extent_free_create_intent,
- .abort_intent = xfs_extent_free_abort_intent,
- .create_done = xfs_extent_free_create_done,
- .finish_item = xfs_agfl_free_finish_item,
- .cancel_item = xfs_extent_free_cancel_item,
-};
-
/* Is this recovered EFI ok? */
static inline bool
xfs_efi_validate_ext(
@@ -675,7 +656,7 @@ xfs_efi_recover_work(
* the log. We need to free the extents that it describes.
*/
STATIC int
-xfs_efi_item_recover(
+xfs_extent_free_recover_work(
struct xfs_defer_pending *dfp,
struct list_head *capture_list)
{
@@ -724,6 +705,27 @@ xfs_efi_item_recover(
return error;
}
+const struct xfs_defer_op_type xfs_extent_free_defer_type = {
+ .max_items = XFS_EFI_MAX_FAST_EXTENTS,
+ .create_intent = xfs_extent_free_create_intent,
+ .abort_intent = xfs_extent_free_abort_intent,
+ .create_done = xfs_extent_free_create_done,
+ .finish_item = xfs_extent_free_finish_item,
+ .cancel_item = xfs_extent_free_cancel_item,
+ .recover_work = xfs_extent_free_recover_work,
+};
+
+/* sub-type with special handling for AGFL deferred frees */
+const struct xfs_defer_op_type xfs_agfl_free_defer_type = {
+ .max_items = XFS_EFI_MAX_FAST_EXTENTS,
+ .create_intent = xfs_extent_free_create_intent,
+ .abort_intent = xfs_extent_free_abort_intent,
+ .create_done = xfs_extent_free_create_done,
+ .finish_item = xfs_agfl_free_finish_item,
+ .cancel_item = xfs_extent_free_cancel_item,
+ .recover_work = xfs_extent_free_recover_work,
+};
+
STATIC bool
xfs_efi_item_match(
struct xfs_log_item *lip,
@@ -766,7 +768,6 @@ static const struct xfs_item_ops xfs_efi_item_ops = {
.iop_format = xfs_efi_item_format,
.iop_unpin = xfs_efi_item_unpin,
.iop_release = xfs_efi_item_release,
- .iop_recover = xfs_efi_item_recover,
.iop_match = xfs_efi_item_match,
.iop_relog = xfs_efi_item_relog,
};
diff --git a/fs/xfs/xfs_log_recover.c b/fs/xfs/xfs_log_recover.c
index 92bf97a6e108..3f343ba36b09 100644
--- a/fs/xfs/xfs_log_recover.c
+++ b/fs/xfs/xfs_log_recover.c
@@ -2565,17 +2565,14 @@ xlog_recover_process_intents(
#endif
list_for_each_entry_safe(dfp, n, &log->r_dfops, dfp_list) {
- struct xfs_log_item *lip = dfp->dfp_intent;
- const struct xfs_item_ops *ops = lip->li_ops;
-
- ASSERT(xlog_item_is_intent(lip));
+ ASSERT(xlog_item_is_intent(dfp->dfp_intent));
/*
* We should never see a redo item with a LSN higher than
* the last transaction we found in the log at the start
* of recovery.
*/
- ASSERT(XFS_LSN_CMP(last_lsn, lip->li_lsn) >= 0);
+ ASSERT(XFS_LSN_CMP(last_lsn, dfp->dfp_intent->li_lsn) >= 0);
/*
* NOTE: If your intent processing routine can create more
@@ -2584,15 +2581,13 @@ xlog_recover_process_intents(
* replayed in the wrong order!
*
* The recovery function can free the log item, so we must not
- * access lip after it returns. It must dispose of @dfp if it
- * returns 0.
+ * access dfp->dfp_intent after it returns. It must dispose of
+ * @dfp if it returns 0.
*/
- error = ops->iop_recover(dfp, &capture_list);
- if (error) {
- trace_xlog_intent_recovery_failed(log->l_mp, error,
- ops->iop_recover);
+ error = xfs_defer_finish_recovery(log->l_mp, dfp,
+ &capture_list);
+ if (error)
break;
- }
}
if (error)
goto err;
diff --git a/fs/xfs/xfs_refcount_item.c b/fs/xfs/xfs_refcount_item.c
index f561ca73c784..48f1a38b272e 100644
--- a/fs/xfs/xfs_refcount_item.c
+++ b/fs/xfs/xfs_refcount_item.c
@@ -433,16 +433,6 @@ xfs_refcount_update_cancel_item(
kmem_cache_free(xfs_refcount_intent_cache, ri);
}
-const struct xfs_defer_op_type xfs_refcount_update_defer_type = {
- .max_items = XFS_CUI_MAX_FAST_EXTENTS,
- .create_intent = xfs_refcount_update_create_intent,
- .abort_intent = xfs_refcount_update_abort_intent,
- .create_done = xfs_refcount_update_create_done,
- .finish_item = xfs_refcount_update_finish_item,
- .finish_cleanup = xfs_refcount_finish_one_cleanup,
- .cancel_item = xfs_refcount_update_cancel_item,
-};
-
/* Is this recovered CUI ok? */
static inline bool
xfs_cui_validate_phys(
@@ -491,7 +481,7 @@ xfs_cui_recover_work(
* We need to update the refcountbt.
*/
STATIC int
-xfs_cui_item_recover(
+xfs_refcount_recover_work(
struct xfs_defer_pending *dfp,
struct list_head *capture_list)
{
@@ -553,6 +543,17 @@ xfs_cui_item_recover(
return error;
}
+const struct xfs_defer_op_type xfs_refcount_update_defer_type = {
+ .max_items = XFS_CUI_MAX_FAST_EXTENTS,
+ .create_intent = xfs_refcount_update_create_intent,
+ .abort_intent = xfs_refcount_update_abort_intent,
+ .create_done = xfs_refcount_update_create_done,
+ .finish_item = xfs_refcount_update_finish_item,
+ .finish_cleanup = xfs_refcount_finish_one_cleanup,
+ .cancel_item = xfs_refcount_update_cancel_item,
+ .recover_work = xfs_refcount_recover_work,
+};
+
STATIC bool
xfs_cui_item_match(
struct xfs_log_item *lip,
@@ -593,7 +594,6 @@ static const struct xfs_item_ops xfs_cui_item_ops = {
.iop_format = xfs_cui_item_format,
.iop_unpin = xfs_cui_item_unpin,
.iop_release = xfs_cui_item_release,
- .iop_recover = xfs_cui_item_recover,
.iop_match = xfs_cui_item_match,
.iop_relog = xfs_cui_item_relog,
};
diff --git a/fs/xfs/xfs_rmap_item.c b/fs/xfs/xfs_rmap_item.c
index 23e736179894..23684bc2ab85 100644
--- a/fs/xfs/xfs_rmap_item.c
+++ b/fs/xfs/xfs_rmap_item.c
@@ -452,16 +452,6 @@ xfs_rmap_update_cancel_item(
kmem_cache_free(xfs_rmap_intent_cache, ri);
}
-const struct xfs_defer_op_type xfs_rmap_update_defer_type = {
- .max_items = XFS_RUI_MAX_FAST_EXTENTS,
- .create_intent = xfs_rmap_update_create_intent,
- .abort_intent = xfs_rmap_update_abort_intent,
- .create_done = xfs_rmap_update_create_done,
- .finish_item = xfs_rmap_update_finish_item,
- .finish_cleanup = xfs_rmap_finish_one_cleanup,
- .cancel_item = xfs_rmap_update_cancel_item,
-};
-
/* Is this recovered RUI ok? */
static inline bool
xfs_rui_validate_map(
@@ -556,7 +546,7 @@ xfs_rui_recover_work(
* We need to update the rmapbt.
*/
STATIC int
-xfs_rui_item_recover(
+xfs_rmap_recover_work(
struct xfs_defer_pending *dfp,
struct list_head *capture_list)
{
@@ -606,6 +596,17 @@ xfs_rui_item_recover(
return error;
}
+const struct xfs_defer_op_type xfs_rmap_update_defer_type = {
+ .max_items = XFS_RUI_MAX_FAST_EXTENTS,
+ .create_intent = xfs_rmap_update_create_intent,
+ .abort_intent = xfs_rmap_update_abort_intent,
+ .create_done = xfs_rmap_update_create_done,
+ .finish_item = xfs_rmap_update_finish_item,
+ .finish_cleanup = xfs_rmap_finish_one_cleanup,
+ .cancel_item = xfs_rmap_update_cancel_item,
+ .recover_work = xfs_rmap_recover_work,
+};
+
STATIC bool
xfs_rui_item_match(
struct xfs_log_item *lip,
@@ -646,7 +647,6 @@ static const struct xfs_item_ops xfs_rui_item_ops = {
.iop_format = xfs_rui_item_format,
.iop_unpin = xfs_rui_item_unpin,
.iop_release = xfs_rui_item_release,
- .iop_recover = xfs_rui_item_recover,
.iop_match = xfs_rui_item_match,
.iop_relog = xfs_rui_item_relog,
};
diff --git a/fs/xfs/xfs_trans.h b/fs/xfs/xfs_trans.h
index ead65f5f8dc3..ae32ffe9a1b1 100644
--- a/fs/xfs/xfs_trans.h
+++ b/fs/xfs/xfs_trans.h
@@ -66,8 +66,6 @@ struct xfs_log_item {
{ (1u << XFS_LI_DIRTY), "DIRTY" }, \
{ (1u << XFS_LI_WHITEOUT), "WHITEOUT" }
-struct xfs_defer_pending;
-
struct xfs_item_ops {
unsigned flags;
void (*iop_size)(struct xfs_log_item *, int *, int *);
@@ -80,8 +78,6 @@ struct xfs_item_ops {
xfs_lsn_t (*iop_committed)(struct xfs_log_item *, xfs_lsn_t);
uint (*iop_push)(struct xfs_log_item *, struct list_head *);
void (*iop_release)(struct xfs_log_item *);
- int (*iop_recover)(struct xfs_defer_pending *dfp,
- struct list_head *capture_list);
bool (*iop_match)(struct xfs_log_item *item, uint64_t id);
struct xfs_log_item *(*iop_relog)(struct xfs_log_item *intent,
struct xfs_trans *tp);
--
2.49.0
^ permalink raw reply related [flat|nested] 7+ messages in thread* Re: [PATCH 6.6 0/4] fix kernel crash for xfs/235 test
2025-03-22 14:34 [PATCH 6.6 0/4] fix kernel crash for xfs/235 test Fedor Pchelkin
` (3 preceding siblings ...)
2025-03-22 14:34 ` [PATCH 6.6 4/4] xfs: move ->iop_recover to xfs_defer_op_type Fedor Pchelkin
@ 2026-06-11 18:39 ` Hamza Mahfooz
2026-06-13 0:20 ` Sasha Levin
4 siblings, 1 reply; 7+ messages in thread
From: Hamza Mahfooz @ 2026-06-11 18:39 UTC (permalink / raw)
To: Fedor Pchelkin
Cc: Leah Rumancik, stable, xfs-stable, Darrick J. Wong,
Christoph Hellwig, Catherine Hoang, Greg Kroah-Hartman,
lvc-project
On Sat, Mar 22, 2025 at 05:34:11PM +0300, Fedor Pchelkin wrote:
> Incomplete backport of series "xfs: log intent item recovery should
> reconstruct defer work state" [1] leads to a kernel crash during the
> xfs/235 test execution on top of 6.6.y stable.
>
> Tested (briefly) with my local xfstests setup. Additional testing would
> be much appreciated.
Any idea what happened to this series? It resolves an issue that I've
hit in a production environment FWIW.
Series is:
Tested-by: Hamza Mahfooz <hamzamahfooz@linux.microsoft.com>
>
> [1]: https://lore.kernel.org/linux-xfs/170191741007.1195961.10092536809136830257.stg-ugh@frogsfrogsfrogs/
>
> XFS (loop1): Corruption of in-memory data (0x8) detected at xfs_trans_cancel+0x4d9/0x610 (fs/xfs/xfs_trans.c:1097). Shutting down filesystem.
> XFS (loop1): Please unmount the filesystem and rectify the problem(s)
> general protection fault, probably for non-canonical address 0xdffffc000000000c: 0000 [#1] PREEMPT SMP KASAN NOPTI
> KASAN: null-ptr-deref in range [0x0000000000000060-0x0000000000000067]
> CPU: 1 PID: 2011 Comm: mount Not tainted 6.6.84-rc2+ #12
> Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.16.3-3.fc41 04/01/2014
> RIP: 0010:xlog_recover_cancel_intents+0xad/0x1b0
> Call Trace:
> <TASK>
> xlog_recover_finish+0x7f6/0x9a0
> xfs_log_mount_finish+0x386/0x650
> xfs_mountfs+0x1405/0x1fb0
> xfs_fs_fill_super+0x11d6/0x1ca0
> get_tree_bdev+0x3b4/0x650
> vfs_get_tree+0x92/0x370
> path_mount+0x13b9/0x1f10
> __x64_sys_mount+0x286/0x310
> do_syscall_64+0x39/0x90
> entry_SYSCALL_64_after_hwframe+0x78/0xe2
> </TASK>
> Modules linked in:
> ---[ end trace 0000000000000000 ]---
> RIP: 0010:xlog_recover_cancel_intents+0xad/0x1b0
>
>
> Link to the original bug report [2].
>
> [2]: https://lore.kernel.org/stable/6pxyzwujo52p4bp2otliyssjcvsfydd6ju32eusdlyhzhpjh4q@eze6eh7rtidg/
>
> Found by Linux Verification Center (linuxtesting.org).
>
> Darrick J. Wong (4):
> xfs: recreate work items when recovering intent items
> xfs: dump the recovered xattri log item if corruption happens
> xfs: use xfs_defer_finish_one to finish recovered work items
> xfs: move ->iop_recover to xfs_defer_op_type
>
> fs/xfs/libxfs/xfs_defer.c | 22 ++++-
> fs/xfs/libxfs/xfs_defer.h | 14 +++
> fs/xfs/libxfs/xfs_log_recover.h | 4 +-
> fs/xfs/xfs_attr_item.c | 115 ++++++++++++------------
> fs/xfs/xfs_bmap_item.c | 92 ++++++++++---------
> fs/xfs/xfs_extfree_item.c | 117 +++++++++++--------------
> fs/xfs/xfs_log_recover.c | 37 ++++----
> fs/xfs/xfs_refcount_item.c | 127 +++++++++------------------
> fs/xfs/xfs_rmap_item.c | 151 ++++++++++++++++----------------
> fs/xfs/xfs_trans.h | 4 -
> 10 files changed, 326 insertions(+), 357 deletions(-)
>
> --
> 2.49.0
>
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [PATCH 6.6 0/4] fix kernel crash for xfs/235 test
2026-06-11 18:39 ` [PATCH 6.6 0/4] fix kernel crash for xfs/235 test Hamza Mahfooz
@ 2026-06-13 0:20 ` Sasha Levin
0 siblings, 0 replies; 7+ messages in thread
From: Sasha Levin @ 2026-06-13 0:20 UTC (permalink / raw)
To: Fedor Pchelkin
Cc: Sasha Levin, Leah Rumancik, stable, xfs-stable, Darrick J. Wong,
Christoph Hellwig, Catherine Hoang, Greg Kroah-Hartman,
lvc-project, Hamza Mahfooz
On Wed, Jun 11, 2026 at 02:39:03PM -0400, Hamza Mahfooz wrote:
> Any idea what happened to this series? It resolves an issue that I've
> hit in a production environment FWIW.
>
> Series is:
>
> Tested-by: Hamza Mahfooz <hamzamahfooz@linux.microsoft.com>
Thanks for the nudge, and thanks Fedor for putting the backport together.
We generally don't take XFS backports without a maintainer signing off on them,
so right now we're waiting for one to do so :)
--
Thanks,
Sasha
^ permalink raw reply [flat|nested] 7+ messages in thread