Date: Mon, 8 Nov 2010 09:04:54 -0800 From: Matthew Fleming <mdf356@gmail.com> To: John Baldwin <jhb@freebsd.org> Cc: freebsd-usb@freebsd.org, Weongyo Jeong <weongyo.jeong@gmail.com>, freebsd-current@freebsd.org, freebsd-arch@freebsd.org Subject: Re: [RFC] Outline of USB process integration in the kernel taskqueue system Message-ID: <AANLkTikocAqmG-xUdRkSxcKRAs13=Nmvyu5QSv0DTi73@mail.gmail.com> In-Reply-To: <AANLkTi=SJMF91%2BxtFuc_hG_K5VWjhCvRGcVuVaBfF173@mail.gmail.com> References: <201011012054.59551.hselasky@c2i.net> <201011080947.00550.jhb@freebsd.org> <AANLkTinfsLB9QqvYZ6gyQL9YyPeVRx10et-oTpR%2B7f3X@mail.gmail.com> <201011081142.46175.jhb@freebsd.org> <AANLkTi=SJMF91%2BxtFuc_hG_K5VWjhCvRGcVuVaBfF173@mail.gmail.com>
next in thread | previous in thread | raw e-mail | index | archive | help
On Mon, Nov 8, 2010 at 8:46 AM, Matthew Fleming <mdf356@gmail.com> wrote:
> On Mon, Nov 8, 2010 at 8:42 AM, John Baldwin <jhb@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 <jhb@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 <hselasky@c2i.ne=
t> wrote:
>>> >> > Hi,
>>> >> >
>>> >> > On Saturday 06 November 2010 14:57:50 Matthew Fleming wrote:
>>> >> >>
>>> >> >> I think you're misunderstanding the existing taskqueue(9) impleme=
ntation.
>>> >> >>
>>> >> >> As long as TQ_LOCK is held, the state of ta->ta_pending cannot ch=
ange,
>>> >> >> nor can the set of running tasks. =A0So 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:
>>> >> >
>>> >> > =A0> +int
>>> >> >> > +taskqueue_cancel(struct taskqueue *queue, struct task *task)
>>> >> >> > +{
>>> >> >> > + =A0 =A0 =A0 int rc;
>>> >> >> > +
>>> >> >> > + =A0 =A0 =A0 TQ_LOCK(queue);
>>> >> >> > + =A0 =A0 =A0 if (!task_is_running(queue, task)) {
>>> >> >> > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 if ((rc =3D task->ta_pending) > 0=
)
>>> >> >> > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 STAILQ_REMOVE(&qu=
eue->tq_queue, task, task,
>>> >> >> > ta_link); + =A0 =A0 =A0 =A0 =A0 =A0 =A0 task->ta_pending =3D 0;
>>> >> >> > + =A0 =A0 =A0 } else {
>>> >> >> > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 rc =3D -EBUSY;
>>> >> >
>>> >> > What happens in this case if ta_pending > 0. Are you saying this i=
s not
>>> >> > possible? If ta_pending > 0, shouldn't we also do a STAILQ_REMOVE(=
) ?
>>> >>
>>> >> Ah! =A0I see what you mean.
>>> >>
>>> >> I'm not quite sure what the best thing to do here is; I agree it wou=
ld
>>> >> be nice if taskqueue_cancel(9) dequeued the task, but I believe it
>>> >> also needs to indicate that the task is currently running. =A0I gues=
s
>>> >> 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. =A0I think it should =
return
>>> > -EBUSY in that case. =A0That 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? =A0In the case
>>> where a task is pending and is also currently running (admittedly a
>>> narrow window), how would we do this? =A0This is why I suggested
>>> returning the old ta_pending by reference. =A0This also allows callers
>>> who don't care about the old pending to pass NULL and ignore it.
>>
>> I would be fine with that then. =A0I wonder if taskqueue_cancel() could =
then
>> return a simple true/false. =A0False 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-pe=
nding-task-wi.patch
Thanks,
matthew
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?AANLkTikocAqmG-xUdRkSxcKRAs13=Nmvyu5QSv0DTi73>