From owner-freebsd-arch@FreeBSD.ORG Sun Jan 10 01:09:50 2010 Return-Path: Delivered-To: freebsd-arch@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id A0FBF106568D; Sun, 10 Jan 2010 01:09:50 +0000 (UTC) (envelope-from asmrookie@gmail.com) Received: from mail-fx0-f227.google.com (mail-fx0-f227.google.com [209.85.220.227]) by mx1.freebsd.org (Postfix) with ESMTP id D7F938FC15; Sun, 10 Jan 2010 01:09:49 +0000 (UTC) Received: by fxm27 with SMTP id 27so5720415fxm.3 for ; Sat, 09 Jan 2010 17:09:44 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=gamma; h=domainkey-signature:mime-version:sender:received:date :x-google-sender-auth:message-id:subject:from:to:content-type; bh=HBAhMQU4OjwEd5hNy/YRwKWDD4m97B5kvV0mBe3cqFg=; b=WspFmjHnOsrzpzoThKL+/55BEtVK7pUP13B6EjaE7lg1Lj+ZfOJ7KQxIOj8c7D8YtT 78tUxY2nBFYxnWCzbFuEyPtt/yiqr4/gpMSZQCEPmBvury8H4qLsTp0JxzHtSzZZQUzc FKxQXOC8g2oRzBhiAcLGNNo5cOaz/KcmBehuE= DomainKey-Signature: a=rsa-sha1; c=nofws; d=gmail.com; s=gamma; h=mime-version:sender:date:x-google-sender-auth:message-id:subject :from:to:content-type; b=XmXZX7XQbGZxBNjbB1RHkPeLRwxHoyXrQzE2UT4xJJg78fm+iiXX/sqyXP43+jsahy LXsF+4pDniTgDhyGxt25fJTOhBC/EwRQRG/M1qr60PS5J1E7d8rbdk9A4ib54jAWEFyP FHNu6zfr/76LlMF+RndHMyWKTalkbcq4CazRk= MIME-Version: 1.0 Sender: asmrookie@gmail.com Received: by 10.223.57.66 with SMTP id b2mr4625771fah.33.1263085783975; Sat, 09 Jan 2010 17:09:43 -0800 (PST) Date: Sun, 10 Jan 2010 02:09:43 +0100 X-Google-Sender-Auth: 1b92f40240ce688b Message-ID: <3bbf2fe11001091709t228c48cft1c17af686e9e9c46@mail.gmail.com> From: Attilio Rao To: freebsd-arch@freebsd.org Content-Type: text/plain; charset=UTF-8 Subject: [PATCH] kthread_{suspend, resume, suspend_check} locking bugs X-BeenThere: freebsd-arch@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: Discussion related to FreeBSD architecture List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sun, 10 Jan 2010 01:09:50 -0000 I think that routines kthread_suspend(), kthread_resume() and kthread_suspend_check() are not adeguately SMP protected. That is because, in particular, the critical path doesn't protect, together, TDF_KTH_SUSP and sleeping activity. The right pattern should be to use the thread lock spinlock as an interlock and use msleep. Such bugs have not been revealed probabilly because there has been a lack of testing of such primitives and there are not, currently, consumers within our stock kernel. Additively, kthread_suspend_check() seems to require to always pass curthread, which is silly (as we don't have to conform to any particular KPI), thus I think it is appropriate for the prototype to change. The following patch should fix the issue: http://www.freebsd.org/~attilio/kthread.diff That patch would have benefit from the priority argument (for PDROP) for msleep_spin(9), and I don't understand why we don't support it (the log message doesn't seem much clearer). If nobody objects, after this patch goes in I would add the priority argument to msleep_spin() too. Thanks, Attilio -- Peace can only be achieved by understanding - A. Einstein