> +
> +/* Sorting hat for log items as they're read in. */
> +enum xlog_recover_reorder {
> + XLOG_REORDER_UNKNOWN,
> + XLOG_REORDER_BUFFER_LIST,
> + XLOG_REORDER_CANCEL_LIST,
> + XLOG_REORDER_INODE_BUFFER_LIST,
> + XLOG_REORDER_INODE_LIST,
XLOG_REORDER_INODE_LIST seems a bit misnamed as it really is the
"misc" or "no reorder" list. I guess the naming comes from the
local inode_list variable, but maybe we need to fix that as well?
> +typedef enum xlog_recover_reorder (*xlog_recover_reorder_fn)(
> + struct xlog_recover_item *item);
This typedef doesn't actually seem to help with anything (neither
with just thіs patch nor the final tree).
> +
> +struct xlog_recover_item_type {
> + /*
> + * These two items decide how to sort recovered log items during
> + * recovery. If reorder_fn is non-NULL it will be called; otherwise,
> + * reorder will be used to decide. See the comment above
> + * xlog_recover_reorder_trans for more details about what the values
> + * mean.
> + */
> + enum xlog_recover_reorder reorder;
> + xlog_recover_reorder_fn reorder_fn;
I'd just use reorder_fn and skip the simple field. Just one way to do
things even if it adds a tiny amount of boilerplate code.
> + case XFS_LI_INODE:
> + return &xlog_inode_item_type;
> + case XFS_LI_DQUOT:
> + return &xlog_dquot_item_type;
> + case XFS_LI_QUOTAOFF:
> + return &xlog_quotaoff_item_type;
> + case XFS_LI_IUNLINK:
> + /* Not implemented? */
Not implemented! I think we need a prep patch to remove this first.
> @@ -1851,41 +1890,34 @@ xlog_recover_reorder_trans(
>
> list_splice_init(&trans->r_itemq, &sort_list);
> list_for_each_entry_safe(item, n, &sort_list, ri_list) {
> - xfs_buf_log_format_t *buf_f = item->ri_buf[0].i_addr;
> + enum xlog_recover_reorder fate = XLOG_REORDER_UNKNOWN;
> +
> + item->ri_type = xlog_item_for_type(ITEM_TYPE(item));
I wonder if just passing the whole item to xlog_item_for_type would
make more sense. It would then need a different name, of course.
> + if (item->ri_type) {
> + if (item->ri_type->reorder_fn)
> + fate = item->ri_type->reorder_fn(item);
> + else
> + fate = item->ri_type->reorder;
> + }
I think for the !item->ri_type we should immediately jump to what
currently is the XLOG_REORDER_UNKNOWN case, and thus avoid even
adding XLOG_REORDER_UNKNOWN to the enum. The added benefit is that
any item without a reorder_fn could then be treated as on what
currently is the inode_list, but needs a btter name.
