From owner-freebsd-current@freebsd.org Thu Dec 17 19:58:07 2015 Return-Path: Delivered-To: freebsd-current@mailman.ysv.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:1900:2254:206a::19:1]) by mailman.ysv.freebsd.org (Postfix) with ESMTP id 911B3A49550 for ; Thu, 17 Dec 2015 19:58:07 +0000 (UTC) (envelope-from mjguzik@gmail.com) Received: from mail-wm0-x22c.google.com (mail-wm0-x22c.google.com [IPv6:2a00:1450:400c:c09::22c]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (Client CN "smtp.gmail.com", Issuer "Google Internet Authority G2" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 2E94E123C; Thu, 17 Dec 2015 19:58:07 +0000 (UTC) (envelope-from mjguzik@gmail.com) Received: by mail-wm0-x22c.google.com with SMTP id p187so36241130wmp.1; Thu, 17 Dec 2015 11:58:07 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=date:from:to:cc:subject:message-id:mail-followup-to:references :mime-version:content-type:content-disposition:in-reply-to :user-agent; bh=vEgWe25PyIJC0z3ADcwn4ljutYMWsEyWPPtAcGFDPwo=; b=CiTMhc0oUnjfvEs5GfKXDsVvxEnHCoCN+Oayit9zXriQsdNzyDRmjkUQy53LulVu6g +Tly5WCPPVLUWv9ztNnkEyinNRsLQRr/4FuLMyCQRa/AqGKSelnLy7iPS+LU3kXEU3bw +fNSKAinwmMIIa6NENAwOfh9N+yFmphxCFPUn5TbXMEKBQJFwgPO99U5L+NdRVWlyU+h GQWELBAM7INMBJmQHMF8zalx1n+qrD1qOAqKC1YLK271uEqFn6gqAQ3iTrZVlC6KlYlI dGuOqv8qOsvBxMgjCYgIvM+Wmm+VM59wNHbEvOpx+Qddgr8/uTOYe68ttum2Opw0+07C Lt7g== X-Received: by 10.194.57.73 with SMTP id g9mr11717252wjq.107.1450382285490; Thu, 17 Dec 2015 11:58:05 -0800 (PST) Received: from dft-labs.eu (n1x0n-1-pt.tunnel.tserv5.lon1.ipv6.he.net. [2001:470:1f08:1f7::2]) by smtp.gmail.com with ESMTPSA id l128sm3678307wmf.10.2015.12.17.11.58.04 (version=TLS1_2 cipher=AES128-SHA bits=128/128); Thu, 17 Dec 2015 11:58:04 -0800 (PST) Date: Thu, 17 Dec 2015 20:58:03 +0100 From: Mateusz Guzik To: Don Lewis Cc: kostikbel@gmail.com, freebsd-current@freebsd.org Subject: Re: fork_findpid() - Fatal trap 12: page fault while in kernel mode Message-ID: <20151217195802.GA29200@dft-labs.eu> Mail-Followup-To: Mateusz Guzik , Don Lewis , kostikbel@gmail.com, freebsd-current@freebsd.org References: <20151217120343.GA3625@kib.kiev.ua> <201512171948.tBHJm80b094518@gw.catspoiler.org> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: <201512171948.tBHJm80b094518@gw.catspoiler.org> User-Agent: Mutt/1.5.21 (2010-09-15) X-BeenThere: freebsd-current@freebsd.org X-Mailman-Version: 2.1.20 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: Thu, 17 Dec 2015 19:58:07 -0000 On Thu, Dec 17, 2015 at 11:48:08AM -0800, Don Lewis wrote: > On 17 Dec, Konstantin Belousov wrote: > > On Wed, Dec 16, 2015 at 11:08:02AM -0800, Don Lewis wrote: > >> I used to have a patch the deferred linking the new process into > >> proctree/allproc until it was fully formed. The motivation was to get > >> rid of all of the PRS_NEW stuff scattered around the source. > >> Unfortunately the patch bit-rotted and I'm pretty sure that I lost it. > > > > I had similar tought for a second as one of the possibilities to fix the > > issue, but rejected it outright due to the way the pid allocator works. > > The loop which faulted is the allocator, it depends on the new pid being > > linked early to detect the duplicated alloc. > > > > What you wrote could be done, but this restructuring requires the separate > > pid allocator, and probably it must repeat all quirks and subtle behaviour > > of the current algorithm. But I do not object, PRS_NEW is a trouble > > on its own. > > I don't think it requires any changes to the allocater. It should only > be necessary to delay the call to fork_findpid() until we are ready to > link the new proc into allproc. Basically, drop the locks at the > beginning of do_fork(), then grab them again somewhere near the end > (probably where we are currently mark the process as PRS_NORMAL) and > move the call to fork_findpid(), the p2->p_pid assignment, and the list > manipulation code to a location after that. > > It's probably not quite that simple though ... That would mean you would need to be able to deconstruct the process because you cannot guarantee there are any pids left, which may or may not be easily doable. The current method is going to bite us performance-wise anyway and an allocater which does not require a walk over the tree is necessary in the long run. Seems like a bitmap (or a bunch of bitmaps) is the way to go here. Meanwhile one can add a special process permanently in PRS_NEW state and poisoned pointers in debug kernels to help ensuring that all loops handle the case. Not signing up for any of this work though. -- Mateusz Guzik