lkml.org 
[lkml]   [2018]   [Apr]   [3]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH 2/2] smp: introduce kick_active_cpus_sync()
Hi Yury,

On Sun, Apr 01, 2018 at 02:11:08PM +0300, Yury Norov wrote:
> +/*
> + * Flush I-cache if CPU is in extended quiescent state
> + */

This comment is misleading. An ISB doesn't touch the I-cache; it forces
a context synchronization event.

> + .macro isb_if_eqs
> +#ifndef CONFIG_TINY_RCU
> + bl rcu_is_watching
> + tst w0, #0xff
> + b.ne 1f

The TST+B.NE can be a CBNZ:

bl rcu_is_watching
cbnz x0, 1f
isb
1:

> + /* Pairs with aarch64_insn_patch_text for EQS CPUs. */
> + isb
> +1:
> +#endif
> + .endm
> +
> el0_sync_invalid:
> inv_entry 0, BAD_SYNC
> ENDPROC(el0_sync_invalid)
> @@ -840,8 +861,10 @@ el0_svc:
> mov wsc_nr, #__NR_syscalls
> el0_svc_naked: // compat entry point
> stp x0, xscno, [sp, #S_ORIG_X0] // save the original x0 and syscall number
> + isb_if_eqs
> enable_dbg_and_irq
> - ct_user_exit 1
> + ct_user_exit

I don't think this is safe. here we issue the ISB *before* exiting a
quiesecent state, so I think we can race with another CPU that calls
kick_all_active_cpus_sync, e.g.

CPU0 CPU1

ISB
patch_some_text()
kick_all_active_cpus_sync()
ct_user_exit

// not synchronized!
use_of_patched_text()

... and therefore the ISB has no effect, which could be disasterous.

I believe we need the ISB *after* we transition into a non-quiescent
state, so that we can't possibly miss a context synchronization event.

Thanks,
Mark.

\
 
 \ /
  Last update: 2018-04-03 15:49    [W:0.095 / U:3.376 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site