Re: sleep bug in taskqueue(9)

On Fri, Nov 12, 2010 at 6:23 AM, Hans Petter Selasky <hselasky_at_c2i.net> wrote:
> On Friday 12 November 2010 15:18:46 mdf_at_freebsd.org wrote:
>> On Fri, Nov 12, 2010 at 12:56 AM, Hans Petter Selasky <hselasky_at_c2i.net>
> wrote:
>> > On Thursday 29 April 2010 01:59:58 Matthew Fleming wrote:
>> >> It looks to me like taskqueue_drain(taskqueue_thread, foo) will not
>> >> correctly detect whether or not a task is currently running.  The check
>> >> is against a field in the taskqueue struct, but for the taskqueue_thread
>> >> queue with more than one thread, multiple threads can simultaneously be
>> >> running a task, thus stomping over the tq_running field.
>> >>
>> >> I have not seen any problem with the code as-is in actual use, so this
>> >> is purely an inspection bug.
>> >>
>> >> The following patch should fix the problem.  Because it changes the size
>> >> of struct task I'm not sure if it would be suitable for MFC.
>> >
>> > 1) The u_char is going to leave a hole in that structure on ARM platforms
>> > for example.
>> >
>> > 2) The existing taskqueue implementation also has a missing check for the
>> > pending count wrapping to zero. I.E. it should stick at 0xFFFF and not
>> > wrap to 0.
>>
>> This commit mail is rather old, and this fix was incorrect, because
>> the task cannot be referenced after it has been run.  Some task
>> handlers will free the task as part of the handler.
>
> Ok, maybe the e-mail got stuck somewhere. Have you fixed the above mentioned
> issues in a newer patch?

If you look at the file history for subr_taskqueue.c:

http://svn.freebsd.org/viewvc/base/head/sys/kern/subr_taskqueue.c

You will see quite a few commits by me.  The most recent relating to
detecting if a task is running is being MFC'd today:


Revision 213813 - (view) (annotate) - [select for diffs]
Modified Wed Oct 13 22:59:04 2010 UTC (4 weeks, 1 day ago) by mdf
File length: 10831 byte(s)
Diff to previous 213739
Use a safer mechanism for determining if a task is currently running,
that does not rely on the lifetime of pointers being the same. This also
restores the task KBI.

Suggested by:	jhb
MFC after:	1 month

Thanks,
matthew