Page MenuHomeFreeBSD

add a kernel gzio interface
ClosedPublic

Authored by markj on Feb 13 2015, 3:13 AM.
Tags
None
Referenced Files
Unknown Object (File)
Jan 30 2024, 9:45 AM
Unknown Object (File)
Jan 6 2024, 3:51 AM
Unknown Object (File)
Nov 15 2023, 1:37 PM
Unknown Object (File)
Nov 9 2023, 12:09 PM
Unknown Object (File)
Nov 9 2023, 3:07 AM
Unknown Object (File)
Nov 8 2023, 1:47 AM
Unknown Object (File)
Nov 6 2023, 5:53 AM
Unknown Object (File)
Nov 1 2023, 5:03 PM
Subscribers
None

Details

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.

Diff Detail

Repository
rS FreeBSD src repository - subversion
Lint
Lint Skipped
Unit
Tests Skipped

Event Timeline

markj retitled this revision from to add a kernel gzio interface.
markj updated this object.
markj edited the test plan for this revision. (Show Details)
markj added a reviewer: rpaulo.

Quick glance and the approach looks reasonable to me.

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

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?

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.

Small fixes based on review comments.

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.

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.

  • Make sure we use the right uio segment argument.

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

rpaulo edited edge metadata.
This revision is now accepted and ready to land.Mar 2 2015, 6:31 AM
markj updated this revision to Diff 4152.

Closed by commit rS279801 (authored by @markj).