Skip site navigation (1)Skip section navigation (2)
Date:      Sat, 4 Feb 2017 14:56:37 -0500
From:      Alexander Kabaev <kabaev@gmail.com>
To:        "Jason A. Harmening" <jah@freebsd.org>
Cc:        src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org, Andreas Tobler <andreast@FreeBSD.org>, Kurt Lidl <lidl@pix.net>
Subject:   Re: svn commit: r313037 - in head/sys: amd64/include kern mips/include net powerpc/include sparc64/include
Message-ID:  <20170204145637.0324f015@kan>
In-Reply-To: <CAM=8qa=h3=sfro02hCQyzqkDnLO7TnQJ8ugCoa5=MfNE_OCgZg@mail.gmail.com>
References:  <201702010332.v113WnYf041362@repo.freebsd.org> <20170203231238.0675c289@kan> <CAM=8qa=h3=sfro02hCQyzqkDnLO7TnQJ8ugCoa5=MfNE_OCgZg@mail.gmail.com>

next in thread | previous in thread | raw e-mail | index | archive | help
--Sig_/tT+G97w/.cXn85LaPanxYVX
Content-Type: text/plain; charset=US-ASCII
Content-Transfer-Encoding: quoted-printable

On Fri, 3 Feb 2017 21:29:33 -0800
Jason Harmening <jason.harmening@gmail.com> wrote:

> Hi,
>=20
> I'm a bit confused as to how this change breaks MIPS.  The new
> function, get_pcpu() is intended to be used only to access the
> per-cpu data pointer locally.  It returns pcpup, which is the per-cpu
> pointer wired into the local TLB to translate to the local CPU's
> physical data region, correct?
>=20
> This is the same value used by the per-CPU accessors such as PCPU_ADD
> and PCPU_GET.  The MI portions of this change only use get_pcpu() to
> access the local CPU's data, e.g. under a critical section in the
> rmlock.  It is not intended to be used for iterating all CPUs.
>=20
> If I've missed something and MIPS is truly broken by this, then I'll
> gladly revert, but (maybe because it's late) I'm not seeing where
> this goes wrong on MIPS.
>=20
> Thanks,
> Jason

The hang is actually due to _rmlock_assert spinning in
rm_trackers_present with interrupts disabled, due to this constructs:

        for (queue =3D pc->pc_rm_queue.rmq_next; queue !=3D
            &pc->pc_rm_queue; queue =3D queue->rmq_next) {

You changed the code to pass get_pcpu() results instead of direct
pointer to the corresponding pcpu storage and that is NOT the pointer
that was being used in pcpu_init. On architectures that rig TLB to make
local PCPU available at constant address from each CPU, these two are
generally not same, so while empty pc_rm_queue is a cyclic list
consisting of just one element pc_rm_queue, the loop terminating
condition will never be met, as &pc->pc_rm_qeue value will be very
different from the one that pc->pc_rm_queue.rmq_next was set at
initialisation time.

This affects MIPS in SMP configuration only, UP configs do not bother
with TLB hackery.

--=20
Alexander Kabaev

--Sig_/tT+G97w/.cXn85LaPanxYVX
Content-Type: application/pgp-signature
Content-Description: Цифровая подпись OpenPGP

-----BEGIN PGP SIGNATURE-----

iQKTBAEBCgB9FiEExffZlZm2QeE8UVaRBxMimZJ5Ln4FAliWMfVfFIAAAAAALgAo
aXNzdWVyLWZwckBub3RhdGlvbnMub3BlbnBncC5maWZ0aGhvcnNlbWFuLm5ldEM1
RjdEOTk1OTlCNjQxRTEzQzUxNTY5MTA3MTMyMjk5OTI3OTJFN0UACgkQBxMimZJ5
Ln7gZg/+OrJ/rUQ+gm0P5DzMAf7aZ4rSOVNe244TWr+3Vu69zCEY9290HzxvKGd1
IhNY2P+4s5OFzbtYm/3ppjSrIziYr/hhIR0ZglunBEzsKtMwMGM2/mmi0n8fkMJW
Ls0ndnQLgpNvInf6hKgvumqoxW/ak+WIet4AgqRfETPl5NfEHmaPKtrXT6qapP4c
8lGD1Lg4Sv/nOI7/VkeULMWXxGY9opFTZ08x68jOJKtxgNO/7g9D9LKg2JlO0FED
GPA50APGetUuzkxLAItOYx1zr6SDeZqkwOokBr8gAbL4OI2oTND7lIFdqn+gkJM0
UCGul7AtgwOOSHsDp31rVok5OEs12S4AGb8RY5aRkG03g6NsCTY06JpvmbLI9S3e
WvC/18ZSqVPjq7HkZnK2bSBwF3pwq2uIAXuKy8+0WCRn8d7OrW0+w4d5HaT58ACh
XfadjjgCf8nBe7cde7AmckORU7cCGqV0ik8RVDUoqVgEGbqbOBHn8xXhCFoON00T
nRUpHgzhYqWRX0MJ8Br800MXbNx/Dv/uljI6tfhF75+39TOOdQItdB7jpllvf+ox
NmFbvQxVAJ5ShYEDprj4yEZuJKmxVZn7v4zaHCIJLAr+KQEl0OQAH2LiXSdPtPa7
eWZ87NIFjHmvIRTa8m7y8JWqXe1Rbvk7Ckq3h28/hUul+q0fALA=
=kIAL
-----END PGP SIGNATURE-----

--Sig_/tT+G97w/.cXn85LaPanxYVX--



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