[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/