On Thu, Apr 13, 2017 at 02:53:11AM -0700, Christoph Hellwig wrote:
> > +static inline unsigned int
> > +XFS_RMAPADD_SPACE_RES(
> > + struct xfs_mount *mp)
> > +{
> > + return xfs_sb_version_hasrmapbt(&mp->m_sb) ? mp->m_rmap_maxlevels : 0;
> > +}
>
> All the other helpers are macros. While I much prefer inlines it might
> be a good idea to do it in one go and also convert them to lower case
> instead of screaming..
>
> > +#define XFS_NRMAPADD_SPACE_RES(mp,b,w)\
> > + (((b + XFS_MAX_CONTIG_RMAPS_PER_BLOCK(mp) - 1) / \
> > + XFS_MAX_CONTIG_RMAPS_PER_BLOCK(mp)) * \
> > + XFS_RMAPADD_SPACE_RES(mp))
>
> But then again the above is so much more readable than this, so maybe
> it's a good idea after all and this one should be an inline as well.
>
> Also I think future developers will really appreciate comments
> explaining what we add up here.
Yeah, I was going to clean this up too but then I distracted myself with
that other bunmapi max_len thing, and by the time I sent it, it was late
evening already. I'll try to squeeze it in today.
--D
>
> Otherwise this looks fine and test fine for me:
>
> Reviewed-by: Christoph Hellwig <hch@xxxxxx>
> Tested-by: Christoph Hellwig <hch@xxxxxx>
> --
> 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
