Experimental Workqueue support

[pfg]

Summary

This is based on the work of Stacey Son (sson@).
It was hammered into 12-current based on the patches
from 2014_06_01 and NextBSD.

It is considered WIP: handle with care, beware of dog.
sson's notes may apply:
https://people.freebsd.org/~sson/thrworkq/TODO.txt

Test Plan

It is only compile-tested on amd64 at this time.

MIPS build fails with a linking issue for the tests.
usr/lib/libpthread.so: undefined reference to `__sync_sub_and_fetch_4'

SPARC64 build fails:
head/sys/kern/kern_thrworkq.c:841: error: too few arguments to function 'cpu_set_upcall'

[wblock]

share/man/man3/pthread_workqueue_np.3

73

s/the following/these/

80

Better without the leading "The".

82

Better without the leading "The".

84

Better without the leading "The".

87

Better without the leading "The".

89

Better without the leading "The".

117

This should be either

...by the
.Fa workq
parameter.

Or better, just
...by
.Fa workq .

141

Avoid pauses when possible. Generally, "may" is for permission, and "might" is for possibility:

More threads will be started as needed when the overcommit flag is set.
This might overcommit the physical resources of the system.

145

s/the following/these/

148

How about:

Queued work items with this attribute will be given higher priority by

151

As above, "Queued..."

154

As above, "Queued..."

168

This is confusing worded and formatted. Is it this way because more error conditions and return codes will be added?

function will fail in the following conditions with these return codes:

(I avoid "the following" because it is usually filler. Here, it replaces the confusing "fail in these conditions".)

176

As above, confusing wording and table. Same for the rest of these.

237

Typo: s/destory/destroy/

This sentence can be written without the pauses:

There is no current way to remove or destroy work queues or pending

[kmacy]

I will incorporate this in to my Mach/Darwin/launchd branch after some further review. This will at least give the change some wider exposure.

[pfg]

In D3189#68797, @kmacy wrote:

I will incorporate this in to my Mach/Darwin/launchd branch after some further review. This will at least give the change some wider exposure.

Great! I expect to be back on this in a couple of weeks, when the GSoC is over.

[pfg]

Update manpage.
(Kicking a tinderbox build in today's -current.)

[pfg]

Build break:
...

> lib/libthr (obj,depend,all,install)

/scratch/tmp/pfg/head/lib/libthr/thread/thr_init.c: In function
'init_private':
/scratch/tmp/pfg/head/lib/libthr/thread/thr_init.c:484: error:
'_thr_stack_initial' undeclared (first use in this function)
/scratch/tmp/pfg/head/lib/libthr/thread/thr_init.c:484: error: (Each
undeclared identifier is reported only once
/scratch/tmp/pfg/head/lib/libthr/thread/thr_init.c:484: error: for each
function it appears in.)

---

It looks like _thr_stack_initial is no more so just drop the assignment
for now.

(This really needs some review by sson@)

[wblock]

share/man/man3/pthread_workqueue_np.3

141

That did not help. The original version of this sentence was missing commas. Corrected, it would be like this:

If the overcommit flag is set, then more threads will be started, if
needed, which might overcommit the physical resources of the system.

It has a lot of pauses and is not clear.

If you really want to keep the if/then structure, how about this:

When the overcommit flag is set, more threads will be started as needed.
This might overcommit the physical resources of the system.

237

Typo:
s/currentlu/currently/

[pfg]

More manpage fixes (Thanks Warren!).

This appears to build but I make no claims ... not after having brutally
hacked it into compilation :-/

[wblock]

share/man/man3/pthread_workqueue_np.3

142

Good, but new sentences should start on a new line. So:

When the overcommit flag is set, more threads will be started as
needed.
This might overcommit the physical resources of the system.

[pfg]

More man page cleanup.

Leave the '_thr_stack_initial' line commented as a reminder that
we may want to preserve rlim.rlim_cur.

[pfg]

share/man/man3/pthread_workqueue_np.3

143

I was considering a colon there

When the overcommit flag is set, more threads will be started as
needed: this might overcommit the physical resources of the system.

I left it as you suggested though.

[wblock]

Approved for the current version of the man page. Remember that it never hurts to check man pages with igor -R and mandoc -Tlint, and also remember to update the .Dd before commit. Thanks!

share/man/man3/pthread_workqueue_np.3

144

A semicolon would be usable there, but breaking the sentence in two is simpler.

[kmacy]

Just a note that this patch has never been compiled with THRWORKQ actually enabled.

[kmacy]

sys/kern/kern_thr.c

338

It's not a difficult change. But the fact that this code could never have compiled concerns me.

[kmacy]

What happened to thr_workq.c from his patch? That appears to be completely missing.

[pfg]

In D3189#70646, @kmacy wrote:

Just a note that this patch has never been compiled with THRWORKQ actually enabled.

I honestly only made sure the patch still applies so that it doesn't get lost and I haven't really gone through enabling it or much less testing it. The code needs real review but I also think it's critical to find a set of tests or at least find examples from Apple.

I am also busy putting together the results of the GSoC 2015 and can't play with this at this time. So .. yes you have reasons for concern: I am just hoping that keeping the patch in some form will make it easier to re-take where it was left off.

[pfg]

In D3189#70648, @kmacy wrote:

What happened to thr_workq.c from his patch? That appears to be completely missing.

I don't know what happened in arcanist ... I will reload the patch.

[pfg]

Add back thr_workq.c which for some reason disappeared.

[kmacy]

Thanks. I've updated it a bit in my tree and am enabling libdispatch to use it. I'll point you at the set of commits when I've finished.

[kmacy]

I've committed this patch in four parts now that it compiles, links, and doesn't cause panics.

Add kernel support:
https://github.com/freebsd/freebsd/commit/1b67658d064193c0e676058eadec0940076bebb6
Add man page:
https://github.com/freebsd/freebsd/commit/228ecc72a86e4e26e43cfc13be8806ba9b5c900a
add user-side support:
https://github.com/freebsd/freebsd/commit/9ecf34aacb8b0bbc789553c91cd968b14c438336
fix workq initialization related panics:
https://github.com/freebsd/freebsd/commit/2ab13e4ec1e283d4c51a8a9f2f31cd712748d272

I'm seeing assertion failures in jemalloc in client programs in tsd_arenas_cache_set. I'll update this again if and when this appears to be a stable implementation.

[kmacy]

And this so that suspend doesn't get confused by unscheduled workq threads:
https://github.com/freebsd/freebsd/commit/3d020b415731cec113f049191a4520e3ffda0580

[kmacy]

and this: https://github.com/freebsd/freebsd/commit/550d458c1108c9e12574fa226c93fc0cb299e95d

[pfg]

Wow, really nice Kip!

I am somewhat busy at this time ... perhaps you could take over the revision? I will still keep around and will be willing to shepherd the code into the tree when it's ready.

[brueffer]

share/man/man3/pthread_workqueue_np.3

103

is
.Dv NULL
then...

142

New sentences should start on a new line.

191

Cannot

244

.An Stacey Son Aq Mt [e-mail] .

[pfg]

Bring Kip Macy's fixes and enhancements.

[pfg]

Address brueffer's comments.

[pfg]

Build on MIPS is broken:

> lib/libc/tests/gen/posix_spawn (all)

2+0 records in
2+0 records out
2048 bytes transferred in 0.000042 secs (48360056 bytes/sec)
/scratch/tmp/pfg/obj/mips.mips64el/scratch/tmp/pfg/head/tmp/usr/lib/libpthread.so: undefined reference to `sync_sub_and_fetch_4'
/scratch/tmp/pfg/obj/mips.mips64el/scratch/tmp/pfg/head/tmp/usr/lib/libpthread.so: undefined reference to `sync_add_and_fetch_4'

• nice_test ---
• [nice_test] Error code 1

make[9]: stopped in /scratch/tmp/pfg/head/lib/libc/tests/gen
1 error

[pfg]

Update just enough to get this to compile in 12-current.
May not pass a universe build just yet.

[pfg]

Missing manpage.

[pfg]

Add missing untracked file: pthread_workqueue.h

[pfg]

Another missing file: thr_workq.c.

[pfg]

Bring some fixes from NextBSD.

Also remove a re-declaration in
sys/i386/include/atomic.h

This builds on AMD64 but I still get failures on MIPS:

...

> lib/libc/tests/db (all)

> lib/libc/tests/gen (all)

dlopen_empty_test.o: In function `atfu_dlopen_empty_test_body':
/scratch/tmp/pfg/head/lib/libc/tests/gen/dlopen_empty_test.c:66: warning:
warning: mktemp() possibly used unsafely; consider using mkstemp()
/scratch/tmp/pfg/obj/mips.mips/scratch/tmp/pfg/head/tmp/usr/lib/libpthread.so:
undefined reference to `sync_sub_and_fetch_4'
/scratch/tmp/pfg/obj/mips.mips/scratch/tmp/pfg/head/tmp/usr/lib/libpthread.so:
undefined reference to `sync_add_and_fetch_4'

• nice_test.full ---

...
This seems to be a bug in the testsuite, but needs more investigation.

[pfg]

This really needs more help.

[wblock]

share/man/man3/pthread_workqueue_np.3

92

"can" rather than "may" sounds better to me.

[adrian]

so it's just missing those atomic ops for MIPS32/MIPS64? It compiles/works fine on other architectures?

[pfg]

In D3189#182628, @adrian wrote:

so it's just missing those atomic ops for MIPS32/MIPS64? It compiles/works fine on other architectures?

It is also failing on SPARC64 :(.

Apparently it works on amd64 and i386, other platforms build but remain untested. I think sson@ told me at BSDCan 2014 that it was done but the TODO list has:

kern_thrworkq code:

• The code to assign CPU affinity needs to be fixed up and activited. I am not sure if this is really needed.

[pfg]

Bring up to date with latest head.

[pfg]

Recent changes cause breakage in lib/libthr/thread/thr_workq.c:

> lib/libthr (obj,all,install)

cc1: warnings being treated as errors
/scratch/tmp/head/lib/libthr/thread/thr_workq.c: In function 'thr_workq_init':
/scratch/tmp/head/lib/libthr/thread/thr_workq.c:266: warning: assignment from incompatible pointer type

• thr_workq.o ---
• [thr_workq.o] Error code 1

make[6]: stopped in /scratch/tmp/head/lib/libthr
1 error

make[6]: stopped in /scratch/tmp/head/lib/libthr

• lib/libthr__L ---
• [lib/libthr__L] Error code 2

make[5]: stopped in /scratch/tmp/head
1 error

make[5]: stopped in /scratch/tmp/head

• libraries ---
• [libraries] Error code 2

make[4]: stopped in /scratch/tmp/head
1 error

make[4]: stopped in /scratch/tmp/head

It actually needs a complete revision.