Re: [RFC] Outline of USB process integration in the kernel taskqueue system
On Saturday, November 06, 2010 4:33:17 pm Matthew Fleming wrote:
On Sat, Nov 6, 2010 at 7:22 AM, Hans Petter Selasky hsela...@c2i.net wrote:
Hi,
On Saturday 06 November 2010 14:57:50 Matthew Fleming wrote:
I think you're misunderstanding the existing taskqueue(9) implementation.
As long as TQ_LOCK is held, the state of ta-ta_pending cannot change,
nor can the set of running tasks. So the order of checks is
irrelevant.
I agree that the order of checks is not important. That is not the problem.
Cut paste from suggested taskqueue patch from Fleming:
+int
+taskqueue_cancel(struct taskqueue *queue, struct task *task)
+{
+ int rc;
+
+ TQ_LOCK(queue);
+ if (!task_is_running(queue, task)) {
+ if ((rc = task-ta_pending) 0)
+ STAILQ_REMOVE(queue-tq_queue, task, task,
ta_link); + task-ta_pending = 0;
+ } else {
+ rc = -EBUSY;
What happens in this case if ta_pending 0. Are you saying this is not
possible? If ta_pending 0, shouldn't we also do a STAILQ_REMOVE() ?
Ah! I see what you mean.
I'm not quite sure what the best thing to do here is; I agree it would
be nice if taskqueue_cancel(9) dequeued the task, but I believe it
also needs to indicate that the task is currently running. I guess
the best thing would be to return the old pending count by reference
parameter, and 0 or EBUSY to also indicate if there is a task
currently running.
Adding jhb@ to this mail since he has good thoughts on interfacing.
I agree we should always dequeue when possible. I think it should return
-EBUSY in that case. That way code that uses 'cancel' followed by a
conditional 'drain' to implement a blocking 'cancel' will DTRT.
--
John Baldwin
___
freebsd-usb@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-usb
To unsubscribe, send any mail to freebsd-usb-unsubscr...@freebsd.org
Re: [RFC] Outline of USB process integration in the kernel taskqueue system
On Mon, Nov 8, 2010 at 6:47 AM, John Baldwin j...@freebsd.org wrote:
On Saturday, November 06, 2010 4:33:17 pm Matthew Fleming wrote:
On Sat, Nov 6, 2010 at 7:22 AM, Hans Petter Selasky hsela...@c2i.net wrote:
Hi,
On Saturday 06 November 2010 14:57:50 Matthew Fleming wrote:
I think you're misunderstanding the existing taskqueue(9) implementation.
As long as TQ_LOCK is held, the state of ta-ta_pending cannot change,
nor can the set of running tasks. So the order of checks is
irrelevant.
I agree that the order of checks is not important. That is not the problem.
Cut paste from suggested taskqueue patch from Fleming:
+int
+taskqueue_cancel(struct taskqueue *queue, struct task *task)
+{
+ int rc;
+
+ TQ_LOCK(queue);
+ if (!task_is_running(queue, task)) {
+ if ((rc = task-ta_pending) 0)
+ STAILQ_REMOVE(queue-tq_queue, task, task,
ta_link); + task-ta_pending = 0;
+ } else {
+ rc = -EBUSY;
What happens in this case if ta_pending 0. Are you saying this is not
possible? If ta_pending 0, shouldn't we also do a STAILQ_REMOVE() ?
Ah! I see what you mean.
I'm not quite sure what the best thing to do here is; I agree it would
be nice if taskqueue_cancel(9) dequeued the task, but I believe it
also needs to indicate that the task is currently running. I guess
the best thing would be to return the old pending count by reference
parameter, and 0 or EBUSY to also indicate if there is a task
currently running.
Adding jhb@ to this mail since he has good thoughts on interfacing.
I agree we should always dequeue when possible. I think it should return
-EBUSY in that case. That way code that uses 'cancel' followed by a
conditional 'drain' to implement a blocking 'cancel' will DTRT.
Do we not also want the old ta_pending to be returned? In the case
where a task is pending and is also currently running (admittedly a
narrow window), how would we do this? This is why I suggested
returning the old ta_pending by reference. This also allows callers
who don't care about the old pending to pass NULL and ignore it.
Thanks,
matthew
___
freebsd-usb@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-usb
To unsubscribe, send any mail to freebsd-usb-unsubscr...@freebsd.org
Re: [RFC] Outline of USB process integration in the kernel taskqueue system
On Monday, November 08, 2010 10:34:33 am Matthew Fleming wrote:
On Mon, Nov 8, 2010 at 6:47 AM, John Baldwin j...@freebsd.org wrote:
On Saturday, November 06, 2010 4:33:17 pm Matthew Fleming wrote:
On Sat, Nov 6, 2010 at 7:22 AM, Hans Petter Selasky hsela...@c2i.net
wrote:
Hi,
On Saturday 06 November 2010 14:57:50 Matthew Fleming wrote:
I think you're misunderstanding the existing taskqueue(9)
implementation.
As long as TQ_LOCK is held, the state of ta-ta_pending cannot change,
nor can the set of running tasks. So the order of checks is
irrelevant.
I agree that the order of checks is not important. That is not the
problem.
Cut paste from suggested taskqueue patch from Fleming:
+int
+taskqueue_cancel(struct taskqueue *queue, struct task *task)
+{
+ int rc;
+
+ TQ_LOCK(queue);
+ if (!task_is_running(queue, task)) {
+ if ((rc = task-ta_pending) 0)
+ STAILQ_REMOVE(queue-tq_queue, task, task,
ta_link); + task-ta_pending = 0;
+ } else {
+ rc = -EBUSY;
What happens in this case if ta_pending 0. Are you saying this is not
possible? If ta_pending 0, shouldn't we also do a STAILQ_REMOVE() ?
Ah! I see what you mean.
I'm not quite sure what the best thing to do here is; I agree it would
be nice if taskqueue_cancel(9) dequeued the task, but I believe it
also needs to indicate that the task is currently running. I guess
the best thing would be to return the old pending count by reference
parameter, and 0 or EBUSY to also indicate if there is a task
currently running.
Adding jhb@ to this mail since he has good thoughts on interfacing.
I agree we should always dequeue when possible. I think it should return
-EBUSY in that case. That way code that uses 'cancel' followed by a
conditional 'drain' to implement a blocking 'cancel' will DTRT.
Do we not also want the old ta_pending to be returned? In the case
where a task is pending and is also currently running (admittedly a
narrow window), how would we do this? This is why I suggested
returning the old ta_pending by reference. This also allows callers
who don't care about the old pending to pass NULL and ignore it.
I would be fine with that then. I wonder if taskqueue_cancel() could then
return a simple true/false. False if the task is running, and true
otherwise?
--
John Baldwin
___
freebsd-usb@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-usb
To unsubscribe, send any mail to freebsd-usb-unsubscr...@freebsd.org
Re: [RFC] Outline of USB process integration in the kernel taskqueue system
On Mon, Nov 8, 2010 at 8:42 AM, John Baldwin j...@freebsd.org wrote:
On Monday, November 08, 2010 10:34:33 am Matthew Fleming wrote:
On Mon, Nov 8, 2010 at 6:47 AM, John Baldwin j...@freebsd.org wrote:
On Saturday, November 06, 2010 4:33:17 pm Matthew Fleming wrote:
On Sat, Nov 6, 2010 at 7:22 AM, Hans Petter Selasky hsela...@c2i.net
wrote:
Hi,
On Saturday 06 November 2010 14:57:50 Matthew Fleming wrote:
I think you're misunderstanding the existing taskqueue(9)
implementation.
As long as TQ_LOCK is held, the state of ta-ta_pending cannot change,
nor can the set of running tasks. So the order of checks is
irrelevant.
I agree that the order of checks is not important. That is not the
problem.
Cut paste from suggested taskqueue patch from Fleming:
+int
+taskqueue_cancel(struct taskqueue *queue, struct task *task)
+{
+ int rc;
+
+ TQ_LOCK(queue);
+ if (!task_is_running(queue, task)) {
+ if ((rc = task-ta_pending) 0)
+ STAILQ_REMOVE(queue-tq_queue, task, task,
ta_link); + task-ta_pending = 0;
+ } else {
+ rc = -EBUSY;
What happens in this case if ta_pending 0. Are you saying this is not
possible? If ta_pending 0, shouldn't we also do a STAILQ_REMOVE() ?
Ah! I see what you mean.
I'm not quite sure what the best thing to do here is; I agree it would
be nice if taskqueue_cancel(9) dequeued the task, but I believe it
also needs to indicate that the task is currently running. I guess
the best thing would be to return the old pending count by reference
parameter, and 0 or EBUSY to also indicate if there is a task
currently running.
Adding jhb@ to this mail since he has good thoughts on interfacing.
I agree we should always dequeue when possible. I think it should return
-EBUSY in that case. That way code that uses 'cancel' followed by a
conditional 'drain' to implement a blocking 'cancel' will DTRT.
Do we not also want the old ta_pending to be returned? In the case
where a task is pending and is also currently running (admittedly a
narrow window), how would we do this? This is why I suggested
returning the old ta_pending by reference. This also allows callers
who don't care about the old pending to pass NULL and ignore it.
I would be fine with that then. I wonder if taskqueue_cancel() could then
return a simple true/false. False if the task is running, and true
otherwise?
Sure, though since we don't really have a bool type in the kernel I'd
still prefer to return an int with EBUSY meaning a task was running.
Thanks,
matthew
___
freebsd-usb@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-usb
To unsubscribe, send any mail to freebsd-usb-unsubscr...@freebsd.org
Re: [RFC] Outline of USB process integration in the kernel taskqueue system
On Mon, Nov 8, 2010 at 8:46 AM, Matthew Fleming mdf...@gmail.com wrote:
On Mon, Nov 8, 2010 at 8:42 AM, John Baldwin j...@freebsd.org wrote:
On Monday, November 08, 2010 10:34:33 am Matthew Fleming wrote:
On Mon, Nov 8, 2010 at 6:47 AM, John Baldwin j...@freebsd.org wrote:
On Saturday, November 06, 2010 4:33:17 pm Matthew Fleming wrote:
On Sat, Nov 6, 2010 at 7:22 AM, Hans Petter Selasky hsela...@c2i.net
wrote:
Hi,
On Saturday 06 November 2010 14:57:50 Matthew Fleming wrote:
I think you're misunderstanding the existing taskqueue(9)
implementation.
As long as TQ_LOCK is held, the state of ta-ta_pending cannot change,
nor can the set of running tasks. So the order of checks is
irrelevant.
I agree that the order of checks is not important. That is not the
problem.
Cut paste from suggested taskqueue patch from Fleming:
+int
+taskqueue_cancel(struct taskqueue *queue, struct task *task)
+{
+ int rc;
+
+ TQ_LOCK(queue);
+ if (!task_is_running(queue, task)) {
+ if ((rc = task-ta_pending) 0)
+ STAILQ_REMOVE(queue-tq_queue, task, task,
ta_link); + task-ta_pending = 0;
+ } else {
+ rc = -EBUSY;
What happens in this case if ta_pending 0. Are you saying this is not
possible? If ta_pending 0, shouldn't we also do a STAILQ_REMOVE() ?
Ah! I see what you mean.
I'm not quite sure what the best thing to do here is; I agree it would
be nice if taskqueue_cancel(9) dequeued the task, but I believe it
also needs to indicate that the task is currently running. I guess
the best thing would be to return the old pending count by reference
parameter, and 0 or EBUSY to also indicate if there is a task
currently running.
Adding jhb@ to this mail since he has good thoughts on interfacing.
I agree we should always dequeue when possible. I think it should return
-EBUSY in that case. That way code that uses 'cancel' followed by a
conditional 'drain' to implement a blocking 'cancel' will DTRT.
Do we not also want the old ta_pending to be returned? In the case
where a task is pending and is also currently running (admittedly a
narrow window), how would we do this? This is why I suggested
returning the old ta_pending by reference. This also allows callers
who don't care about the old pending to pass NULL and ignore it.
I would be fine with that then. I wonder if taskqueue_cancel() could then
return a simple true/false. False if the task is running, and true
otherwise?
Sure, though since we don't really have a bool type in the kernel I'd
still prefer to return an int with EBUSY meaning a task was running.
I'll commit this later today unless there are objections.
http://people.freebsd.org/~mdf/0001-Add-a-taskqueue_cancel-9-to-cancel-a-pending-task-wi.patch
Thanks,
matthew
___
freebsd-usb@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-usb
To unsubscribe, send any mail to freebsd-usb-unsubscr...@freebsd.org
Re: [RFC] Outline of USB process integration in the kernel taskqueue system
On Monday, November 08, 2010 11:46:58 am Matthew Fleming wrote:
On Mon, Nov 8, 2010 at 8:42 AM, John Baldwin j...@freebsd.org wrote:
On Monday, November 08, 2010 10:34:33 am Matthew Fleming wrote:
On Mon, Nov 8, 2010 at 6:47 AM, John Baldwin j...@freebsd.org wrote:
On Saturday, November 06, 2010 4:33:17 pm Matthew Fleming wrote:
On Sat, Nov 6, 2010 at 7:22 AM, Hans Petter Selasky hsela...@c2i.net
wrote:
Hi,
On Saturday 06 November 2010 14:57:50 Matthew Fleming wrote:
I think you're misunderstanding the existing taskqueue(9)
implementation.
As long as TQ_LOCK is held, the state of ta-ta_pending cannot
change,
nor can the set of running tasks. So the order of checks is
irrelevant.
I agree that the order of checks is not important. That is not the
problem.
Cut paste from suggested taskqueue patch from Fleming:
+int
+taskqueue_cancel(struct taskqueue *queue, struct task *task)
+{
+ int rc;
+
+ TQ_LOCK(queue);
+ if (!task_is_running(queue, task)) {
+ if ((rc = task-ta_pending) 0)
+ STAILQ_REMOVE(queue-tq_queue, task, task,
ta_link); + task-ta_pending = 0;
+ } else {
+ rc = -EBUSY;
What happens in this case if ta_pending 0. Are you saying this is
not
possible? If ta_pending 0, shouldn't we also do a STAILQ_REMOVE() ?
Ah! I see what you mean.
I'm not quite sure what the best thing to do here is; I agree it would
be nice if taskqueue_cancel(9) dequeued the task, but I believe it
also needs to indicate that the task is currently running. I guess
the best thing would be to return the old pending count by reference
parameter, and 0 or EBUSY to also indicate if there is a task
currently running.
Adding jhb@ to this mail since he has good thoughts on interfacing.
I agree we should always dequeue when possible. I think it should return
-EBUSY in that case. That way code that uses 'cancel' followed by a
conditional 'drain' to implement a blocking 'cancel' will DTRT.
Do we not also want the old ta_pending to be returned? In the case
where a task is pending and is also currently running (admittedly a
narrow window), how would we do this? This is why I suggested
returning the old ta_pending by reference. This also allows callers
who don't care about the old pending to pass NULL and ignore it.
I would be fine with that then. I wonder if taskqueue_cancel() could then
return a simple true/false. False if the task is running, and true
otherwise?
Sure, though since we don't really have a bool type in the kernel I'd
still prefer to return an int with EBUSY meaning a task was running.
The only reason I would prefer the other sense (false if already running)
is that is what callout_stop()'s return value is (true if it stopped the
callout, false if it couldn't stop it)).
However, returning EBUSY is ok. One difference is that callout_stop()
returns false if the callout is not pending (i.e. not on softclock). Right
now taskqueue_cancel() returns 0 in that case (ta_pending == 0), but that is
probably fine as long as it is documented. If you wanted to return EINVAL
instead, that would be fine, too.
--
John Baldwin
___
freebsd-usb@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-usb
To unsubscribe, send any mail to freebsd-usb-unsubscr...@freebsd.org
Re: [RFC] Outline of USB process integration in the kernel taskqueue system
On Mon, Nov 8, 2010 at 10:24 AM, John Baldwin j...@freebsd.org wrote:
On Monday, November 08, 2010 11:46:58 am Matthew Fleming wrote:
On Mon, Nov 8, 2010 at 8:42 AM, John Baldwin j...@freebsd.org wrote:
On Monday, November 08, 2010 10:34:33 am Matthew Fleming wrote:
On Mon, Nov 8, 2010 at 6:47 AM, John Baldwin j...@freebsd.org wrote:
On Saturday, November 06, 2010 4:33:17 pm Matthew Fleming wrote:
On Sat, Nov 6, 2010 at 7:22 AM, Hans Petter Selasky hsela...@c2i.net
wrote:
Hi,
On Saturday 06 November 2010 14:57:50 Matthew Fleming wrote:
I think you're misunderstanding the existing taskqueue(9)
implementation.
As long as TQ_LOCK is held, the state of ta-ta_pending cannot
change,
nor can the set of running tasks. So the order of checks is
irrelevant.
I agree that the order of checks is not important. That is not the
problem.
Cut paste from suggested taskqueue patch from Fleming:
+int
+taskqueue_cancel(struct taskqueue *queue, struct task *task)
+{
+ int rc;
+
+ TQ_LOCK(queue);
+ if (!task_is_running(queue, task)) {
+ if ((rc = task-ta_pending) 0)
+ STAILQ_REMOVE(queue-tq_queue, task,
task,
ta_link); + task-ta_pending = 0;
+ } else {
+ rc = -EBUSY;
What happens in this case if ta_pending 0. Are you saying this is
not
possible? If ta_pending 0, shouldn't we also do a STAILQ_REMOVE() ?
Ah! I see what you mean.
I'm not quite sure what the best thing to do here is; I agree it would
be nice if taskqueue_cancel(9) dequeued the task, but I believe it
also needs to indicate that the task is currently running. I guess
the best thing would be to return the old pending count by reference
parameter, and 0 or EBUSY to also indicate if there is a task
currently running.
Adding jhb@ to this mail since he has good thoughts on interfacing.
I agree we should always dequeue when possible. I think it should
return
-EBUSY in that case. That way code that uses 'cancel' followed by a
conditional 'drain' to implement a blocking 'cancel' will DTRT.
Do we not also want the old ta_pending to be returned? In the case
where a task is pending and is also currently running (admittedly a
narrow window), how would we do this? This is why I suggested
returning the old ta_pending by reference. This also allows callers
who don't care about the old pending to pass NULL and ignore it.
I would be fine with that then. I wonder if taskqueue_cancel() could then
return a simple true/false. False if the task is running, and true
otherwise?
Sure, though since we don't really have a bool type in the kernel I'd
still prefer to return an int with EBUSY meaning a task was running.
The only reason I would prefer the other sense (false if already running)
is that is what callout_stop()'s return value is (true if it stopped the
callout, false if it couldn't stop it)).
I think the symmetry with callout(9) can't go very far, mostly because
callout generally takes a specified mtx before calling the callback,
and taskqueue(9) does not. That means fundamentally that, when using
a taskqueue(9) the consumer has much less knowledge of the exact state
of a task.
Thanks,
matthew
However, returning EBUSY is ok. One difference is that callout_stop()
returns false if the callout is not pending (i.e. not on softclock). Right
now taskqueue_cancel() returns 0 in that case (ta_pending == 0), but that is
probably fine as long as it is documented. If you wanted to return EINVAL
instead, that would be fine, too.
--
John Baldwin
___
freebsd-usb@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-usb
To unsubscribe, send any mail to freebsd-usb-unsubscr...@freebsd.org
Re: [RFC] Outline of USB process integration in the kernel taskqueue system
On Sat, Nov 6, 2010 at 1:37 AM, Hans Petter Selasky hsela...@c2i.net wrote:
On Friday 05 November 2010 20:06:12 John Baldwin wrote:
On Friday, November 05, 2010 3:00:37 pm Hans Petter Selasky wrote:
On Friday 05 November 2010 19:48:05 Matthew Fleming wrote:
On Fri, Nov 5, 2010 at 11:45 AM, Hans Petter Selasky hsela...@c2i.net
wrote:
On Friday 05 November 2010 19:39:45 Matthew Fleming wrote:
True, but no taskqueue(9) code can handle that. Only the caller can
prevent a task from becoming enqueued again. The same issue exists
with taskqueue_drain().
I find that strange, because that means if I queue a task again while
it is running, then I doesn't get run? Are you really sure?
If a task is currently running when enqueued, the task struct will be
re-enqueued to the taskqueue. When that task comes up as the head of
the queue, it will be run again.
Right, and the taskqueue_cancel has to cancel in that state to, but it
doesn't because it only checks pending if !running() :-) ??
You can't close that race in taskqueue_cancel(). You have to manage that
race yourself in your task handler. For the callout(9) API we are only
able to close that race if you use callout_init_mtx() so that the code
managing the callout wheel can make use of your lock to resolve the races.
If you use callout_init() you have to explicitly manage these races in your
callout handler.
Hi,
I think you are getting me wrong! In the initial 0001-Implement-
taskqueue_cancel-9-to-cancel-a-task-from-a.patch you have the following code-
chunk:
+int
+taskqueue_cancel(struct taskqueue *queue, struct task *task)
+{
+ int rc;
+
+ TQ_LOCK(queue);
+ if (!task_is_running(queue, task)) {
+ if ((rc = task-ta_pending) 0)
+ STAILQ_REMOVE(queue-tq_queue, task, task, ta_link);
+ task-ta_pending = 0;
+ } else
+ rc = -EBUSY;
+ TQ_UNLOCK(queue);
+ return (rc);
+}
This code should be written like this, having the if (!task_is_running())
after checking the ta_pending variable.
+int
+taskqueue_cancel(struct taskqueue *queue, struct task *task)
+{
+ int rc;
+
+ TQ_LOCK(queue);
+ if ((rc = task-ta_pending) 0) {
+ STAILQ_REMOVE(queue-tq_queue, task, task, ta_link);
+ task-ta_pending = 0;
+ }
+ if (!task_is_running(queue, task))
+ rc = -EBUSY;
+ TQ_UNLOCK(queue);
+ return (rc);
+}
Or completely skip the !task_is_running() check. Else you are opening up a
race in this function! Do I need to explain that more? Isn't it obvious?
I think you're misunderstanding the existing taskqueue(9) implementation.
As long as TQ_LOCK is held, the state of ta-ta_pending cannot change,
nor can the set of running tasks. So the order of checks is
irrelevant.
As John said, the taskqueue(9) implementation cannot protect consumers
of it from re-queueing a task; that kind of serialization needs to
happen at a higher level.
taskqueue(9) is not quite like callout(9); the taskqueue(9)
implementation drops all locks before calling the task's callback
function. So once the task is running, taskqueue(9) can do nothing
about it until the task stops running. This is why Jeff's
implementation of taskqueue_cancel(9) slept if the task was running,
and why mine returns an error code. The only way to know for sure
that a task is about to run is to ask taskqueue(9), because there's
a window where the TQ_LOCK is dropped before the callback is entered.
Thanks,
matthew
___
freebsd-usb@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-usb
To unsubscribe, send any mail to freebsd-usb-unsubscr...@freebsd.org
Re: [RFC] Outline of USB process integration in the kernel taskqueue system
Hi,
On Saturday 06 November 2010 14:57:50 Matthew Fleming wrote:
I think you're misunderstanding the existing taskqueue(9) implementation.
As long as TQ_LOCK is held, the state of ta-ta_pending cannot change,
nor can the set of running tasks. So the order of checks is
irrelevant.
I agree that the order of checks is not important. That is not the problem.
Cut paste from suggested taskqueue patch from Fleming:
+int
+taskqueue_cancel(struct taskqueue *queue, struct task *task)
+{
+ int rc;
+
+ TQ_LOCK(queue);
+ if (!task_is_running(queue, task)) {
+ if ((rc = task-ta_pending) 0)
+ STAILQ_REMOVE(queue-tq_queue, task, task,
ta_link); + task-ta_pending = 0;
+ } else {
+ rc = -EBUSY;
What happens in this case if ta_pending 0. Are you saying this is not
possible? If ta_pending 0, shouldn't we also do a STAILQ_REMOVE() ?
+ }
+ TQ_UNLOCK(queue);
+ return (rc);
+}
As John said, the taskqueue(9) implementation cannot protect consumers
of it from re-queueing a task; that kind of serialization needs to
happen at a higher level.
Agreed, but that is not what I'm pointing at. I'm pointing at what happens if
you re-queue a task and then cancel while it is actually running. Will the
task still be queued for execution after taskqueue_cancel()?
taskqueue(9) is not quite like callout(9); the taskqueue(9)
implementation drops all locks before calling the task's callback
function. So once the task is running, taskqueue(9) can do nothing
about it until the task stops running.
This is not the problem.
This is why Jeff's
implementation of taskqueue_cancel(9) slept if the task was running,
and why mine returns an error code. The only way to know for sure
that a task is about to run is to ask taskqueue(9), because there's
a window where the TQ_LOCK is dropped before the callback is entered.
And if you re-queue and cancel in this window, shouldn't this also be handled
like in the other cases?
Cut paste from kern/subr_taskqueue.c:
task-ta_pending = 0;
tb.tb_running = task;
TQ_UNLOCK(queue);
If a concurrent thread at exactly this point in time calls taskqueue_enqueue()
on this task, then we re-add the task to the queue-tq_queue. So far we
agree. Imagine now that for some reason a following call to taskqueue_cancel()
on this task at same point in time. Now, shouldn't taskqueue_cancel() also
remove the task from the queue-tq_queue in this case aswell? Because in
your implementation you only remove the task if we are not running, and that
is not true when we are at exactly this point in time.
task-ta_func(task-ta_context, pending);
TQ_LOCK(queue);
tb.tb_running = NULL;
wakeup(task);
Another issue I noticed is that the ta_pending counter should have a wrap
protector.
--HPS
___
freebsd-usb@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-usb
To unsubscribe, send any mail to freebsd-usb-unsubscr...@freebsd.org
Re: [RFC] Outline of USB process integration in the kernel taskqueue system
On Sat, Nov 6, 2010 at 7:22 AM, Hans Petter Selasky hsela...@c2i.net wrote:
Hi,
On Saturday 06 November 2010 14:57:50 Matthew Fleming wrote:
I think you're misunderstanding the existing taskqueue(9) implementation.
As long as TQ_LOCK is held, the state of ta-ta_pending cannot change,
nor can the set of running tasks. So the order of checks is
irrelevant.
I agree that the order of checks is not important. That is not the problem.
Cut paste from suggested taskqueue patch from Fleming:
+int
+taskqueue_cancel(struct taskqueue *queue, struct task *task)
+{
+ int rc;
+
+ TQ_LOCK(queue);
+ if (!task_is_running(queue, task)) {
+ if ((rc = task-ta_pending) 0)
+ STAILQ_REMOVE(queue-tq_queue, task, task,
ta_link); + task-ta_pending = 0;
+ } else {
+ rc = -EBUSY;
What happens in this case if ta_pending 0. Are you saying this is not
possible? If ta_pending 0, shouldn't we also do a STAILQ_REMOVE() ?
Ah! I see what you mean.
I'm not quite sure what the best thing to do here is; I agree it would
be nice if taskqueue_cancel(9) dequeued the task, but I believe it
also needs to indicate that the task is currently running. I guess
the best thing would be to return the old pending count by reference
parameter, and 0 or EBUSY to also indicate if there is a task
currently running.
Adding jhb@ to this mail since he has good thoughts on interfacing.
Thanks,
matthew
+ }
+ TQ_UNLOCK(queue);
+ return (rc);
+}
As John said, the taskqueue(9) implementation cannot protect consumers
of it from re-queueing a task; that kind of serialization needs to
happen at a higher level.
Agreed, but that is not what I'm pointing at. I'm pointing at what happens if
you re-queue a task and then cancel while it is actually running. Will the
task still be queued for execution after taskqueue_cancel()?
taskqueue(9) is not quite like callout(9); the taskqueue(9)
implementation drops all locks before calling the task's callback
function. So once the task is running, taskqueue(9) can do nothing
about it until the task stops running.
This is not the problem.
This is why Jeff's
implementation of taskqueue_cancel(9) slept if the task was running,
and why mine returns an error code. The only way to know for sure
that a task is about to run is to ask taskqueue(9), because there's
a window where the TQ_LOCK is dropped before the callback is entered.
And if you re-queue and cancel in this window, shouldn't this also be handled
like in the other cases?
Cut paste from kern/subr_taskqueue.c:
task-ta_pending = 0;
tb.tb_running = task;
TQ_UNLOCK(queue);
If a concurrent thread at exactly this point in time calls taskqueue_enqueue()
on this task, then we re-add the task to the queue-tq_queue. So far we
agree. Imagine now that for some reason a following call to taskqueue_cancel()
on this task at same point in time. Now, shouldn't taskqueue_cancel() also
remove the task from the queue-tq_queue in this case aswell? Because in
your implementation you only remove the task if we are not running, and that
is not true when we are at exactly this point in time.
task-ta_func(task-ta_context, pending);
TQ_LOCK(queue);
tb.tb_running = NULL;
wakeup(task);
Another issue I noticed is that the ta_pending counter should have a wrap
protector.
--HPS
___
freebsd-usb@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-usb
To unsubscribe, send any mail to freebsd-usb-unsubscr...@freebsd.org
Re: [RFC] Outline of USB process integration in the kernel taskqueue system
On Fri, Nov 05, 2010 at 07:30:38PM +0100, Hans Petter Selasky wrote: Hi, In the patch attached to this e-mail I included Matthew Fleming's patch aswell. 1) I renamed taskqueue_cancel() into taskqueue_stop(), hence that resembles the words of the callout and USB API's terminology for doing the same. 2) I turns out I need to have code in subr_taskqueue.c to be able to make the operations atomic. 3) I did not update the manpage in this patch. Will do that before a commit. 4) My patch implements separate state keeping in struct task_pair, which avoids having to change any KPI's for now, like suggested by John Baldwin I think. 5) In my implementation I hard-coded the priority argument to zero, so that enqueuing is fast. Comments are welcome! The patch looks almost you moved usb_process.c code into taskqueue(9) that I means it still follows that a entry which enqueued at last should be executed at last. It seems that at least it's not a general for taskqueue(9). In my humble opinion it looks a trick. I think it'd better to find a general solution to solve it though I used sx(9) lock in my patches. regards, Weongyo Jeong ___ freebsd-usb@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/freebsd-usb To unsubscribe, send any mail to freebsd-usb-unsubscr...@freebsd.org
Re: [RFC] Outline of USB process integration in the kernel taskqueue system
On Thursday, November 04, 2010 5:49:22 pm Matthew Fleming wrote: On Thu, Nov 4, 2010 at 2:22 PM, John Baldwin j...@freebsd.org wrote: On Thursday, November 04, 2010 4:15:16 pm Hans Petter Selasky wrote: I think that if a task is currently executing, then there should be a drain method for that. I.E. two methods: One to stop and one to cancel/drain. Can you implement this? I agree, this would also be consistent with the callout_*() API if you had both stop() and drain() methods. Here's my proposed code. Note that this builds but is not yet tested. Implement a taskqueue_cancel(9), to cancel a task from a queue. Requested by: hps Original code: jeff MFC after: 1 week http://people.freebsd.org/~mdf/bsd-taskqueue-cancel.diff For FreeBSD taskqueue_cancel() should return EBUSY, not -EBUSY. However, I would prefer that it follow the semantics of callout_stop() and return true if it stopped the task and false otherwise. The Linux wrapper for taskqueue_cancel() can convert the return value. I'm not sure I like reusing the memory allocation flags (M_NOWAIT / M_WAITOK) for this blocking flag. In the case of callout(9) we just have two functions that pass an internal boolean to the real routine (callout_stop() and callout_drain() are wrappers for _callout_stop_safe()). It is a bit unfortunate that taskqueue_drain() already exists and has different semantics than callout_drain(). It would have been nice to have the two APIs mirror each other instead. Hmm, I wonder if the blocking behavior cannot safely be provided by just doing: if (!taskqueue_cancel(queue, task, M_NOWAIT) taskqueue_drain(queue, task); If that works ok (I think it does), I would rather have taskqueue_cancel() always be non-blocking. Even though there is a race where the task could be rescheduled by another thread in between cancel and drain, the race still exists since if the task could be scheduled between the two, it could also be scheduled just before the call to taskqueue_cancel() (in which case a taskqueue_cancel(queue, task, M_WAITOK) would have blocked to wait for it matching the taskqueue_drain() above). The caller still always has to provide synchronization for preventing a task's execution outright via their own locking. -- John Baldwin ___ freebsd-usb@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/freebsd-usb To unsubscribe, send any mail to freebsd-usb-unsubscr...@freebsd.org
Re: [RFC] Outline of USB process integration in the kernel taskqueue system
On Fri, Nov 5, 2010 at 5:58 AM, John Baldwin j...@freebsd.org wrote: On Thursday, November 04, 2010 5:49:22 pm Matthew Fleming wrote: On Thu, Nov 4, 2010 at 2:22 PM, John Baldwin j...@freebsd.org wrote: On Thursday, November 04, 2010 4:15:16 pm Hans Petter Selasky wrote: I think that if a task is currently executing, then there should be a drain method for that. I.E. two methods: One to stop and one to cancel/drain. Can you implement this? I agree, this would also be consistent with the callout_*() API if you had both stop() and drain() methods. Here's my proposed code. Note that this builds but is not yet tested. Implement a taskqueue_cancel(9), to cancel a task from a queue. Requested by: hps Original code: jeff MFC after: 1 week http://people.freebsd.org/~mdf/bsd-taskqueue-cancel.diff For FreeBSD taskqueue_cancel() should return EBUSY, not -EBUSY. However, I would prefer that it follow the semantics of callout_stop() and return true if it stopped the task and false otherwise. The Linux wrapper for taskqueue_cancel() can convert the return value. I used -EBUSY since positive return values reflect the old pending count. ta_pending was zero'd, and I think needs to be to keep the task sane, because all of taskqueue(9) assumes a non-zero ta_pending means the task is queued. I don't know that the caller often needs to know the old value of ta_pending, but it seems simpler to return that as the return value and use -EBUSY than to use an optional pointer to a place to store the old ta_pending just so we can keep the error return positive. Note that phk (IIRC) suggested using -error in the returns for sbuf_drain to indicate the difference between success ( 0 bytes drained) and an error, so FreeBSD now has precedent. I'm not entirely sure that's a good thing, since I am not generally fond of Linux's use of -error, but for some cases it is convenient. But, I'll do this one either way, just let me know if the above hasn't convinced you. I'm not sure I like reusing the memory allocation flags (M_NOWAIT / M_WAITOK) for this blocking flag. In the case of callout(9) we just have two functions that pass an internal boolean to the real routine (callout_stop() and callout_drain() are wrappers for _callout_stop_safe()). It is a bit unfortunate that taskqueue_drain() already exists and has different semantics than callout_drain(). It would have been nice to have the two APIs mirror each other instead. Hmm, I wonder if the blocking behavior cannot safely be provided by just doing: if (!taskqueue_cancel(queue, task, M_NOWAIT) taskqueue_drain(queue, task); This seems reasonable and correct. I will add a note to the manpage about this. Thanks, matthew If that works ok (I think it does), I would rather have taskqueue_cancel() always be non-blocking. Even though there is a race where the task could be rescheduled by another thread in between cancel and drain, the race still exists since if the task could be scheduled between the two, it could also be scheduled just before the call to taskqueue_cancel() (in which case a taskqueue_cancel(queue, task, M_WAITOK) would have blocked to wait for it matching the taskqueue_drain() above). The caller still always has to provide synchronization for preventing a task's execution outright via their own locking. -- John Baldwin ___ freebsd-usb@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/freebsd-usb To unsubscribe, send any mail to freebsd-usb-unsubscr...@freebsd.org
Re: [RFC] Outline of USB process integration in the kernel taskqueue system
On Friday, November 05, 2010 9:50:10 am Matthew Fleming wrote: On Fri, Nov 5, 2010 at 5:58 AM, John Baldwin j...@freebsd.org wrote: On Thursday, November 04, 2010 5:49:22 pm Matthew Fleming wrote: On Thu, Nov 4, 2010 at 2:22 PM, John Baldwin j...@freebsd.org wrote: On Thursday, November 04, 2010 4:15:16 pm Hans Petter Selasky wrote: I think that if a task is currently executing, then there should be a drain method for that. I.E. two methods: One to stop and one to cancel/drain. Can you implement this? I agree, this would also be consistent with the callout_*() API if you had both stop() and drain() methods. Here's my proposed code. Note that this builds but is not yet tested. Implement a taskqueue_cancel(9), to cancel a task from a queue. Requested by: hps Original code: jeff MFC after: 1 week http://people.freebsd.org/~mdf/bsd-taskqueue-cancel.diff For FreeBSD taskqueue_cancel() should return EBUSY, not -EBUSY. However, I would prefer that it follow the semantics of callout_stop() and return true if it stopped the task and false otherwise. The Linux wrapper for taskqueue_cancel() can convert the return value. I used -EBUSY since positive return values reflect the old pending count. ta_pending was zero'd, and I think needs to be to keep the task sane, because all of taskqueue(9) assumes a non-zero ta_pending means the task is queued. I don't know that the caller often needs to know the old value of ta_pending, but it seems simpler to return that as the return value and use -EBUSY than to use an optional pointer to a place to store the old ta_pending just so we can keep the error return positive. Note that phk (IIRC) suggested using -error in the returns for sbuf_drain to indicate the difference between success ( 0 bytes drained) and an error, so FreeBSD now has precedent. I'm not entirely sure that's a good thing, since I am not generally fond of Linux's use of -error, but for some cases it is convenient. But, I'll do this one either way, just let me know if the above hasn't convinced you. Hmm, I hadn't considered if callers would want to know the pending count of the cancelled task. I'm not sure I like reusing the memory allocation flags (M_NOWAIT / M_WAITOK) for this blocking flag. In the case of callout(9) we just have two functions that pass an internal boolean to the real routine (callout_stop() and callout_drain() are wrappers for _callout_stop_safe()). It is a bit unfortunate that taskqueue_drain() already exists and has different semantics than callout_drain(). It would have been nice to have the two APIs mirror each other instead. Hmm, I wonder if the blocking behavior cannot safely be provided by just doing: if (!taskqueue_cancel(queue, task, M_NOWAIT) taskqueue_drain(queue, task); This seems reasonable and correct. I will add a note to the manpage about this. In that case, would you be fine with dropping the blocking functionality from taskqueue_cancel() completely and requiring code that wants the blocking semantics to use a cancel followed by a drain? -- John Baldwin ___ freebsd-usb@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/freebsd-usb To unsubscribe, send any mail to freebsd-usb-unsubscr...@freebsd.org
Re: [RFC] Outline of USB process integration in the kernel taskqueue system
On Fri, Nov 5, 2010 at 7:18 AM, John Baldwin j...@freebsd.org wrote: On Friday, November 05, 2010 9:50:10 am Matthew Fleming wrote: On Fri, Nov 5, 2010 at 5:58 AM, John Baldwin j...@freebsd.org wrote: On Thursday, November 04, 2010 5:49:22 pm Matthew Fleming wrote: On Thu, Nov 4, 2010 at 2:22 PM, John Baldwin j...@freebsd.org wrote: On Thursday, November 04, 2010 4:15:16 pm Hans Petter Selasky wrote: I think that if a task is currently executing, then there should be a drain method for that. I.E. two methods: One to stop and one to cancel/drain. Can you implement this? I agree, this would also be consistent with the callout_*() API if you had both stop() and drain() methods. Here's my proposed code. Note that this builds but is not yet tested. Implement a taskqueue_cancel(9), to cancel a task from a queue. Requested by: hps Original code: jeff MFC after: 1 week http://people.freebsd.org/~mdf/bsd-taskqueue-cancel.diff For FreeBSD taskqueue_cancel() should return EBUSY, not -EBUSY. However, I would prefer that it follow the semantics of callout_stop() and return true if it stopped the task and false otherwise. The Linux wrapper for taskqueue_cancel() can convert the return value. I used -EBUSY since positive return values reflect the old pending count. ta_pending was zero'd, and I think needs to be to keep the task sane, because all of taskqueue(9) assumes a non-zero ta_pending means the task is queued. I don't know that the caller often needs to know the old value of ta_pending, but it seems simpler to return that as the return value and use -EBUSY than to use an optional pointer to a place to store the old ta_pending just so we can keep the error return positive. Note that phk (IIRC) suggested using -error in the returns for sbuf_drain to indicate the difference between success ( 0 bytes drained) and an error, so FreeBSD now has precedent. I'm not entirely sure that's a good thing, since I am not generally fond of Linux's use of -error, but for some cases it is convenient. But, I'll do this one either way, just let me know if the above hasn't convinced you. Hmm, I hadn't considered if callers would want to know the pending count of the cancelled task. I'm not sure I like reusing the memory allocation flags (M_NOWAIT / M_WAITOK) for this blocking flag. In the case of callout(9) we just have two functions that pass an internal boolean to the real routine (callout_stop() and callout_drain() are wrappers for _callout_stop_safe()). It is a bit unfortunate that taskqueue_drain() already exists and has different semantics than callout_drain(). It would have been nice to have the two APIs mirror each other instead. Hmm, I wonder if the blocking behavior cannot safely be provided by just doing: if (!taskqueue_cancel(queue, task, M_NOWAIT) taskqueue_drain(queue, task); This seems reasonable and correct. I will add a note to the manpage about this. In that case, would you be fine with dropping the blocking functionality from taskqueue_cancel() completely and requiring code that wants the blocking semantics to use a cancel followed by a drain? New patch is at http://people.freebsd.org/~mdf/0001-Implement-taskqueue_cancel-9-to-cancel-a-task-from-a.patch I'll try to set up something to test it today too. Thanks, matthew ___ freebsd-usb@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/freebsd-usb To unsubscribe, send any mail to freebsd-usb-unsubscr...@freebsd.org
Re: [RFC] Outline of USB process integration in the kernel taskqueue system
On Friday 05 November 2010 18:15:01 Matthew Fleming wrote:
On Fri, Nov 5, 2010 at 7:18 AM, John Baldwin j...@freebsd.org wrote:
On Friday, November 05, 2010 9:50:10 am Matthew Fleming wrote:
On Fri, Nov 5, 2010 at 5:58 AM, John Baldwin j...@freebsd.org wrote:
On Thursday, November 04, 2010 5:49:22 pm Matthew Fleming wrote:
On Thu, Nov 4, 2010 at 2:22 PM, John Baldwin j...@freebsd.org wrote:
On Thursday, November 04, 2010 4:15:16 pm Hans Petter Selasky wrote:
I think that if a task is currently executing, then there should
be a drain method for that. I.E. two methods: One to stop and one
to cancel/drain. Can you implement this?
I agree, this would also be consistent with the callout_*() API if
you had both stop() and drain() methods.
Here's my proposed code. Note that this builds but is not yet
tested.
Implement a taskqueue_cancel(9), to cancel a task from a queue.
Requested by: hps
Original code: jeff
MFC after: 1 week
http://people.freebsd.org/~mdf/bsd-taskqueue-cancel.diff
For FreeBSD taskqueue_cancel() should return EBUSY, not -EBUSY.
However, I would prefer that it follow the semantics of
callout_stop() and return true if it stopped the task and false
otherwise. The Linux wrapper for taskqueue_cancel() can convert the
return value.
I used -EBUSY since positive return values reflect the old pending
count. ta_pending was zero'd, and I think needs to be to keep the
task sane, because all of taskqueue(9) assumes a non-zero ta_pending
means the task is queued.
I don't know that the caller often needs to know the old value of
ta_pending, but it seems simpler to return that as the return value
and use -EBUSY than to use an optional pointer to a place to store the
old ta_pending just so we can keep the error return positive.
Note that phk (IIRC) suggested using -error in the returns for
sbuf_drain to indicate the difference between success ( 0 bytes
drained) and an error, so FreeBSD now has precedent. I'm not entirely
sure that's a good thing, since I am not generally fond of Linux's use
of -error, but for some cases it is convenient.
But, I'll do this one either way, just let me know if the above hasn't
convinced you.
Hmm, I hadn't considered if callers would want to know the pending count
of the cancelled task.
I'm not sure I like reusing the memory allocation flags (M_NOWAIT /
M_WAITOK) for this blocking flag. In the case of callout(9) we just
have two functions that pass an internal boolean to the real routine
(callout_stop() and callout_drain() are wrappers for
_callout_stop_safe()). It is a bit unfortunate that
taskqueue_drain() already exists and has different semantics than
callout_drain(). It would have been nice to have the two APIs mirror
each other instead.
Hmm, I wonder if the blocking behavior cannot safely be provided by
just doing:
if (!taskqueue_cancel(queue, task, M_NOWAIT)
taskqueue_drain(queue, task);
This seems reasonable and correct. I will add a note to the manpage
about this.
In that case, would you be fine with dropping the blocking functionality
from taskqueue_cancel() completely and requiring code that wants the
blocking semantics to use a cancel followed by a drain?
New patch is at
http://people.freebsd.org/~mdf/0001-Implement-taskqueue_cancel-9-to-cancel-
a-task-from-a.patch
I think the:
+ if (!task_is_running(queue, task)) {
check needs to be omitted. Else you block the possibility of enqueue and
cancel a task while it is actually executing/running ??
--HPS
___
freebsd-usb@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-usb
To unsubscribe, send any mail to freebsd-usb-unsubscr...@freebsd.org
Re: [RFC] Outline of USB process integration in the kernel taskqueue system
On Fri, Nov 5, 2010 at 10:36 AM, Hans Petter Selasky hsela...@c2i.net wrote:
On Friday 05 November 2010 18:15:01 Matthew Fleming wrote:
On Fri, Nov 5, 2010 at 7:18 AM, John Baldwin j...@freebsd.org wrote:
On Friday, November 05, 2010 9:50:10 am Matthew Fleming wrote:
On Fri, Nov 5, 2010 at 5:58 AM, John Baldwin j...@freebsd.org wrote:
On Thursday, November 04, 2010 5:49:22 pm Matthew Fleming wrote:
On Thu, Nov 4, 2010 at 2:22 PM, John Baldwin j...@freebsd.org wrote:
On Thursday, November 04, 2010 4:15:16 pm Hans Petter Selasky wrote:
I think that if a task is currently executing, then there should
be a drain method for that. I.E. two methods: One to stop and one
to cancel/drain. Can you implement this?
I agree, this would also be consistent with the callout_*() API if
you had both stop() and drain() methods.
Here's my proposed code. Note that this builds but is not yet
tested.
Implement a taskqueue_cancel(9), to cancel a task from a queue.
Requested by: hps
Original code: jeff
MFC after: 1 week
http://people.freebsd.org/~mdf/bsd-taskqueue-cancel.diff
For FreeBSD taskqueue_cancel() should return EBUSY, not -EBUSY.
However, I would prefer that it follow the semantics of
callout_stop() and return true if it stopped the task and false
otherwise. The Linux wrapper for taskqueue_cancel() can convert the
return value.
I used -EBUSY since positive return values reflect the old pending
count. ta_pending was zero'd, and I think needs to be to keep the
task sane, because all of taskqueue(9) assumes a non-zero ta_pending
means the task is queued.
I don't know that the caller often needs to know the old value of
ta_pending, but it seems simpler to return that as the return value
and use -EBUSY than to use an optional pointer to a place to store the
old ta_pending just so we can keep the error return positive.
Note that phk (IIRC) suggested using -error in the returns for
sbuf_drain to indicate the difference between success ( 0 bytes
drained) and an error, so FreeBSD now has precedent. I'm not entirely
sure that's a good thing, since I am not generally fond of Linux's use
of -error, but for some cases it is convenient.
But, I'll do this one either way, just let me know if the above hasn't
convinced you.
Hmm, I hadn't considered if callers would want to know the pending count
of the cancelled task.
I'm not sure I like reusing the memory allocation flags (M_NOWAIT /
M_WAITOK) for this blocking flag. In the case of callout(9) we just
have two functions that pass an internal boolean to the real routine
(callout_stop() and callout_drain() are wrappers for
_callout_stop_safe()). It is a bit unfortunate that
taskqueue_drain() already exists and has different semantics than
callout_drain(). It would have been nice to have the two APIs mirror
each other instead.
Hmm, I wonder if the blocking behavior cannot safely be provided by
just doing:
if (!taskqueue_cancel(queue, task, M_NOWAIT)
taskqueue_drain(queue, task);
This seems reasonable and correct. I will add a note to the manpage
about this.
In that case, would you be fine with dropping the blocking functionality
from taskqueue_cancel() completely and requiring code that wants the
blocking semantics to use a cancel followed by a drain?
New patch is at
http://people.freebsd.org/~mdf/0001-Implement-taskqueue_cancel-9-to-cancel-
a-task-from-a.patch
I think the:
+ if (!task_is_running(queue, task)) {
check needs to be omitted. Else you block the possibility of enqueue and
cancel a task while it is actually executing/running ??
Huh? If the task is currently running, there's nothing to do except
return failure. Task running means it can't be canceled, because...
it's running.
Thanks,
matthew
___
freebsd-usb@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-usb
To unsubscribe, send any mail to freebsd-usb-unsubscr...@freebsd.org
Re: [RFC] Outline of USB process integration in the kernel taskqueue system
Hi,
In the patch attached to this e-mail I included Matthew Fleming's patch
aswell.
1) I renamed taskqueue_cancel() into taskqueue_stop(), hence that resembles
the words of the callout and USB API's terminology for doing the same.
2) I turns out I need to have code in subr_taskqueue.c to be able to make the
operations atomic.
3) I did not update the manpage in this patch. Will do that before a commit.
4) My patch implements separate state keeping in struct task_pair, which
avoids having to change any KPI's for now, like suggested by John Baldwin I
think.
5) In my implementation I hard-coded the priority argument to zero, so that
enqueuing is fast.
Comments are welcome!
--HPS
=== kern/subr_taskqueue.c
==
--- kern/subr_taskqueue.c (revision 214796)
+++ kern/subr_taskqueue.c (local)
@@ -275,6 +275,25 @@
return (0);
}
+int
+taskqueue_stop(struct taskqueue *queue, struct task *task)
+{
+ int retval = 0;
+
+ TQ_LOCK(queue);
+
+ if (task-ta_pending != 0) {
+ STAILQ_REMOVE(queue-tq_queue, task, task, ta_link);
+ task-ta_pending = 0;
+ }
+ if (task_is_running(queue, task))
+ retval = EBUSY;
+
+ TQ_UNLOCK(queue);
+
+ return (retval);
+}
+
void
taskqueue_drain(struct taskqueue *queue, struct task *task)
{
@@ -288,6 +307,113 @@
TQ_UNLOCK(queue);
}
+
+int
+taskqueue_pair_enqueue(struct taskqueue *queue, struct task_pair *tp)
+{
+ struct task *task;
+ int retval;
+ int j;
+
+ TQ_LOCK(queue);
+
+ j = 0;
+ if (tp-tp_task[0].ta_pending 0)
+ j |= 1;
+ if (tp-tp_task[1].ta_pending 0)
+ j |= 2;
+
+ if (j == 0) {
+ /* No entries are queued. Just pick a last task. */
+ tp-tp_last = 0;
+ /* Re-queue the last queued task. */
+ task = tp-tp_task[0];
+ } else if (j == 1) {
+ /* There is only one task pending and the other becomes last. */
+ tp-tp_last = 1;
+ /* Re-queue the last queued task. */
+ task = tp-tp_task[1];
+ } else if (j == 2) {
+ /* There is only one task pending and the other becomes last. */
+ tp-tp_last = 0;
+ /* Re-queue the last queued task. */
+ task = tp-tp_task[0];
+ } else {
+ /* Re-queue the last queued task. */
+ task = tp-tp_task[tp-tp_last];
+ STAILQ_REMOVE(queue-tq_queue, task, task, ta_link);
+ }
+
+ STAILQ_INSERT_TAIL(queue-tq_queue, task, ta_link);
+
+ retval = tp-tp_last + 1;
+ /* store the actual order in the pending count */
+ task-ta_pending = retval;
+
+ if ((queue-tq_flags TQ_FLAGS_BLOCKED) == 0)
+ queue-tq_enqueue(queue-tq_context);
+ else
+ queue-tq_flags |= TQ_FLAGS_PENDING;
+
+ TQ_UNLOCK(queue);
+
+ return (retval);
+}
+
+int
+taskqueue_pair_stop(struct taskqueue *queue, struct task_pair *tp)
+{
+ struct task *task;
+ int retval = 0;
+
+ TQ_LOCK(queue);
+
+ task = tp-tp_task[0];
+ if (task-ta_pending != 0) {
+ STAILQ_REMOVE(queue-tq_queue, task, task, ta_link);
+ task-ta_pending = 0;
+ }
+ if (task_is_running(queue, task))
+ retval = EBUSY;
+
+ task = tp-tp_task[1];
+ if (task-ta_pending != 0) {
+ STAILQ_REMOVE(queue-tq_queue, task, task, ta_link);
+ task-ta_pending = 0;
+ }
+ if (task_is_running(queue, task))
+ retval = EBUSY;
+
+ TQ_UNLOCK(queue);
+
+ return (retval);
+}
+
+void
+taskqueue_pair_drain(struct taskqueue *queue, struct task_pair *tp)
+{
+ struct task *task;
+
+ if (!queue-tq_spin)
+ WITNESS_WARN(WARN_GIANTOK | WARN_SLEEPOK, NULL, __func__);
+
+ TQ_LOCK(queue);
+top:
+ task = tp-tp_task[0];
+ if (task-ta_pending != 0 || task_is_running(queue, task)) {
+ TQ_SLEEP(queue, task, queue-tq_mutex, PWAIT, -, 0);
+ goto top;
+ }
+
+ task = tp-tp_task[1];
+ if (task-ta_pending != 0 || task_is_running(queue, task)) {
+ TQ_SLEEP(queue, task, queue-tq_mutex, PWAIT, -, 0);
+ goto top;
+ }
+
+ TQ_UNLOCK(queue);
+}
+
static void
taskqueue_swi_enqueue(void *context)
{
=== sys/_task.h
==
--- sys/_task.h (revision 214796)
+++ sys/_task.h (local)
@@ -51,4 +51,9 @@
void *ta_context; /* (c) argument for handler */
};
+struct task_pair {
+ struct task tp_task[2];
+ int tp_last; /* (q) index of last queued task */
+};
+
#endif /* !_SYS__TASK_H_ */
=== sys/taskqueue.h
==
--- sys/taskqueue.h (revision 214796)
+++ sys/taskqueue.h (local)
@@ -53,7 +53,11 @@
void *context);
int taskqueue_start_threads(struct taskqueue **tqp, int count, int pri,
const char *name, ...) __printflike(4, 5);
+int taskqueue_pair_enqueue(struct taskqueue *queue, struct task_pair *tp);
+int taskqueue_pair_stop(struct taskqueue *queue, struct task_pair *tp);
+void taskqueue_pair_drain(struct taskqueue *queue, struct task_pair *tp);
int taskqueue_enqueue(struct taskqueue *queue, struct task *task);
+int taskqueue_stop(struct taskqueue *queue, struct task *task);
void taskqueue_drain(struct taskqueue *queue, struct task *task);
void taskqueue_free(struct taskqueue *queue);
void taskqueue_run(struct taskqueue *queue);
@@ -78,6 +82,15 @@
} while (0)
Re: [RFC] Outline of USB process integration in the kernel taskqueue system
On Friday 05 November 2010 19:13:08 Matthew Fleming wrote:
On Fri, Nov 5, 2010 at 10:36 AM, Hans Petter Selasky hsela...@c2i.net
wrote:
On Friday 05 November 2010 18:15:01 Matthew Fleming wrote:
On Fri, Nov 5, 2010 at 7:18 AM, John Baldwin j...@freebsd.org wrote:
On Friday, November 05, 2010 9:50:10 am Matthew Fleming wrote:
On Fri, Nov 5, 2010 at 5:58 AM, John Baldwin j...@freebsd.org wrote:
On Thursday, November 04, 2010 5:49:22 pm Matthew Fleming wrote:
On Thu, Nov 4, 2010 at 2:22 PM, John Baldwin j...@freebsd.org
wrote:
On Thursday, November 04, 2010 4:15:16 pm Hans Petter Selasky
wrote:
I think that if a task is currently executing, then there
should be a drain method for that. I.E. two methods: One to
stop and one to cancel/drain. Can you implement this?
I agree, this would also be consistent with the callout_*() API
if you had both stop() and drain() methods.
Here's my proposed code. Note that this builds but is not yet
tested.
Implement a taskqueue_cancel(9), to cancel a task from a queue.
Requested by: hps
Original code: jeff
MFC after: 1 week
http://people.freebsd.org/~mdf/bsd-taskqueue-cancel.diff
For FreeBSD taskqueue_cancel() should return EBUSY, not -EBUSY.
However, I would prefer that it follow the semantics of
callout_stop() and return true if it stopped the task and false
otherwise. The Linux wrapper for taskqueue_cancel() can convert
the return value.
I used -EBUSY since positive return values reflect the old pending
count. ta_pending was zero'd, and I think needs to be to keep the
task sane, because all of taskqueue(9) assumes a non-zero ta_pending
means the task is queued.
I don't know that the caller often needs to know the old value of
ta_pending, but it seems simpler to return that as the return value
and use -EBUSY than to use an optional pointer to a place to store
the old ta_pending just so we can keep the error return positive.
Note that phk (IIRC) suggested using -error in the returns for
sbuf_drain to indicate the difference between success ( 0 bytes
drained) and an error, so FreeBSD now has precedent. I'm not
entirely sure that's a good thing, since I am not generally fond of
Linux's use of -error, but for some cases it is convenient.
But, I'll do this one either way, just let me know if the above
hasn't convinced you.
Hmm, I hadn't considered if callers would want to know the pending
count of the cancelled task.
I'm not sure I like reusing the memory allocation flags (M_NOWAIT /
M_WAITOK) for this blocking flag. In the case of callout(9) we
just have two functions that pass an internal boolean to the real
routine (callout_stop() and callout_drain() are wrappers for
_callout_stop_safe()). It is a bit unfortunate that
taskqueue_drain() already exists and has different semantics than
callout_drain(). It would have been nice to have the two APIs
mirror each other instead.
Hmm, I wonder if the blocking behavior cannot safely be provided by
just doing:
if (!taskqueue_cancel(queue, task, M_NOWAIT)
taskqueue_drain(queue, task);
This seems reasonable and correct. I will add a note to the manpage
about this.
In that case, would you be fine with dropping the blocking
functionality from taskqueue_cancel() completely and requiring code
that wants the blocking semantics to use a cancel followed by a
drain?
New patch is at
http://people.freebsd.org/~mdf/0001-Implement-taskqueue_cancel-9-to-canc
el- a-task-from-a.patch
I think the:
+ if (!task_is_running(queue, task)) {
If it is running, it is dequeued from the the taskqueue, right? And while it
is running it can be queued again, which your initial code didn't handle?
--HPS
___
freebsd-usb@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-usb
To unsubscribe, send any mail to freebsd-usb-unsubscr...@freebsd.org
Re: [RFC] Outline of USB process integration in the kernel taskqueue system
On Fri, Nov 5, 2010 at 11:35 AM, Hans Petter Selasky hsela...@c2i.net wrote:
On Friday 05 November 2010 19:13:08 Matthew Fleming wrote:
On Fri, Nov 5, 2010 at 10:36 AM, Hans Petter Selasky hsela...@c2i.net
wrote:
On Friday 05 November 2010 18:15:01 Matthew Fleming wrote:
On Fri, Nov 5, 2010 at 7:18 AM, John Baldwin j...@freebsd.org wrote:
On Friday, November 05, 2010 9:50:10 am Matthew Fleming wrote:
On Fri, Nov 5, 2010 at 5:58 AM, John Baldwin j...@freebsd.org wrote:
On Thursday, November 04, 2010 5:49:22 pm Matthew Fleming wrote:
On Thu, Nov 4, 2010 at 2:22 PM, John Baldwin j...@freebsd.org
wrote:
On Thursday, November 04, 2010 4:15:16 pm Hans Petter Selasky
wrote:
I think that if a task is currently executing, then there
should be a drain method for that. I.E. two methods: One to
stop and one to cancel/drain. Can you implement this?
I agree, this would also be consistent with the callout_*() API
if you had both stop() and drain() methods.
Here's my proposed code. Note that this builds but is not yet
tested.
Implement a taskqueue_cancel(9), to cancel a task from a queue.
Requested by: hps
Original code: jeff
MFC after: 1 week
http://people.freebsd.org/~mdf/bsd-taskqueue-cancel.diff
For FreeBSD taskqueue_cancel() should return EBUSY, not -EBUSY.
However, I would prefer that it follow the semantics of
callout_stop() and return true if it stopped the task and false
otherwise. The Linux wrapper for taskqueue_cancel() can convert
the return value.
I used -EBUSY since positive return values reflect the old pending
count. ta_pending was zero'd, and I think needs to be to keep the
task sane, because all of taskqueue(9) assumes a non-zero ta_pending
means the task is queued.
I don't know that the caller often needs to know the old value of
ta_pending, but it seems simpler to return that as the return value
and use -EBUSY than to use an optional pointer to a place to store
the old ta_pending just so we can keep the error return positive.
Note that phk (IIRC) suggested using -error in the returns for
sbuf_drain to indicate the difference between success ( 0 bytes
drained) and an error, so FreeBSD now has precedent. I'm not
entirely sure that's a good thing, since I am not generally fond of
Linux's use of -error, but for some cases it is convenient.
But, I'll do this one either way, just let me know if the above
hasn't convinced you.
Hmm, I hadn't considered if callers would want to know the pending
count of the cancelled task.
I'm not sure I like reusing the memory allocation flags (M_NOWAIT /
M_WAITOK) for this blocking flag. In the case of callout(9) we
just have two functions that pass an internal boolean to the real
routine (callout_stop() and callout_drain() are wrappers for
_callout_stop_safe()). It is a bit unfortunate that
taskqueue_drain() already exists and has different semantics than
callout_drain(). It would have been nice to have the two APIs
mirror each other instead.
Hmm, I wonder if the blocking behavior cannot safely be provided by
just doing:
if (!taskqueue_cancel(queue, task, M_NOWAIT)
taskqueue_drain(queue, task);
This seems reasonable and correct. I will add a note to the manpage
about this.
In that case, would you be fine with dropping the blocking
functionality from taskqueue_cancel() completely and requiring code
that wants the blocking semantics to use a cancel followed by a
drain?
New patch is at
http://people.freebsd.org/~mdf/0001-Implement-taskqueue_cancel-9-to-canc
el- a-task-from-a.patch
I think the:
+ if (!task_is_running(queue, task)) {
If it is running, it is dequeued from the the taskqueue, right? And while it
is running it can be queued again, which your initial code didn't handle?
True, but no taskqueue(9) code can handle that. Only the caller can
prevent a task from becoming enqueued again. The same issue exists
with taskqueue_drain().
BTW, I planned to commit the patch I sent today after testing,
assuming jhb@ has no more issues.
Thanks,
matthew
___
freebsd-usb@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-usb
To unsubscribe, send any mail to freebsd-usb-unsubscr...@freebsd.org
Re: [RFC] Outline of USB process integration in the kernel taskqueue system
On Friday 05 November 2010 19:39:45 Matthew Fleming wrote: True, but no taskqueue(9) code can handle that. Only the caller can prevent a task from becoming enqueued again. The same issue exists with taskqueue_drain(). I find that strange, because that means if I queue a task again while it is running, then I doesn't get run? Are you really sure? --HPS ___ freebsd-usb@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/freebsd-usb To unsubscribe, send any mail to freebsd-usb-unsubscr...@freebsd.org
Re: [RFC] Outline of USB process integration in the kernel taskqueue system
On Fri, Nov 5, 2010 at 11:45 AM, Hans Petter Selasky hsela...@c2i.net wrote: On Friday 05 November 2010 19:39:45 Matthew Fleming wrote: True, but no taskqueue(9) code can handle that. Only the caller can prevent a task from becoming enqueued again. The same issue exists with taskqueue_drain(). I find that strange, because that means if I queue a task again while it is running, then I doesn't get run? Are you really sure? If a task is currently running when enqueued, the task struct will be re-enqueued to the taskqueue. When that task comes up as the head of the queue, it will be run again. Thanks, matthew ___ freebsd-usb@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/freebsd-usb To unsubscribe, send any mail to freebsd-usb-unsubscr...@freebsd.org
Re: [RFC] Outline of USB process integration in the kernel taskqueue system
On Friday 05 November 2010 19:48:05 Matthew Fleming wrote: On Fri, Nov 5, 2010 at 11:45 AM, Hans Petter Selasky hsela...@c2i.net wrote: On Friday 05 November 2010 19:39:45 Matthew Fleming wrote: True, but no taskqueue(9) code can handle that. Only the caller can prevent a task from becoming enqueued again. The same issue exists with taskqueue_drain(). I find that strange, because that means if I queue a task again while it is running, then I doesn't get run? Are you really sure? If a task is currently running when enqueued, the task struct will be re-enqueued to the taskqueue. When that task comes up as the head of the queue, it will be run again. Right, and the taskqueue_cancel has to cancel in that state to, but it doesn't because it only checks pending if !running() :-) ?? --HPS ___ freebsd-usb@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/freebsd-usb To unsubscribe, send any mail to freebsd-usb-unsubscr...@freebsd.org
Re: [RFC] Outline of USB process integration in the kernel taskqueue system
On Friday, November 05, 2010 3:00:37 pm Hans Petter Selasky wrote: On Friday 05 November 2010 19:48:05 Matthew Fleming wrote: On Fri, Nov 5, 2010 at 11:45 AM, Hans Petter Selasky hsela...@c2i.net wrote: On Friday 05 November 2010 19:39:45 Matthew Fleming wrote: True, but no taskqueue(9) code can handle that. Only the caller can prevent a task from becoming enqueued again. The same issue exists with taskqueue_drain(). I find that strange, because that means if I queue a task again while it is running, then I doesn't get run? Are you really sure? If a task is currently running when enqueued, the task struct will be re-enqueued to the taskqueue. When that task comes up as the head of the queue, it will be run again. Right, and the taskqueue_cancel has to cancel in that state to, but it doesn't because it only checks pending if !running() :-) ?? You can't close that race in taskqueue_cancel(). You have to manage that race yourself in your task handler. For the callout(9) API we are only able to close that race if you use callout_init_mtx() so that the code managing the callout wheel can make use of your lock to resolve the races. If you use callout_init() you have to explicitly manage these races in your callout handler. -- John Baldwin ___ freebsd-usb@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/freebsd-usb To unsubscribe, send any mail to freebsd-usb-unsubscr...@freebsd.org
Re: [RFC] Outline of USB process integration in the kernel taskqueue system
On Tuesday 02 November 2010 08:39:45 Hans Petter Selasky wrote: On Monday 01 November 2010 22:14:49 John Baldwin wrote: On Monday, November 01, 2010 3:54:59 pm Hans Petter Selasky wrote: Hi! I've wrapped up an outline patch for what needs to be done to integrate the USB process framework into the kernel taskqueue system in a more direct way that to wrap it. The limitation of the existing taskqueue system is that it only guarantees execution at a given priority level. USB requires more. USB also requires a guarantee that the last task queued task also gets executed last. This is for example so that a deferred USB detach event does not happen before any pending deferred I/O for example in case of multiple occurring events. Mostly this new feature is targeted for GPIO-alike system using slow busses like the USB. Typical use case: 2 tasks to program GPIO on. 2 tasks to program GPIO off. Example: a) taskqueue_enqueue_odd(sc-sc_taskqueue, sc-sc_task_on[0], sc- sc_task_on[1]); b) taskqueue_enqueue_odd(sc-sc_taskqueue, sc-sc_task_off[0], sc- sc_task_off[1]); No matter how the call ordering of code-line a) and b), we are always guaranteed that the last queued state on or off is reached before the head of the taskqueue empties. In lack of a better name, the new function was called taskqueue_enqueue_odd [some people obviously think that USB processes are odd, but not taskqueues :-)] It feels like this should be something you could manage with a state machine internal to USB rather than forcing that state into the taskqueue code itself. Hi John, No, this is not possible without keeping my own queue, which I want to avoid. By state-machine you mean remembering the last state as a separate variable and checking that in the task-callback, right? Yes, I do that in addition to the new queuing mechanism. A task barrier does not solve my problem. The barrier in my case is always last in the queue. I need to pull out previously queued tasks and put them last. That is currently not supported. I do this because I don't want to have a FIFO signalling model, and a neither want the pure taskqueue, which only ensures execution, not order of execution when at the same priority. Another issue: Won't the barrier model lead to blocking the caller once the task in question is being issued the second time? --HPS If you wanted a simple barrier task (where a barrier task is always queued at the tail of the list and all subsequent tasks are queued after the barrier task) then I would be fine with adding that. You could manage this without having to alter the task KBI by having the taskqueue maintain a separate pointer to the current barrier task and always enqueue entries after that task (the barrier would be NULL before a barrier is queued, and set to NULL when a barrier executes). I think this sort of semantic is a bit simpler and also used in other parts of the tree (e.g. in bio queues). Any more comments on this matter or someone still doing review? --HPS ___ freebsd-usb@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/freebsd-usb To unsubscribe, send any mail to freebsd-usb-unsubscr...@freebsd.org
Re: [RFC] Outline of USB process integration in the kernel taskqueue system
On Mon, Nov 1, 2010 at 1:15 PM, Hans Petter Selasky hsela...@c2i.net wrote: On Monday 01 November 2010 21:07:29 Matthew Fleming wrote: On Mon, Nov 1, 2010 at 12:54 PM, Hans Petter Selasky hsela...@c2i.net wrote: Hi! I've wrapped up an outline patch for what needs to be done to integrate the USB process framework into the kernel taskqueue system in a more direct way that to wrap it. The limitation of the existing taskqueue system is that it only guarantees execution at a given priority level. USB requires more. USB also requires a guarantee that the last task queued task also gets executed last. This is for example so that a deferred USB detach event does not happen before any pending deferred I/O for example in case of multiple occurring events. Mostly this new feature is targeted for GPIO-alike system using slow busses like the USB. Typical use case: 2 tasks to program GPIO on. 2 tasks to program GPIO off. Example: a) taskqueue_enqueue_odd(sc-sc_taskqueue, sc-sc_task_on[0], sc- sc_task_on[1]); b) taskqueue_enqueue_odd(sc-sc_taskqueue, sc-sc_task_off[0], sc- sc_task_off[1]); No matter how the call ordering of code-line a) and b), we are always guaranteed that the last queued state on or off is reached before the head of the taskqueue empties. In lack of a better name, the new function was called taskqueue_enqueue_odd [some people obviously think that USB processes are odd, but not taskqueues :-)] I'd like to make sure I understand the USB requirements. (1) does USB need the task priority field? Many taskqueue(9) consumers do not. No, USB does not need a task priority field, but a sequence field associated with the task and task queue head to figure out which task was queued first without having to scan all the tasks queued. (2) if there was a working taskqueue_remove(9) that removed the task if pending or returned error if the task was currently running, would that be sufficient to implement the required USB functionality? (assuming that taskqueue_enqueue(9) put all tasks with equal priority in order of queueing). No, not completely. See comment above. I also need information about which task was queued first, or else I have to keep this information separately, which again, confuse people. The more layers the more confusion? I don't follow why keeping the information about which task was queued first privately rather than having taskqueue(9) maintain it is confusing. So far, USB seems to be the only taskqueue consumer which needs this information, so it makes a lot more sense to me for it to be USB private. To my mind, there's primary operations, and secondary ones. A secondary operation can be built from the primary ones. It reads to me that, if there was a taskqueue_cancel(9) (and there is in Jeff's OFED branch) then you could build the functionality you want (and maybe you don't need cancel, even). While there is sometimes an argument for making secondary operations available in a library, in this case you need extra data which most other taskqueue consumers do not. That would break the KBI. That is another argument in favor of keeping the implementation private to USB. Thanks, matthew ___ freebsd-usb@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/freebsd-usb To unsubscribe, send any mail to freebsd-usb-unsubscr...@freebsd.org
Re: [RFC] Outline of USB process integration in the kernel taskqueue system
On Thursday 04 November 2010 14:55:09 Matthew Fleming wrote: On Mon, Nov 1, 2010 at 1:15 PM, Hans Petter Selasky hsela...@c2i.net wrote: On Monday 01 November 2010 21:07:29 Matthew Fleming wrote: On Mon, Nov 1, 2010 at 12:54 PM, Hans Petter Selasky hsela...@c2i.net wrote: Hi! I've wrapped up an outline patch for what needs to be done to integrate the USB process framework into the kernel taskqueue system in a more direct way that to wrap it. The limitation of the existing taskqueue system is that it only guarantees execution at a given priority level. USB requires more. USB also requires a guarantee that the last task queued task also gets executed last. This is for example so that a deferred USB detach event does not happen before any pending deferred I/O for example in case of multiple occurring events. Mostly this new feature is targeted for GPIO-alike system using slow busses like the USB. Typical use case: 2 tasks to program GPIO on. 2 tasks to program GPIO off. Example: a) taskqueue_enqueue_odd(sc-sc_taskqueue, sc-sc_task_on[0], sc- sc_task_on[1]); b) taskqueue_enqueue_odd(sc-sc_taskqueue, sc-sc_task_off[0], sc- sc_task_off[1]); No matter how the call ordering of code-line a) and b), we are always guaranteed that the last queued state on or off is reached before the head of the taskqueue empties. In lack of a better name, the new function was called taskqueue_enqueue_odd [some people obviously think that USB processes are odd, but not taskqueues :-)] I'd like to make sure I understand the USB requirements. (1) does USB need the task priority field? Many taskqueue(9) consumers do not. No, USB does not need a task priority field, but a sequence field associated with the task and task queue head to figure out which task was queued first without having to scan all the tasks queued. (2) if there was a working taskqueue_remove(9) that removed the task if pending or returned error if the task was currently running, would that be sufficient to implement the required USB functionality? (assuming that taskqueue_enqueue(9) put all tasks with equal priority in order of queueing). No, not completely. See comment above. I also need information about which task was queued first, or else I have to keep this information separately, which again, confuse people. The more layers the more confusion? Hi, I don't follow why keeping the information about which task was queued first privately rather than having taskqueue(9) maintain it is confusing. So far, USB seems to be the only taskqueue consumer which needs this information, so it makes a lot more sense to me for it to be USB private. Probably I can check which task is pending when I queue them and store that in a separate variable. Still I need a way to remove a task from the queue, which becomes very slow due to the fact that STAILQ() is used. To my mind, there's primary operations, and secondary ones. A secondary operation can be built from the primary ones. That is right, if there is a way to remove a task from a queue without draining. It reads to me that, if there was a taskqueue_cancel(9) (and there is in Jeff's OFED branch) then you could build the functionality you want (and maybe you don't need cancel, even). While there is sometimes an argument for making secondary operations available in a library, in this case you need extra data which most other taskqueue consumers do not. That would break the KBI. That is another argument in favor of keeping the implementation private to USB. The only reason I want to break the KBI is because it is slow to remove a task from the taskqueue using STAILQ's when you don't know the previous task- element in the queue. --HPS ___ freebsd-usb@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/freebsd-usb To unsubscribe, send any mail to freebsd-usb-unsubscr...@freebsd.org
Re: [RFC] Outline of USB process integration in the kernel taskqueue system
On Thursday, November 04, 2010 9:55:09 am Matthew Fleming wrote: On Mon, Nov 1, 2010 at 1:15 PM, Hans Petter Selasky hsela...@c2i.net wrote: On Monday 01 November 2010 21:07:29 Matthew Fleming wrote: On Mon, Nov 1, 2010 at 12:54 PM, Hans Petter Selasky hsela...@c2i.net wrote: Hi! I've wrapped up an outline patch for what needs to be done to integrate the USB process framework into the kernel taskqueue system in a more direct way that to wrap it. The limitation of the existing taskqueue system is that it only guarantees execution at a given priority level. USB requires more. USB also requires a guarantee that the last task queued task also gets executed last. This is for example so that a deferred USB detach event does not happen before any pending deferred I/O for example in case of multiple occurring events. Mostly this new feature is targeted for GPIO-alike system using slow busses like the USB. Typical use case: 2 tasks to program GPIO on. 2 tasks to program GPIO off. Example: a) taskqueue_enqueue_odd(sc-sc_taskqueue, sc-sc_task_on[0], sc- sc_task_on[1]); b) taskqueue_enqueue_odd(sc-sc_taskqueue, sc-sc_task_off[0], sc- sc_task_off[1]); No matter how the call ordering of code-line a) and b), we are always guaranteed that the last queued state on or off is reached before the head of the taskqueue empties. In lack of a better name, the new function was called taskqueue_enqueue_odd [some people obviously think that USB processes are odd, but not taskqueues :-)] I'd like to make sure I understand the USB requirements. (1) does USB need the task priority field? Many taskqueue(9) consumers do not. No, USB does not need a task priority field, but a sequence field associated with the task and task queue head to figure out which task was queued first without having to scan all the tasks queued. (2) if there was a working taskqueue_remove(9) that removed the task if pending or returned error if the task was currently running, would that be sufficient to implement the required USB functionality? (assuming that taskqueue_enqueue(9) put all tasks with equal priority in order of queueing). No, not completely. See comment above. I also need information about which task was queued first, or else I have to keep this information separately, which again, confuse people. The more layers the more confusion? I don't follow why keeping the information about which task was queued first privately rather than having taskqueue(9) maintain it is confusing. So far, USB seems to be the only taskqueue consumer which needs this information, so it makes a lot more sense to me for it to be USB private. To my mind, there's primary operations, and secondary ones. A secondary operation can be built from the primary ones. It reads to me that, if there was a taskqueue_cancel(9) (and there is in Jeff's OFED branch) then you could build the functionality you want (and maybe you don't need cancel, even). While there is sometimes an argument for making secondary operations available in a library, in this case you need extra data which most other taskqueue consumers do not. That would break the KBI. That is another argument in favor of keeping the implementation private to USB. My sense is that I certainly agree. The fact that you can't think of a good name for the operation certainly indicates that it is not generic. Unfortunately, I don't really understand the problem that is being solved. I do agree that due to the 'pending' feature and automatic-coalescing of task enqueue operations, taskqueues do not lend themselves to a barrier operation. -- John Baldwin ___ freebsd-usb@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/freebsd-usb To unsubscribe, send any mail to freebsd-usb-unsubscr...@freebsd.org
Re: [RFC] Outline of USB process integration in the kernel taskqueue system
On Thursday 04 November 2010 15:29:51 John Baldwin wrote: (and there is in Jeff's OFED branch) Is there a link to this branch? I would certainly have a look at his work and re-base my patch. --HPS ___ freebsd-usb@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/freebsd-usb To unsubscribe, send any mail to freebsd-usb-unsubscr...@freebsd.org
Re: [RFC] Outline of USB process integration in the kernel taskqueue system
On Thu, Nov 4, 2010 at 11:41 AM, Hans Petter Selasky hsela...@c2i.net wrote: On Thursday 04 November 2010 15:29:51 John Baldwin wrote: (and there is in Jeff's OFED branch) Is there a link to this branch? I would certainly have a look at his work and re-base my patch. It's on svn.freebsd.org: http://svn.freebsd.org/viewvc/base/projects/ofed/head/sys/kern/subr_taskqueue.c?view=log http://svn.freebsd.org/viewvc/base?view=revisionrevision=209422 For the purpose of speed, I'm not opposed to breaking the KBI by using a doubly-linked TAILQ, but I don't think the difference will matter all that often (perhaps I'm wrong and some taskqueues have dozens of pending tasks?) Thanks, matthew ___ freebsd-usb@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/freebsd-usb To unsubscribe, send any mail to freebsd-usb-unsubscr...@freebsd.org
Re: [RFC] Outline of USB process integration in the kernel taskqueue system
On Thursday 04 November 2010 20:01:57 Matthew Fleming wrote: For the purpose of speed, I'm not opposed to breaking the KBI by using a doubly-linked TAILQ, but I don't think the difference will matter all that often (perhaps I'm wrong and some taskqueues have dozens of pending tasks?) Hi, In my case we are talking about 10-15 tasks at maximum. But still (10*9) / 2 = 45 iterations is much more than 2 steps to do the unlink. Anyway. I will have a look at your work and suggest a new patch for my needs. --HPS ___ freebsd-usb@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/freebsd-usb To unsubscribe, send any mail to freebsd-usb-unsubscr...@freebsd.org
Re: [RFC] Outline of USB process integration in the kernel taskqueue system
On Thursday 04 November 2010 20:01:57 Matthew Fleming wrote: On Thu, Nov 4, 2010 at 11:41 AM, Hans Petter Selasky hsela...@c2i.net wrote: On Thursday 04 November 2010 15:29:51 John Baldwin wrote: (and there is in Jeff's OFED branch) Is there a link to this branch? I would certainly have a look at his work and re-base my patch. It's on svn.freebsd.org: http://svn.freebsd.org/viewvc/base/projects/ofed/head/sys/kern/subr_taskque ue.c?view=log http://svn.freebsd.org/viewvc/base?view=revisionrevision=209422 For the purpose of speed, I'm not opposed to breaking the KBI by using a doubly-linked TAILQ, but I don't think the difference will matter all that often (perhaps I'm wrong and some taskqueues have dozens of pending tasks?) Thanks, matthew At first look I see that I need a non-blocking version of: taskqueue_cancel( At the point in the code where these functions are called I cannot block. Is this impossible to implement? --HPS ___ freebsd-usb@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/freebsd-usb To unsubscribe, send any mail to freebsd-usb-unsubscr...@freebsd.org
Re: [RFC] Outline of USB process integration in the kernel taskqueue system
On Thu, Nov 4, 2010 at 12:11 PM, Hans Petter Selasky hsela...@c2i.net wrote: On Thursday 04 November 2010 20:01:57 Matthew Fleming wrote: On Thu, Nov 4, 2010 at 11:41 AM, Hans Petter Selasky hsela...@c2i.net wrote: On Thursday 04 November 2010 15:29:51 John Baldwin wrote: (and there is in Jeff's OFED branch) Is there a link to this branch? I would certainly have a look at his work and re-base my patch. It's on svn.freebsd.org: http://svn.freebsd.org/viewvc/base/projects/ofed/head/sys/kern/subr_taskque ue.c?view=log http://svn.freebsd.org/viewvc/base?view=revisionrevision=209422 For the purpose of speed, I'm not opposed to breaking the KBI by using a doubly-linked TAILQ, but I don't think the difference will matter all that often (perhaps I'm wrong and some taskqueues have dozens of pending tasks?) Thanks, matthew At first look I see that I need a non-blocking version of: taskqueue_cancel( At the point in the code where these functions are called I cannot block. Is this impossible to implement? It depends on whether the queue uses a MTX_SPIN or MTX_DEF. It is not possible to determine whether a task is running without taking the taskqueue lock. And it is certainly impossible to dequeue a task without the lock that was used to enqueue it. However, a variant that dequeued if the task was still pending, and returned failure otherwise (rather than sleeping) is definitely possible. Thanks, matthew ___ freebsd-usb@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/freebsd-usb To unsubscribe, send any mail to freebsd-usb-unsubscr...@freebsd.org
Re: [RFC] Outline of USB process integration in the kernel taskqueue system
On Thursday 04 November 2010 21:11:38 Matthew Fleming wrote: On Thu, Nov 4, 2010 at 12:11 PM, Hans Petter Selasky hsela...@c2i.net wrote: On Thursday 04 November 2010 20:01:57 Matthew Fleming wrote: On Thu, Nov 4, 2010 at 11:41 AM, Hans Petter Selasky hsela...@c2i.net wrote: On Thursday 04 November 2010 15:29:51 John Baldwin wrote: (and there is in Jeff's OFED branch) Is there a link to this branch? I would certainly have a look at his work and re-base my patch. It's on svn.freebsd.org: http://svn.freebsd.org/viewvc/base/projects/ofed/head/sys/kern/subr_task que ue.c?view=log http://svn.freebsd.org/viewvc/base?view=revisionrevision=209422 For the purpose of speed, I'm not opposed to breaking the KBI by using a doubly-linked TAILQ, but I don't think the difference will matter all that often (perhaps I'm wrong and some taskqueues have dozens of pending tasks?) Thanks, matthew At first look I see that I need a non-blocking version of: taskqueue_cancel( At the point in the code where these functions are called I cannot block. Is this impossible to implement? It depends on whether the queue uses a MTX_SPIN or MTX_DEF. It is not possible to determine whether a task is running without taking the taskqueue lock. And it is certainly impossible to dequeue a task without the lock that was used to enqueue it. However, a variant that dequeued if the task was still pending, and returned failure otherwise (rather than sleeping) is definitely possible. I think that if a task is currently executing, then there should be a drain method for that. I.E. two methods: One to stop and one to cancel/drain. Can you implement this? --HPS ___ freebsd-usb@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/freebsd-usb To unsubscribe, send any mail to freebsd-usb-unsubscr...@freebsd.org
Re: [RFC] Outline of USB process integration in the kernel taskqueue system
On Thursday, November 04, 2010 4:15:16 pm Hans Petter Selasky wrote: On Thursday 04 November 2010 21:11:38 Matthew Fleming wrote: On Thu, Nov 4, 2010 at 12:11 PM, Hans Petter Selasky hsela...@c2i.net wrote: On Thursday 04 November 2010 20:01:57 Matthew Fleming wrote: On Thu, Nov 4, 2010 at 11:41 AM, Hans Petter Selasky hsela...@c2i.net wrote: On Thursday 04 November 2010 15:29:51 John Baldwin wrote: (and there is in Jeff's OFED branch) Is there a link to this branch? I would certainly have a look at his work and re-base my patch. It's on svn.freebsd.org: http://svn.freebsd.org/viewvc/base/projects/ofed/head/sys/kern/subr_task que ue.c?view=log http://svn.freebsd.org/viewvc/base?view=revisionrevision=209422 For the purpose of speed, I'm not opposed to breaking the KBI by using a doubly-linked TAILQ, but I don't think the difference will matter all that often (perhaps I'm wrong and some taskqueues have dozens of pending tasks?) Thanks, matthew At first look I see that I need a non-blocking version of: taskqueue_cancel( At the point in the code where these functions are called I cannot block. Is this impossible to implement? It depends on whether the queue uses a MTX_SPIN or MTX_DEF. It is not possible to determine whether a task is running without taking the taskqueue lock. And it is certainly impossible to dequeue a task without the lock that was used to enqueue it. However, a variant that dequeued if the task was still pending, and returned failure otherwise (rather than sleeping) is definitely possible. I think that if a task is currently executing, then there should be a drain method for that. I.E. two methods: One to stop and one to cancel/drain. Can you implement this? I agree, this would also be consistent with the callout_*() API if you had both stop() and drain() methods. -- John Baldwin ___ freebsd-usb@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/freebsd-usb To unsubscribe, send any mail to freebsd-usb-unsubscr...@freebsd.org
Re: [RFC] Outline of USB process integration in the kernel taskqueue system
On Thu, Nov 4, 2010 at 2:22 PM, John Baldwin j...@freebsd.org wrote: On Thursday, November 04, 2010 4:15:16 pm Hans Petter Selasky wrote: I think that if a task is currently executing, then there should be a drain method for that. I.E. two methods: One to stop and one to cancel/drain. Can you implement this? I agree, this would also be consistent with the callout_*() API if you had both stop() and drain() methods. Here's my proposed code. Note that this builds but is not yet tested. Implement a taskqueue_cancel(9), to cancel a task from a queue. Requested by: hps Original code: jeff MFC after: 1 week http://people.freebsd.org/~mdf/bsd-taskqueue-cancel.diff Thanks, matthew ___ freebsd-usb@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/freebsd-usb To unsubscribe, send any mail to freebsd-usb-unsubscr...@freebsd.org
Re: [RFC] Outline of USB process integration in the kernel taskqueue system
On Monday 01 November 2010 22:14:49 John Baldwin wrote: On Monday, November 01, 2010 3:54:59 pm Hans Petter Selasky wrote: Hi! I've wrapped up an outline patch for what needs to be done to integrate the USB process framework into the kernel taskqueue system in a more direct way that to wrap it. The limitation of the existing taskqueue system is that it only guarantees execution at a given priority level. USB requires more. USB also requires a guarantee that the last task queued task also gets executed last. This is for example so that a deferred USB detach event does not happen before any pending deferred I/O for example in case of multiple occurring events. Mostly this new feature is targeted for GPIO-alike system using slow busses like the USB. Typical use case: 2 tasks to program GPIO on. 2 tasks to program GPIO off. Example: a) taskqueue_enqueue_odd(sc-sc_taskqueue, sc-sc_task_on[0], sc- sc_task_on[1]); b) taskqueue_enqueue_odd(sc-sc_taskqueue, sc-sc_task_off[0], sc- sc_task_off[1]); No matter how the call ordering of code-line a) and b), we are always guaranteed that the last queued state on or off is reached before the head of the taskqueue empties. In lack of a better name, the new function was called taskqueue_enqueue_odd [some people obviously think that USB processes are odd, but not taskqueues :-)] It feels like this should be something you could manage with a state machine internal to USB rather than forcing that state into the taskqueue code itself. Hi John, No, this is not possible without keeping my own queue, which I want to avoid. By state-machine you mean remembering the last state as a separate variable and checking that in the task-callback, right? Yes, I do that in addition to the new queuing mechanism. A task barrier does not solve my problem. The barrier in my case is always last in the queue. I need to pull out previously queued tasks and put them last. That is currently not supported. I do this because I don't want to have a FIFO signalling model, and a neither want the pure taskqueue, which only ensures execution, not order of execution when at the same priority. Another issue: Won't the barrier model lead to blocking the caller once the task in question is being issued the second time? --HPS If you wanted a simple barrier task (where a barrier task is always queued at the tail of the list and all subsequent tasks are queued after the barrier task) then I would be fine with adding that. You could manage this without having to alter the task KBI by having the taskqueue maintain a separate pointer to the current barrier task and always enqueue entries after that task (the barrier would be NULL before a barrier is queued, and set to NULL when a barrier executes). I think this sort of semantic is a bit simpler and also used in other parts of the tree (e.g. in bio queues). ___ freebsd-usb@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/freebsd-usb To unsubscribe, send any mail to freebsd-usb-unsubscr...@freebsd.org
[RFC] Outline of USB process integration in the kernel taskqueue system
Hi!
I've wrapped up an outline patch for what needs to be done to integrate the
USB process framework into the kernel taskqueue system in a more direct way
that to wrap it.
The limitation of the existing taskqueue system is that it only guarantees
execution at a given priority level. USB requires more. USB also requires a
guarantee that the last task queued task also gets executed last. This is for
example so that a deferred USB detach event does not happen before any pending
deferred I/O for example in case of multiple occurring events.
Mostly this new feature is targeted for GPIO-alike system using slow busses
like the USB. Typical use case:
2 tasks to program GPIO on.
2 tasks to program GPIO off.
Example:
a) taskqueue_enqueue_odd(sc-sc_taskqueue, sc-sc_task_on[0], sc-
sc_task_on[1]);
b) taskqueue_enqueue_odd(sc-sc_taskqueue, sc-sc_task_off[0], sc-
sc_task_off[1]);
No matter how the call ordering of code-line a) and b), we are always
guaranteed that the last queued state on or off is reached before the head
of the taskqueue empties.
In lack of a better name, the new function was called taskqueue_enqueue_odd
[some people obviously think that USB processes are odd, but not taskqueues
:-)]
Manpage:
.Ft int
.Fn taskqueue_enqueue_odd struct taskqueue *queue struct task *t0 struct
task *t1
..
The function
.Fn taskqueue_enqueue_odd
should be used if it is important that the last queued task is also
executed last in the task queue at the given priority level. This
function requires two valid task pointers. Depending on which of the
tasks passed are queued at the calling moment, the last queued task of
the two will get dequeued and put last in the task queue, while not
touching the first queued task. If no tasks are queued at the calling
moment, this function behaves like
.Fn taskqueue_enqueue .
This function returns zero if the first task was queued last, one if
the second task was queued last.
Preliminary patch - see e-mail attachment.
Comments are welcome!
--HPS
More docs: Also see talk about the new USB stack in FreeBSD on Youtube.
=== share/man/man9/taskqueue.9
==
--- share/man/man9/taskqueue.9 (revision 214211)
+++ share/man/man9/taskqueue.9 (local)
@@ -46,11 +46,15 @@
typedef void (*taskqueue_enqueue_fn)(void *context);
struct task {
- STAILQ_ENTRY(task) ta_link;/* link for queue */
- u_short ta_pending; /* count times queued */
- u_short ta_priority;/* priority of task in queue */
- task_fn_t ta_func;/* task handler */
- void*ta_context;/* argument for handler */
+ TAILQ_ENTRY(task) ta_link; /* link for queue */
+#defineTASKQUEUE_SEQUENCE_MAX 255
+ u_char ta_sequence;/* sequence number */
+#defineTASKQUEUE_PENDING_MAX 255
+ u_char ta_pending; /* count times queued */
+#defineTASKQUEUE_PRIORITY_MAX 65535U
+ u_short ta_priority;/* priority of task in queue */
+ task_fn_t *ta_func; /* task handler */
+ void*ta_context;/* argument for handler */
};
.Ed
.Ft struct taskqueue *
@@ -62,6 +66,8 @@
.Ft int
.Fn taskqueue_enqueue struct taskqueue *queue struct task *task
.Ft int
+.Fn taskqueue_enqueue_odd struct taskqueue *queue struct task *t0 struct
task *t1
+.Ft int
.Fn taskqueue_enqueue_fast struct taskqueue *queue struct task *task
.Ft void
.Fn taskqueue_drain struct taskqueue *queue struct task *task
@@ -134,6 +140,19 @@
if the queue is being freed.
.Pp
The function
+.Fn taskqueue_enqueue_odd
+should be used if it is important that the last queued task is also
+executed last in the task queue at the given priority level. This
+function requires two valid task pointers. Depending on which of the
+tasks passed are queued at the calling moment, the last queued task of
+the two will get dequeued and put last in the task queue, while not
+touching the first queued task. If no tasks are queued at the calling
+moment, this function behaves like
+.Fn taskqueue_enqueue .
+This function returns zero if the first task was queued last, one if
+the second task was queued last.
+.Pp
+The function
.Fn taskqueue_enqueue_fast
should be used in place of
.Fn taskqueue_enqueue
=== sys/sys/_task.h
==
--- sys/sys/_task.h (revision 214433)
+++ sys/sys/_task.h (local)
@@ -44,9 +44,13 @@
typedef void task_fn_t(void *context, int pending);
struct task {
- STAILQ_ENTRY(task) ta_link; /* (q) link for queue */
- u_short ta_pending; /* (q) count times queued */
- u_short ta_priority;/* (c) Priority */
+ TAILQ_ENTRY(task) ta_link; /* (q) link for queue */
+#defineTASKQUEUE_SEQUENCE_MAX 255U
+ u_char ta_sequence;
Re: [RFC] Outline of USB process integration in the kernel taskqueue system
On Monday 01 November 2010 21:07:29 Matthew Fleming wrote: On Mon, Nov 1, 2010 at 12:54 PM, Hans Petter Selasky hsela...@c2i.net wrote: Hi! I've wrapped up an outline patch for what needs to be done to integrate the USB process framework into the kernel taskqueue system in a more direct way that to wrap it. The limitation of the existing taskqueue system is that it only guarantees execution at a given priority level. USB requires more. USB also requires a guarantee that the last task queued task also gets executed last. This is for example so that a deferred USB detach event does not happen before any pending deferred I/O for example in case of multiple occurring events. Mostly this new feature is targeted for GPIO-alike system using slow busses like the USB. Typical use case: 2 tasks to program GPIO on. 2 tasks to program GPIO off. Example: a) taskqueue_enqueue_odd(sc-sc_taskqueue, sc-sc_task_on[0], sc- sc_task_on[1]); b) taskqueue_enqueue_odd(sc-sc_taskqueue, sc-sc_task_off[0], sc- sc_task_off[1]); No matter how the call ordering of code-line a) and b), we are always guaranteed that the last queued state on or off is reached before the head of the taskqueue empties. In lack of a better name, the new function was called taskqueue_enqueue_odd [some people obviously think that USB processes are odd, but not taskqueues :-)] I'd like to make sure I understand the USB requirements. (1) does USB need the task priority field? Many taskqueue(9) consumers do not. No, USB does not need a task priority field, but a sequence field associated with the task and task queue head to figure out which task was queued first without having to scan all the tasks queued. (2) if there was a working taskqueue_remove(9) that removed the task if pending or returned error if the task was currently running, would that be sufficient to implement the required USB functionality? (assuming that taskqueue_enqueue(9) put all tasks with equal priority in order of queueing). No, not completely. See comment above. I also need information about which task was queued first, or else I have to keep this information separately, which again, confuse people. The more layers the more confusion? --HPS ___ freebsd-usb@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/freebsd-usb To unsubscribe, send any mail to freebsd-usb-unsubscr...@freebsd.org
Re: [RFC] Outline of USB process integration in the kernel taskqueue system
On Mon, Nov 1, 2010 at 12:54 PM, Hans Petter Selasky hsela...@c2i.net wrote:
Hi!
I've wrapped up an outline patch for what needs to be done to integrate the
USB process framework into the kernel taskqueue system in a more direct way
that to wrap it.
The limitation of the existing taskqueue system is that it only guarantees
execution at a given priority level. USB requires more. USB also requires a
guarantee that the last task queued task also gets executed last. This is for
example so that a deferred USB detach event does not happen before any pending
deferred I/O for example in case of multiple occurring events.
Mostly this new feature is targeted for GPIO-alike system using slow busses
like the USB. Typical use case:
2 tasks to program GPIO on.
2 tasks to program GPIO off.
Example:
a) taskqueue_enqueue_odd(sc-sc_taskqueue, sc-sc_task_on[0], sc-
sc_task_on[1]);
b) taskqueue_enqueue_odd(sc-sc_taskqueue, sc-sc_task_off[0], sc-
sc_task_off[1]);
No matter how the call ordering of code-line a) and b), we are always
guaranteed that the last queued state on or off is reached before the head
of the taskqueue empties.
In lack of a better name, the new function was called taskqueue_enqueue_odd
[some people obviously think that USB processes are odd, but not taskqueues
:-)]
I'd like to make sure I understand the USB requirements.
(1) does USB need the task priority field? Many taskqueue(9) consumers do not.
(2) if there was a working taskqueue_remove(9) that removed the task
if pending or returned error if the task was currently running, would
that be sufficient to implement the required USB functionality?
(assuming that taskqueue_enqueue(9) put all tasks with equal priority
in order of queueing).
Thanks,
matthew
Manpage:
.Ft int
.Fn taskqueue_enqueue_odd struct taskqueue *queue struct task *t0 struct
task *t1
..
The function
.Fn taskqueue_enqueue_odd
should be used if it is important that the last queued task is also
executed last in the task queue at the given priority level. This
function requires two valid task pointers. Depending on which of the
tasks passed are queued at the calling moment, the last queued task of
the two will get dequeued and put last in the task queue, while not
touching the first queued task. If no tasks are queued at the calling
moment, this function behaves like
.Fn taskqueue_enqueue .
This function returns zero if the first task was queued last, one if
the second task was queued last.
Preliminary patch - see e-mail attachment.
Comments are welcome!
--HPS
More docs: Also see talk about the new USB stack in FreeBSD on Youtube.
=== share/man/man9/taskqueue.9
==
--- share/man/man9/taskqueue.9 (revision 214211)
+++ share/man/man9/taskqueue.9 (local)
@@ -46,11 +46,15 @@
typedef void (*taskqueue_enqueue_fn)(void *context);
struct task {
- STAILQ_ENTRY(task) ta_link; /* link for queue */
- u_short ta_pending; /* count times queued */
- u_short ta_priority; /* priority of task in queue
*/
- task_fn_t ta_func; /* task handler */
- void *ta_context; /* argument for handler */
+ TAILQ_ENTRY(task) ta_link; /* link for queue */
+#define TASKQUEUE_SEQUENCE_MAX 255
+ u_char ta_sequence; /* sequence number */
+#define TASKQUEUE_PENDING_MAX 255
+ u_char ta_pending; /* count times queued */
+#define TASKQUEUE_PRIORITY_MAX 65535U
+ u_short ta_priority; /* priority of task in queue */
+ task_fn_t *ta_func; /* task handler */
+ void *ta_context; /* argument for handler */
};
.Ed
.Ft struct taskqueue *
@@ -62,6 +66,8 @@
.Ft int
.Fn taskqueue_enqueue struct taskqueue *queue struct task *task
.Ft int
+.Fn taskqueue_enqueue_odd struct taskqueue *queue struct task *t0
struct task *t1
+.Ft int
.Fn taskqueue_enqueue_fast struct taskqueue *queue struct task *task
.Ft void
.Fn taskqueue_drain struct taskqueue *queue struct task *task
@@ -134,6 +140,19 @@
if the queue is being freed.
.Pp
The function
+.Fn taskqueue_enqueue_odd
+should be used if it is important that the last queued task is also
+executed last in the task queue at the given priority level. This
+function requires two valid task pointers. Depending on which of the
+tasks passed are queued at the calling moment, the last queued task of
+the two will get dequeued and put last in the task queue, while not
+touching the first queued task. If no tasks are queued at the calling
+moment, this function behaves like
+.Fn taskqueue_enqueue .
+This function returns zero if the first task was queued last, one if
+the second task was queued last.
+.Pp
+The function
.Fn taskqueue_enqueue_fast
should be used in place
Re: [RFC] Outline of USB process integration in the kernel taskqueue system
On Monday, November 01, 2010 3:54:59 pm Hans Petter Selasky wrote: Hi! I've wrapped up an outline patch for what needs to be done to integrate the USB process framework into the kernel taskqueue system in a more direct way that to wrap it. The limitation of the existing taskqueue system is that it only guarantees execution at a given priority level. USB requires more. USB also requires a guarantee that the last task queued task also gets executed last. This is for example so that a deferred USB detach event does not happen before any pending deferred I/O for example in case of multiple occurring events. Mostly this new feature is targeted for GPIO-alike system using slow busses like the USB. Typical use case: 2 tasks to program GPIO on. 2 tasks to program GPIO off. Example: a) taskqueue_enqueue_odd(sc-sc_taskqueue, sc-sc_task_on[0], sc- sc_task_on[1]); b) taskqueue_enqueue_odd(sc-sc_taskqueue, sc-sc_task_off[0], sc- sc_task_off[1]); No matter how the call ordering of code-line a) and b), we are always guaranteed that the last queued state on or off is reached before the head of the taskqueue empties. In lack of a better name, the new function was called taskqueue_enqueue_odd [some people obviously think that USB processes are odd, but not taskqueues :-)] It feels like this should be something you could manage with a state machine internal to USB rather than forcing that state into the taskqueue code itself. If you wanted a simple barrier task (where a barrier task is always queued at the tail of the list and all subsequent tasks are queued after the barrier task) then I would be fine with adding that. You could manage this without having to alter the task KBI by having the taskqueue maintain a separate pointer to the current barrier task and always enqueue entries after that task (the barrier would be NULL before a barrier is queued, and set to NULL when a barrier executes). I think this sort of semantic is a bit simpler and also used in other parts of the tree (e.g. in bio queues). -- John Baldwin ___ freebsd-usb@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/freebsd-usb To unsubscribe, send any mail to freebsd-usb-unsubscr...@freebsd.org
