Date: Fri, 12 Nov 2010 08:38:38 -0800 From: mdf@FreeBSD.org To: Hans Petter Selasky <hselasky@c2i.net> Cc: freebsd-current@freebsd.org Subject: Re: sleep bug in taskqueue(9) Message-ID: <AANLkTimoKgWAPv0aUOcCO9NFEWByKSuDekkxppOwGcd5@mail.gmail.com> In-Reply-To: <201011121523.18044.hselasky@c2i.net> References: <06D5F9F6F655AD4C92E28B662F7F853E039E389A@seaxch09.desktop.isilon.com> <201011120956.04501.hselasky@c2i.net> <AANLkTinVN=RsMQSvKCKmPg0M9vTZXk5mKmRSRR%2BVSzhh@mail.gmail.com> <201011121523.18044.hselasky@c2i.net>
next in thread | previous in thread | raw e-mail | index | archive | help
On Fri, Nov 12, 2010 at 6:23 AM, Hans Petter Selasky <hselasky@c2i.net> wro= te: > On Friday 12 November 2010 15:18:46 mdf@freebsd.org wrote: >> On Fri, Nov 12, 2010 at 12:56 AM, Hans Petter Selasky <hselasky@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. =A0The c= heck >> >> is against a field in the taskqueue struct, but for the taskqueue_thr= ead >> >> 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 thi= s >> >> is purely an inspection bug. >> >> >> >> The following patch should fix the problem. =A0Because 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 platfo= rms >> > 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. =A0Some task >> handlers will free the task as part of the handler. > > Ok, maybe the e-mail got stuck somewhere. Have you fixed the above mentio= ned > 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
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?AANLkTimoKgWAPv0aUOcCO9NFEWByKSuDekkxppOwGcd5>