Messages in this thread | | | Date | Thu, 17 Feb 2022 18:33:07 +0100 | From | Sebastian Andrzej Siewior <> | Subject | Re: [PATCH v3] random: remove batched entropy locking |
| |
On 2022-02-16 21:58:14 [+0100], Jason A. Donenfeld wrote: > > Why raw_cpu_ptr? include/linux/percpu-defs.h says about raw_*() operations: > > > > * Operations for contexts where we do not want to do any checks for > > * preemptions. Unless strictly necessary, always use [__]this_cpu_*() > > * instead. > > > > So when I see a raw_*() percpu thing, I read it as "it is expected > > that we can migrate after this point (or we're in some really weird > > context where the normal context check doesn't work)". Is that > > incorrect? > > If it says "contexts where we do not want to do any checks for > preemptions", then that would apply here I would think? We're taking a > local lock, which means afterwards there are no preemptions. For > context, the code that got committed after Sebastian's final review > is: > > local_lock_irqsave(&batched_entropy_u32.lock, flags); > batch = raw_cpu_ptr(&batched_entropy_u32); > > However, I think most other code uses this_cpu_ptr() instead? So not sure.
It depends what you are looking for. raw_cpu_ptr(&batched_entropy_u32) give you the pointer to &batched_entropy_u32 of _this_ CPU - the CPU you are currently running. this_cpu_ptr() does the same except that it has smp_processor_id() in it. smp_processor_id() will yell at you (given you enabled CONFIG_DEBUG_PREEMPT) if the code can migrate to another CPU. So it will yell at you unless: - you disable preemption / migration so code can't migrate - you run on a per-CPU thread i.e. a thread which is bound to a single CPU and therefore can't migrate to another.
The local_lock_irqsave() acquires a lock / disables interrupt and therefore can't migrate. So I suggested to use raw_cpu_ptr() as an optimisation in the debug-case.
If you disable preemption before accessing a per-CPU variable with this_cpu_ptr() then you _need_ to ensure that nobody is accessing the variable from in-IRQ context. Nobody will yell at you. But if you use a local-lock then you get lockdep annotation and lockdep will complain if you use that variable always in process context and sometimes in-IRQ.
> Jason
Sebastian
| |