----- On Sep 15, 2020, at 2:55 PM, Peter Oskolkov [email protected] wrote: [...] > > -static int membarrier_private_expedited(int flags) > +static int membarrier_private_expedited(int flags, int cpu_id) > { > - int cpu; > cpumask_var_t tmpmask; > struct mm_struct *mm = current->mm; > + smp_call_func_t ipi_func = ipi_mb; > > - if (flags & MEMBARRIER_FLAG_SYNC_CORE) { > + if (flags == MEMBARRIER_FLAG_SYNC_CORE) { > if (!IS_ENABLED(CONFIG_ARCH_HAS_MEMBARRIER_SYNC_CORE)) > return -EINVAL; > if (!(atomic_read(&mm->membarrier_state) & > MEMBARRIER_STATE_PRIVATE_EXPEDITED_SYNC_CORE_READY)) > return -EPERM; > + } else if (flags == MEMBARRIER_FLAG_RSEQ) { > + if (!IS_ENABLED(CONFIG_RSEQ)) > + return -EINVAL; > + if (!(atomic_read(&mm->membarrier_state) & > + MEMBARRIER_STATE_PRIVATE_EXPEDITED_RSEQ_READY)) > + return -EPERM; > + ipi_func = ipi_rseq; > } else { > + BUG_ON(flags != 0);
Usually BUG_ON() is really for utterly unrecoverable situations, which is not the case here. I am tempted to just use WARN_ON_ONCE(flags) instead to make dmesg yell (once) if an unexpected flags parameter is received. This is not a user-space input, so it should never trigger unless we change the code. The rest looks good to me, thanks! Reviewed-by: Mathieu Desnoyers <[email protected]> Mathieu -- Mathieu Desnoyers EfficiOS Inc. http://www.efficios.com
