Fix kernel build with GEOM_LABEL and no FFS

[mckusick]

Summary

After rG1111a44301da, MINIMAL kernel stopped building.

linking kernel.full
ld: error: undefined symbol: sysctl___vfs_ffs
>>> referenced by ffs_subr.c
>>>               ffs_subr.o:(sysctl___vfs_ffs_prttimechgs)
*** [kernel.full] Error code 1

make[2]: stopped making "all" in /usr/obj/usr/src/amd64.amd64/sys/MINIMAL
make[2]: 1 error

make[2]: stopped making "all" in /usr/obj/usr/src/amd64.amd64/sys/MINIMAL
      143.83 real      2018.96 user       165.62 sys

make[1]: stopped making "buildkernel" in /usr/src

make: stopped making "buildkernel" in /usr/src

This happens because sys/conf/files has this:

ufs/ffs/ffs_subr.c              optional ffs | geom_label

This patch fixes the problem by moving it to a separate file.

Test Plan

Build both GENERIC and MINIMAL kernels. Build libufs.

[mckusick]

This will cause libufs to fail to build. Also, ffs_subr.c is supposed to contain all the kernel functionality needed by filesystem utilities. And ffs_oldfscompat_inode_read() is one of those functions.

I propose just doing this change instead

diff --git a/sys/ufs/ffs/ffs_subr.c b/sys/ufs/ffs/ffs_subr.c
index e2b09da86ae5..ef488185e026 100644
--- a/sys/ufs/ffs/ffs_subr.c
+++ b/sys/ufs/ffs/ffs_subr.c
@@ -405,9 +405,12 @@ ffs_oldfscompat_write(struct fs *fs)
  */
 static int prttimechgs = 0;
 #ifdef _KERNEL
+#include "opt_dontuse.h" /* Check to see if FFS is in this kernel */
+#ifdef FFS
 SYSCTL_DECL(_vfs_ffs);
 SYSCTL_INT(_vfs_ffs, OID_AUTO, prttimechgs, CTLFLAG_RWTUN, &prttimechgs, 0,
 	"print UFS1 time changes made to inodes");
+#endif /* FFS */
 #endif /* _KERNEL */
 bool
 ffs_oldfscompat_inode_read(struct fs *fs, union dinodep dp, time_t now)

[mckusick]

Actually, there is an even easier fix. Just move the declaration of vfs_ffs from ffs_alloc.c to ffs_subr.c. Then everything just works.

diff --git a/sys/ufs/ffs/ffs_alloc.c b/sys/ufs/ffs/ffs_alloc.c
index 265daef14812..61fb6ccbfc68 100644
--- a/sys/ufs/ffs/ffs_alloc.c
+++ b/sys/ufs/ffs/ffs_alloc.c
@@ -489,8 +489,7 @@ ffs_realloccg(struct inode *ip,
  * allocation will be used.
  */
 
-SYSCTL_NODE(_vfs, OID_AUTO, ffs, CTLFLAG_RW | CTLFLAG_MPSAFE, 0,
-    "FFS filesystem");
+SYSCTL_DECL(_vfs_ffs);
 
 static int doasyncfree = 1;
 SYSCTL_INT(_vfs_ffs, OID_AUTO, doasyncfree, CTLFLAG_RW, &doasyncfree, 0,
diff --git a/sys/ufs/ffs/ffs_subr.c b/sys/ufs/ffs/ffs_subr.c
index e2b09da86ae5..b87516429f12 100644
--- a/sys/ufs/ffs/ffs_subr.c
+++ b/sys/ufs/ffs/ffs_subr.c
@@ -405,7 +405,8 @@ ffs_oldfscompat_write(struct fs *fs)
  */
 static int prttimechgs = 0;
 #ifdef _KERNEL
-SYSCTL_DECL(_vfs_ffs);
+SYSCTL_NODE(_vfs, OID_AUTO, ffs, CTLFLAG_RW | CTLFLAG_MPSAFE, 0,
+	"FFS filesystem");
 SYSCTL_INT(_vfs_ffs, OID_AUTO, prttimechgs, CTLFLAG_RWTUN, &prttimechgs, 0,
 	"print UFS1 time changes made to inodes");
 #endif /* _KERNEL */

[imp]

In D48794#1112862, @mckusick wrote:

This will cause libufs to fail to build. Also, ffs_subr.c is supposed to contain all the kernel functionality needed by filesystem utilities. And ffs_oldfscompat_inode_read() is one of those functions.

I propose just doing this change instead

diff --git a/sys/ufs/ffs/ffs_subr.c b/sys/ufs/ffs/ffs_subr.c
index e2b09da86ae5..ef488185e026 100644
--- a/sys/ufs/ffs/ffs_subr.c
+++ b/sys/ufs/ffs/ffs_subr.c
@@ -405,9 +405,12 @@ ffs_oldfscompat_write(struct fs *fs)
  */
 static int prttimechgs = 0;
 #ifdef _KERNEL
+#include "opt_dontuse.h" /* Check to see if FFS is in this kernel */
+#ifdef FFS
 SYSCTL_DECL(_vfs_ffs);
 SYSCTL_INT(_vfs_ffs, OID_AUTO, prttimechgs, CTLFLAG_RWTUN, &prttimechgs, 0,
 	"print UFS1 time changes made to inodes");
+#endif /* FFS */
 #endif /* _KERNEL */
 bool
 ffs_oldfscompat_inode_read(struct fs *fs, union dinodep dp, time_t now)

Why not define vfs_ffs here rather than just declare it to use? Then it just doesn't matter. Right now it is defined in ffs_alloc.c:

sys/ufs/ffs/ffs_alloc.c:SYSCTL_NODE(_vfs, OID_AUTO, ffs, CTLFLAG_RW | CTLFLAG_MPSAFE, 0,

which seems pretty random to me.

Then you don't care if FFS is in the kernel or not.

[jkim]

Gleb added a stopgap in rGe1ebda4458bb but I don't like it because we will have vfs.ffs node in MINIMAL kernel.

[imp]

In D48794#1113055, @jkim wrote:

Gleb added a stopgap in rGe1ebda4458bb but I don't like it because we will have vfs.ffs node in MINIMAL kernel.

It's the least bad outcome. We are doing ffs things, even if they are minimal.

Ideally we'd have a way to demand load geom tasters

[jkim]

I updated the patch to revert rGe1ebda4458bb and re-apply my patch.

[glebius]

I will let Kirk to decide what solution is better. I just had fixed the build.

[mckusick]

I have gone with Gleb's fix. It is simple and as Warner has pointed out `It's the least bad outcome. We are doing ffs things, even if they are minimal.' As I have pointed out, your change breaks libufs and would require many additional changes after making the needed fixes in libufs to the clients of libufs.

[mckusick]

I have accepted gleb's changes, so this issue is resolved.