On Fri, Nov 5, 2010 at 7:18 AM, John Baldwin <jhb_at_freebsd.org> wrote: > On Friday, November 05, 2010 9:50:10 am Matthew Fleming wrote: >> On Fri, Nov 5, 2010 at 5:58 AM, John Baldwin <jhb_at_freebsd.org> wrote: >> > On Thursday, November 04, 2010 5:49:22 pm Matthew Fleming wrote: >> >> On Thu, Nov 4, 2010 at 2:22 PM, John Baldwin <jhb_at_freebsd.org> 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-cancel-a-task-from-a.patch I'll try to set up something to test it today too. Thanks, matthewReceived on Fri Nov 05 2010 - 16:15:03 UTC
This archive was generated by hypermail 2.4.0 : Wed May 19 2021 - 11:40:08 UTC