I've looked a bit over the total diff and finaly result and really like
it.
A few comments from that without going into the individual patches:
- I don't think the buffer cancellation table should remain in
xfs_log_recovery.c. I can either move them into a new file
as part of resending my prep series, or you could move them into
xfs_buf_item_recover.c. Let me know what you prefer.
- Should the match callback also move into struct xfs_item_ops? That
would also match iop_recover.
- Based on that we could also kill XFS_ITEM_TYPE_IS_INTENT by just
checking for iop_recover and/or iop_match.
- Setting XFS_LI_RECOVERED could also move to common code, basically
set it whenever iop_recover returns. Also we can remove the
XFS_LI_RECOVERED asserts in ->iop_recovery when the caller checks
it just before.
- we are still having a few redundant ri_type checks.
- ri_type maybe should be ri_ops?
See this patch below for my take on cleaning up the recovery ops
handling a bit:
diff --git a/fs/xfs/libxfs/xfs_log_recover.h b/fs/xfs/libxfs/xfs_log_recover.h
index ba172eb454c8f..f97946cf94f11 100644
--- a/fs/xfs/libxfs/xfs_log_recover.h
+++ b/fs/xfs/libxfs/xfs_log_recover.h
@@ -7,7 +7,7 @@
#define __XFS_LOG_RECOVER_H__
/*
- * Each log item type (XFS_LI_*) gets its own xlog_recover_item_type to
+ * Each log item type (XFS_LI_*) gets its own xlog_recover_item_ops to
* define how recovery should work for that type of log item.
*/
struct xlog_recover_item;
@@ -20,7 +20,9 @@ enum xlog_recover_reorder {
XLOG_REORDER_CANCEL_LIST,
};
-struct xlog_recover_item_type {
+struct xlog_recover_item_ops {
+ uint16_t item_type;
+
/*
* Help sort recovered log items into the order required to replay them
* correctly. Log item types that always use XLOG_REORDER_ITEM_LIST do
@@ -58,19 +60,19 @@ struct xlog_recover_item_type {
struct xlog_recover_item *item, xfs_lsn_t lsn);
};
-extern const struct xlog_recover_item_type xlog_icreate_item_type;
-extern const struct xlog_recover_item_type xlog_buf_item_type;
-extern const struct xlog_recover_item_type xlog_inode_item_type;
-extern const struct xlog_recover_item_type xlog_dquot_item_type;
-extern const struct xlog_recover_item_type xlog_quotaoff_item_type;
-extern const struct xlog_recover_item_type xlog_bmap_intent_item_type;
-extern const struct xlog_recover_item_type xlog_bmap_done_item_type;
-extern const struct xlog_recover_item_type xlog_extfree_intent_item_type;
-extern const struct xlog_recover_item_type xlog_extfree_done_item_type;
-extern const struct xlog_recover_item_type xlog_rmap_intent_item_type;
-extern const struct xlog_recover_item_type xlog_rmap_done_item_type;
-extern const struct xlog_recover_item_type xlog_refcount_intent_item_type;
-extern const struct xlog_recover_item_type xlog_refcount_done_item_type;
+extern const struct xlog_recover_item_ops xlog_icreate_item_type;
+extern const struct xlog_recover_item_ops xlog_buf_item_type;
+extern const struct xlog_recover_item_ops xlog_inode_item_type;
+extern const struct xlog_recover_item_ops xlog_dquot_item_type;
+extern const struct xlog_recover_item_ops xlog_quotaoff_item_type;
+extern const struct xlog_recover_item_ops xlog_bmap_intent_item_type;
+extern const struct xlog_recover_item_ops xlog_bmap_done_item_type;
+extern const struct xlog_recover_item_ops xlog_extfree_intent_item_type;
+extern const struct xlog_recover_item_ops xlog_extfree_done_item_type;
+extern const struct xlog_recover_item_ops xlog_rmap_intent_item_type;
+extern const struct xlog_recover_item_ops xlog_rmap_done_item_type;
+extern const struct xlog_recover_item_ops xlog_refcount_intent_item_type;
+extern const struct xlog_recover_item_ops xlog_refcount_done_item_type;
/*
* Macros, structures, prototypes for internal log manager use.
@@ -93,7 +95,7 @@ typedef struct xlog_recover_item {
int ri_cnt; /* count of regions found */
int ri_total; /* total regions */
xfs_log_iovec_t *ri_buf; /* ptr to regions buffer */
- const struct xlog_recover_item_type *ri_type;
+ const struct xlog_recover_item_ops *ri_ops;
} xlog_recover_item_t;
struct xlog_recover {
diff --git a/fs/xfs/xfs_bmap_item.c b/fs/xfs/xfs_bmap_item.c
index 58f0904e4504d..952b4ce40433e 100644
--- a/fs/xfs/xfs_bmap_item.c
+++ b/fs/xfs/xfs_bmap_item.c
@@ -667,10 +667,12 @@ xlog_recover_bmap_done_commit_pass2(
return 0;
}
-const struct xlog_recover_item_type xlog_bmap_intent_item_type = {
+const struct xlog_recover_item_ops xlog_bmap_intent_item_type = {
+ .item_type = XFS_LI_BUI,
.commit_pass2_fn = xlog_recover_bmap_intent_commit_pass2,
};
-const struct xlog_recover_item_type xlog_bmap_done_item_type = {
+const struct xlog_recover_item_ops xlog_bmap_done_item_type = {
+ .item_type = XFS_LI_BUD,
.commit_pass2_fn = xlog_recover_bmap_done_commit_pass2,
};
diff --git a/fs/xfs/xfs_buf_item_recover.c b/fs/xfs/xfs_buf_item_recover.c
index d324f810819df..954e0e96af5dc 100644
--- a/fs/xfs/xfs_buf_item_recover.c
+++ b/fs/xfs/xfs_buf_item_recover.c
@@ -857,7 +857,8 @@ xlog_recover_buffer_commit_pass2(
return 0;
}
-const struct xlog_recover_item_type xlog_buf_item_type = {
+const struct xlog_recover_item_ops xlog_buf_item_type = {
+ .item_type = XFS_LI_BUF,
.reorder_fn = xlog_buf_reorder_fn,
.ra_pass2_fn = xlog_recover_buffer_ra_pass2,
.commit_pass1_fn = xlog_recover_buffer_commit_pass1,
diff --git a/fs/xfs/xfs_dquot_item.c b/fs/xfs/xfs_dquot_item.c
index 83bd7ded9185f..6c6216bdc432c 100644
--- a/fs/xfs/xfs_dquot_item.c
+++ b/fs/xfs/xfs_dquot_item.c
@@ -527,7 +527,8 @@ xlog_recover_dquot_commit_pass2(
return 0;
}
-const struct xlog_recover_item_type xlog_dquot_item_type = {
+const struct xlog_recover_item_ops xlog_dquot_item_type = {
+ .item_type = XFS_LI_DQUOT,
.ra_pass2_fn = xlog_recover_dquot_ra_pass2,
.commit_pass2_fn = xlog_recover_dquot_commit_pass2,
};
@@ -559,6 +560,7 @@ xlog_recover_quotaoff_commit_pass1(
return 0;
}
-const struct xlog_recover_item_type xlog_quotaoff_item_type = {
+const struct xlog_recover_item_ops xlog_quotaoff_item_type = {
+ .item_type = XFS_LI_QUOTAOFF,
.commit_pass1_fn = xlog_recover_quotaoff_commit_pass1,
};
diff --git a/fs/xfs/xfs_extfree_item.c b/fs/xfs/xfs_extfree_item.c
index d6f2c88570de1..5d1fb5e05b781 100644
--- a/fs/xfs/xfs_extfree_item.c
+++ b/fs/xfs/xfs_extfree_item.c
@@ -729,10 +729,12 @@ xlog_recover_extfree_done_commit_pass2(
return 0;
}
-const struct xlog_recover_item_type xlog_extfree_intent_item_type = {
+const struct xlog_recover_item_ops xlog_extfree_intent_item_type = {
+ .item_type = XFS_LI_EFI,
.commit_pass2_fn = xlog_recover_extfree_intent_commit_pass2,
};
-const struct xlog_recover_item_type xlog_extfree_done_item_type = {
+const struct xlog_recover_item_ops xlog_extfree_done_item_type = {
+ .item_type = XFS_LI_EFD,
.commit_pass2_fn = xlog_recover_extfree_done_commit_pass2,
};
diff --git a/fs/xfs/xfs_icreate_item.c b/fs/xfs/xfs_icreate_item.c
index 602a8c91371fe..34805bdbc2e12 100644
--- a/fs/xfs/xfs_icreate_item.c
+++ b/fs/xfs/xfs_icreate_item.c
@@ -248,7 +248,8 @@ xlog_recover_do_icreate_commit_pass2(
length, be32_to_cpu(icl->icl_gen));
}
-const struct xlog_recover_item_type xlog_icreate_item_type = {
+const struct xlog_recover_item_ops xlog_icreate_item_type = {
+ .item_type = XFS_LI_ICREATE,
.reorder_fn = xlog_icreate_reorder,
.commit_pass2_fn = xlog_recover_do_icreate_commit_pass2,
};
diff --git a/fs/xfs/xfs_inode_item_recover.c b/fs/xfs/xfs_inode_item_recover.c
index 46fc8a4b9ac61..9dff80783fe12 100644
--- a/fs/xfs/xfs_inode_item_recover.c
+++ b/fs/xfs/xfs_inode_item_recover.c
@@ -393,7 +393,8 @@ xlog_recover_inode_commit_pass2(
return error;
}
-const struct xlog_recover_item_type xlog_inode_item_type = {
+const struct xlog_recover_item_ops xlog_inode_item_type = {
+ .item_type = XFS_LI_INODE,
.ra_pass2_fn = xlog_recover_inode_ra_pass2,
.commit_pass2_fn = xlog_recover_inode_commit_pass2,
};
diff --git a/fs/xfs/xfs_log_recover.c b/fs/xfs/xfs_log_recover.c
index 09dd514a34980..e3f13866deb08 100644
--- a/fs/xfs/xfs_log_recover.c
+++ b/fs/xfs/xfs_log_recover.c
@@ -1828,55 +1828,35 @@ xlog_recover_insert_ail(
******************************************************************************
*/
-static int
-xlog_set_item_type(
- struct xlog_recover_item *item)
-{
- switch (ITEM_TYPE(item)) {
- case XFS_LI_ICREATE:
- item->ri_type = &xlog_icreate_item_type;
- return 0;
- case XFS_LI_BUF:
- item->ri_type = &xlog_buf_item_type;
- return 0;
- case XFS_LI_EFI:
- item->ri_type = &xlog_extfree_intent_item_type;
- return 0;
- case XFS_LI_EFD:
- item->ri_type = &xlog_extfree_done_item_type;
- return 0;
- case XFS_LI_RUI:
- item->ri_type = &xlog_rmap_intent_item_type;
- return 0;
- case XFS_LI_RUD:
- item->ri_type = &xlog_rmap_done_item_type;
- return 0;
- case XFS_LI_CUI:
- item->ri_type = &xlog_refcount_intent_item_type;
- return 0;
- case XFS_LI_CUD:
- item->ri_type = &xlog_refcount_done_item_type;
- return 0;
- case XFS_LI_BUI:
- item->ri_type = &xlog_bmap_intent_item_type;
- return 0;
- case XFS_LI_BUD:
- item->ri_type = &xlog_bmap_done_item_type;
- return 0;
- case XFS_LI_INODE:
- item->ri_type = &xlog_inode_item_type;
- return 0;
+static const struct xlog_recover_item_ops *xlog_recover_item_ops[] = {
+ &xlog_icreate_item_type,
+ &xlog_buf_item_type,
+ &xlog_extfree_intent_item_type,
+ &xlog_extfree_done_item_type,
+ &xlog_rmap_intent_item_type,
+ &xlog_rmap_done_item_type,
+ &xlog_refcount_intent_item_type,
+ &xlog_refcount_done_item_type,
+ &xlog_bmap_intent_item_type,
+ &xlog_bmap_done_item_type,
+ &xlog_inode_item_type,
#ifdef CONFIG_XFS_QUOTA
- case XFS_LI_DQUOT:
- item->ri_type = &xlog_dquot_item_type;
- return 0;
- case XFS_LI_QUOTAOFF:
- item->ri_type = &xlog_quotaoff_item_type;
- return 0;
+ &xlog_dquot_item_type,
+ &xlog_quotaoff_item_type,
#endif /* CONFIG_XFS_QUOTA */
- default:
- return -EFSCORRUPTED;
- }
+};
+
+static const struct xlog_recover_item_ops *
+xlog_find_item_ops(
+ struct xlog_recover_item *item)
+{
+ int i;
+
+ for (i = 0; i < ARRAY_SIZE(xlog_recover_item_ops); i++)
+ if (ITEM_TYPE(item) == xlog_recover_item_ops[i]->item_type)
+ return xlog_recover_item_ops[i];
+
+ return NULL;
}
/*
@@ -1946,8 +1926,8 @@ xlog_recover_reorder_trans(
list_for_each_entry_safe(item, n, &sort_list, ri_list) {
enum xlog_recover_reorder fate = XLOG_REORDER_ITEM_LIST;
- error = xlog_set_item_type(item);
- if (error) {
+ item->ri_ops = xlog_find_item_ops(item);
+ if (!item->ri_ops) {
xfs_warn(log->l_mp,
"%s: unrecognized type of log operation (%d)",
__func__, ITEM_TYPE(item));
@@ -1958,11 +1938,12 @@ xlog_recover_reorder_trans(
*/
if (!list_empty(&sort_list))
list_splice_init(&sort_list, &trans->r_itemq);
+ error = -EFSCORRUPTED;
break;
}
- if (item->ri_type->reorder_fn)
- fate = item->ri_type->reorder_fn(item);
+ if (item->ri_ops->reorder_fn)
+ fate = item->ri_ops->reorder_fn(item);
switch (fate) {
case XLOG_REORDER_BUFFER_LIST:
@@ -2098,46 +2079,6 @@ xlog_buf_readahead(
xfs_buf_readahead(log->l_mp->m_ddev_targp, blkno, len, ops);
}
-STATIC int
-xlog_recover_commit_pass1(
- struct xlog *log,
- struct xlog_recover *trans,
- struct xlog_recover_item *item)
-{
- trace_xfs_log_recover_item_recover(log, trans, item, XLOG_RECOVER_PASS1);
-
- if (!item->ri_type) {
- xfs_warn(log->l_mp, "%s: invalid item type (%d)",
- __func__, ITEM_TYPE(item));
- ASSERT(0);
- return -EFSCORRUPTED;
- }
- if (!item->ri_type->commit_pass1_fn)
- return 0;
- return item->ri_type->commit_pass1_fn(log, item);
-}
-
-STATIC int
-xlog_recover_commit_pass2(
- struct xlog *log,
- struct xlog_recover *trans,
- struct list_head *buffer_list,
- struct xlog_recover_item *item)
-{
- trace_xfs_log_recover_item_recover(log, trans, item, XLOG_RECOVER_PASS2);
-
- if (!item->ri_type) {
- xfs_warn(log->l_mp, "%s: invalid item type (%d)",
- __func__, ITEM_TYPE(item));
- ASSERT(0);
- return -EFSCORRUPTED;
- }
- if (!item->ri_type->commit_pass2_fn)
- return 0;
- return item->ri_type->commit_pass2_fn(log, buffer_list, item,
- trans->r_lsn);
-}
-
STATIC int
xlog_recover_items_pass2(
struct xlog *log,
@@ -2146,16 +2087,18 @@ xlog_recover_items_pass2(
struct list_head *item_list)
{
struct xlog_recover_item *item;
- int error = 0;
+ int error;
list_for_each_entry(item, item_list, ri_list) {
- error = xlog_recover_commit_pass2(log, trans,
- buffer_list, item);
+ if (!item->ri_ops->commit_pass2_fn)
+ continue;
+ error = item->ri_ops->commit_pass2_fn(log, buffer_list, item,
+ trans->r_lsn);
if (error)
return error;
}
- return error;
+ return 0;
}
/*
@@ -2187,13 +2130,16 @@ xlog_recover_commit_trans(
return error;
list_for_each_entry_safe(item, next, &trans->r_itemq, ri_list) {
+ trace_xfs_log_recover_item_recover(log, trans, item, pass);
+
switch (pass) {
case XLOG_RECOVER_PASS1:
- error = xlog_recover_commit_pass1(log, trans, item);
+ if (item->ri_ops->commit_pass1_fn)
+ error = item->ri_ops->commit_pass1_fn(log, item);
break;
case XLOG_RECOVER_PASS2:
- if (item->ri_type && item->ri_type->ra_pass2_fn)
- item->ri_type->ra_pass2_fn(log, item);
+ if (item->ri_ops->ra_pass2_fn)
+ item->ri_ops->ra_pass2_fn(log, item);
list_move_tail(&item->ri_list, &ra_list);
items_queued++;
if (items_queued >= XLOG_RECOVER_COMMIT_QUEUE_MAX) {
diff --git a/fs/xfs/xfs_refcount_item.c b/fs/xfs/xfs_refcount_item.c
index 53a79dc618f76..5703d5fdf4eeb 100644
--- a/fs/xfs/xfs_refcount_item.c
+++ b/fs/xfs/xfs_refcount_item.c
@@ -690,10 +690,12 @@ xlog_recover_refcount_done_commit_pass2(
return 0;
}
-const struct xlog_recover_item_type xlog_refcount_intent_item_type = {
+const struct xlog_recover_item_ops xlog_refcount_intent_item_type = {
+ .item_type = XFS_LI_CUI,
.commit_pass2_fn = xlog_recover_refcount_intent_commit_pass2,
};
-const struct xlog_recover_item_type xlog_refcount_done_item_type = {
+const struct xlog_recover_item_ops xlog_refcount_done_item_type = {
+ .item_type = XFS_LI_CUD,
.commit_pass2_fn = xlog_recover_refcount_done_commit_pass2,
};
diff --git a/fs/xfs/xfs_rmap_item.c b/fs/xfs/xfs_rmap_item.c
index cee5c61550321..12e035ff7bb2d 100644
--- a/fs/xfs/xfs_rmap_item.c
+++ b/fs/xfs/xfs_rmap_item.c
@@ -680,10 +680,12 @@ xlog_recover_rmap_done_commit_pass2(
return 0;
}
-const struct xlog_recover_item_type xlog_rmap_intent_item_type = {
+const struct xlog_recover_item_ops xlog_rmap_intent_item_type = {
+ .item_type = XFS_LI_RUI,
.commit_pass2_fn = xlog_recover_rmap_intent_commit_pass2,
};
-const struct xlog_recover_item_type xlog_rmap_done_item_type = {
+const struct xlog_recover_item_ops xlog_rmap_done_item_type = {
+ .item_type = XFS_LI_RUD,
.commit_pass2_fn = xlog_recover_rmap_done_commit_pass2,
};
