On Wed, Jul 25, 2018 at 03:48:51PM -0700, Eric Sandeen wrote:
>
>
> On 7/25/18 2:33 PM, Bill O'Donnell wrote:
> > + /* Additional sb sanity checks for writes */
> > + if (!error) {
> > + if (sb.sb_fdblocks > sb.sb_dblocks ||
> > + sb.sb_ifree > sb.sb_icount) {
> > + xfs_notice(mp, "SB sanity check failed");
> > + error = -EFSCORRUPTED;
> > + }
> > + }
>
> I had kind of had it in my head that Dave suggested testing not
> only sb_fdblocks & sb_ifree but also validating sb_icount against
> sb_dblocks ... would that make sense? something like:
>
> + if (sb.sb_fdblocks > sb.sb_dblocks ||
> + sb.sb_icount / sb.sb_inopblock > sb.sb_dblocks) ||
That would make sense, but perhaps we should have a xfs_verify_icount
instead of open-coding a 64-bit division? :)
Granted, I /was/ planning to add all that as part of fs summary counter
scrubbing next cycle.
--D
> + sb.sb_ifree > sb.sb_icount) {
> + xfs_notice(mp, "SB sanity check failed");
>
> because all 3 go into the statfs calculations which went wonky
> in the original report? (xfs_sb_verify has done some sanity
> checks on sb_inopblock by the time we get here.)
>
> Also, a comment about why these checks are only for write, and are not
> simply in xfs_mount_validate_sb() would be good, since that obviously
> wasn't obvious to me at first. o_O :)
>
> -Eric
> --
> To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
> the body of a message to majordomo@xxxxxxxxxxxxxxx
> More majordomo info at http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
