On Fri, Oct 09, 2020 at 01:05:46PM +0800, Gao Xiang wrote: > Introduce a common helper to consolidate stripe validation process. > Also make kernel code xfs_validate_sb_common() use it first. > > Signed-off-by: Gao Xiang <hsiangkao@xxxxxxxxxx> > --- > > kernel side of: > https://lore.kernel.org/r/20201007140402.14295-3-hsiangkao@xxxxxxx > with update suggested by Darrick: > - stretch columns for commit message; > - add comments to hasdalign check; > - break old sunit / swidth != 0 check into two seperate checks; > - update an error message description. > > also use bytes for sunit / swidth representation, so users can > see values in the unique unit. > > see > https://lore.kernel.org/r/20201007140402.14295-1-hsiangkao@xxxxxxx > for the background. > > fs/xfs/libxfs/xfs_sb.c | 65 +++++++++++++++++++++++++++++++++++------- > fs/xfs/libxfs/xfs_sb.h | 3 ++ > 2 files changed, 57 insertions(+), 11 deletions(-) > > diff --git a/fs/xfs/libxfs/xfs_sb.c b/fs/xfs/libxfs/xfs_sb.c > index 5aeafa59ed27..cb2a7aa0ad51 100644 > --- a/fs/xfs/libxfs/xfs_sb.c > +++ b/fs/xfs/libxfs/xfs_sb.c ... > @@ -1233,3 +1230,49 @@ xfs_sb_get_secondary( > *bpp = bp; > return 0; > } > + > +/* > + * sunit, swidth, sectorsize(optional with 0) should be all in bytes, > + * so users won't be confused by values in error messages. > + */ > +bool > +xfs_validate_stripe_factors( xfs_validate_stripe_geometry() perhaps? > + struct xfs_mount *mp, > + __s64 sunit, > + __s64 swidth, > + int sectorsize) > +{ > + if (sectorsize && sunit % sectorsize) { > + xfs_notice(mp, > +"stripe unit (%lld) must be a multiple of the sector size (%d)", > + sunit, sectorsize); > + return false; > + } > + > + if (sunit && !swidth) { > + xfs_notice(mp, > +"invalid stripe unit (%lld) and stripe width of 0", sunit); > + return false; > + } > + > + if (!sunit && swidth) { > + xfs_notice(mp, > +"invalid stripe width (%lld) and stripe unit of 0", swidth); > + return false; > + } Seems like these two could be combined into one check that prints something like: invalid stripe width (%lld) and stripe unit (%lld) > + > + if (sunit > swidth) { > + xfs_notice(mp, > +"stripe unit (%lld) is larger than the stripe width (%lld)", sunit, swidth); > + return false; > + } > + > + if (sunit && (swidth % sunit)) { > + xfs_notice(mp, > +"stripe width (%lld) must be a multiple of the stripe unit (%lld)", > + swidth, sunit); > + return false; > + } > + return true; > +} > + Trailing whitespace here. Otherwise looks reasonable outside of those nits. Brian > diff --git a/fs/xfs/libxfs/xfs_sb.h b/fs/xfs/libxfs/xfs_sb.h > index 92465a9a5162..2d3504eb9886 100644 > --- a/fs/xfs/libxfs/xfs_sb.h > +++ b/fs/xfs/libxfs/xfs_sb.h > @@ -42,4 +42,7 @@ extern int xfs_sb_get_secondary(struct xfs_mount *mp, > struct xfs_trans *tp, xfs_agnumber_t agno, > struct xfs_buf **bpp); > > +extern bool xfs_validate_stripe_factors(struct xfs_mount *mp, > + __s64 sunit, __s64 swidth, int sectorsize); > + > #endif /* __XFS_SB_H__ */ > -- > 2.18.1 >
