On Thu, Aug 17, 2017 at 04:35:55PM -0700, Ram Pai wrote: > On Wed, Aug 02, 2017 at 07:40:46PM +1000, Michael Ellerman wrote: > > Thiago Jung Bauermann <bauerman@xxxxxxxxxxxxxxxxxx> writes: > > > > > Michael Ellerman <mpe@xxxxxxxxxxxxxx> writes: > > > > > >> Thiago Jung Bauermann <bauerman@xxxxxxxxxxxxxxxxxx> writes: > > >>> Ram Pai <linuxram@xxxxxxxxxx> writes: > > >> ... > > >>>> + > > >>>> + /* We got one, store it and use it from here on out */ > > >>>> + if (need_to_set_mm_pkey) > > >>>> + mm->context.execute_only_pkey = execute_only_pkey; > > >>>> + return execute_only_pkey; > > >>>> +} > > >>> > > >>> If you follow the code flow in __execute_only_pkey, the AMR and UAMOR > > >>> are read 3 times in total, and AMR is written twice. IAMR is read and > > >>> written twice. Since they are SPRs and access to them is slow (or isn't > > >>> it?), > > >> > > >> SPRs read/writes are slow, but they're not *that* slow in comparison to > > >> a system call (which I think is where this code is being called?). > > > > > > Yes, this code runs on mprotect and mmap syscalls if the memory is > > > requested to have execute but not read nor write permissions. > > > > Yep. That's not in the fast path for key usage, ie. the fast path is > > userspace changing the AMR itself, and the overhead of a syscall is > > already hundreds of cycles. > > > > >> So we should try to avoid too many SPR read/writes, but at the same time > > >> we can accept more than the minimum if it makes the code much easier to > > >> follow. > > > > > > Ok. Ram had asked me to suggest a way to optimize the SPR reads and > > > writes and I came up with the patch below. Do you think it's worth it? > > > > At a glance no I don't think it is. Sorry you spent that much time on it. > > > > I think we can probably reduce the number of SPR accesses without > > needing to go to that level of complexity. > > > > But don't throw the patch away, I may eat my words once I have the full > > series applied and am looking at it hard - at the moment I'm just > > reviewing the patches piecemeal as I get time. > Thiago's patch does save some cycles. I dont feel like throwing his work. I agree, It should be considered after applying all the patches. RP -- Ram Pai -- To unsubscribe from this list: send the line "unsubscribe linux-kselftest" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html