On Mon, Feb 26, 2018 at 2:06 AM, Jan Kara [off-list ref] wrote:

On Fri 23-02-18 16:43:27, Dan Williams wrote:

quoted

Given that S_DAX is non-zero in the FS_DAX=n + DEV_DAX=y case, another
mechanism besides the plain IS_DAX() check to compile out dead
filesystem-dax code paths. Without IS_FSDAX() xfs will fail at link time
with:

    ERROR: "dax_finish_sync_fault" [fs/xfs/xfs.ko] undefined!
    ERROR: "dax_iomap_fault" [fs/xfs/xfs.ko] undefined!
    ERROR: "dax_iomap_rw" [fs/xfs/xfs.ko] undefined!

This compile failure was previously hidden by the fact that S_DAX was
erroneously defined to '0' in the FS_DAX=n + DEV_DAX=y case.

Cc: "Darrick J. Wong" <redacted>
Cc: linux-xfs@vger.kernel.org
Cc: <redacted>
Reported-by: kbuild test robot <redacted>
Signed-off-by: Dan Williams <redacted>

As much as I appreciate that relying on compiler to optimize out dead
branches results in nicer looking code this is an example where it
backfires. Also having IS_DAX() and IS_FSDAX() doing almost the same, just
not exactly the same, is IMHO a recipe for confusion (e.g. a casual reader
could think why does ext4 get away with using IS_DAX while XFS has to use
IS_FSDAX?). So I'd just prefer to handle this as is usual in other kernel
areas - define empty stubs for all exported functions when CONFIG_FS_DAX is
not enabled. That way code can stay without ugly ifdefs and we don't have
to bother with IS_FSDAX vs IS_DAX distinction in filesystem code. Thoughts?

I think my patch is incomplete either way, because the current
IS_DAX() usages handle more than just compiling out calls to fs/dax.c
symbols. I.e. even if there were stubs for all fs/dax.c call outs call
there are still local usages of the helper. Lets kill IS_DAX() and
only have IS_FSDAX() and IS_DEVDAX() with the S_ISCHR() check. Any
issues with that?

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>