From owner-freebsd-usb@FreeBSD.ORG Fri Nov 5 18:34:21 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 D16E0106566B; Fri, 5 Nov 2010 18:34:21 +0000 (UTC) (envelope-from hselasky@c2i.net) Received: from swip.net (mailfe06.swip.net [212.247.154.161]) by mx1.freebsd.org (Postfix) with ESMTP id C03C88FC16; Fri, 5 Nov 2010 18:34:20 +0000 (UTC) X-Cloudmark-Score: 0.000000 [] X-Cloudmark-Analysis: v=1.1 cv=5UXFHLfkiY5XrCDma5uYm2T9fyMGz6t0cyN4hLfZsqg= c=1 sm=1 a=FberXtVRn-wA:10 a=8nJEP1OIZ-IA:10 a=CL8lFSKtTFcA:10 a=i9M/sDlu2rpZ9XS819oYzg==:17 a=8kQB0OdkAAAA:8 a=6I5d2MoRAAAA:8 a=NQlS8b5mwfIYmOo8uroA:9 a=E_rvnGVBWsNcG74Lc3UA:7 a=2RIT3NFf099GxrwdX1yFkcDJl_4A:4 a=wPNLvfGTeEIA:10 a=9aOQ2cSd83gA:10 a=SV7veod9ZcQA:10 a=i9M/sDlu2rpZ9XS819oYzg==:117 Received: from [188.126.198.129] (account mc467741@c2i.net HELO laptop002.hselasky.homeunix.org) by mailfe06.swip.net (CommuniGate Pro SMTP 5.2.19) with ESMTPA id 44710663; Fri, 05 Nov 2010 19:34:19 +0100 From: Hans Petter Selasky To: freebsd-arch@freebsd.org Date: Fri, 5 Nov 2010 19:35:27 +0100 User-Agent: KMail/1.13.5 (FreeBSD/8.1-STABLE; KDE/4.4.5; amd64; ; ) References: <201011012054.59551.hselasky@c2i.net> <201011051836.39697.hselasky@c2i.net> In-Reply-To: X-Face: +~\`s("[*|O,="7?X@L.elg*F"OA\I/3%^p8g?ab%RN'( =?iso-8859-1?q?=3B=5FIjlA=3A=0A=09hGE=2E=2EEw?=, =?iso-8859-1?q?XAQ*o=23=5C/M=7ESC=3DS1-f9=7BEzRfT=27=7CHhll5Q=5Dha5Bt-s=7Co?= =?iso-8859-1?q?TlKMusi=3A1e=5BwJl=7Dkd=7DGR=0A=09Z0adGx-x=5F0zGbZj=27e?=(Y[(UNle~)8CQWXW@:DX+9)_YlB[tIccCPN$7/L' MIME-Version: 1.0 Content-Type: Text/Plain; charset="iso-8859-1" Content-Transfer-Encoding: 7bit Message-Id: <201011051935.27579.hselasky@c2i.net> Cc: freebsd-current@freebsd.org, Matthew Fleming , freebsd-usb@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 18:34:22 -0000 On Friday 05 November 2010 19:13:08 Matthew Fleming wrote: > On Fri, Nov 5, 2010 at 10:36 AM, Hans Petter Selasky wrote: > > On Friday 05 November 2010 18:15:01 Matthew Fleming wrote: > >> On Fri, Nov 5, 2010 at 7:18 AM, John Baldwin wrote: > >> > 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? > >> > >> New patch is at > >> http://people.freebsd.org/~mdf/0001-Implement-taskqueue_cancel-9-to-canc > >> el- a-task-from-a.patch > > > > I think the: > > > > + if (!task_is_running(queue, task)) { > > If it is running, it is dequeued from the the taskqueue, right? And while it is running it can be queued again, which your initial code didn't handle? --HPS