From owner-freebsd-current@FreeBSD.ORG Fri Nov 12 16:38:40 2010 Return-Path: Delivered-To: freebsd-current@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id 5CCF0106564A for ; Fri, 12 Nov 2010 16:38:40 +0000 (UTC) (envelope-from mdf356@gmail.com) Received: from mail-gw0-f54.google.com (mail-gw0-f54.google.com [74.125.83.54]) by mx1.freebsd.org (Postfix) with ESMTP id 116E48FC08 for ; Fri, 12 Nov 2010 16:38:39 +0000 (UTC) Received: by gwj20 with SMTP id 20so1584352gwj.13 for ; Fri, 12 Nov 2010 08:38:39 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=gamma; h=domainkey-signature:mime-version:received:sender:received :in-reply-to:references:date:x-google-sender-auth:message-id:subject :from:to:cc:content-type:content-transfer-encoding; bh=2QYyZvlkiIbmEfEuzWDjp6GhSSUP90Tt3k6d8hNnjhI=; b=swOZU7jr1bPRQzGqcL88jtftSksZbAeacPMmoB40ZdqnrLI+7vQs4cH7Xx9zz12/vf MCAbWU/2NV6J5vWX9MOV4lHNFqNtWZ/55E/ejKW44CaXYL5cimIift/apD1fEwbDCMv7 pwYQSOQwc8xhP/edVdc+jBtEb+4wXJrCYnNX4= DomainKey-Signature: a=rsa-sha1; c=nofws; d=gmail.com; s=gamma; h=mime-version:sender:in-reply-to:references:date :x-google-sender-auth:message-id:subject:from:to:cc:content-type :content-transfer-encoding; b=tjpyZi6hOXf9gK89t31bpKv9IJBlq56aqcE77fiNNTZ586Ji1ozPKisXmAjpMS5crD 6ue5x8p6INsNKVmKgCqyXSkDlyd5vZKEcHsMI/6v04qLUQ3SUgd1y5KrnZHOgtXt0LE6 8i9pJdC4TSUQt+PjuPrt9NU2CE2aAv7QCdS9s= MIME-Version: 1.0 Received: by 10.231.156.139 with SMTP id x11mr2246825ibw.22.1289579918775; Fri, 12 Nov 2010 08:38:38 -0800 (PST) Sender: mdf356@gmail.com Received: by 10.231.21.35 with HTTP; Fri, 12 Nov 2010 08:38:38 -0800 (PST) In-Reply-To: <201011121523.18044.hselasky@c2i.net> References: <06D5F9F6F655AD4C92E28B662F7F853E039E389A@seaxch09.desktop.isilon.com> <201011120956.04501.hselasky@c2i.net> <201011121523.18044.hselasky@c2i.net> Date: Fri, 12 Nov 2010 08:38:38 -0800 X-Google-Sender-Auth: TneMxbln8ifCI6N6_2JAkM2Zzi0 Message-ID: From: mdf@FreeBSD.org To: Hans Petter Selasky Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: quoted-printable Cc: freebsd-current@freebsd.org Subject: Re: sleep bug in taskqueue(9) X-BeenThere: freebsd-current@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: Discussions about the use of FreeBSD-current List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 12 Nov 2010 16:38:40 -0000 On Fri, Nov 12, 2010 at 6:23 AM, Hans Petter Selasky 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 > 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