Fix config issues: frame size and headers

[chrisrd]

Description

Fix config issues: frame size and headers

Motivation and Context

1. With various (debug and/or tracing?) kernel options enabled, it's possible for struct inode and struct super_block to exceed the default frame size, leaving errors like this in config.log:

build/conftest.c:116:1: error: the frame size of 1048 bytes is larger than 1024 bytes [-Werror=frame-larger-than=]

Fix this by moving the struct declarations off the stack.

2. Without the correct headers included, it's possible for declarations to be missed, leaving errors like this in config.log:

build/conftest.c:131:14: error: ‘struct nameidata’ declared inside parameter list [-Werror]

Fix this by adding appropriate headers.

These issues found by:

grep -E 'error: (the frame size|.*declared inside)' config.log

Note:

Both these issues can result in silent config failures because the compile failure is taken to mean "this option is not supported by this kernel" rather than "there's something wrong with the config test". This can lead to something merely annoying (compile failures) to something potentially serious (miscompiled or misused kernel primitives or functions).

This type of error is likely to reoccur because it only shows up with, perhaps, specific kernel versions and non-standard options, and the failures are "silent" per above.

Is there some way of flagging these sorts of errors via the buildbots? Maybe run the grep -E 'pattern' config.log, where pattern can be extended to include any further config issues found?

How Has This Been Tested?

$ rm -f config.log
$ ./configure --with-linux=${linux_source_dir} --with-linux-obj=${linux_build_dir} &&
{ grep -E 'error: (the frame size|.*declared inside parameter list)' config.log || echo ' => clear of frame size and declaration issues'; }

...fix errors, wash, rinse, repeat until config.log is clear. Then:

$ make -C module

Types of changes

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Performance enhancement (non-breaking change which improves efficiency)
• Code cleanup (non-breaking change which makes code smaller or more readable)
• Breaking change (fix or feature that would cause existing functionality to change)
• Documentation (a change to man pages or other documentation)

Checklist:

• My code follows the ZFS on Linux code style requirements.
• I have updated the documentation accordingly.
• I have read the CONTRIBUTING document.
• I have added tests to cover my changes.
• All new and existing tests passed.
• All commit messages are properly formatted and contain Signed-off-by.
• Change has been approved by a ZFS on Linux member.

[behlendorf]

Alternately, we could suppress the frame_size warnings entirely in the configure checks by adding $FRAME_LARGER_THAN to the default compile flags used in ZFS_LINUX_COMPILE_IFELSE. Something like this (untested).

diff --git a/config/kernel.m4 b/config/kernel.m4
index 805ec60a14..e9eec70d44 100644
--- a/config/kernel.m4
+++ b/config/kernel.m4
@@ -711,7 +711,7 @@ AC_DEFUN([ZFS_LINUX_COMPILE_IFELSE], [
        modpost_flag=''
        test "x$enable_linux_builtin" = xyes && modpost_flag='modpost=true' # fake modpost stage
        AS_IF(
-               [AC_TRY_COMMAND(cp conftest.c conftest.h build && make [$2] -C $LINUX_OBJ EXTRA_CFLAGS="-Werror $EXTRA_KCFLAGS" $ARCH_UM M=$PWD/build $modpost_flag) >/
+               [AC_TRY_COMMAND(cp conftest.c conftest.h build && make [$2] -C $LINUX_OBJ EXTRA_CFLAGS="-Werror $FRAME_LARGER_THAN $EXTRA_KCFLAGS" $ARCH_UM M=$PWD/buil
                [$4],
                [_AC_MSG_LOG_CONFTEST m4_ifvaln([$5],[$5])]
        )

[chrisrd]

Your $FRAME_LARGER_THAN suggestion worked a treat and of course remove the frame site issue for config tests permanently. Feel free to add your own S-O-B!

Some means of catching any missing declarations in future could still be worthwhile.

Note the new commit message includes the before/after differences in zfs_config.h (for v4.14.19):

#define HAVE_CURRENT_TIME 1
#define HAVE_INODE_SET_FLAGS 1
#define HAVE_SET_NLINK 1

I.e. these issues aren't just theoretical!

[codecov]

Codecov Report

Merging #7169 into master will increase coverage by 0.06%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master    #7169      +/-   ##
==========================================
+ Coverage   76.31%   76.37%   +0.06%     
==========================================
  Files         327      327              
  Lines      103764   103765       +1     
==========================================
+ Hits        79183    79249      +66     
+ Misses      24581    24516      -65

Flag	Coverage Δ
#kernel	76.15% <ø> (-0.2%)	⬇️
#user	65.73% <ø> (+0.11%)	⬆️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update ddc751d...6f6c10c. Read the comment docs.

[behlendorf]

Thanks! This looks good, I'll tag it for the next 0.7 release too.