From owner-freebsd-usb@FreeBSD.ORG Fri Nov 5 14:22:17 2010 Return-Path: Delivered-To: freebsd-usb@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id 56EDB106564A; Fri, 5 Nov 2010 14:22:17 +0000 (UTC) (envelope-from jhb@freebsd.org) Received: from cyrus.watson.org (cyrus.watson.org [65.122.17.42]) by mx1.freebsd.org (Postfix) with ESMTP id 0FA618FC15; Fri, 5 Nov 2010 14:22:17 +0000 (UTC) Received: from bigwig.baldwin.cx (66.111.2.69.static.nyinternet.net [66.111.2.69]) by cyrus.watson.org (Postfix) with ESMTPSA id 84A0246B37; Fri, 5 Nov 2010 10:22:16 -0400 (EDT) Received: from jhbbsd.localnet (smtp.hudson-trading.com [209.249.190.9]) by bigwig.baldwin.cx (Postfix) with ESMTPSA id 806658A009; Fri, 5 Nov 2010 10:22:15 -0400 (EDT) From: John Baldwin To: Matthew Fleming Date: Fri, 5 Nov 2010 10:18:28 -0400 User-Agent: KMail/1.13.5 (FreeBSD/7.3-CBSD-20100819; KDE/4.4.5; amd64; ; ) References: <201011012054.59551.hselasky@c2i.net> <201011050858.33568.jhb@freebsd.org> In-Reply-To: MIME-Version: 1.0 Content-Type: Text/Plain; charset="iso-8859-1" Content-Transfer-Encoding: 7bit Message-Id: <201011051018.28860.jhb@freebsd.org> X-Greylist: Sender succeeded SMTP AUTH, not delayed by milter-greylist-4.2.6 (bigwig.baldwin.cx); Fri, 05 Nov 2010 10:22:15 -0400 (EDT) X-Virus-Scanned: clamav-milter 0.96.3 at bigwig.baldwin.cx X-Virus-Status: Clean X-Spam-Status: No, score=-1.9 required=4.2 tests=BAYES_00 autolearn=ham version=3.3.1 X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on bigwig.baldwin.cx Cc: freebsd-usb@freebsd.org, freebsd-current@freebsd.org, freebsd-arch@freebsd.org, Weongyo Jeong Subject: Re: [RFC] Outline of USB process integration in the kernel taskqueue system X-BeenThere: freebsd-usb@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: FreeBSD support for USB List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 05 Nov 2010 14:22:17 -0000 On Friday, November 05, 2010 9:50:10 am Matthew Fleming wrote: > On Fri, Nov 5, 2010 at 5:58 AM, John Baldwin wrote: > > On Thursday, November 04, 2010 5:49:22 pm Matthew Fleming wrote: > >> On Thu, Nov 4, 2010 at 2:22 PM, John Baldwin 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