Messages in this thread | ![/](/images/icornerl.gif) | | Date | Tue, 23 Apr 2019 12:23:19 +0100 | From | Dave Martin <> | Subject | Re: [PATCH v2 3/3] arm64/fpsimd: Don't disable softirq when touching FPSIMD/SVE state |
| |
On Tue, Apr 23, 2019 at 11:59:44AM +0100, Julien Grall wrote: > Hi Dave, > > On 4/17/19 3:01 PM, Dave Martin wrote: > >On Wed, Apr 17, 2019 at 12:37:57PM +0100, Julien Grall wrote: > >>Hi Dave, > >> > >>On 16/04/2019 13:30, Dave Martin wrote: > >>>On Fri, Apr 12, 2019 at 06:14:20PM +0100, Julien Grall wrote:
[...]
> >>>>@@ -1125,19 +1194,18 @@ static void fpsimd_flush_cpu_state(void) > >>>> /* > >>>> * Save the FPSIMD state to memory and invalidate cpu view. > >>>>- * This function must be called with softirqs (and preemption) disabled. > >>>>+ * This function must be called with preemption disabled. > >>>> */ > >>>> void fpsimd_save_and_flush_cpu_state(void) > >>>> { > >>>>+ __get_cpu_fpsimd_context(); > >>>> fpsimd_save(); > >>>> fpsimd_flush_cpu_state(); > >>>>+ __put_cpu_fpsimd_context(); > >>> > >>>It may be cleaner to avoid the assumption about preemption already being > >>>disabled here. fpsimd_thread_switch() is rather a special case, but for > >>>this one is this really used on a hot path that justifies the assumption? > >> > >>It is currently only called with preemption disabled. So I thought it would > >>be better to avoid disabling preemption again. But I am happy to use the > >>non-__ version if you think it is better. > > > >Hmmm, this works either way. Since this is not fast-path and has an > >external caller, it might be worth adding a WARN_ON(preemptible()) here > >if you want to stick with the __ functions. > > As it is not a fastpath, I will use the non-underscore version.
OK, either is good for me.
Cheers ---Dave
| ![\](/images/icornerr.gif) |