[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <f39d240b-7abe-45d6-9d87-553ce8c6cf41@oracle.com>
Date: Wed, 7 Aug 2024 13:33:49 +0100
From: John Garry <john.g.garry@...cle.com>
To: "Darrick J. Wong" <djwong@...nel.org>
Cc: chandan.babu@...cle.com, dchinner@...hat.com, hch@....de,
viro@...iv.linux.org.uk, brauner@...nel.org, jack@...e.cz,
linux-xfs@...r.kernel.org, linux-kernel@...r.kernel.org,
linux-fsdevel@...r.kernel.org, catherine.hoang@...cle.com,
martin.petersen@...cle.com
Subject: Re: [PATCH v3 10/14] xfs: Do not free EOF blocks for forcealign
>> +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
Powered by blists - more mailing lists