Date: Mon, 8 Nov 2010 07:34:33 -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, Andrew Thompson <thompsa@freebsd.org>, freebsd-arch@freebsd.org, Hans Petter Selasky <hselasky@c2i.net> Subject: Re: [RFC] Outline of USB process integration in the kernel taskqueue system Message-ID: <AANLkTinfsLB9QqvYZ6gyQL9YyPeVRx10et-oTpR%2B7f3X@mail.gmail.com> In-Reply-To: <201011080947.00550.jhb@freebsd.org> References: <201011012054.59551.hselasky@c2i.net> <201011061522.26533.hselasky@c2i.net> <AANLkTincy=wkJ5ZPMc7FK73kRnN8uakkO1Ld6W=9ntos@mail.gmail.com> <201011080947.00550.jhb@freebsd.org>
next in thread | previous in thread | raw e-mail | index | archive | help
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.net> w=
rote:
>> > Hi,
>> >
>> > On Saturday 06 November 2010 14:57:50 Matthew Fleming wrote:
>> >>
>> >> I think you're misunderstanding the existing taskqueue(9) implementat=
ion.
>> >>
>> >> As long as TQ_LOCK is held, the state of ta->ta_pending cannot change=
,
>> >> 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 pro=
blem.
>> >
>> > 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(&queue-=
>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 is no=
t
>> > 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 would
>> be nice if taskqueue_cancel(9) dequeued the task, but I believe it
>> also needs to indicate that the task is currently running. =A0I 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. =A0I think it should retu=
rn
> -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? 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
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?AANLkTinfsLB9QqvYZ6gyQL9YyPeVRx10et-oTpR%2B7f3X>