On Thu, Mar 18, 2021 at 04:48:43PM -0700, Sami Tolvanen wrote:

On Thu, Mar 18, 2021 at 3:29 PM Peter Zijlstra [off-list ref] wrote:

quoted

On Thu, Mar 18, 2021 at 10:10:55AM -0700, Sami Tolvanen wrote:

quoted

+static void update_shadow(struct module *mod, unsigned long base_addr,
+             update_shadow_fn fn)
+{
+     struct cfi_shadow *prev;
+     struct cfi_shadow *next;
+     unsigned long min_addr, max_addr;
+
+     next = vmalloc(SHADOW_SIZE);
+
+     mutex_lock(&shadow_update_lock);
+     prev = rcu_dereference_protected(cfi_shadow,
+                                      mutex_is_locked(&shadow_update_lock));
+
+     if (next) {
+             next->base = base_addr >> PAGE_SHIFT;
+             prepare_next_shadow(prev, next);
+
+             min_addr = (unsigned long)mod->core_layout.base;
+             max_addr = min_addr + mod->core_layout.text_size;
+             fn(next, mod, min_addr & PAGE_MASK, max_addr & PAGE_MASK);
+
+             set_memory_ro((unsigned long)next, SHADOW_PAGES);
+     }
+
+     rcu_assign_pointer(cfi_shadow, next);
+     mutex_unlock(&shadow_update_lock);
+     synchronize_rcu_expedited();

expedited is BAD(tm), why is it required and why doesn't it have a
comment?

Ah, this uses synchronize_rcu_expedited() because we have a case where
synchronize_rcu() hangs here with a specific SoC family after the
vendor's cpu_pm driver powers down CPU cores.

Broken vendor drivers seem like an exceedingly poor reason for this.

Would you say expedited is bad enough that we should avoid it here?
The function is called only when kernel modules are loaded or
unloaded, so not very frequently.

Module unload is pretty crap (it has stop_machine), so an expedited
would not really be noticable, but module load isn't nearly as bad.

Also, getting the vendor to fix their driver seems like a good thing :-)

So please consider using regular synchronize_rcu() here.

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel