Re: [PATCH v5 2/5] mm: memory_failure: Allow truncate_error_folio to return MF_DELAYED

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] AB-BA deadlock between `folio_lock` and `mapping->invalidate_lock` involving the memory failure handler and guest_memfd file truncation.
--

--- Patch [2]: [PATCH v5 2/5] mm: memory_failure: Allow truncate_error_folio to return MF_DELAYED ---
commit 1e7b00ec8e2bc22a20c74d21aa034f3f7f88aba8
Author: Lisa Wang <[email protected]>
Subject: mm: memory_failure: Allow truncate_error_folio to return MF_DELAYED

This commit modifies truncate_error_folio() to check if the
error_remove_folio callback returns MF_DELAYED. If it does, the
function now propagates MF_DELAYED back to the caller instead of
treating it as an error.

> diff --git a/mm/memory-failure.c b/mm/memory-failure.c
> --- a/mm/memory-failure.c
> +++ b/mm/memory-failure.c
> @@ -939,10 +939,12 @@ static int truncate_error_folio(struct folio *folio, unsigned long pfn,
>  	if (mapping->a_ops->error_remove_folio) {
>  		int err = mapping->a_ops->error_remove_folio(mapping, folio);

[Severity: High]
This is a pre-existing issue, but can this callback trigger an AB-BA deadlock
between the folio lock and the mapping invalidate_lock?

The memory failure path acquires the folio lock before reaching this point:

memory_failure()
  folio_lock(folio);
  ...
  me_pagecache_clean()
    truncate_error_folio()

When the callback is kvm_gmem_error_folio(), it then acquires the
invalidate_lock for read:

virt/kvm/guest_memfd.c:kvm_gmem_error_folio() {
    ...
    filemap_invalidate_lock_shared(mapping);
    ...
}

However, the file truncation path acquires these locks in the reverse order.
It acquires the invalidate_lock for write before taking the folio lock:

virt/kvm/guest_memfd.c:kvm_gmem_punch_hole() {
    ...
    filemap_invalidate_lock(inode->i_mapping);
    kvm_gmem_invalidate_start(inode, start, end);
    truncate_inode_pages_range(inode->i_mapping, offset, ...);
}

Since truncate_inode_pages_range() acquires the folio lock, could a
concurrent fallocate hole punch and a memory failure on the same guest_memfd
file lead to a system-wide deadlock?

> -		if (err != 0)
> +		if (err == MF_DELAYED)
> +			ret = err;
> +		else if (err != 0)
>  			pr_info("%#lx: Failed to punch page: %d\n", pfn, err);
[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260823-memory-failure-mf-delayed-fix-v5-0-d97a5703e45a@google.com?part=2