From owner-freebsd-toolchain@FreeBSD.ORG Tue Jan 8 06:21:54 2013 Return-Path: Delivered-To: toolchain@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:1900:2254:206a::19:1]) by hub.freebsd.org (Postfix) with ESMTP id DB7C570D; Tue, 8 Jan 2013 06:21:54 +0000 (UTC) (envelope-from bright@mu.org) Received: from elvis.mu.org (elvis.mu.org [192.203.228.196]) by mx1.freebsd.org (Postfix) with ESMTP id CAE67D30; Tue, 8 Jan 2013 06:21:54 +0000 (UTC) Received: from Alfreds-MacBook-Pro-9.local (unknown [207.238.187.242]) by elvis.mu.org (Postfix) with ESMTPSA id 501121A3CC4; Mon, 7 Jan 2013 22:21:51 -0800 (PST) Message-ID: <50EBBAFC.9070803@mu.org> Date: Tue, 08 Jan 2013 01:21:48 -0500 From: Alfred Perlstein User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.7; rv:17.0) Gecko/17.0 Thunderbird/17.0 MIME-Version: 1.0 To: Konstantin Belousov Subject: Re: Fast sigblock (AKA rtld speedup) References: <20130107182235.GA65279@kib.kiev.ua> <50EB3319.6060303@mu.org> <20130108000840.GA82219@kib.kiev.ua> In-Reply-To: <20130108000840.GA82219@kib.kiev.ua> Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Cc: arch@freebsd.org, toolchain@freebsd.org X-BeenThere: freebsd-toolchain@freebsd.org X-Mailman-Version: 2.1.14 Precedence: list List-Id: Maintenance of FreeBSD's integrated toolchain List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 08 Jan 2013 06:21:54 -0000 On 1/7/13 7:08 PM, Konstantin Belousov wrote: > On Mon, Jan 07, 2013 at 03:42:01PM -0500, Alfred Perlstein wrote: >> On 1/7/13 1:22 PM, Konstantin Belousov wrote: >>> Below is the forward of the patch for which I failed to obtain a private >>> review. Might be, the list generates more responses. >> Very cool. >> >> Sorry for being rusty here, but is it safe to call fuword in the middle >> of issignal()? >> >> The reason I ask is because it looks like proc lock is required for this >> block, and I forget it fuword(9) can fault. >> >> If fuword(9) can fault then I don't think you can do this. >> >> You'll need to use something like fuswintr(9) or wire the page down >> using vslock(9) or something. >> >> I am probably missing something? > No, you are completely right. This is a serious bug. Thank you. > > I was careful to code the td_sigblock_val prefetch etc, but failed > to do the update properly. There is no reason to execute the update > in the issignal() at all, the setting of the pending bit can be postponed > to the moment after the postsig() loop was executed. > > I uploaded the updated but not yet tested patch at > http://people.freebsd.org/~kib/misc/rtld-sigblock.1.patch This new patch looks like it may have issues with PSTOP. there is a lot of code in issignal() that is missed when this code is in place, I have not audited the effect of this, are you sure this is what will work for the majority of cases? Honestly, if I were coding it for correctness I would vslock the pages (or otherwise wire them in) and let issignal() behave normally and not early exit from it. That said I may be missing something. -Alfred