free old file descriptor tables when not shared

[rew]

Summary

The idea for for freeing old file descriptor tables was taken from the FreeBSD Junior Jobs
website: https://wiki.freebsd.org/JuniorJobs#Free_old_file_descriptor_tables.

During the life of a process, new file descriptor tables may be allocated. When a new table
is allocated, the old table is placed in a free list and held onto until all processes
referencing them exit.

When a new file descriptor table is allocated, the old file descriptor table can be freed when
the current process has a single-thread and the file descriptor table is not being shared
by any other processes.

based off of r342237

Test Plan

I've included tests in the diff, it uses kvm to transfer the relevant bits from kernel space
to userspace. For all tests, a sufficient amount of files are opened to initiate fdgrowtable
into creating new file descriptor tables.

1. Test a one thread process that doesn't have a shared file descriptor table. There should be no old file descriptor tables in the free list.

2. Test a process with two threads, that doesn't have a shared file descriptor table. Old file descriptor tables should be in the free list.

3. Test a process with one thread and shared file descriptor table. The old file descriptor tables should be in the free list.

[kevans]

sys/kern/kern_descrip.c

1689 ↗

(On Diff #52188)

I would expand this to mention the more complex scenario where they'll get shelved for the freelist still.

tests/sys/kern/fdgrowtable_test.c

2 ↗

(On Diff #52188)

Copyright line + SPDX tag?

152 ↗

(On Diff #52188)

All three tests should have a ATF_TC_HEAD() that at least sets require.user=root since they munge around in /dev/mem, (examples in ^/lib/libkvm/tests/)

[rew]

Fixed ATF test cases to require root user.

Clarifed comments on when a file descriptor table is
placed on the freelist.

Fixed typo for an unrelated comment in the vicinity.

[kevans]

I'd also like to get @mjg to confirm that the conditions for immediate free are fine; seems reasonable at a glance, at least.

sys/kern/kern_descrip.c

1802 ↗

(On Diff #74428)

These should all be indented with tabs (w/ tabstop=8) rather than spaces.

1815 ↗

(On Diff #74428)

Same w.r.t indentation, and style(9) indicates dropping the extra parentheses here; ("Unary operators do not require spaces, binary operators do. Do not use parentheses unless they are required for precedence or unless the statement is confusing without them.")

tests/sys/kern/fdgrowtable_test.c

59 ↗

(On Diff #74428)

These should all be formatted with a tab immediately after #define rather than a space, as per style(9).

67 ↗

(On Diff #74428)

Spaces creeped into indentation here and various other places further down in the file.

[rew]

• de-butcher the indenting

• style(9) changes as necessary

• In the test-case helper function, openfiles(): The order of precedence for expressions evaluated within ATF_REQUIRE() was incorrect. Fixed misplaced parenthesis to ensure proper evaluation. This change doesn't impact the test results. Tested with patched and un-patched kernel, results were as expected.

[kevans]

This seems to look good to me, and the tests do what I expect before and after. Give @mjg a week to object, and if we don't hear from him please feel free to commit:

Approved by: kevans (mentor)