Skip site navigation (1)Skip section navigation (2)
Date:      Wed, 20 Feb 2013 21:27:39 +0200
From:      Konstantin Belousov <kostikbel@gmail.com>
To:        John Baldwin <jhb@freebsd.org>
Cc:        freebsd-current@freebsd.org, Svatopluk Kraus <onwahe@gmail.com>
Subject:   Re: [patch] i386 pmap sysmaps_pcpu[] atomic access
Message-ID:  <20130220192739.GM2598@kib.kiev.ua>
In-Reply-To: <201302201022.29294.jhb@freebsd.org>
References:  <CAFHCsPUVTM9jfrnzY72YsPszLWkg-UaJcycTR4xXcS%2BfPzS1Vg@mail.gmail.com> <20130219185129.GC2598@kib.kiev.ua> <CAFHCsPV0p13yjs65i6P17JocoGhQCbVEaaTb443C=c9AJNO%2Bww@mail.gmail.com> <201302201022.29294.jhb@freebsd.org>

next in thread | previous in thread | raw e-mail | index | archive | help

--+0mKm/ENadSkQxF+
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
Content-Transfer-Encoding: quoted-printable

On Wed, Feb 20, 2013 at 10:22:29AM -0500, John Baldwin wrote:
> On Wednesday, February 20, 2013 7:31:08 am Svatopluk Kraus wrote:
> > On Tue, Feb 19, 2013 at 7:51 PM, Konstantin Belousov
> > <kostikbel@gmail.com> 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
> > >> <kostikbel@gmail.com> wrote:
> > >> Well, I'm taking a part on porting FreeBSD to ARM11mpcore. UP case w=
as
> > >> 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, t=
he
> > >> 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 t=
he code
> > > which uses its result would not be atomic (single-instruction or what=
ever, see
> > > below). Thus, either the preemption should not matter since the actio=
n with
> > > the per-cpu data is advisory, as is in the case of i386 pmap you note=
d.
> > > or some external measures should be applied in advance to the contain=
ing
> > > region (which you proposed, by bracing with sched_pin()).
> >=20
> > So, it's advisory in the case of i386 pmap... Well, if you can live
> > with that, I can too.
> >=20
> > >
> > > Also, note that it is not interrupts but preemption which is concern.
> >=20
> > 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.
>=20
> curthread is a bit magic. :)  If you perform a context switch during an
> interrupt (which will change 'curthread') you also change your register s=
tate.
> When you resume, the register state is also restored.  This means that wh=
ile
> something like 'PCPU_GET(cpuid)' might be stale after you read it, 'curth=
read'
> never is.  However, it is true that actually reading curthread has to be
> atomic.  If your read of curthread looks like:
>=20
> 	mov <pcpu reg>, r0
> 	add <offset of pc_curthread>, r0
> 	ld r0, r1
>=20
> Then that will indeed break.  Alpha used a fixed register for 'pcpu_reg'
> (as does ia64 IIRC).  OTOH, you might also be able to depend on the fact =
that
> pc_curthread is the first thing in 'struct pcpu' (and always will be, you=
 could
> add a CTASSERT to future-proof).  In that case, you can remove the 'add'
> instruction and instead just do:
>=20
> 	ld <pcpu reg>, r1
>=20
> which is fine.

I just looked at the arm pcpu.h, and indeed the curthread falls
back to the default implementation from sys/pcpu.h, which is
get_pcpu()->pc_curthread. This looks buggy for SMP, does our arm port
support any multi-cpu configuration ?


--+0mKm/ENadSkQxF+
Content-Type: application/pgp-signature

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2.0.19 (FreeBSD)

iQIcBAEBAgAGBQJRJSOrAAoJEJDCuSvBvK1BqskP/jtTAH9+tb4wLITbSbQBnrPc
Lldv1hIMLti5Upo/bCzmyj0HgNMFPuYEtAFCKglamiwq0IsnWF631UPuXoewhdZL
KIk0vzNt2awp9n0KDC+NS15DiDzjgyD2jioxTuT79gSUT7mhyz0P7YVdjRlsED4f
0Dil6sAUvzZsVV7O2ZMT0WPii87gPdY2l+QRT49zbfwuPmi6USVwyxUTE6d3yWPw
P/PeL/WVBEb2z+8dp5vrN8SWQR214nJ021hnxRtCN//qYlKCZzV2BvWp++lV5er6
zuv9udCBAGSdVU5CZwFQ8faGpncpJrWcfZD1u31rPvwobR54mpG0ySYdFTCSXQRi
Fajtujl45BCCaXW27D+4bDc40nAmwqvI/Ivoj/qbc6t1Tq1khGIiQWv/lObbRruF
EipM/VEnn8IlVB6QLm0i010lLFgMevPbjOyzdNZzRXrcThH6pbA7E9Z1RqL5J1jT
gCbPvsKUywxpcwKrr/DOsCU4jBS3bt1pL7ADx38yrEDDSNyGGSHP22zwEWcoWXui
TGBdXleEubrwGrasbvSLaa59ewBlm7i/rgDwxuLP8U3Qdg3YSHdu7vYlv01reREI
OnlNYXaytHQSpK3VM9vsFBP5gRBgdLBSGCnNbyQ8oN8AATg+GmRZJ3+1b2BiRUXz
q87LBvl5Tg6yLVxzMPsW
=QLoH
-----END PGP SIGNATURE-----

--+0mKm/ENadSkQxF+--



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20130220192739.GM2598>