Re: [PATCH v2 3/3] drivers/base/memory: fix locking for poison accounting lookup

[Muchun Song]

> On Apr 29, 2026, at 11:32, Oscar Salvador <osalvador@xxxxxxx> wrote:
> 
> On Wed, Apr 29, 2026 at 11:08:51AM +0800, Miaohe Lin wrote:
>> Right, I missed that. Thanks. But I'm still worried that there might be potential issues.
>> For example, this function could be called while lock_page is held. Acquiring lock_device_hotplug
>> while already holding lock_page might cause problems, though I haven't seen any specific issues yet.
>> Also there might be some other potential scenarios that haven't been considered. Hope I'm just
>> overthinking it. :)
> 
> lock_device_hotplug is a mutex lock, and we already take other mutex locks while
> holding lock_folio in other paths, so I am not sure I see what should be special
> in this case.

Hi Oscar and Miaohe,

I saw sashiko's report [1] related to folio lock and lock_device_hotplug.
Seems it is possible. You can correct me if I am wrong.

[1] https://sashiko.dev/#/patchset/20260428085219.1316047-1-songmuchun%40bytedance.com

We could fix this by calling action_result() without holding folio lock.
What do you think?

diff --git a/mm/memory-failure.c b/mm/memory-failure.c
index ee42d4361309..bc76066547ce 100644
--- a/mm/memory-failure.c
+++ b/mm/memory-failure.c
@@ -2534,8 +2534,9 @@ int memory_failure(unsigned long pfn, int flags)
         * Abort on fail: __filemap_remove_folio() assumes unmapped page.
         */
        if (!hwpoison_user_mappings(folio, p, pfn, flags)) {
+               folio_unlock(folio);
                res = action_result(pfn, MF_MSG_UNMAP_FAILED, MF_FAILED);
-               goto unlock_page;
+               goto unlock_mutex;
        }

        /*
@@ -2543,16 +2544,15 @@ int memory_failure(unsigned long pfn, int flags)
         */
        if (folio_test_lru(folio) && !folio_test_swapcache(folio) &&
            folio->mapping == NULL) {
+               folio_unlock(folio);
                res = action_result(pfn, MF_MSG_TRUNCATED_LRU, MF_IGNORED);
-               goto unlock_page;
+               goto unlock_mutex;
        }

 identify_page_state:
        res = identify_page_state(pfn, p, page_flags);
        mutex_unlock(&mf_mutex);
        return res;
-unlock_page:
-       folio_unlock(folio);
 unlock_mutex:
        mutex_unlock(&mf_mutex);
        return res;

> 
> 
> -- 
> Oscar Salvador
> SUSE Labs