add a kernel gzio interface

[markj]

Summary

This change replaces the current kern_gzio.c with a more general callback-based interface, and converts the existing userland core compression code to use it.

The interface is in sys/gzio.h. It currently only supports compression, no decompression.

This change also modifies the kernel configuration needed to enable userland core compression support. Before, you needed

device gzio
options COMPRESS_USER_CORES

now it's just

options GZIO

with a sysctl, kern.compress_user_cores, which defaults to off when GZIO is defined.

TODO:

• write a small man page for this interface
• the interface needs some affordances to make it easy to implement decompression support if someone wants it later on - probably an extra flag parameter to kern_gz_init() is sufficient
• the kernel config changes might warrant an UPDATING entry

I'll work on these over the next few days if the overall approach seems reasonable.

[rpaulo]

Quick glance and the approach looks reasonable to me.

[markj]

• Define the interface only with _KERNEL.
• Miscellaneous cleanup:
• gzio man page
• Less indentation in the header.

[rpaulo]

I think the code is fine and what I pointed out is minor. You could try to find someone else to review this, but it might be difficult since it's been broken so long and no one cared :-/

share/man/man5/core.5

111 ↗

(On Diff #3770)

Maybe add a comma here after "included".

116 ↗

(On Diff #3770)

Is this going to be a problem when upgrading with an old sysctl.conf?

share/man/man9/gzio.9

72 ↗

(On Diff #3770)

The description of gzio_init should move up because: it's the first function and you just wrote "Finally" in line 67.

sys/kern/imgact_elf.c

1111 ↗

(On Diff #3770)

I think we're depreciating u_int & friends.

1150 ↗

(On Diff #3770)

Can we abstract the call to vn_rdwr_inchunks with a static function? I never really liked the fact that we call vn_rdwr_inchunks so many times when the only thing that changes is the the address space parameter.

1183 ↗

(On Diff #3770)

Idem.

1252 ↗

(On Diff #3770)

It would be safer if you bzero it. I think "gzs" might be uninitialised.

sys/kern/kern_gzio.c

195 ↗

(On Diff #3770)

Can 'room' be zero?

[markj]

share/man/man5/core.5

111 ↗

(On Diff #3770)

Ok.

116 ↗

(On Diff #3770)

-1 is just the default zlib value, which happens to be 6. I just thought it would be clearer to do that. So this change has no actual effect.

share/man/man9/gzio.9

72 ↗

(On Diff #3770)

Ok. I tried to write it so that the first paragraph was a summary of all the functions, and then subsequent paragraphs described the arguments of gzio_init() and friends. It's kind of confusing though, I'll rework it.

sys/kern/imgact_elf.c

1111 ↗

(On Diff #3770)

I think that only applies to u_intXX_t. There's no uint_t in FreeBSD to replace u_int either.

1150 ↗

(On Diff #3770)

Good point. I'll rename core_gz_output_cb to core_write() without changing the body, and convert all the callers of vn_rdwr*.

1252 ↗

(On Diff #3770)

Ok.

sys/kern/kern_gzio.c

195 ↗

(On Diff #3770)

Yes. That case (room < 2) is handled by the if statement right at the end of the loop. It's a bit ugly, but I want to minimize invocations of the callback, so I try to pack the trailer into the output buffer whenever possible (which is most of the time) instead of always invoking the callback once for the trailer.

[markj]

Small fixes based on review comments.

[rpaulo]

sys/kern/imgact_elf.c

1150 ↗

(On Diff #3850)

This now uses UIO_SYSSPACE instead of UIO_USERSPACE. Is that ok?

1111 ↗

(On Diff #3770)

I meant unsigned int. :-)

sys/kern/kern_gzio.c

195 ↗

(On Diff #3770)

Got it.

[markj]

sys/kern/imgact_elf.c

1150 ↗

(On Diff #3850)

Nope. Thanks for catching that - I forgot to test the last revision with compression disabled.

1111 ↗

(On Diff #3770)

Ok, I'll change it to unsigned int.

[markj]

• Make sure we use the right uio segment argument.

[markj]

Oops, the last upload accidentally included some kernel configuration files. Sorry about that - they can be ignored.

[markj]

Closed by commit rS279801 (authored by @markj).