From owner-freebsd-arch@FreeBSD.ORG Sat Feb 13 17:22:38 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 479001065676; Sat, 13 Feb 2010 17:22:38 +0000 (UTC) (envelope-from asmrookie@gmail.com) Received: from mail-iw0-f180.google.com (mail-iw0-f180.google.com [209.85.223.180]) by mx1.freebsd.org (Postfix) with ESMTP id 026B08FC0A; Sat, 13 Feb 2010 17:22:37 +0000 (UTC) Received: by iwn10 with SMTP id 10so512329iwn.13 for ; Sat, 13 Feb 2010 09:22:37 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=gamma; h=domainkey-signature:mime-version:sender:received:in-reply-to :references:date:x-google-sender-auth:message-id:subject:from:to:cc :content-type; bh=9YzCuYyL4sOOrXoPVYjvoZUrKeRsctOGSEU62cJbOEw=; b=NzTJHdkgm0iJs2AIQfDRCOEx8iw0dFGpoQYu7lCmhw7BbirQfSY+G8GAe3XR7FOXyB CabEHQ9JyD3Ug6dLi4TXdnnc7zKb1zxaOvUA05CEJpAu21ByBv7Ug7rmXLcDSFvmUYqq pyVk0hDmZEKrMKvr0CZYfPwSEldF85LbUDrK4= 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; b=cfzfP8syYx23Mo7h/KYniLIf+UP4nesQLwmT+OPZetbgkVPD+NqDfjuKOGmMlzORzV KOYcy8U1qr8aQHwrVa0Emt7IZqmqxYbc3L1rjg9GlIlrlbyXKlmO880YOVjogEsIDEsa CBozuJysngYGJl/RJitGZ1djMCOynAaWCCdMQ= MIME-Version: 1.0 Sender: asmrookie@gmail.com Received: by 10.231.154.213 with SMTP id p21mr3308702ibw.42.1266081756365; Sat, 13 Feb 2010 09:22:36 -0800 (PST) In-Reply-To: <4e6cba831001280238x6a86e9f8vf5b7858b4bb82178@mail.gmail.com> References: <4e6cba831001280238x6a86e9f8vf5b7858b4bb82178@mail.gmail.com> Date: Sat, 13 Feb 2010 18:22:36 +0100 X-Google-Sender-Auth: f81a0a35428aa50f Message-ID: <3bbf2fe11002130922n2ca90130x4da8d47185d04978@mail.gmail.com> From: Attilio Rao To: Giovanni Trematerra Content-Type: text/plain; charset=UTF-8 Cc: freebsd-arch@freebsd.org Subject: Re: kthread interface 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: Sat, 13 Feb 2010 17:22:38 -0000 2010/1/28 Giovanni Trematerra : > Hi all, > There is a race in kthread_exit when all the threads of a kernel > process exit at same time. I came up with a quick and dirty patch that > resolve the issue at least in my test case. > > http://www.trematerra.net/patches/kthread_exit.diff > > Nonetheless I see space for improvement into kthread interface. > At present, with kproc_kthread_add you could have a kernel process > without a main thread and that seems to me only a way to logical > grouping threads and pretty useless. > I propose to remove kproc_kthread_add and don't let kthread_exit call > kproc_exit on the last exiting thread but demand user to handle > process termination. > If you need kernel threads but no reason to have a kernel process with > a main thread that acts as a coordinator you can attach them to proc0 > by kthread_add passing NULL for (struct proc *) argument. The patch is right but for a different reason than what you expose. What really happens is that kproc_kthread_add() has mostly an evil semantic. In the common case you define a precise callback, but you are not going to know if the callback you are offering will end up being the first thread (thus 'reconduited' to kproc_create()) or just one of the others (thus added via kthread_add()). The problem is that in the former case the callback should close with a kproc_exit() call, while on the latter an ideal thread_exit() should happen. As long as the callback can't know in which case it is, the kthread_exit() (used in this context, but as I can see, not in all of them) needs to take care of this problem. This is only possible, however, if the callbacks also ensure (and so far I don't see any similar safety belt somewhere) that the threads adding is correctly drained when the threads are going to die. This is a race the primitive itself can't handle (aka: the count of threads within the kproc can't grow up when the first thread start dying) because it needs of external help. The normal proc/thread working model is quite different because the process is created with an explicit interface (fork1()) and it is closed by an explicit interface (eventually reused, but exit1() takes explicitly place in the interested paths) and the threads are added with a explicit logic (via thr interfaces). kproc_kthread_add() does complicate all this simple logic, of course. As a first band-aid your patch is ok, but what I really would like is that the whole semantic gets sanitized and gets a better logic (in other words: explicit ordering of the kproc/kthreads creation and destruction). Attilio -- Peace can only be achieved by understanding - A. Einstein