Source-Changes-D archive
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]
Re: CVS commit: src
Thanks, looks much nicer, and much easier to diff from my drafts!
I agree it would be nice if we had a first-class way in C to mark a
struct as opaque outside some module, but that's a nice-to-have rather
than something that makes the aliasing hacks and manual ABI analysis
worthwhile.
Job reference counting is currently slightly busted -- my draft for
threadpool_schedule_job didn't increment it correctly, and your
threadpool_schedule_job doesn't handle failure in threadpool_job_hold.
I see two obvious ways to address it:
1. Use a 64-bit reference count without atomics, under the threadpool
lock. Requires moving a couple holds and reles, but it looks like
this should work.
Only question is whether the reference count is handled by only a
single threadpool at a time. Maybe we could kassert that, at the
expense of a slightly larger struct threadpool_job.
2. Chuck the job reference count altogether. Just use job_thread as a
proxy for whether it's in use. User must not call
threadpool_schedule_job concurrently with threadpool_job_destroy;
if a job may have been scheduled, user must cancel it before
destroying it.
Bonus: No need to wait in threadpool_job_destroy. Caveat: Need to
take care of the lwp name in threadpool_job_done so that we never
point at the job_name of a job in oblivion. Secondary bonus: We
can easily KASSERTMSG((curlwp->l_name == thread->tpt_saved_name),
"you forgot to call threadpool_job_done!").
I'm inclined toward option (2) since it should need less logic, but I
haven't thought through the consequences enough to be confident either
one will work.
Home |
Main Index |
Thread Index |
Old Index