Re: [PATCH v2 0/5] mm/hwpoison: Fix regressions in memory failure handling

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 





在 2025/2/18 16:27, Borislav Petkov 写道:
On Mon, Feb 17, 2025 at 02:33:30PM +0800, Shuai Xue wrote:
changes singce v1:
- Patch 1: Fix cur_sev and sev type to `int` per Tony
- Patch 4: Fix return value to 0 for clean pages per Miaohe
- Patch 5: pick return value comments of memory-failure()

This patch addresses three regressions identified in memory failure
handling, as discovered using ras-tools[1]:

- `./einj_mem_uc copyin -f`
- `./einj_mem_uc futex -f`
- `./einj_mem_uc instr`

This is not how you write a problem statement and explain why your patches
exist.

You need to state:

1. What are you trying to do
2. What is the expected outcome and why
3. What actually happens and why
4. The fix, in your opinion, should be X or Y

Not quote some ras tools commands. Show me that you actually know what you're
doing and explain the problem in human understandable way.  And then we can
talk fixes.

Thx.


Sorry for the confusion.

1. What are you trying to do

I am tring to fix two memory failure regression in upstream kernel compared
with 5.10 LTS.

- copyin case: poison found while copying from user space.
- instr case: poison found while instruction fetching in user space

2. What is the expected outcome and why

For copyin case:

Kernel can recover from poison found while copying from user space.  MCE check
the fixup handler type to decide whether an in kernel #MC can be recovered.
When EX_TYPE_UACCESS is found, the PC jumps to recovery code specified in
_ASM_EXTABLE_FAULT() and return a -EFAULT to user space.

For instr case:

If a poison found while instruction fetching in user space, full recovery is
possible. User process takes #PF, Linux allocates a new page and fills by
reading from storage.

3. What actually happens and why

For copyin case: kernel panic since v5.17

Commit 4c132d1d844a ("x86/futex: Remove .fixup usage") introduced a new extable
fixup type, EX_TYPE_EFAULT_REG, and later patches updated the extable fixup
type for copy-from-user operations, changing it from EX_TYPE_UACCESS to
EX_TYPE_EFAULT_REG.

For instr case: user process is killed by a SIGBUS signal

Commit 046545a661af ("mm/hwpoison: fix error page recovered but reported "not
recovered"") introduced a bug that kill_accessing_process() return -EHWPOISON
for instr case, as result, kill_me_maybe() send a SIGBUS to user process.

4. The fix, in your opinion, should be X or Y

For copyin case: add EX_TYPE_EFAULT_REG as a recovery type.
For instr case: let kill_accessing_process return 0 to prevent a SIGBUS.

For patch 1 and 2:

While debuging the two regression, I found `msg` in predefined `severities`, e.g.

    MCESEV(
        AO, "Action optional: last level cache writeback error",
        SER, MASK(MCI_UC_AR|MCACOD, MCI_STATUS_UC|MCACOD_L3WB)
        ),

is helpful for me to know what kind of MCE is happened. For a fatal machine
check, kernel panic use the message and I want to extend to collect the message
and print it out for non-fatal one.

For patch 5:

The return value of memory_failure() is quite important while discussed instr
case regression with Tony and Miaohe for patch 4, so move comment to the place
it belongs to.

I hope the information provided above effectively addresses your concerns.
Please feel free to let me know if you have any further questions or need
additional clarification.

Thanks.
Shuai




[Index of Archives]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [IETF Annouce]     [Bugtraq]     [Linux OMAP]     [Linux MIPS]     [eCos]     [Asterisk Internet PBX]     [Linux API]

  Powered by Linux