On Wed, Apr 29, 2020 at 10:53:09PM -0700, Christoph Hellwig wrote:
> On Wed, Apr 29, 2020 at 05:47:41PM -0700, Darrick J. Wong wrote:
> > +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;
>
> I'd prefer if we didn't have to expose these structures, but had a
> xlog_register_recovery_item helper that just adds them to a list or
> array.
I can look into making a register function and do lookups, but that's
a lot of indirection to save ~15 or so externs.
> > typedef struct xlog_recover_item {
> > struct list_head ri_list;
> > - int ri_type;
> > 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;
> > } xlog_recover_item_t;
>
> Btw, killing the xlog_recover_item_t typedef might be a worthwhile prep
> patch.
Hrm, ok....
> > --- a/fs/xfs/xfs_buf_item.c
> > +++ b/fs/xfs/xfs_buf_item.c
> > @@ -17,7 +17,6 @@
> > #include "xfs_trace.h"
> > #include "xfs_log.h"
> >
> > -
> > kmem_zone_t *xfs_buf_item_zone;
> >
> > static inline struct xfs_buf_log_item *BUF_ITEM(struct xfs_log_item *lip)
>
> Spurious whitespace change in a file not otherwise touched.
Fixed.
> >
> > @@ -107,3 +109,14 @@ xfs_icreate_log(
> > tp->t_flags |= XFS_TRANS_DIRTY;
> > set_bit(XFS_LI_DIRTY, &icp->ic_item.li_flags);
> > }
> > +
> > +static enum xlog_recover_reorder
> > +xlog_icreate_reorder(
> > + struct xlog_recover_item *item)
> > +{
> > + return XLOG_REORDER_BUFFER_LIST;
> > +}
>
> It might be worth to throw in a comment why icreate items got to
> the buffer list.
>
> > + return 0;
> > +#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;
> > +#endif /* CONFIG_XFS_QUOTA */
> > + default:
> > + return -EFSCORRUPTED;
> > + }
> > +}
>
> Quote recovery support currently is unconditionalẏ Making it
> conditional on CONFIG_XFS_QUOTA means a kernel without that config
> will now fail to recover a file system with quota updates in the log.
Heh, I hadn't realized that quota recovery always works even if quotas
are disabled. Ok, xfs_dquot_recovery.c it is...
--D
