Re: [linux-next:master] BUILD REGRESSION 34d1d36073ea4d4c532e8c8345627a9702be799e

[Dave Chinner <david@fromorbit.com>]

[trimmed cc list]

On Wed, Jun 22, 2022 at 06:10:27AM +0800, kernel test robot wrote:
> tree/branch: https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git master
> branch HEAD: 34d1d36073ea4d4c532e8c8345627a9702be799e  Add linux-next specific files for 20220621
> 
> Error/Warning reports:
> 
> https://lore.kernel.org/linux-mm/202206212029.Yr5m7Cd3-lkp@intel.com
> https://lore.kernel.org/linux-mm/202206212033.3lgl72Fw-lkp@intel.com

SEP. (Networking)

> https://lore.kernel.org/lkml/202206071511.FI7WLdZo-lkp@intel.com

The XFS warnings are in on this code:

  1258	#ifdef CONFIG_FS_DAX
  1259	int
> 1260	xfs_dax_fault(
  1261		struct vm_fault		*vmf,
  1262		enum page_entry_size	pe_size,
  1263		bool			write_fault,
  1264		pfn_t			*pfn)
  1265	{
> 1266		return dax_iomap_fault(vmf, pe_size, pfn, NULL,
  1267				(write_fault && !vmf->cow_page) ?
  1268					&xfs_dax_write_iomap_ops :
  1269					&xfs_read_iomap_ops);
  1270	}
  1271	#else
  1272	int
  1273	xfs_dax_fault(
  1274		struct vm_fault		*vmf,
  1275		enum page_entry_size	pe_size,
  1276		bool			write_fault,
  1277		pfn_t			*pfn)
  1278	{
  1279		return 0;
  1280	}
  1281	#endif

As it's not declared static and the return type should be vm_fault_t.

This code is not in the XFS tree, so I have to assume that it is in
Andrew's tree and needs fixing there.

But I don't think just changing 'int' to 'static vm_fault_t' is
right. This CONFIG_FS_DAX wrapper just smells wrong. That is, the
call to xfs_dax_fault() should be completely elided by the compiler
if CONFIG_FS_DAX is not set because it is only called from inside a
if (IS_DAX(inode))  {...} context.  IS_DAX(inode) will always
evaluate as zero when CONFIG_FS_DAX is not set, and so
xfs_dax_fault() becomes unreacheable and gets elided.

That's the way the current code in 5.19-rc3 works, and has for
years. We can call dax_iomap_fault() directly as long as it is only
in a (IS_DAX(inode) == true) conditional context. We do this all
over the place with fs-dax code - we did quite a bit of work to set
up the FSDAX code to avoid needing config conditional code in the
filesystems like the above wrappers. I can't see anything that has
changes this, so I'm at a loss to explain why this wrapper is needed
in the current linux-next tree...

I think these wrappers should go away (needing them would be a
potentially serious regression!) and be reverted to the
existing 5.19-rc3 code, not just be patched to make the prototypes
correct.

Ruan, Andrew, can you two work together to massage the changes
that are pending in Andrew's tree to get them fixed?

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com