From owner-freebsd-current@FreeBSD.ORG Wed Feb 20 12:31:10 2013 Return-Path: Delivered-To: freebsd-current@freebsd.org Received: from mx1.freebsd.org (mx1.FreeBSD.org [8.8.178.115]) by hub.freebsd.org (Postfix) with ESMTP id 7D2D482F for ; Wed, 20 Feb 2013 12:31:10 +0000 (UTC) (envelope-from onwahe@gmail.com) Received: from mail-qa0-f52.google.com (mail-qa0-f52.google.com [209.85.216.52]) by mx1.freebsd.org (Postfix) with ESMTP id 3AD5EE26 for ; Wed, 20 Feb 2013 12:31:09 +0000 (UTC) Received: by mail-qa0-f52.google.com with SMTP id bs12so2357759qab.18 for ; Wed, 20 Feb 2013 04:31:09 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=mime-version:x-received:in-reply-to:references:date:message-id :subject:from:to:cc:content-type; bh=W2kel/G4N+35SEqvmEgnguyp4lirXsRVpp7WK72xUeo=; b=RV87rJldrpjZl5SWQF0zn+H9KQ3MflMyariXQSVpbjlxfpZKErSvgENhZ3sLDMo79y LOng/ILnHiHmV82MeZtklWX8nUG06rmQPBx4qeovima0FDkcCLyxpcFgpkTHVqJEGBmL jQNYDNsVRNjPCUuks2BlavCs/pTTyTHgul/9ndsQanF3oDxdD9cHnzjTj6qeRo1TsL5Q qZxpySFtPe4TH5mbwz0A8u3ROa6iup7LwjlPJOc8zY/pe/Qtk2kY6Zu4KLhi+E/b7axD qYgK4FZ3JcuZH7pjBETDeLKDKT9YUeH1TWB4CSBiupr8rJipBZkHqH0q7KCqMor19gz+ wneg== MIME-Version: 1.0 X-Received: by 10.49.118.138 with SMTP id km10mr9309492qeb.18.1361363468921; Wed, 20 Feb 2013 04:31:08 -0800 (PST) Received: by 10.49.121.198 with HTTP; Wed, 20 Feb 2013 04:31:08 -0800 (PST) In-Reply-To: <20130219185129.GC2598@kib.kiev.ua> References: <20130218150809.GG2598@kib.kiev.ua> <20130218170957.GJ2598@kib.kiev.ua> <20130218203630.GO2598@kib.kiev.ua> <20130219185129.GC2598@kib.kiev.ua> Date: Wed, 20 Feb 2013 13:31:08 +0100 Message-ID: Subject: Re: [patch] i386 pmap sysmaps_pcpu[] atomic access From: Svatopluk Kraus To: Konstantin Belousov Content-Type: text/plain; charset=ISO-8859-1 Cc: freebsd-current@freebsd.org X-BeenThere: freebsd-current@freebsd.org X-Mailman-Version: 2.1.14 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: Wed, 20 Feb 2013 12:31:10 -0000 On Tue, Feb 19, 2013 at 7:51 PM, Konstantin Belousov wrote: > On Mon, Feb 18, 2013 at 11:18:16PM +0100, Svatopluk Kraus wrote: >> On Mon, Feb 18, 2013 at 9:36 PM, Konstantin Belousov >> wrote: >> Well, I'm taking a part on porting FreeBSD to ARM11mpcore. UP case was >> simple. SMP case is more complex and rather new for me. Recently, I >> was solving a problem with PCPU stuff. For example, PCPU_GET is >> implemented by one instruction on i386 arch. So, a need of atomicity >> with respect to interrupts can be overlooked. On load-store archs, the >> implementation which works in SMP case is not so simple. And what >> works in UP case (single PCPU), not works in SMP case. Believe me, >> mysterious and sporadic 'mutex not owned' assertions and others ones >> caused by curthreads mess, it takes a while ... > Note that PCPU_GET() is not needed to be atomic. The reason is that the code > which uses its result would not be atomic (single-instruction or whatever, see > below). Thus, either the preemption should not matter since the action with > the per-cpu data is advisory, as is in the case of i386 pmap you noted. > or some external measures should be applied in advance to the containing > region (which you proposed, by bracing with sched_pin()). So, it's advisory in the case of i386 pmap... Well, if you can live with that, I can too. > > Also, note that it is not interrupts but preemption which is concern. Yes and no. In theory, yes, a preemption is a concern. In FreeBSD, however, sched_pin() and critical_enter() and their counterparts are implemented with help of curthread. And curthread definition falls to PCPU_GET(curthread) if not defined in other way. So, curthread should be atomic with respect to interrupts and in general, PCPU_GET() should be too. Note that spinlock_enter() definitions often (always) use curthread too. Anyhow, it's defined in MD code and can be defined for each arch separately. >> >> After this, I took a look at how PCPU stuff is used in whole kernel >> and found out mentioned here i386 pmap issue. So, my concern is >> following: >> >> 1. to verify my newly gained ideas how per CPU data should be used, >> 2. to decide how to change my implementation of pmap on ARM11mpcore, >> as it's based on i386 pmap, >> 3. to make FreeBSD code better. >> >> In the meanwhile, it looks that using data dedicated to one CPU on >> another one is OK. However, I can't agree. At least, without comments, >> it is misleading for anyone new in FreeBSD and makes code misty. >> >> > Both threads in your description make useful progress, and computation >> > proceeds correctly. >> >> I thought, there is only one thread in my example. One thread running >> on CPU1, but holding sysmaps dedicated to CPU0 instead of holding >> sysmaps dedicated to CPU1. So, any thread running on CPU0 must wait >> because the thread running on CPU1 is a thief. Futhermore, the idea >> that a thread on CPU1 should hold data for CPU1 is not valid. So, >> either some comment is missing in the code that it's OK or the using >> of PCPU_GET(cpuid) is unneeded and some kind of free sysmaps list can >> be used and it will serve better. > > What is wrong on almost all architectures except x86 is PCPU_ADD(), which > is usually supposed by the MD code to be atomic in regard to _both_ > interrupts and preemption. I said almost all, but in fact I am not aware > of any architecture except x86 where it is right. > > And, RISCs with the load-link and store-conditional (ll/sc) primitives > are well capable of doing the PCPU_ADD() correctly. > > You could look at the projects/counters branch, where single-instruction > increment is used on x86 for dynamic per-cpu counters, and critical_enter() > for other architectures. I might do some work for other arches, but the > counters are correct but slower that it could be, on !x86. Thanks to help me to settle my thoughts. Svata