Skip site navigation (1)Skip section navigation (2)
Date:      Fri, 24 Jan 2014 15:51:46 -0500 (EST)
From:      Benjamin Kaduk <bjk@freebsd.org>
To:        Andriy Gapon <avg@freebsd.org>
Cc:        hackers@freebsd.org, doc@freebsd.org
Subject:   Re: Fwd: Re: svn commit: r258713 - in head/sys: kern sys
Message-ID:  <alpine.GSO.1.10.1401241544320.27579@multics.mit.edu>
In-Reply-To: <52E21F94.5000104@FreeBSD.org>
References:  <52D93F91.502@FreeBSD.org> <52E21F94.5000104@FreeBSD.org>

next in thread | previous in thread | raw e-mail | index | archive | help
On Fri, 24 Jan 2014, Andriy Gapon wrote:

>
> I would like to commit the following manual page update.
> Could you please review it?
> Thank you very much!
>
> -------- Original Message --------
> Message-ID: <52D93F91.502@FreeBSD.org>
> Date: Fri, 17 Jan 2014 16:34:57 +0200
> From: Andriy Gapon <avg@FreeBSD.org>
> To: Justin T. Gibbs <gibbs@scsiguy.com>
> CC: src-committers@freebsd.org, svn-src-all@freebsd.org,  svn-src-head@freebsd.org
> Subject: Re: svn commit: r258713 - in head/sys: kern sys
> References: <201311281856.rASIuZu8059699@svn.freebsd.org>
> <3784C560-3648-4E03-93DA-9A60E3AC401D@scsiguy.com>
> In-Reply-To: <3784C560-3648-4E03-93DA-9A60E3AC401D@scsiguy.com>
>
> on 30/11/2013 08:46 Justin T. Gibbs said the following:
>> Man page update?
>
> I came up with the following update that
> - adds taskqueue_drain_all documentation
> - extends taskqueue_drain description
> - adds description for previously undocumented taskqueue_block /
> taskqueue_unblock, including a warning on taskqueue_block + taskqueue_drain
> combination
>
> All reviews and suggestions are welcome.  Both for content and language.
>
> diff --git a/share/man/man9/taskqueue.9 b/share/man/man9/taskqueue.9
> index 5f6131a..4db3d7a 100644
> --- a/share/man/man9/taskqueue.9
> +++ b/share/man/man9/taskqueue.9
> @@ -86,6 +86,12 @@ struct timeout_task;
> .Fn taskqueue_drain "struct taskqueue *queue" "struct task *task"
> .Ft void
> .Fn taskqueue_drain_timeout "struct taskqueue *queue" "struct timeout_task
> *timeout_task"
> +.Ft void
> +.Fn taskqueue_drain_all "struct taskqueue *queue"
> +.Ft void
> +.Fn taskqueue_block "struct taskqueue *queue"
> +.Ft void
> +.Fn taskqueue_unblock "struct taskqueue *queue"
> .Ft int
> .Fn taskqueue_member "struct taskqueue *queue" "struct thread *td"
> .Ft void
> @@ -255,6 +261,74 @@ function is used to wait for the scheduled task to finish.
> There is no guarantee that the task will not be
> enqueued after call to
> .Fn taskqueue_drain .
> +Because the caller typically would use
> +.Fn taskqueue_drain
> +to put the task into a known state, then the caller should use

This "then" is a bit odd, I think the sentence is better without it.

> +out-of-band means to ensure, before calling
> +.Fn taskqueue_drain ,
> +that the task would not be enqueued.
> +For example, if the task is enqueued by an interrupt filter, then
> +the interrupt could be disabled.
> +.Pp
> +The
> +.Fn taskqueue_drain_all
> +function is used to wait for all the pending and running tasks that
> +are enqueued on the taskqueue to finish.
> +The caller must arrange that the tasks are not re-enqueued.
> +Note that
> +.Fn taskqueue_drain_all
> +currently does not handle tasks with delayed enqueueing.
> +.Pp
> +The
> +.Fn taskqueue_block
> +function blocks the taskqueue.
> +It prevents any enqueued but not running tasks from being executed.
> +Future calls to
> +.Fn taskqueue_enqueue
> +will enqueue tasks, but the tasks will not be run until
> +.Fn taskqueue_unblock
> +is called.
> +Please note that
> +.Fn taskqueue_block
> +does not wait for any currently running tasks to finish.
> +Thus, the
> +.Fn taskqueue_block
> +does not provide a guarantee that
> +.Fn taskqueue_run
> +is not running after
> +.Fn taskqueue_block
> +returns, but it does provide a guarantee that
> +.Fn taskqueue_run
> +will not be called again
> +until
> +.Fn taskqueue_unblock
> +is called.
> +If the caller requires a guarantee that
> +.Fn taskqueue_run
> +is not running, then this must be arranged by the caller.
> +Note that if
> +.Fn taskqueue_drain
> +is called on a task that is enqueued on a taskqueue that is blocked by
> +.Fn taskqueue_block ,
> +then
> +.Fn taskqueue_drain
> +can not return until the taskqueue is unblocked.

I would prefer "cannot" as a single word here, though I don't think our 
style guide says anything about this case.

> +This can result in a deadlock if the thread blocked in
> +.Fn taskqueue_drain
> +is a thread that is supposed to call

Is s/a/the/ correct?  That is, by my reading, if the thread blocked in 
taskqueue_drain is only one of several that could call taskqueue_unblock, 
the other threads would still (eventually) do so and there is no deadlock. 
So the risk of deadlock would (only?) be if there is only one thread that 
is supposed to call taskqueue_unblock.

> +.Fn taskqueue_unblock .
> +Thus, use of
> +.Fn taskqueue_drain
> +after
> +.Fn taskqueue_block
> +is discouraged, because a state of the task can not be known in advance.

s/ a / the / here is needed, though, I am certain.

> +The same applies to

I would put another noun here, as "same caveat" or "same warning" or 
something like that.

-Ben

> +.Fn taskqueue_drain_all .
> +.Pp
> +The
> +.Fn taskqueue_unblock
> +function unblocks the previously blocked taskqueue.
> +All enqueued tasks can be run after this call.
> .Pp
> The
> .Fn taskqueue_member
>
>
> -- 
> Andriy Gapon
>
>
> _______________________________________________
> freebsd-doc@freebsd.org mailing list
> http://lists.freebsd.org/mailman/listinfo/freebsd-doc
> To unsubscribe, send any mail to "freebsd-doc-unsubscribe@freebsd.org"
>



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?alpine.GSO.1.10.1401241544320.27579>