Date: Thu, 21 Feb 2013 07:08:24 -0700 From: Warner Losh <imp@bsdimp.com> To: Gleb Smirnoff <glebius@FreeBSD.org> Cc: svn-src-head@freebsd.org, Andrey Zonov <zont@FreeBSD.org>, src-committers@freebsd.org, Warner Losh <imp@FreeBSD.org>, svn-src-all@freebsd.org Subject: Re: svn commit: r247066 - head/sys/dev/ppc Message-ID: <63ACAB54-24F4-4EB7-B7F6-54683E016848@bsdimp.com> In-Reply-To: <20130221132659.GS72813@FreeBSD.org> References: <201302210027.r1L0Rqv3091748@svn.freebsd.org> <5125E465.20700@FreeBSD.org> <20130221132659.GS72813@FreeBSD.org>
next in thread | previous in thread | raw e-mail | index | archive | help
On Feb 21, 2013, at 6:26 AM, Gleb Smirnoff wrote: > On Thu, Feb 21, 2013 at 01:09:57PM +0400, Andrey Zonov wrote: > A> > Log: > A> > Replace splhigh() with critical_enter()/leave() to ensure we = write the > A> > config mode unlock sequence quickly enough. This likely isn't = too critical, > A> > since splhigh() has been a noop for a decade... > A> >=20 > A> > Modified: > A> > head/sys/dev/ppc/ppc.c > A> >=20 > A> > Modified: head/sys/dev/ppc/ppc.c > A> > = =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D > A> > --- head/sys/dev/ppc/ppc.c Thu Feb 21 00:26:31 2013 = (r247065) > A> > +++ head/sys/dev/ppc/ppc.c Thu Feb 21 00:27:51 2013 = (r247066) > A> > @@ -74,6 +74,22 @@ static void ppcintr(void *arg); > A> > =20 > A> > #define DEVTOSOFTC(dev) ((struct ppc_data = *)device_get_softc(dev)) > A> > =20 > A> > +/* > A> > + * We use critical enter/leave for the simple config locking = needed to > A> > + * detect the devices. We just want to make sure that both of = our writes > A> > + * happen without someone else also writing to those config = registers. Since > A> > + * we just do this at startup, Giant keeps multiple threads from = executing, > A> > + * and critical_enter() then is all that's needed to keep us = from being preempted > A> > + * during the critical sequences with the hardware. > A> > + * > A> > + * Note: this doesn't prevent multiple threads from putting the = chips into > A> > + * config mode, but since we only do that to detect the type at = startup the > A> > + * extra overhead isn't needed since Giant protects us from = multiple entry > A> > + * and no other code changes these registers. > A> > + */ > A> > +#define PPC_CONFIG_LOCK(ppc) critical_enter() > A> > +#define PPC_CONFIG_UNLOCK(ppc) critical_leave() > A> > + > A>=20 > A> s/critical_leave/critical_exit/? >=20 > Already fixed. >=20 > However, question to Warner. >=20 > Since code already executes under Giant, what is the reason for = critical_section? > What's the problem if couple of outb instructions are split (assuming = they are > properly serialized) across two CPUs? There's a timing window that needs to be hit with these chips. Warner=
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?63ACAB54-24F4-4EB7-B7F6-54683E016848>