Page MenuHomeFreeBSD

Prevent live-lock and access of destroyed data in taskqueue_drain_all().
ClosedPublic

Authored by gibbs on Nov 30 2014, 9:53 PM.
Tags
None
Referenced Files
Unknown Object (File)
Sat, Feb 17, 5:14 AM
Unknown Object (File)
Sat, Feb 17, 5:08 AM
Unknown Object (File)
Sat, Feb 17, 4:54 AM
Unknown Object (File)
Sat, Feb 17, 4:35 AM
Unknown Object (File)
Sat, Feb 17, 4:35 AM
Unknown Object (File)
Sat, Feb 17, 4:35 AM
Unknown Object (File)
Sat, Feb 17, 4:35 AM
Unknown Object (File)
Sat, Feb 17, 4:19 AM
Subscribers

Details

Reviewers
jhb
avg
imp
Summary

sys/kern_subr_taskqueue.c:

Modify taskqueue_drain_all() processing to use a temporary               
"barrier task", rather than rely on a user task that may                 
be destroyed during taskqueue_drain_all()'s execution.  The              
barrier task is queued behind all previously queued tasks                
and then has its priority elevated so that future tasks                  
cannot pass it in the queue.                                             
                                                                         
Use a similar barrier scheme to drain threads processing                 
current tasks.  This requires taskqueue_run_locked() to                  
insert and remove the taskqueue_busy object for the running              
thread for every task processed.

share/man/man9/taskqueue.9:

Remove warning about live-lock issues with taskqueue_drain_all().

Diff Detail

Repository
rS FreeBSD src repository - subversion
Lint
No Lint Coverage
Unit
No Test Coverage

Event Timeline

gibbs retitled this revision from to Prevent live-lock and access of destroyed data in taskqueue_drain_all()..
gibbs updated this object.
gibbs edited the test plan for this revision. (Show Details)
gibbs added reviewers: jhb, avg.

Fix typo in man page: .Pr -> .Pp.

gibbs edited edge metadata.

Ignore the second diff. It was pulled from the wrong tree.

share/man/man9/taskqueue.9
290โ€“297

Do you handle tasks with delayed enqueueing? Also, you might want to reword this line a bit. "are ignored" might be interpreted to mean that the tasks are dropped rather than queued behind the barrier. I think what you want to say is that the drain doesn't wait for tasks posted after the call to complete, but I'm not sure of the best wording. Maybe something like "Tasks posted to the taskqueue while waiting may execute after taskqueue_drain_all has returned"

sys/kern/subr_taskqueue.c
374

s/interveining/intervening/

Improve man page documentation for taskqueue_drain_all().

Fix spelling errors in comments.

share/man/man9/taskqueue.9
290โ€“297

Delayed enqueuing tasks are ignored. However, the previous text that I removed was confusing (at least it confused me). I've added more explicit text about tasqueue_enqueue_timeout() to address this. I've also tried to clarify that additional enqueues do not extent the wait time of the function.

sys/kern/subr_taskqueue.c
374

Thanks for catching that. Happened was misspelled too. Both fixed.

imp added a reviewer: imp.
imp added a subscriber: imp.

comitted

This revision is now accepted and ready to land.Jan 5 2015, 5:11 PM

II think that this change has aleady been committed.
So the review should be closed?

Should have been closed by rS276665

In D1247#55369, @avg wrote:

Should have been closed by rS276665

Thanks for following up. Yes, the Differential Revision: tag has to be at the end for Phabricator to pick it up.