From: Ira Weiny <ira.weiny@intel.com> To: Dave Hansen <dave.hansen@intel.com> Cc: Thomas Gleixner <tglx@linutronix.de>, Ingo Molnar <mingo@redhat.com>, Borislav Petkov <bp@alien8.de>, Andy Lutomirski <luto@kernel.org>, Peter Zijlstra <peterz@infradead.org>, Dave Hansen <dave.hansen@linux.intel.com>, Fenghua Yu <fenghua.yu@intel.com>, x86@kernel.org, linux-kernel@vger.kernel.org, Andrew Morton <akpm@linux-foundation.org>, linux-doc@vger.kernel.org, linux-nvdimm@lists.01.org, linux-mm@kvack.org, linux-kselftest@vger.kernel.org, Greg KH <gregkh@linuxfoundation.org> Subject: Re: [PATCH V3 10/10] x86/pks: Add PKS test code Date: Thu, 17 Dec 2020 20:05:09 -0800 [thread overview] Message-ID: <20201218040509.GD1563847@iweiny-DESK2.sc.intel.com> (raw) In-Reply-To: <570ead2a-ff41-e730-d61d-0f59c67b1903@intel.com> On Thu, Dec 17, 2020 at 12:55:39PM -0800, Dave Hansen wrote: > On 11/6/20 3:29 PM, ira.weiny@intel.com wrote: > > + /* Arm for context switch test */ > > + write(fd, "1", 1); > > + > > + /* Context switch out... */ > > + sleep(4); > > + > > + /* Check msr restored */ > > + write(fd, "2", 1); > > These are always tricky. What you ideally want here is: > > 1. Switch away from this task to a non-PKS task, or > 2. Switch from this task to a PKS-using task, but one which has a > different PKS value Or both... > > then, switch back to this task and make sure PKS maintained its value. > > *But*, there's no absolute guarantee that another task will run. It > would not be totally unreasonable to have the kernel just sit in a loop > without context switching here if no other tasks can run. > > The only way you *know* there is a context switch is by having two tasks > bound to the same logical CPU and make sure they run one after another. Ah... We do that. ... + CPU_ZERO(&cpuset); + CPU_SET(0, &cpuset); + /* Two processes run on CPU 0 so that they go through context switch. */ + sched_setaffinity(getpid(), sizeof(cpu_set_t), &cpuset); ... I think this should be ensuring that both the parent and the child are running on CPU 0. At least according to the man page they should be. <man> A child created via fork(2) inherits its parent's CPU affinity mask. </man> Perhaps a better method would be to synchronize the 2 threads more to ensure that we are really running at the 'same time' and forcing the context switch. > This just gets itself into a state where it *CAN* context switch and > prays that one will happen. Not sure what you mean by 'This'? Do you mean that running on the same CPU will sometimes not force a context switch? Or do you mean that the sleeps could be badly timed and the 2 threads could run 1 after the other on the same CPU? The latter is AFAICT the most likely case. > > You can also run a bunch of these in parallel bound to a single CPU. > That would also give you higher levels of assurance that *some* context > switch happens at sleep(). I think more cycles is a good idea for sure. But I'm more comfortable with forcing the test to be more synchronized so that it is actually running in the order we think/want it to be. > > One critical thing with these tests is to sabotage the kernel and then > run them and make *sure* they fail. Basically, if you screw up, do they > actually work to catch it? I'll try and come up with a more stressful test. Ira _______________________________________________ Linux-nvdimm mailing list -- linux-nvdimm@lists.01.org To unsubscribe send an email to linux-nvdimm-leave@lists.01.org
WARNING: multiple messages have this Message-ID (diff)
From: Ira Weiny <ira.weiny@intel.com> To: Dave Hansen <dave.hansen@intel.com> Cc: Thomas Gleixner <tglx@linutronix.de>, Ingo Molnar <mingo@redhat.com>, Borislav Petkov <bp@alien8.de>, Andy Lutomirski <luto@kernel.org>, Peter Zijlstra <peterz@infradead.org>, Dave Hansen <dave.hansen@linux.intel.com>, Fenghua Yu <fenghua.yu@intel.com>, x86@kernel.org, linux-kernel@vger.kernel.org, Andrew Morton <akpm@linux-foundation.org>, linux-doc@vger.kernel.org, linux-nvdimm@lists.01.org, linux-mm@kvack.org, linux-kselftest@vger.kernel.org, Dan Williams <dan.j.williams@intel.com>, Greg KH <gregkh@linuxfoundation.org> Subject: Re: [PATCH V3 10/10] x86/pks: Add PKS test code Date: Thu, 17 Dec 2020 20:05:09 -0800 [thread overview] Message-ID: <20201218040509.GD1563847@iweiny-DESK2.sc.intel.com> (raw) In-Reply-To: <570ead2a-ff41-e730-d61d-0f59c67b1903@intel.com> On Thu, Dec 17, 2020 at 12:55:39PM -0800, Dave Hansen wrote: > On 11/6/20 3:29 PM, ira.weiny@intel.com wrote: > > + /* Arm for context switch test */ > > + write(fd, "1", 1); > > + > > + /* Context switch out... */ > > + sleep(4); > > + > > + /* Check msr restored */ > > + write(fd, "2", 1); > > These are always tricky. What you ideally want here is: > > 1. Switch away from this task to a non-PKS task, or > 2. Switch from this task to a PKS-using task, but one which has a > different PKS value Or both... > > then, switch back to this task and make sure PKS maintained its value. > > *But*, there's no absolute guarantee that another task will run. It > would not be totally unreasonable to have the kernel just sit in a loop > without context switching here if no other tasks can run. > > The only way you *know* there is a context switch is by having two tasks > bound to the same logical CPU and make sure they run one after another. Ah... We do that. ... + CPU_ZERO(&cpuset); + CPU_SET(0, &cpuset); + /* Two processes run on CPU 0 so that they go through context switch. */ + sched_setaffinity(getpid(), sizeof(cpu_set_t), &cpuset); ... I think this should be ensuring that both the parent and the child are running on CPU 0. At least according to the man page they should be. <man> A child created via fork(2) inherits its parent's CPU affinity mask. </man> Perhaps a better method would be to synchronize the 2 threads more to ensure that we are really running at the 'same time' and forcing the context switch. > This just gets itself into a state where it *CAN* context switch and > prays that one will happen. Not sure what you mean by 'This'? Do you mean that running on the same CPU will sometimes not force a context switch? Or do you mean that the sleeps could be badly timed and the 2 threads could run 1 after the other on the same CPU? The latter is AFAICT the most likely case. > > You can also run a bunch of these in parallel bound to a single CPU. > That would also give you higher levels of assurance that *some* context > switch happens at sleep(). I think more cycles is a good idea for sure. But I'm more comfortable with forcing the test to be more synchronized so that it is actually running in the order we think/want it to be. > > One critical thing with these tests is to sabotage the kernel and then > run them and make *sure* they fail. Basically, if you screw up, do they > actually work to catch it? I'll try and come up with a more stressful test. Ira
next prev parent reply other threads:[~2020-12-18 4:05 UTC|newest] Thread overview: 100+ messages / expand[flat|nested] mbox.gz Atom feed top 2020-11-06 23:28 [PATCH V3 00/10] PKS: Add Protection Keys Supervisor (PKS) support V3 ira.weiny 2020-11-06 23:28 ` ira.weiny 2020-11-06 23:28 ` [PATCH V3 01/10] x86/pkeys: Create pkeys_common.h ira.weiny 2020-11-06 23:28 ` ira.weiny 2020-11-06 23:29 ` [PATCH V3 02/10] x86/fpu: Refactor arch_set_user_pkey_access() for PKS support ira.weiny 2020-11-06 23:29 ` ira.weiny 2020-11-06 23:29 ` [PATCH V3 03/10] x86/pks: Add PKS defines and Kconfig options ira.weiny 2020-11-06 23:29 ` ira.weiny 2020-11-06 23:29 ` [PATCH V3 04/10] x86/pks: Preserve the PKRS MSR on context switch ira.weiny 2020-11-06 23:29 ` ira.weiny 2020-12-17 14:50 ` Thomas Gleixner 2020-12-17 14:50 ` Thomas Gleixner 2020-12-17 22:43 ` Thomas Gleixner 2020-12-17 22:43 ` Thomas Gleixner 2020-12-18 13:57 ` Thomas Gleixner 2020-12-18 13:57 ` Thomas Gleixner 2020-12-18 19:20 ` Dan Williams 2020-12-18 19:20 ` Dan Williams 2020-12-18 19:20 ` Dan Williams 2020-12-18 21:06 ` Thomas Gleixner 2020-12-18 21:06 ` Thomas Gleixner 2020-12-18 21:58 ` Dan Williams 2020-12-18 21:58 ` Dan Williams 2020-12-18 21:58 ` Dan Williams 2020-12-18 22:44 ` Thomas Gleixner 2020-12-18 22:44 ` Thomas Gleixner 2020-12-18 19:42 ` Ira Weiny 2020-12-18 19:42 ` Ira Weiny 2020-12-18 20:10 ` Dave Hansen 2020-12-18 20:10 ` Dave Hansen 2020-12-18 21:30 ` Thomas Gleixner 2020-12-18 21:30 ` Thomas Gleixner 2020-12-18 4:05 ` Ira Weiny 2020-12-18 4:05 ` Ira Weiny 2020-12-17 20:41 ` [NEEDS-REVIEW] " Dave Hansen 2020-12-17 20:41 ` Dave Hansen 2020-12-18 4:10 ` Ira Weiny 2020-12-18 4:10 ` Ira Weiny 2020-12-18 15:33 ` Dave Hansen 2020-12-18 15:33 ` Dave Hansen 2020-11-06 23:29 ` [PATCH V3 05/10] x86/entry: Pass irqentry_state_t by reference ira.weiny 2020-11-06 23:29 ` ira.weiny 2020-11-15 18:58 ` Thomas Gleixner 2020-11-15 18:58 ` Thomas Gleixner 2020-11-16 18:49 ` Ira Weiny 2020-11-16 18:49 ` Ira Weiny 2020-11-16 20:36 ` Thomas Gleixner 2020-11-16 20:36 ` Thomas Gleixner 2020-11-24 6:09 ` [PATCH V3.1] entry: " ira.weiny 2020-11-24 6:09 ` ira.weiny 2020-12-11 22:14 ` Andy Lutomirski 2020-12-11 22:14 ` Andy Lutomirski 2020-12-11 22:14 ` Andy Lutomirski 2020-12-16 1:32 ` Ira Weiny 2020-12-16 1:32 ` Ira Weiny 2020-12-16 1:32 ` Ira Weiny 2020-12-16 2:09 ` Andy Lutomirski 2020-12-16 2:09 ` Andy Lutomirski 2020-12-16 2:09 ` Andy Lutomirski 2020-12-17 0:38 ` Ira Weiny 2020-12-17 0:38 ` Ira Weiny 2020-12-17 0:38 ` Ira Weiny 2020-12-17 13:07 ` Thomas Gleixner 2020-12-17 13:07 ` Thomas Gleixner 2020-12-17 13:07 ` Thomas Gleixner 2020-12-17 13:19 ` Peter Zijlstra 2020-12-17 13:19 ` Peter Zijlstra 2020-12-17 13:19 ` Peter Zijlstra 2020-12-17 15:35 ` Andy Lutomirski 2020-12-17 15:35 ` Andy Lutomirski 2020-12-17 15:35 ` Andy Lutomirski 2020-12-17 16:58 ` Thomas Gleixner 2020-12-17 16:58 ` Thomas Gleixner 2020-11-06 23:29 ` [PATCH V3 06/10] x86/entry: Preserve PKRS MSR across exceptions ira.weiny 2020-11-06 23:29 ` ira.weiny 2020-12-17 15:28 ` Thomas Gleixner 2020-12-17 15:28 ` Thomas Gleixner 2020-11-06 23:29 ` [PATCH V3 07/10] x86/fault: Report the PKRS state on fault ira.weiny 2020-11-06 23:29 ` ira.weiny 2020-11-06 23:29 ` [PATCH V3 08/10] x86/pks: Add PKS kernel API ira.weiny 2020-11-06 23:29 ` ira.weiny 2020-12-23 20:39 ` Randy Dunlap 2020-12-23 20:39 ` Randy Dunlap 2020-11-06 23:29 ` [PATCH V3 09/10] x86/pks: Enable Protection Keys Supervisor (PKS) ira.weiny 2020-11-06 23:29 ` ira.weiny 2020-11-06 23:29 ` [PATCH V3 10/10] x86/pks: Add PKS test code ira.weiny 2020-11-06 23:29 ` ira.weiny 2020-12-17 20:55 ` Dave Hansen 2020-12-17 20:55 ` Dave Hansen 2020-12-18 4:05 ` Ira Weiny [this message] 2020-12-18 4:05 ` Ira Weiny 2020-12-18 16:59 ` Dan Williams 2020-12-18 16:59 ` Dan Williams 2020-12-18 16:59 ` Dan Williams 2020-12-07 22:14 ` [PATCH V3 00/10] PKS: Add Protection Keys Supervisor (PKS) support V3 Ira Weiny 2020-12-07 22:14 ` Ira Weiny 2020-12-08 15:55 ` Thomas Gleixner 2020-12-08 15:55 ` Thomas Gleixner 2020-12-08 17:22 ` Ira Weiny 2020-12-08 17:22 ` Ira Weiny
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=20201218040509.GD1563847@iweiny-DESK2.sc.intel.com \ --to=ira.weiny@intel.com \ --cc=akpm@linux-foundation.org \ --cc=bp@alien8.de \ --cc=dave.hansen@intel.com \ --cc=dave.hansen@linux.intel.com \ --cc=fenghua.yu@intel.com \ --cc=gregkh@linuxfoundation.org \ --cc=linux-doc@vger.kernel.org \ --cc=linux-kernel@vger.kernel.org \ --cc=linux-kselftest@vger.kernel.org \ --cc=linux-mm@kvack.org \ --cc=linux-nvdimm@lists.01.org \ --cc=luto@kernel.org \ --cc=mingo@redhat.com \ --cc=peterz@infradead.org \ --cc=tglx@linutronix.de \ --cc=x86@kernel.org \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.