From owner-freebsd-current@FreeBSD.ORG Mon Nov 8 20:37:08 2010 Return-Path: Delivered-To: freebsd-current@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id 473841065672; Mon, 8 Nov 2010 20:37:08 +0000 (UTC) (envelope-from mdf356@gmail.com) Received: from mail-iw0-f182.google.com (mail-iw0-f182.google.com [209.85.214.182]) by mx1.freebsd.org (Postfix) with ESMTP id C93C28FC24; Mon, 8 Nov 2010 20:37:07 +0000 (UTC) Received: by iwn39 with SMTP id 39so6506949iwn.13 for ; Mon, 08 Nov 2010 12:37:07 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=gamma; h=domainkey-signature:mime-version:received:received:in-reply-to :references:date:message-id:subject:from:to:cc:content-type :content-transfer-encoding; bh=I155nDbwEt9h3lLLqiGD6cPgOWFcZiWcgJYvp7RPhsU=; b=RkXnSGymT0weEJez7fPVi2VchCnFcI7CbVq/eXn+fjhtm2TwNQNJE9LwYX9AmxTk8V JpTBvsqdofAU37j2eysXW9lEXoslGeHilZ5TWAlcc1XIGL/7ehMbaA1vA9hl4daEnMsm gqWgXt+U+Tzk4J+3Lb8+U36e1HdDWdUuMpIwQ= DomainKey-Signature: a=rsa-sha1; c=nofws; d=gmail.com; s=gamma; h=mime-version:in-reply-to:references:date:message-id:subject:from:to :cc:content-type:content-transfer-encoding; b=W4z+VGmu0W515mdZi+6jYRIUdfkyVjgtTyHgm1out//T3IZkL/su7ShIJSFLDZuyMT YPPt9d/4Dmima9ji7r6BrcJ+HwADcYlx4f0xMgTyLdRFmB5+8rjWjM/oQol5ePxIJwu4 ck22MS1URXyF8oNtwYSSNJ5hIgB/rN1ic9QE0= MIME-Version: 1.0 Received: by 10.231.192.73 with SMTP id dp9mr4536957ibb.72.1289248626670; Mon, 08 Nov 2010 12:37:06 -0800 (PST) Received: by 10.231.21.35 with HTTP; Mon, 8 Nov 2010 12:37:06 -0800 (PST) In-Reply-To: <201011081324.05514.jhb@freebsd.org> References: <201011012054.59551.hselasky@c2i.net> <201011081142.46175.jhb@freebsd.org> <201011081324.05514.jhb@freebsd.org> Date: Mon, 8 Nov 2010 12:37:06 -0800 Message-ID: From: Matthew Fleming To: John Baldwin Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: quoted-printable Cc: freebsd-usb@freebsd.org, Weongyo Jeong , freebsd-current@freebsd.org, freebsd-arch@freebsd.org, Hans Petter Selasky Subject: Re: [RFC] Outline of USB process integration in the kernel taskqueue system X-BeenThere: freebsd-current@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: Discussions about the use of FreeBSD-current List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 08 Nov 2010 20:37:08 -0000 On Mon, Nov 8, 2010 at 10:24 AM, John Baldwin wrote: > On Monday, November 08, 2010 11:46:58 am Matthew Fleming wrote: >> On Mon, Nov 8, 2010 at 8:42 AM, John Baldwin wrote: >> > On Monday, November 08, 2010 10:34:33 am Matthew Fleming wrote: >> >> On Mon, Nov 8, 2010 at 6:47 AM, John Baldwin 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 wrote: >> >> >> > Hi, >> >> >> > >> >> >> > On Saturday 06 November 2010 14:57:50 Matthew Fleming wrote: >> >> >> >> >> >> >> >> I think you're misunderstanding the existing taskqueue(9) imple= mentation. >> >> >> >> >> >> >> >> 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 t= he 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(&= 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 not >> >> >> > possible? If ta_pending > 0, shouldn't we also do a STAILQ_REMOV= E() ? >> >> >> >> >> >> Ah! =A0I see what you mean. >> >> >> >> >> >> I'm not quite sure what the best thing to do here is; I agree it w= ould >> >> >> be nice if taskqueue_cancel(9) dequeued the task, but I believe it >> >> >> also needs to indicate that the task is currently running. =A0I gu= ess >> >> >> the best thing would be to return the old pending count by referen= ce >> >> >> 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 shoul= d return >> >> > -EBUSY in that case. =A0That way code that uses 'cancel' followed b= y 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 caller= s >> >> 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() coul= d 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. > > 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" th= e > 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. =A0One difference is that callout_stop() > returns false if the callout is not pending (i.e. not on softclock). =A0R= ight > now taskqueue_cancel() returns 0 in that case (ta_pending =3D=3D 0), but = that is > probably fine as long as it is documented. =A0If you wanted to return EIN= VAL > instead, that would be fine, too. > > -- > John Baldwin >