WIP: Punt zfs_dbgmsg into zcommon

[rincebrain]

(WIP because I just want to see if the CI screams anywhere about this...)

e: Oh, I didn't mark it as draft on creation. Oops.

Motivation and Context

It's much more useful to be able to call zfs_dbgmsg from anything
that depends on zcommon - zzstd, icp...

(Using it from icp means that people can't link libicp on its own, which is unfortunate, but since the only consumer in our tree that did that is the checksum tests in the test suite, I think it's an acceptable tradeoff...if anybody really minds this, I imagine you could implement weak symbols for the dbgmsg family in libicp that stub it out and get overridden if you have libzpool instead.)

(I tried punting it into SPL, but that would involve reimplementing a whole bunch in userland because you no longer get libzpool/kernel.c if you're in libspl...oh well, if someone really wants to call it from SPL, they can do it.)

Description

git mv module/os/linux/{zfs,zcommon}/zfs_debug.c and then cleanup of fallout.

(FreeBSD never had a problem with this, since it shoves everything into one module anyway...)

How Has This Been Tested?

This branch? It built.

The branch I pulled these changes out of, I've been running tests on and using this for a couple weeks.

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)
• Library ABI change (libzfs, libzfs_core, libnvpair, libuutil and libzfsbootenv)
• Documentation (a change to man pages or other documentation)

Checklist:

• My code follows the OpenZFS code style requirements.
• I have updated the documentation accordingly.
• I have read the contributing document.
• I have added tests to cover my changes.
• I have run the ZFS Test Suite with this change applied.
• All commit messages are properly formatted and contain Signed-off-by.

[nabijaczleweli]

Now that you mention it – is there a Good Reason we couldn't just shove everything into zfs.ko for Linux, too, nowadays?

[rincebrain]

Other than if Linux has an upper limit on single module size (I'm not aware of one) or the fact that the LICENSE field on the module would become "It's Complicated...", I'm not aware of any reason we couldn't.

[adamdmoss]

It's much more useful to be able to call zfs_dbgmsg from anything
that depends on zcommon - zzstd, icp...

Yes please! I spent too long trying to figure out why this doesn't work from zzstd and ended up doing my own stupid printk() wrapper 😬

[behlendorf]

Pulling everything we can together in to a single zfs.ko would be nice. Merging the icp, znvpair, zavl, zcommon, zunicode, and zfs modules would be straight forward. Keeping zzstd, zlua, and spl modules split off might still be desirable sole to keep the license information clear.

[rincebrain]

Wouldn't merging zfs.ko into zcommon leave a cyclic dependency between zzstd and zfs?

The rest could probably get flattened, but I think you're stuck with at least some parts of zfs.ko being separate from zcommon unless you want to flatten the whole enchilada.

[behlendorf]

Good point, I was only really commenting on it from the stand point that the licenses are identical. Seems like we'd have to pull in zzstd and zlua as well. This would be fine, and it's already what we do on the FreeBSD side.

[nabijaczleweli]

So, I've spent the last 4 hours making a unified 94-megabyte zfs.ko (well, okay, 4M stripped), and it'd be perfect if:

• all of CPU hotplugging weren't GPL-only
• dequeue_signal() weren't GPL-only
• __alloc_percpu()/free_percpu() weren't GPL-only
• schedule_hrtimeout_range() weren't GPL-only

As it stands, these are all laundered via the SPL, so if you don't want to lie, it's impossible to merge it into the image. And at that point there's no point.

[rincebrain]

Oh no, I'm somehow not surprised that A) it's infeasible because of GPL-only laundering symbols (though I thought the Linux kernel started enforcing some sins-of-the-father dependency tracking on those to stop that) and B) you got to doing the experiment before me.

That said, I don't think I agree that it has no value to flatten some but not all of the modules together when there's no external consumers of them or inherent value in keeping them apart.

[behlendorf]

That said, I don't think I agree that it has no value to flatten some but not all of the modules together when there's no external consumers of them or inherent value in keeping them apart.

This is my feeling as well. Early on I thought there might be some value in keeping the modules split up but in practice that never panned out. We do have one external consumer, Lustre, however it mostly used the zfs.ko symbols so I doubt merging the kmods would cause it any serious problems.

We've also gone out of our way to avoid using GPL symbols in the SPL module, even though it is GPL licensed. So I don't think it's fair to characterize its usage as laundering. It's purpose was never to make that possible, but to simply implement the Solaris interfaces we needed.

[behlendorf]

Closing. The plan is this will be addressed by #13224 #13274 .

[rincebrain]

Did you mean #13274?

I mentioned my rationale for wanting to still see this land separately, but that's fine, I'll just keep cherrypicking it into my 2.0 and 2.1 branches.

[behlendorf]

I do, thanks for catching that.

Right, I can see the appeal. My feeling is cherrypicking this as needed is probably for the best, this strikes me as too significant a change to backport.