Re: [PATCH v3 10/14] xfs: Do not free EOF blocks for forcealign

[John Garry <john.g.garry@oracle.com>]

>> +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?

> 	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