Hi Brian,
On Fri, Sep 04, 2020 at 07:25:48AM -0400, Brian Foster wrote:
> On Fri, Sep 04, 2020 at 04:25:16PM +0800, Gao Xiang wrote:
...
> >
> > +static inline int xlog_logrecv2_hblks(struct xlog_rec_header *rh)
> > +{
> > + int h_size = be32_to_cpu(rh->h_size);
> > +
> > + if ((be32_to_cpu(rh->h_version) & XLOG_VERSION_2) &&
> > + h_size > XLOG_HEADER_CYCLE_SIZE)
> > + return DIV_ROUND_UP(h_size, XLOG_HEADER_CYCLE_SIZE);
> > + return 1;
> > +}
> > +
> > +static inline int xlog_logrec_hblks(struct xlog *log, xlog_rec_header_t *rh)
> > +{
> > + if (!xfs_sb_version_haslogv2(&log->l_mp->m_sb))
> > + return 1;
> > + return xlog_logrecv2_hblks(rh);
> > +}
> > +
>
> h_version is assigned based on xfs_sb_version_haslogv2() in the first
> place so I'm not sure I see the need for multiple helpers like this, at
> least with the current code. I can't really speak to why some code
> checks the feature bit and/or the record header version and not the
> other way around, but perhaps there's some historical reason I'm not
> aware of. Regardless, is there ever a case where
> xfs_sb_version_haslogv2() == true and h_version != 2? That strikes me as
> more of a corruption scenario than anything..
Thanks for this.
Honestly, I'm not quite sure if xfs_sb_version_haslogv2() == true and
h_version != 2 is useful (or existed historially)... anyway, that is
another seperate topic though...
Could you kindly give me some code flow on your preferred way about
this so I could update this patch proper (since we have a complex
case in xlog_do_recovery_pass(), I'm not sure how the unique helper
will be like because there are 3 cases below...)
- for the first 2 cases, we already have rhead read in-memory,
so the logic is like:
....
log_bread (somewhere in advance)
....
if (xfs_sb_version_haslogv2(&log->l_mp->m_sb)) {
...
} else {
...
}
(so I folded this two cases in xlog_logrec_hblks())
- for xlog_do_recovery_pass, it behaves like
if (xfs_sb_version_haslogv2(&log->l_mp->m_sb)) {
log_bread (another extra bread to get h_size for
allocated buffer and hblks).
...
} else {
...
}
so in this case we don't have rhead until
xfs_sb_version_haslogv2(&log->l_mp->m_sb) is true...
Thanks in advance!
Thanks,
Gao Xiang
>
> Brian
