[RFC] Outline of USB process integration in the kernel taskqueue system

Matthew Fleming mdf356 at gmail.com
Fri Nov 5 18:13:10 UTC 2010


On Fri, Nov 5, 2010 at 10:36 AM, Hans Petter Selasky <hselasky at 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 <jhb at 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 <jhb at 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 <jhb at 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


More information about the freebsd-arch mailing list