From mboxrd@z Thu Jan 1 00:00:00 1970 From: Alex =?utf-8?Q?Benn=C3=A9e?= Subject: Re: [PATCH v10 07/18] arm64: fpsimd: Eliminate task->mm checks Date: Thu, 24 May 2018 10:19:26 +0100 Message-ID: <87bmd5tnq9.fsf@linaro.org> References: <1527005119-6842-1-git-send-email-Dave.Martin@arm.com> <1527005119-6842-8-git-send-email-Dave.Martin@arm.com> Mime-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: base64 Return-path: In-reply-to: <1527005119-6842-8-git-send-email-Dave.Martin@arm.com> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=m.gmane.org@lists.infradead.org To: Dave Martin Cc: Christoffer Dall , Ard Biesheuvel , Marc Zyngier , Catalin Marinas , Will Deacon , kvmarm@lists.cs.columbia.edu, linux-arm-kernel@lists.infradead.org List-Id: kvmarm@lists.cs.columbia.edu CkRhdmUgTWFydGluIDxEYXZlLk1hcnRpbkBhcm0uY29tPiB3cml0ZXM6Cgo+IEN1cnJlbnRseSB0 aGUgRlBTSU1EIGhhbmRsaW5nIGNvZGUgdXNlcyB0aGUgY29uZGl0aW9uIHRhc2stPm1tID09Cj4g TlVMTCBhcyBhIGhpbnQgdGhhdCB0YXNrIGhhcyBubyBGUFNJTUQgcmVnaXN0ZXIgY29udGV4dC4K Pgo+IFRoZSAtPm1tIGNoZWNrIGlzIG9ubHkgdGhlcmUgdG8gZmlsdGVyIG91dCB0YXNrcyB0aGF0 IGNhbm5vdAo+IHBvc3NpYmx5IGhhdmUgRlBTSU1EIGNvbnRleHQgbG9hZGVkLCBmb3Igb3B0aW1p c2F0aW9uIHB1cnBvc2VzLgo+IEFsc28sIFRJRl9GT1JFSUdOX0ZQU1RBVEUgbXVzdCBhbHdheXMg YmUgY2hlY2tlZCBhbnl3YXkgYmVmb3JlCj4gc2F2aW5nIEZQU0lNRCBjb250ZXh0IGJhY2sgdG8g bWVtb3J5LiAgRm9yIHRoZXNlIHJlYXNvbnMsIHRoZSAtPm1tCj4gY2hlY2tzIGFyZSBub3QgdXNl ZnVsLCBwcm92aWRpbmcgdGhhdCB0aGF0IFRJRl9GT1JFSUdOX0ZQU1RBVEUgaXMKPiBtYWludGFp bmVkIGluIGEgY29uc2lzdGVudCB3YXkgZm9yIGtlcm5lbCB0aHJlYWRzLgo+Cj4gVGhpcyBpcyB0 cnVlIGJ5IGNvbnN0cnVjdGlvbiBob3dldmVyOiBUSUZfRk9SRUlHTl9GUFNUQVRFIGlzIG5ldmVy Cj4gY2xlYXJlZCBleGNlcHQgd2hlbiByZXR1cm5pbmcgdG8gdXNlcnNwYWNlIG9yIHJldHVybmlu ZyBmcm9tIGEKPiBzaWduYWw6IHRodXMsIGZvciBhIHRydWUga2VybmVsIHRocmVhZCBubyBGUFNJ TUQgY29udGV4dCBpcyBldmVyCj4gbG9hZGVkLCBUSUZfRk9SRUlHTl9GUFNUQVRFIHdpbGwgcmVt YWluIHNldCBhbmQgbm8gY29udGV4dCB3aWxsCj4gZXZlciBiZSBzYXZlZC4KPgo+IFRoaXMgcGF0 Y2ggcmVtb3ZlcyB0aGUgcmVkdW5kYW50IGNoZWNrcyBhbmQgc3BlY2lhbC1jYXNlIGNvZGUuCj4K PiBTaWduZWQtb2ZmLWJ5OiBEYXZlIE1hcnRpbiA8RGF2ZS5NYXJ0aW5AYXJtLmNvbT4KPiBDYzog Q2F0YWxpbiBNYXJpbmFzIDxjYXRhbGluLm1hcmluYXNAYXJtLmNvbT4KPiBDYzogV2lsbCBEZWFj b24gPHdpbGwuZGVhY29uQGFybS5jb20+Cj4gQ2M6IEFyZCBCaWVzaGV1dmVsIDxhcmQuYmllc2hl dXZlbEBsaW5hcm8ub3JnPgoKV2l0aCBDaHJpc3RvZmZlcidzIGNvbW1pdCB0ZXh0OgoKUmV2aWV3 ZWQtYnk6IEFsZXggQmVubsOpZSA8YWxleC5iZW5uZWVAbGluYXJvLm9yZz4KCj4KPiAtLS0KPgo+ IENoYW5nZXMgc2luY2Ugdjk6Cj4KPiAgKiBOZXcgcGF0Y2guICBJbnRyb2R1Y2VkIGR1cmluZyBk ZWJ1Z2dpbmcsIHNpbmNlIHRoZSAtPm1tIGNoZWNrcwo+ICAgIGFwcGVhciBib2d1cyBhbmQvb3Ig cmVkdW5kYW50LCBzbyBhcmUgbGlrZWx5IHRvIGJlIGhpZGluZyBvcgo+ICAgIGNhdXNpbmcgYnVn cy4KPiAtLS0KPiAgYXJjaC9hcm02NC9pbmNsdWRlL2FzbS90aHJlYWRfaW5mby5oIHwgIDEgKwo+ ICBhcmNoL2FybTY0L2tlcm5lbC9mcHNpbWQuYyAgICAgICAgICAgfCAzOCArKysrKysrKysrKyst LS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0KPiAgMiBmaWxlcyBjaGFuZ2VkLCAxNCBpbnNlcnRpb25z KCspLCAyNSBkZWxldGlvbnMoLSkKPgo+IGRpZmYgLS1naXQgYS9hcmNoL2FybTY0L2luY2x1ZGUv YXNtL3RocmVhZF9pbmZvLmggYi9hcmNoL2FybTY0L2luY2x1ZGUvYXNtL3RocmVhZF9pbmZvLmgK PiBpbmRleCA3NDBhYTAzYy4uYTJhYzkxNCAxMDA2NDQKPiAtLS0gYS9hcmNoL2FybTY0L2luY2x1 ZGUvYXNtL3RocmVhZF9pbmZvLmgKPiArKysgYi9hcmNoL2FybTY0L2luY2x1ZGUvYXNtL3RocmVh ZF9pbmZvLmgKPiBAQCAtNDcsNiArNDcsNyBAQCBzdHJ1Y3QgdGhyZWFkX2luZm8gewo+Cj4gICNk ZWZpbmUgSU5JVF9USFJFQURfSU5GTyh0c2spCQkJCQkJXAo+ICB7CQkJCQkJCQkJXAo+ICsJLmZs YWdzCQk9IF9USUZfRk9SRUlHTl9GUFNUQVRFLAkJCQlcCj4gIAkucHJlZW1wdF9jb3VudAk9IElO SVRfUFJFRU1QVF9DT1VOVCwJCQkJXAo+ICAJLmFkZHJfbGltaXQJPSBLRVJORUxfRFMsCQkJCQlc Cj4gIH0KPiBkaWZmIC0tZ2l0IGEvYXJjaC9hcm02NC9rZXJuZWwvZnBzaW1kLmMgYi9hcmNoL2Fy bTY0L2tlcm5lbC9mcHNpbWQuYwo+IGluZGV4IDNhYTEwMGEuLjEyMjI0OTEgMTAwNjQ0Cj4gLS0t IGEvYXJjaC9hcm02NC9rZXJuZWwvZnBzaW1kLmMKPiArKysgYi9hcmNoL2FybTY0L2tlcm5lbC9m cHNpbWQuYwo+IEBAIC04OTEsMzEgKzg5MSwyMSBAQCBhc21saW5rYWdlIHZvaWQgZG9fZnBzaW1k X2V4Yyh1bnNpZ25lZCBpbnQgZXNyLCBzdHJ1Y3QgcHRfcmVncyAqcmVncykKPgo+ICB2b2lkIGZw c2ltZF90aHJlYWRfc3dpdGNoKHN0cnVjdCB0YXNrX3N0cnVjdCAqbmV4dCkKPiAgewo+ICsJYm9v bCB3cm9uZ190YXNrLCB3cm9uZ19jcHU7Cj4gKwo+ICAJaWYgKCFzeXN0ZW1fc3VwcG9ydHNfZnBz aW1kKCkpCj4gIAkJcmV0dXJuOwo+IC0JLyoKPiAtCSAqIFNhdmUgdGhlIGN1cnJlbnQgRlBTSU1E IHN0YXRlIHRvIG1lbW9yeSwgYnV0IG9ubHkgaWYgd2hhdGV2ZXIgaXMgaW4KPiAtCSAqIHRoZSBy ZWdpc3RlcnMgaXMgaW4gZmFjdCB0aGUgbW9zdCByZWNlbnQgdXNlcmxhbmQgRlBTSU1EIHN0YXRl IG9mCj4gLQkgKiAnY3VycmVudCcuCj4gLQkgKi8KPiAtCWlmIChjdXJyZW50LT5tbSkKPiAtCQlm cHNpbWRfc2F2ZSgpOwo+Cj4gLQlpZiAobmV4dC0+bW0pIHsKPiAtCQkvKgo+IC0JCSAqIElmIHdl IGFyZSBzd2l0Y2hpbmcgdG8gYSB0YXNrIHdob3NlIG1vc3QgcmVjZW50IHVzZXJsYW5kCj4gLQkJ ICogRlBTSU1EIHN0YXRlIGlzIGFscmVhZHkgaW4gdGhlIHJlZ2lzdGVycyBvZiAqdGhpcyogY3B1 LAo+IC0JCSAqIHdlIGNhbiBza2lwIGxvYWRpbmcgdGhlIHN0YXRlIGZyb20gbWVtb3J5LiBPdGhl cndpc2UsIHNldAo+IC0JCSAqIHRoZSBUSUZfRk9SRUlHTl9GUFNUQVRFIGZsYWcgc28gdGhlIHN0 YXRlIHdpbGwgYmUgbG9hZGVkCj4gLQkJICogdXBvbiB0aGUgbmV4dCByZXR1cm4gdG8gdXNlcmxh bmQuCj4gLQkJICovCj4gLQkJYm9vbCB3cm9uZ190YXNrID0gX190aGlzX2NwdV9yZWFkKGZwc2lt ZF9sYXN0X3N0YXRlLnN0KSAhPQo+ICsJLyogU2F2ZSB1bnNhdmVkIGZwc2ltZCBzdGF0ZSwgaWYg YW55OiAqLwo+ICsJZnBzaW1kX3NhdmUoKTsKPiArCj4gKwkvKiBGaXggdXAgVElGX0ZPUkVJR05f RlBTVEFURSB0byBjb3JyZWN0bHkgZGVzY3JpYmUgbmV4dCdzIHN0YXRlOiAqLwo+ICsJd3Jvbmdf dGFzayA9IF9fdGhpc19jcHVfcmVhZChmcHNpbWRfbGFzdF9zdGF0ZS5zdCkgIT0KPiAgCQkJCQkm bmV4dC0+dGhyZWFkLnV3LmZwc2ltZF9zdGF0ZTsKPiAtCQlib29sIHdyb25nX2NwdSA9IG5leHQt PnRocmVhZC5mcHNpbWRfY3B1ICE9IHNtcF9wcm9jZXNzb3JfaWQoKTsKPiArCXdyb25nX2NwdSA9 IG5leHQtPnRocmVhZC5mcHNpbWRfY3B1ICE9IHNtcF9wcm9jZXNzb3JfaWQoKTsKPgo+IC0JCXVw ZGF0ZV90c2tfdGhyZWFkX2ZsYWcobmV4dCwgVElGX0ZPUkVJR05fRlBTVEFURSwKPiAtCQkJCSAg ICAgICB3cm9uZ190YXNrIHx8IHdyb25nX2NwdSk7Cj4gLQl9Cj4gKwl1cGRhdGVfdHNrX3RocmVh ZF9mbGFnKG5leHQsIFRJRl9GT1JFSUdOX0ZQU1RBVEUsCj4gKwkJCSAgICAgICB3cm9uZ190YXNr IHx8IHdyb25nX2NwdSk7Cj4gIH0KPgo+ICB2b2lkIGZwc2ltZF9mbHVzaF90aHJlYWQodm9pZCkK PiBAQCAtMTEyMCw5ICsxMTEwLDggQEAgdm9pZCBrZXJuZWxfbmVvbl9iZWdpbih2b2lkKQo+Cj4g IAlfX3RoaXNfY3B1X3dyaXRlKGtlcm5lbF9uZW9uX2J1c3ksIHRydWUpOwo+Cj4gLQkvKiBTYXZl IHVuc2F2ZWQgdGFzayBmcHNpbWQgc3RhdGUsIGlmIGFueTogKi8KPiAtCWlmIChjdXJyZW50LT5t bSkKPiAtCQlmcHNpbWRfc2F2ZSgpOwo+ICsJLyogU2F2ZSB1bnNhdmVkIGZwc2ltZCBzdGF0ZSwg aWYgYW55OiAqLwo+ICsJZnBzaW1kX3NhdmUoKTsKPgo+ICAJLyogSW52YWxpZGF0ZSBhbnkgdGFz ayBzdGF0ZSByZW1haW5pbmcgaW4gdGhlIGZwc2ltZCByZWdzOiAqLwo+ICAJZnBzaW1kX2ZsdXNo X2NwdV9zdGF0ZSgpOwo+IEBAIC0xMjQ0LDggKzEyMzMsNyBAQCBzdGF0aWMgaW50IGZwc2ltZF9j cHVfcG1fbm90aWZpZXIoc3RydWN0IG5vdGlmaWVyX2Jsb2NrICpzZWxmLAo+ICB7Cj4gIAlzd2l0 Y2ggKGNtZCkgewo+ICAJY2FzZSBDUFVfUE1fRU5URVI6Cj4gLQkJaWYgKGN1cnJlbnQtPm1tKQo+ IC0JCQlmcHNpbWRfc2F2ZSgpOwo+ICsJCWZwc2ltZF9zYXZlKCk7Cj4gIAkJZnBzaW1kX2ZsdXNo X2NwdV9zdGF0ZSgpOwo+ICAJCWJyZWFrOwo+ICAJY2FzZSBDUFVfUE1fRVhJVDoKCgotLQpBbGV4 IEJlbm7DqWUKCl9fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19f CmxpbnV4LWFybS1rZXJuZWwgbWFpbGluZyBsaXN0CmxpbnV4LWFybS1rZXJuZWxAbGlzdHMuaW5m cmFkZWFkLm9yZwpodHRwOi8vbGlzdHMuaW5mcmFkZWFkLm9yZy9tYWlsbWFuL2xpc3RpbmZvL2xp bnV4LWFybS1rZXJuZWwK From mboxrd@z Thu Jan 1 00:00:00 1970 From: alex.bennee@linaro.org (Alex =?utf-8?Q?Benn=C3=A9e?=) Date: Thu, 24 May 2018 10:19:26 +0100 Subject: [PATCH v10 07/18] arm64: fpsimd: Eliminate task->mm checks In-Reply-To: <1527005119-6842-8-git-send-email-Dave.Martin@arm.com> References: <1527005119-6842-1-git-send-email-Dave.Martin@arm.com> <1527005119-6842-8-git-send-email-Dave.Martin@arm.com> Message-ID: <87bmd5tnq9.fsf@linaro.org> To: linux-arm-kernel@lists.infradead.org List-Id: linux-arm-kernel.lists.infradead.org Dave Martin writes: > Currently the FPSIMD handling code uses the condition task->mm == > NULL as a hint that task has no FPSIMD register context. > > The ->mm check is only there to filter out tasks that cannot > possibly have FPSIMD context loaded, for optimisation purposes. > Also, TIF_FOREIGN_FPSTATE must always be checked anyway before > saving FPSIMD context back to memory. For these reasons, the ->mm > checks are not useful, providing that that TIF_FOREIGN_FPSTATE is > maintained in a consistent way for kernel threads. > > This is true by construction however: TIF_FOREIGN_FPSTATE is never > cleared except when returning to userspace or returning from a > signal: thus, for a true kernel thread no FPSIMD context is ever > loaded, TIF_FOREIGN_FPSTATE will remain set and no context will > ever be saved. > > This patch removes the redundant checks and special-case code. > > Signed-off-by: Dave Martin > Cc: Catalin Marinas > Cc: Will Deacon > Cc: Ard Biesheuvel With Christoffer's commit text: Reviewed-by: Alex Benn?e > > --- > > Changes since v9: > > * New patch. Introduced during debugging, since the ->mm checks > appear bogus and/or redundant, so are likely to be hiding or > causing bugs. > --- > arch/arm64/include/asm/thread_info.h | 1 + > arch/arm64/kernel/fpsimd.c | 38 ++++++++++++------------------------ > 2 files changed, 14 insertions(+), 25 deletions(-) > > diff --git a/arch/arm64/include/asm/thread_info.h b/arch/arm64/include/asm/thread_info.h > index 740aa03c..a2ac914 100644 > --- a/arch/arm64/include/asm/thread_info.h > +++ b/arch/arm64/include/asm/thread_info.h > @@ -47,6 +47,7 @@ struct thread_info { > > #define INIT_THREAD_INFO(tsk) \ > { \ > + .flags = _TIF_FOREIGN_FPSTATE, \ > .preempt_count = INIT_PREEMPT_COUNT, \ > .addr_limit = KERNEL_DS, \ > } > diff --git a/arch/arm64/kernel/fpsimd.c b/arch/arm64/kernel/fpsimd.c > index 3aa100a..1222491 100644 > --- a/arch/arm64/kernel/fpsimd.c > +++ b/arch/arm64/kernel/fpsimd.c > @@ -891,31 +891,21 @@ asmlinkage void do_fpsimd_exc(unsigned int esr, struct pt_regs *regs) > > void fpsimd_thread_switch(struct task_struct *next) > { > + bool wrong_task, wrong_cpu; > + > if (!system_supports_fpsimd()) > return; > - /* > - * Save the current FPSIMD state to memory, but only if whatever is in > - * the registers is in fact the most recent userland FPSIMD state of > - * 'current'. > - */ > - if (current->mm) > - fpsimd_save(); > > - if (next->mm) { > - /* > - * If we are switching to a task whose most recent userland > - * FPSIMD state is already in the registers of *this* cpu, > - * we can skip loading the state from memory. Otherwise, set > - * the TIF_FOREIGN_FPSTATE flag so the state will be loaded > - * upon the next return to userland. > - */ > - bool wrong_task = __this_cpu_read(fpsimd_last_state.st) != > + /* Save unsaved fpsimd state, if any: */ > + fpsimd_save(); > + > + /* Fix up TIF_FOREIGN_FPSTATE to correctly describe next's state: */ > + wrong_task = __this_cpu_read(fpsimd_last_state.st) != > &next->thread.uw.fpsimd_state; > - bool wrong_cpu = next->thread.fpsimd_cpu != smp_processor_id(); > + wrong_cpu = next->thread.fpsimd_cpu != smp_processor_id(); > > - update_tsk_thread_flag(next, TIF_FOREIGN_FPSTATE, > - wrong_task || wrong_cpu); > - } > + update_tsk_thread_flag(next, TIF_FOREIGN_FPSTATE, > + wrong_task || wrong_cpu); > } > > void fpsimd_flush_thread(void) > @@ -1120,9 +1110,8 @@ void kernel_neon_begin(void) > > __this_cpu_write(kernel_neon_busy, true); > > - /* Save unsaved task fpsimd state, if any: */ > - if (current->mm) > - fpsimd_save(); > + /* Save unsaved fpsimd state, if any: */ > + fpsimd_save(); > > /* Invalidate any task state remaining in the fpsimd regs: */ > fpsimd_flush_cpu_state(); > @@ -1244,8 +1233,7 @@ static int fpsimd_cpu_pm_notifier(struct notifier_block *self, > { > switch (cmd) { > case CPU_PM_ENTER: > - if (current->mm) > - fpsimd_save(); > + fpsimd_save(); > fpsimd_flush_cpu_state(); > break; > case CPU_PM_EXIT: -- Alex Benn?e