Re: [PATCH v6] um: Enable preemption in UML

[Anton Ivanov]

On 22/09/2023 12:22, Johannes Berg wrote:

4. UML TLB flush is also invoked during a fork. This happens
with interrupts and preempt disabled which disagrees with the
standard mm locking via rwsem. The mm lock for this code path
had to be replaced with an rcu.

For the record, even if I figured out this gets rid of the complaints,
I'm not entirely happy with this - yeah it's safe now, but it still
feels entirely wrong.

But I guess we can also do this and then remove it entirely like the
patch I just posted. Order probably doesn't matter much.

Indeed.

We opened the can of worms. It will be a while until they are all in bigger
and more comfortable new quarters.

(Note my patch removes the locking completely since it's now invoked by
the kernel even under write mmap lock.)

Ack.

diff --git a/arch/um/kernel/fpu.c b/arch/um/kernel/fpu.c
new file mode 100644
index 000000000000..4817276b2a26
--- /dev/null
+++ b/arch/um/kernel/fpu.c

+#ifdef CONFIG_64BIT
+       if (likely(cpu_has(&boot_cpu_data, X86_FEATURE_XSAVEOPT)))
+               __builtin_ia32_xsaveopt64(&current->thread.fpu, 
KNOWN_387_FEATURES);
+       else {
+               if (likely(cpu_has(&boot_cpu_data, X86_FEATURE_XSAVE)))
+                       __builtin_ia32_xsave64(&current->thread.fpu, 
KNOWN_387_FEATURES);
+               else
+                       __builtin_ia32_fxsave64(&current->thread.fpu);

:)

OK, I'll stop mentioning it ;-)

+EXPORT_SYMBOL_GPL(kernel_fpu_end);
+

git am complained here about blank line at EOF

@@ -597,8 +609,13 @@ void force_flush_all(void)
        struct vm_area_struct *vma;
        VMA_ITERATOR(vmi, mm, 0);

- mmap_read_lock(mm);

+       /* We use a RCU lock instead of a mm lock, because
+        * this can be invoked out of critical/atomic sections
+        * and that does not agree with the sleepable semantics
+        * of the standard semaphore based mm lock.
+        */
+       rcu_read_lock();

Yeah I guess ... Seems like a very mechanical description of what's
going on, rather than a description of why this is correct (which
assumes no preempt and no SMP)?

I'd have preferred that, but with the patch I just posted we'll just
kill this entirely so it doesn't matter in the end.

johannes

--
Anton R. Ivanov
Cambridgegreys Limited. Registered in England. Company Number 10273661
https://www.cambridgegreys.com/

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