Re: [uml-devel] [PATCH] Fix for "occasional userspace process in D/Z state" bug

[Anton Ivanov <anton.ivanov@kot-begemot.co.uk>]

On 26/09/14 12:56, Richard Weinberger wrote:
> Am 26.09.2014 13:49, schrieb anton.ivanov@kot-begemot.co.uk:
>> From: Anton Ivanov <antivano@cisco.com>
>>
>> This is a fix for a very old UML bug which can be triggered with stock 
>> UML. It takes a lot of effort to trigger it there because the 
>> lseek()/read() | write() mechanics of the UBD driver implicitly sync the 
>> memory all the time by hitting the appropriate barrier implementation in 
>> the host kernel. 
>>
>> By improving the disk susbsystem we make this bug raise its ugly head
>> with a vengeance - you can get a process in D (with an occasional child
>> in Z state) simply by running an apt-get on 30-40 large packages. 
>>
>> Is this correct place to have the sync - no idea. It may need to move
>> to somewhere inside tlb.c. With the fence in exec.c it works (TM).
>>
>> If I understand this correctly, this also needs to be an instruction 
>> appropriate for the underlying host so just a barrier() will not cut 
>> it. You have to fence. En-guarde... Touche... :)
>>
>> Signed-off-by: Anton Ivanov <antivano@cisco.com>
>> ---
>>  arch/um/kernel/exec.c |    5 +++++
>>  1 file changed, 5 insertions(+)
>>
>> diff --git a/arch/um/kernel/exec.c b/arch/um/kernel/exec.c
>> index 0d7103c..7cb6805 100644
>> --- a/arch/um/kernel/exec.c
>> +++ b/arch/um/kernel/exec.c
>> @@ -27,6 +27,11 @@ void flush_thread(void)
>>  	ret = unmap(&current->mm->context.id, 0, STUB_START, 0, &data);
>>  	ret = ret || unmap(&current->mm->context.id, STUB_END,
>>  			   host_task_size - STUB_END, 1, &data);
>> +#ifdef CONFIG_X86_32
>> +	alternative("lock; addl $0,0(%%esp)", "mfence", X86_FEATURE_XMM2);
>> +#else
>> +	asm volatile("mfence":::"memory");
>> +#endif
> Why not mb()?
> I'm not sure whether this fix is correct.

As I said before - neither am I.

I have tried to narrow it down and trace it - the right place is
probably somewhere further downstream in tlb.c (or even further
downstream from that).

While this place (exec.c) is probably not the best place,  it improves
things quite a bit (I am not 100% sure it covers all failure cases).

There is a need to find it and fix it though. You start hitting the bug
quite a lot, the moment you start changing older calls which implicitly
synchronize memory by hitting a mb() in the host (lseek, fsync, etc) for
calls that do not do that.

I did not see that during my initial testing, because I always pin UML
to a single core for performance reasons.

IMHO long term performance improvement of UML depends on finding and
fixing the root cause for this. I will try to look at it during whatever
spare time I have in the next few months.  Without a viable fix, you
cannot do something as trivial as sorting out the disk subsystem (the
pwrite/pread/async-fsync fixes are from the realm of bleeding obvious,
they should not create any races by themselves).

A
>
> Thanks,
> //richard


------------------------------------------------------------------------------
Meet PCI DSS 3.0 Compliance Requirements with EventLog Analyzer
Achieve PCI DSS 3.0 Compliant Status with Out-of-the-box PCI DSS Reports
Are you Audit-Ready for PCI DSS 3.0 Compliance? Download White paper
Comply to PCI DSS 3.0 Requirement 10 and 11.5 with EventLog Analyzer
http://pubads.g.doubleclick.net/gampad/clk?id=154622311&iu=/4140/ostg.clktrk
_______________________________________________
User-mode-linux-devel mailing list
User-mode-linux-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/user-mode-linux-devel