On Wed, Aug 07, 2024 at 01:33:49PM +0100, John Garry wrote:
> > > +void
> > > +xfs_roundout_to_alloc_fsbsize(
> > > + struct xfs_inode *ip,
> > > + xfs_fileoff_t *start,
> > > + xfs_fileoff_t *end)
> > > +{
> > > + unsigned int blocks = xfs_inode_alloc_fsbsize(ip);
> > > +
> > > + if (blocks == 1)
> > > + return;
> > > + *start = rounddown_64(*start, blocks);
> > > + *end = roundup_64(*end, blocks);
> > > +}
> >
> > This is probably going to start another round of shouting, but I think
> > it's silly to do two rounding operations when you only care about one
> > value.
>
> Sure, but the "in" version does use the 2x values and I wanted to be
> consistent. Anyway, I really don't feel strongly about this.
>
> > In patch 12 it results in a bunch more dummy variables that you
> > then ignore.
> >
> > Can't this be:
> >
> > static inline xfs_fileoff_t
> > xfs_inode_rounddown_alloc_unit(
>
> Just a question about the naming:
> xfs_inode_alloc_unitsize() returns bytes, so I would expect
> xfs_inode_rounddown_alloc_unit() to deal in bytes. Would you be satisfied
> with xfs_rounddown_alloc_fsbsize()? Or any other suggestion?
xfs_fileoff_t is the unit for file logical blocks, no need to append
stuff to the name. It's clear that this function takes a file block
offset and returns another one. If we need a second function for file
byte offsets then we can add another function and maybe wrap the whole
mess in _Generic.
--D
> > struct xfs_inode *ip,
> > xfs_fileoff off)
> > {
> > unsigned int rounding = xfs_inode_alloc_fsbsize(ip);
> >
> > if (rounding == 1)
> > return off;
> > return rounddown_64(off, rounding);
> > }
> >
> > static inline xfs_fileoff_t
> > xfs_inode_roundup_alloc_unit(
> > struct xfs_inode *ip,
> > xfs_fileoff off)
> > {
> > unsigned int rounding = xfs_inode_alloc_fsbsize(ip);
> >
> > if (rounding == 1)
> > return off;
> > return roundup_64(off, rounding);
> > }
> >
> > Then that callsite can be:
> >
> > end_fsb = xfs_inode_roundup_alloc_unit(ip,
> > XFS_B_TO_FSB(mp, (xfs_ufsize_t)XFS_ISIZE(ip)));
>
>
> Thanks,
> John
>
