Date: Thu, 18 Jul 2013 14:38:14 +0300 From: Konstantin Belousov <kostikbel@gmail.com> To: Bruce Evans <brde@optusnet.com.au> Cc: freebsd-acpi@FreeBSD.org Subject: Re: Revisiting FPU context resume on i386 Message-ID: <20130718113814.GB5991@kib.kiev.ua> In-Reply-To: <20130718150806.J857@besplex.bde.org> References: <20130716070716.15b7282b9dca2cbc8a767631@tackymt.homeip.net> <20130716052641.GE91021@kib.kiev.ua> <20130716164612.P1088@besplex.bde.org> <20130717205656.GV5991@kib.kiev.ua> <20130718150806.J857@besplex.bde.org>
next in thread | previous in thread | raw e-mail | index | archive | help
--SBKwXYz1xY/kcGv0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Thu, Jul 18, 2013 at 03:33:59PM +1000, Bruce Evans wrote: > On Wed, 17 Jul 2013, Konstantin Belousov wrote: >=20 > > On Tue, Jul 16, 2013 at 06:54:57PM +1000, Bruce Evans wrote: > >> ISTR disagreeing with jkim on using a special save area. > > I believe the normal save area cannot be used there at all, since > > the suspension is async and fpu.c could perform some operation on > > the PCB-pointed save area while suspend IPI is received. >=20 > If that happens, then suspension is broken anyway. Any npx operation Yes, it is broken, but not for FPU. For the fpu.c, the actions in the IPI handler and later in resume path are invisible. > on the pcb (or any other operation on the pcb) between savectx() and > resumectx() makes the state saved by savectx() out of date. In normal > kernel operations, the npx state is changed from volatile to non-volatile > by pushing it to the pcb. This prevents context switches from changing > it underneath you by doing the same push to the pcb. (The state is not > really changed, but the place where it lives and pointers and flags that > say where it lives are changed.) I think this works perfectly for > suspend/resume too. It is less clear what happens for non-npx parts of > the pcb. Hopefully there are no races for them (and the special save > area is not needed) because there is no lazy context switching for them > -- they live either in the CPU or the pcb, and are switched between the > CPU and the pcu atomically on every context switch. Clearly context > switches must not be allowed between suspend and resume, or the resume > must be on the same thread as the suspend. Otherwise resume restores > state for a wrong thread, which is a much larger bug than races and > inconsistencies in accessing separate save areas for the same thread. I do not understand what you described there. Assume that e.g. the CPU was in fpudna() code and performs the bcopy() from fpu_initialstate when the suspend IPI was delivered. Then saving the current HW state into the same PCB save area causes corruption. I do not disagree that the current async suspension is broken, but it is for different reasons. Basically, it delegates too much work to the drivers device_suspend methods, requiring the routine to be mutually exclusive with any access to the hardware. I do not think that any driver gets this right. IMO sleep state should be only entered when all threads are parked either at safe sleep point or at the kernel->user bounday. Even this is not enough if there is in-flight sleepable operation in some device, but that part is indeed in the competence of the driver suspend method. >=20 > > ... > > Also, the comment before the %cr0 manipulations to set CR0_PG was > > copied from amd64 version and is irrelevant there. >=20 > I can't find this comment or any setting of CR0_PG in either swtch.s > file. I referred to the sys/i386/acpica/acpi_wakecode.S, which is the caller of restorectx(). --SBKwXYz1xY/kcGv0 Content-Type: application/pgp-signature -----BEGIN PGP SIGNATURE----- Version: GnuPG v2.0.20 (FreeBSD) iQIcBAEBAgAGBQJR59OmAAoJEJDCuSvBvK1B6zMQAIJYIj0OERQETNiRzCLAP8dn zUMxeDeENBwb7TL96wzwZyzkzDEUv3qIAiwhMKj0axG7B5KdwxvWSEYFPxYXMyzx yK/TpPXyHAGQ1/9MH89gVh6CZgR+ceQJ0dAgYaAC+koRornlUWnn0efzOn9AKsvL ZHK++v7+As6nTTjSSx5qb/YecRw8k5BdYEG0bgLo61gDxEZteG1Z3BQ0x1Mk1ray aqbVI9mPtn2E0j0Cy3GtYtquKuheKeZiemHuJ9l0r+zSsODTKAIBwXFt/k1mU6Uf IsKd6+XMXm5YlqHiR0J8L+SJOmKUrXohQNaEnfsRkvk9E59hhNhMx+9hvcgKUXFE NDEY57rUexbPYQL87IXk+O2jt4a6QXiMi7cfXxRS70VKeFrEl9dKVsB3wr3kADD2 END+tdhvV84zGFx4cLeGud1G+MnRwzzHsUbQjrDhxXpUlWzteK1B+NbgaKeWyCT1 TMqS0XvJGyTGoWkFKXAauggCbY4KzbZtCGSdbcf18YHIaPiWiGxRMcfmsXsjyGXd dsFACuwmUxF0daPpe3GyHuuUhHvbwz+EOKA1jHMGTTuqTh/FwRXFN43bD+kyd40k Bf41FdhyPO5qhOydjqO+DWsQzqC4yc7vxczwOhPjJrraS4Rni9zLZbgj9LtNeUFZ 59qs2cjfQS5UDPkO4Cry =zmDl -----END PGP SIGNATURE----- --SBKwXYz1xY/kcGv0--
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20130718113814.GB5991>