Re: Question about SEA handling process happened in user space

[James Morse <james.morse@arm.com>]

Hi Xiaofei,

On 09/04/2020 10:17, Xiaofei Tan wrote:
> On 2020/4/8 0:37, James Morse wrote:
>> On 02/04/2020 07:35, Xiaofei Tan wrote:
>>> On 2020/3/31 0:49, James Morse wrote:
>>>> If the CPU doesn't tell us the address, we can't tell user-space what it is. The
>>>> alternative is to upgrade to SIGKILL in that case.
>>>>
>>>>
>>>> If you see this instead of the address provided via firmware-first, there is a
>>>> series to improve that here:
>>>> https://lore.kernel.org/linux-acpi/20200228174817.74278-1-james.morse@arm.com/
>>>>
>>>> (We skip this signal code of APEI promises it did all the work. This lets you
>>>> take the signal from memory_failure() instead, which may have better information.)
>>
>>> There may be an competition issue.
>>> APEI run memory_failure() in an bottom half for memory errors. Then it may be not finished
>>> before here SEA handling end, and application process may back to run.

>> With that series, it runs in process-context as task-work. memory_failure() needs to
>> sleep, so it has to run in process-context. 
> 
> 
>> Doing it as task-work means it runs before the thread returns to user-space.
> 
> Sorry, i don't understand this. i thought the task-work need to reschedule, and current thread should
> have returned to user-space before it.

ret_to_user has a loop around do_notify_resume(), if the _TIF_NOTIFY_RESUME flag is set
and we call tracehook_notify_resume() which ends up in task_work_run()...

That TIF flag effectively prevents this thread returning to user-space until that task
work has run.


> BTW, What context synchronous exception abort is? I thought it was process-context.

It depends what you interrupted.
32bit had different CPU modes for different contexts, we don't have that in 64bit. Instead
we mask asynchronous interrupts, and tinker with the preempt count to track the context.
Synchronous exceptions can't be masked, so they happen in whatever context you were
already in.
This means the exception handlers have to be be prepared for each eventuality.
(which is why that code is starting to look complex)


> Because in_interrupt() return false called in do_sea().

If you took the exception from EL0, or EL1 process context, yes. If you took the exception
from an IRQ handler, in_interrupt() would return true.


>> If another thread in the same process accesses the affected memory, I'd expect to take a
>> second external abort. If another process had the page mapped, it could access the
>> affected memory, again taking an external abort.

> Yes, it is hard to avoid another thread to access the affected memory.
> I just worry the same thread access it again.

This is the race that that series fixes.
It can't happen with mainline as the arch code unconditionally signals the affected
process, which was the pre-RAS behaviour.

>> These two could happen while the first CPU was in firmware generating the CPER records, so
>> its not a race we can fix. It should be harmless, the recovery action is the same, its
>> just the error counters that count more events than errors. If you actually see it happen,
>> we can try and make it smaller...

> Hmm, maybe this double SEA handling is an solution.

It assumes you get a second external-abort. We know this thread is affected, and will try
and consume the error again if we restart it. We shouldn't restart it until we've given
the recovery our best shot.
Letting it loose is a poor choice if you have any kind of threshold for error-counts. They
may jump NR_CPUs at a time until every CPU is waiting in memory_failure()...


Thanks,

James

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