diff mbox series

[1/2] xfs: Fix xfs_flush_unmap_range() range for RT

Message ID 20240509104057.1197846-2-john.g.garry@oracle.com (mailing list archive)
State Superseded
Headers show
Series xfs: fallocate RT flush unmap range fixes | expand

Commit Message

John Garry May 9, 2024, 10:40 a.m. UTC
Currently xfs_flush_unmap_range() does unmap for a full RT extent range,
which we also want to ensure is clean and idle.

This code change is originally from Dave Chinner.

Reviewed-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: John Garry <john.g.garry@oracle.com>
---
 fs/xfs/xfs_bmap_util.c | 12 ++++++++----
 1 file changed, 8 insertions(+), 4 deletions(-)

Comments

kernel test robot May 9, 2024, 10:59 p.m. UTC | #1
Hi John,

kernel test robot noticed the following build errors:

[auto build test ERROR on xfs-linux/for-next]
[also build test ERROR on next-20240509]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/John-Garry/xfs-Fix-xfs_flush_unmap_range-range-for-RT/20240509-184217
base:   https://git.kernel.org/pub/scm/fs/xfs/xfs-linux.git for-next
patch link:    https://lore.kernel.org/r/20240509104057.1197846-2-john.g.garry%40oracle.com
patch subject: [PATCH 1/2] xfs: Fix xfs_flush_unmap_range() range for RT
config: mips-randconfig-r053-20240510 (https://download.01.org/0day-ci/archive/20240510/202405100640.0Xf0XLtT-lkp@intel.com/config)
compiler: mips-linux-gcc (GCC) 13.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240510/202405100640.0Xf0XLtT-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202405100640.0Xf0XLtT-lkp@intel.com/

All errors (new ones prefixed by >>):

   mips-linux-ld: fs/xfs/xfs_bmap_util.o: in function `xfs_flush_unmap_range':
>> xfs_bmap_util.c:(.text+0x2934): undefined reference to `__moddi3'
>> mips-linux-ld: xfs_bmap_util.c:(.text+0x2978): undefined reference to `__moddi3'
Dave Chinner May 9, 2024, 11:40 p.m. UTC | #2
On Thu, May 09, 2024 at 10:40:56AM +0000, John Garry wrote:
> Currently xfs_flush_unmap_range() does unmap for a full RT extent range,
> which we also want to ensure is clean and idle.
> 
> This code change is originally from Dave Chinner.
> 
> Reviewed-by: Christoph Hellwig <hch@lst.de>
> Signed-off-by: John Garry <john.g.garry@oracle.com>
> ---
>  fs/xfs/xfs_bmap_util.c | 12 ++++++++----
>  1 file changed, 8 insertions(+), 4 deletions(-)
> 
> diff --git a/fs/xfs/xfs_bmap_util.c b/fs/xfs/xfs_bmap_util.c
> index ac2e77ebb54c..5d4aac50cbf5 100644
> --- a/fs/xfs/xfs_bmap_util.c
> +++ b/fs/xfs/xfs_bmap_util.c
> @@ -794,14 +794,18 @@ xfs_flush_unmap_range(
>  	xfs_off_t		offset,
>  	xfs_off_t		len)
>  {
> -	struct xfs_mount	*mp = ip->i_mount;
>  	struct inode		*inode = VFS_I(ip);
>  	xfs_off_t		rounding, start, end;
>  	int			error;
>  
> -	rounding = max_t(xfs_off_t, mp->m_sb.sb_blocksize, PAGE_SIZE);
> -	start = round_down(offset, rounding);
> -	end = round_up(offset + len, rounding) - 1;
> +	/*
> +	 * Make sure we extend the flush out to extent alignment
> +	 * boundaries so any extent range overlapping the start/end
> +	 * of the modification we are about to do is clean and idle.
> +	 */
> +	rounding = max_t(xfs_off_t, xfs_inode_alloc_unitsize(ip), PAGE_SIZE);
> +	start = rounddown(offset, rounding);
> +	end = roundup(offset + len, rounding) - 1;

These are 64 bit values, so roundup_64() and rounddown_64().

-Dave.
John Garry May 10, 2024, 11:10 a.m. UTC | #3
On 10/05/2024 00:40, Dave Chinner wrote:
>>   
>> -	rounding = max_t(xfs_off_t, mp->m_sb.sb_blocksize, PAGE_SIZE);
>> -	start = round_down(offset, rounding);
>> -	end = round_up(offset + len, rounding) - 1;
>> +	/*
>> +	 * Make sure we extend the flush out to extent alignment
>> +	 * boundaries so any extent range overlapping the start/end
>> +	 * of the modification we are about to do is clean and idle.
>> +	 */
>> +	rounding = max_t(xfs_off_t, xfs_inode_alloc_unitsize(ip), PAGE_SIZE);
>> +	start = rounddown(offset, rounding);
>> +	end = roundup(offset + len, rounding) - 1;

I have to admit that I am not the biggest fan of this rounding API.

So round_{down, up}() happens to handle 64b, but round{down, up} doesn't :(

And that is not to mention the vague naming.

> These are 64 bit values, so roundup_64() and rounddown_64().

yeah, thanks.

I can't help but think such a thing should be part of core kernel API.

Thanks,
John
diff mbox series

Patch

diff --git a/fs/xfs/xfs_bmap_util.c b/fs/xfs/xfs_bmap_util.c
index ac2e77ebb54c..5d4aac50cbf5 100644
--- a/fs/xfs/xfs_bmap_util.c
+++ b/fs/xfs/xfs_bmap_util.c
@@ -794,14 +794,18 @@  xfs_flush_unmap_range(
 	xfs_off_t		offset,
 	xfs_off_t		len)
 {
-	struct xfs_mount	*mp = ip->i_mount;
 	struct inode		*inode = VFS_I(ip);
 	xfs_off_t		rounding, start, end;
 	int			error;
 
-	rounding = max_t(xfs_off_t, mp->m_sb.sb_blocksize, PAGE_SIZE);
-	start = round_down(offset, rounding);
-	end = round_up(offset + len, rounding) - 1;
+	/*
+	 * Make sure we extend the flush out to extent alignment
+	 * boundaries so any extent range overlapping the start/end
+	 * of the modification we are about to do is clean and idle.
+	 */
+	rounding = max_t(xfs_off_t, xfs_inode_alloc_unitsize(ip), PAGE_SIZE);
+	start = rounddown(offset, rounding);
+	end = roundup(offset + len, rounding) - 1;
 
 	error = filemap_write_and_wait_range(inode->i_mapping, start, end);
 	if (error)