* [PATCH 1/3] mm: memory-failure: make put_ref_page() more useful @ 2022-10-21 8:46 Kefeng Wang 2022-10-21 8:46 ` [PATCH 2/3] mm: memory-failure: avoid pfn_valid() twice in soft_offline_page() Kefeng Wang ` (3 more replies) 0 siblings, 4 replies; 12+ messages in thread From: Kefeng Wang @ 2022-10-21 8:46 UTC (permalink / raw) To: Naoya Horiguchi, Andrew Morton Cc: Miaohe Lin, linux-mm, linux-kernel, Kefeng Wang Pass pfn/flags to put_ref_page(), then check MF_COUNT_INCREASED and drop refcount to make the code look cleaner. Signed-off-by: Kefeng Wang <wangkefeng.wang@huawei.com> --- mm/memory-failure.c | 34 +++++++++++++++++----------------- 1 file changed, 17 insertions(+), 17 deletions(-) diff --git a/mm/memory-failure.c b/mm/memory-failure.c index bead6bccc7f2..b94152abb1c9 100644 --- a/mm/memory-failure.c +++ b/mm/memory-failure.c @@ -1913,17 +1913,25 @@ static inline unsigned long free_raw_hwp_pages(struct page *hpage, bool flag) } #endif /* CONFIG_HUGETLB_PAGE */ +/* Drop the extra refcount in case we come from madvise() */ +static void put_ref_page(unsigned long pfn, int flags) +{ + struct page *page; + + if (!(flags & MF_COUNT_INCREASED)) + return; + + page = pfn_to_page(pfn); + if (page) + put_page(page); +} + static int memory_failure_dev_pagemap(unsigned long pfn, int flags, struct dev_pagemap *pgmap) { - struct page *page = pfn_to_page(pfn); int rc = -ENXIO; - if (flags & MF_COUNT_INCREASED) - /* - * Drop the extra refcount in case we come from madvise(). - */ - put_page(page); + put_ref_page(pfn, flags); /* device metadata space is not recoverable */ if (!pgmap_pfn_valid(pgmap, pfn)) @@ -2516,12 +2524,6 @@ static int soft_offline_in_use_page(struct page *page) return ret; } -static void put_ref_page(struct page *page) -{ - if (page) - put_page(page); -} - /** * soft_offline_page - Soft offline a page. * @pfn: pfn to soft-offline @@ -2550,19 +2552,17 @@ int soft_offline_page(unsigned long pfn, int flags) { int ret; bool try_again = true; - struct page *page, *ref_page = NULL; + struct page *page; WARN_ON_ONCE(!pfn_valid(pfn) && (flags & MF_COUNT_INCREASED)); if (!pfn_valid(pfn)) return -ENXIO; - if (flags & MF_COUNT_INCREASED) - ref_page = pfn_to_page(pfn); /* Only online pages can be soft-offlined (esp., not ZONE_DEVICE). */ page = pfn_to_online_page(pfn); if (!page) { - put_ref_page(ref_page); + put_ref_page(pfn, flags); return -EIO; } @@ -2570,7 +2570,7 @@ int soft_offline_page(unsigned long pfn, int flags) if (PageHWPoison(page)) { pr_info("%s: %#lx page already poisoned\n", __func__, pfn); - put_ref_page(ref_page); + put_ref_page(pfn, flags); mutex_unlock(&mf_mutex); return 0; } -- 2.35.3 ^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH 2/3] mm: memory-failure: avoid pfn_valid() twice in soft_offline_page() 2022-10-21 8:46 [PATCH 1/3] mm: memory-failure: make put_ref_page() more useful Kefeng Wang @ 2022-10-21 8:46 ` Kefeng Wang 2022-10-23 23:39 ` HORIGUCHI NAOYA(堀口 直也) 2022-10-25 2:55 ` Miaohe Lin 2022-10-21 8:46 ` [PATCH 3/3] mm: memory-failure: make action_result() return int Kefeng Wang ` (2 subsequent siblings) 3 siblings, 2 replies; 12+ messages in thread From: Kefeng Wang @ 2022-10-21 8:46 UTC (permalink / raw) To: Naoya Horiguchi, Andrew Morton Cc: Miaohe Lin, linux-mm, linux-kernel, Kefeng Wang Simplify WARN_ON_ONCE(flags & MF_COUNT_INCREASED) under !pfn_valid(). Signed-off-by: Kefeng Wang <wangkefeng.wang@huawei.com> --- mm/memory-failure.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/mm/memory-failure.c b/mm/memory-failure.c index b94152abb1c9..ca0199d0f79d 100644 --- a/mm/memory-failure.c +++ b/mm/memory-failure.c @@ -2554,10 +2554,10 @@ int soft_offline_page(unsigned long pfn, int flags) bool try_again = true; struct page *page; - WARN_ON_ONCE(!pfn_valid(pfn) && (flags & MF_COUNT_INCREASED)); - - if (!pfn_valid(pfn)) + if (!pfn_valid(pfn)) { + WARN_ON_ONCE(flags & MF_COUNT_INCREASED); return -ENXIO; + } /* Only online pages can be soft-offlined (esp., not ZONE_DEVICE). */ page = pfn_to_online_page(pfn); -- 2.35.3 ^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH 2/3] mm: memory-failure: avoid pfn_valid() twice in soft_offline_page() 2022-10-21 8:46 ` [PATCH 2/3] mm: memory-failure: avoid pfn_valid() twice in soft_offline_page() Kefeng Wang @ 2022-10-23 23:39 ` HORIGUCHI NAOYA(堀口 直也) 2022-10-25 2:55 ` Miaohe Lin 1 sibling, 0 replies; 12+ messages in thread From: HORIGUCHI NAOYA(堀口 直也) @ 2022-10-23 23:39 UTC (permalink / raw) To: Kefeng Wang; +Cc: Andrew Morton, Miaohe Lin, linux-mm, linux-kernel On Fri, Oct 21, 2022 at 04:46:10PM +0800, Kefeng Wang wrote: > Simplify WARN_ON_ONCE(flags & MF_COUNT_INCREASED) under !pfn_valid(). > > Signed-off-by: Kefeng Wang <wangkefeng.wang@huawei.com> Acked-by: Naoya Horiguchi <naoya.horiguchi@nec.com> ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 2/3] mm: memory-failure: avoid pfn_valid() twice in soft_offline_page() 2022-10-21 8:46 ` [PATCH 2/3] mm: memory-failure: avoid pfn_valid() twice in soft_offline_page() Kefeng Wang 2022-10-23 23:39 ` HORIGUCHI NAOYA(堀口 直也) @ 2022-10-25 2:55 ` Miaohe Lin 1 sibling, 0 replies; 12+ messages in thread From: Miaohe Lin @ 2022-10-25 2:55 UTC (permalink / raw) To: Kefeng Wang; +Cc: linux-mm, linux-kernel, Naoya Horiguchi, Andrew Morton On 2022/10/21 16:46, Kefeng Wang wrote: > Simplify WARN_ON_ONCE(flags & MF_COUNT_INCREASED) under !pfn_valid(). > > Signed-off-by: Kefeng Wang <wangkefeng.wang@huawei.com> Reviewed-by: Miaohe Lin <linmiaohe@huawei.com> Thanks, Miaohe Lin ^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH 3/3] mm: memory-failure: make action_result() return int 2022-10-21 8:46 [PATCH 1/3] mm: memory-failure: make put_ref_page() more useful Kefeng Wang 2022-10-21 8:46 ` [PATCH 2/3] mm: memory-failure: avoid pfn_valid() twice in soft_offline_page() Kefeng Wang @ 2022-10-21 8:46 ` Kefeng Wang 2022-10-23 23:56 ` HORIGUCHI NAOYA(堀口 直也) 2022-10-24 3:51 ` [PATCH v2] " Kefeng Wang 2022-10-23 23:38 ` [PATCH 1/3] mm: memory-failure: make put_ref_page() more useful HORIGUCHI NAOYA(堀口 直也) 2022-10-25 2:52 ` Miaohe Lin 3 siblings, 2 replies; 12+ messages in thread From: Kefeng Wang @ 2022-10-21 8:46 UTC (permalink / raw) To: Naoya Horiguchi, Andrew Morton Cc: Miaohe Lin, linux-mm, linux-kernel, Kefeng Wang Check mf_result in action_result(), only return 0 when MF_RECOVERED, or return -EBUSY, which will simplify code a bit. Signed-off-by: Kefeng Wang <wangkefeng.wang@huawei.com> --- mm/memory-failure.c | 42 ++++++++++++++++-------------------------- 1 file changed, 16 insertions(+), 26 deletions(-) diff --git a/mm/memory-failure.c b/mm/memory-failure.c index ca0199d0f79d..3f469e2da047 100644 --- a/mm/memory-failure.c +++ b/mm/memory-failure.c @@ -1182,14 +1182,16 @@ static struct page_state error_states[] = { * "Dirty/Clean" indication is not 100% accurate due to the possibility of * setting PG_dirty outside page lock. See also comment above set_page_dirty(). */ -static void action_result(unsigned long pfn, enum mf_action_page_type type, - enum mf_result result) +static int action_result(unsigned long pfn, enum mf_action_page_type type, + enum mf_result result) { trace_memory_failure_event(pfn, type, result); num_poisoned_pages_inc(); pr_err("%#lx: recovery action for %s: %s\n", pfn, action_page_types[type], action_name[result]); + + return result == MF_RECOVERED ? 0 : -EBUSY; } static int page_action(struct page_state *ps, struct page *p, @@ -1856,8 +1858,7 @@ static int try_memory_failure_hugetlb(unsigned long pfn, int flags, int *hugetlb flags |= MF_NO_RETRY; goto retry; } - action_result(pfn, MF_MSG_UNKNOWN, MF_IGNORED); - return res; + return action_result(pfn, MF_MSG_UNKNOWN, MF_IGNORED); } head = compound_head(p); @@ -1883,22 +1884,18 @@ static int try_memory_failure_hugetlb(unsigned long pfn, int flags, int *hugetlb } else { res = MF_FAILED; } - action_result(pfn, MF_MSG_FREE_HUGE, res); - return res == MF_RECOVERED ? 0 : -EBUSY; + return action_result(pfn, MF_MSG_FREE_HUGE, res); } page_flags = head->flags; if (!hwpoison_user_mappings(p, pfn, flags, head)) { - action_result(pfn, MF_MSG_UNMAP_FAILED, MF_IGNORED); - res = -EBUSY; - goto out; + res = action_result(pfn, MF_MSG_UNMAP_FAILED, MF_IGNORED); + unlock_page(head); + return res; } return identify_page_state(pfn, p, page_flags); -out: - unlock_page(head); - return res; } #else @@ -2063,16 +2060,13 @@ int memory_failure(unsigned long pfn, int flags) } res = MF_FAILED; } - action_result(pfn, MF_MSG_BUDDY, res); - res = res == MF_RECOVERED ? 0 : -EBUSY; + res = action_result(pfn, MF_MSG_BUDDY, res); } else { - action_result(pfn, MF_MSG_KERNEL_HIGH_ORDER, MF_IGNORED); - res = -EBUSY; + res = action_result(pfn, MF_MSG_KERNEL_HIGH_ORDER, MF_IGNORED); } goto unlock_mutex; } else if (res < 0) { - action_result(pfn, MF_MSG_UNKNOWN, MF_IGNORED); - res = -EBUSY; + res = action_result(pfn, MF_MSG_UNKNOWN, MF_IGNORED); goto unlock_mutex; } } @@ -2093,8 +2087,7 @@ int memory_failure(unsigned long pfn, int flags) */ SetPageHasHWPoisoned(hpage); if (try_to_split_thp_page(p) < 0) { - action_result(pfn, MF_MSG_UNSPLIT_THP, MF_IGNORED); - res = -EBUSY; + res = action_result(pfn, MF_MSG_UNSPLIT_THP, MF_IGNORED); goto unlock_mutex; } VM_BUG_ON_PAGE(!page_count(p), p); @@ -2127,8 +2120,7 @@ int memory_failure(unsigned long pfn, int flags) retry = false; goto try_again; } - action_result(pfn, MF_MSG_DIFFERENT_COMPOUND, MF_IGNORED); - res = -EBUSY; + res = action_result(pfn, MF_MSG_DIFFERENT_COMPOUND, MF_IGNORED); goto unlock_page; } @@ -2168,8 +2160,7 @@ int memory_failure(unsigned long pfn, int flags) * Abort on fail: __filemap_remove_folio() assumes unmapped page. */ if (!hwpoison_user_mappings(p, pfn, flags, p)) { - action_result(pfn, MF_MSG_UNMAP_FAILED, MF_IGNORED); - res = -EBUSY; + res = action_result(pfn, MF_MSG_UNMAP_FAILED, MF_IGNORED); goto unlock_page; } @@ -2177,8 +2168,7 @@ int memory_failure(unsigned long pfn, int flags) * Torn down by someone else? */ if (PageLRU(p) && !PageSwapCache(p) && p->mapping == NULL) { - action_result(pfn, MF_MSG_TRUNCATED_LRU, MF_IGNORED); - res = -EBUSY; + res = action_result(pfn, MF_MSG_TRUNCATED_LRU, MF_IGNORED); goto unlock_page; } -- 2.35.3 ^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH 3/3] mm: memory-failure: make action_result() return int 2022-10-21 8:46 ` [PATCH 3/3] mm: memory-failure: make action_result() return int Kefeng Wang @ 2022-10-23 23:56 ` HORIGUCHI NAOYA(堀口 直也) 2022-10-24 1:39 ` Kefeng Wang 2022-10-24 3:51 ` [PATCH v2] " Kefeng Wang 1 sibling, 1 reply; 12+ messages in thread From: HORIGUCHI NAOYA(堀口 直也) @ 2022-10-23 23:56 UTC (permalink / raw) To: Kefeng Wang; +Cc: Andrew Morton, Miaohe Lin, linux-mm, linux-kernel On Fri, Oct 21, 2022 at 04:46:11PM +0800, Kefeng Wang wrote: > Check mf_result in action_result(), only return 0 when MF_RECOVERED, > or return -EBUSY, which will simplify code a bit. > > Signed-off-by: Kefeng Wang <wangkefeng.wang@huawei.com> Thanks for the cleanup, Kefeng. I basically agree with the change. I have one comment below ... > --- > mm/memory-failure.c | 42 ++++++++++++++++-------------------------- > 1 file changed, 16 insertions(+), 26 deletions(-) > > diff --git a/mm/memory-failure.c b/mm/memory-failure.c > index ca0199d0f79d..3f469e2da047 100644 > --- a/mm/memory-failure.c > +++ b/mm/memory-failure.c > @@ -1182,14 +1182,16 @@ static struct page_state error_states[] = { > * "Dirty/Clean" indication is not 100% accurate due to the possibility of > * setting PG_dirty outside page lock. See also comment above set_page_dirty(). > */ > -static void action_result(unsigned long pfn, enum mf_action_page_type type, > - enum mf_result result) > +static int action_result(unsigned long pfn, enum mf_action_page_type type, > + enum mf_result result) > { > trace_memory_failure_event(pfn, type, result); > > num_poisoned_pages_inc(); > pr_err("%#lx: recovery action for %s: %s\n", > pfn, action_page_types[type], action_name[result]); > + > + return result == MF_RECOVERED ? 0 : -EBUSY; I think that MF_DELAYED may be considered as success (returning 0), then page_action() can be cleaned up a little more (like below?) static int page_action(struct page_state *ps, struct page *p, unsigned long pfn) { int result; /* page p should be unlocked after returning from ps->action(). */ result = ps->action(ps, p); /* Could do more checks here if page looks ok */ /* * Could adjust zone counters here to correct for the missing page. */ return action_result(pfn, ps->type, result); } Existing direct callers (I mean action_result() called from other than page_action()) are never called with result==MF_DELAYED, so this change should not affect them. Does it make sense for you? Thanks, Naoya Horiguchi ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 3/3] mm: memory-failure: make action_result() return int 2022-10-23 23:56 ` HORIGUCHI NAOYA(堀口 直也) @ 2022-10-24 1:39 ` Kefeng Wang 0 siblings, 0 replies; 12+ messages in thread From: Kefeng Wang @ 2022-10-24 1:39 UTC (permalink / raw) To: HORIGUCHI NAOYA(堀口 直也) Cc: Andrew Morton, Miaohe Lin, linux-mm, linux-kernel On 2022/10/24 7:56, HORIGUCHI NAOYA(堀口 直也) wrote: > On Fri, Oct 21, 2022 at 04:46:11PM +0800, Kefeng Wang wrote: >> Check mf_result in action_result(), only return 0 when MF_RECOVERED, >> or return -EBUSY, which will simplify code a bit. >> >> Signed-off-by: Kefeng Wang <wangkefeng.wang@huawei.com> > Thanks for the cleanup, Kefeng. > I basically agree with the change. I have one comment below ... > >> --- >> mm/memory-failure.c | 42 ++++++++++++++++-------------------------- >> 1 file changed, 16 insertions(+), 26 deletions(-) >> >> diff --git a/mm/memory-failure.c b/mm/memory-failure.c >> index ca0199d0f79d..3f469e2da047 100644 >> --- a/mm/memory-failure.c >> +++ b/mm/memory-failure.c >> @@ -1182,14 +1182,16 @@ static struct page_state error_states[] = { >> * "Dirty/Clean" indication is not 100% accurate due to the possibility of >> * setting PG_dirty outside page lock. See also comment above set_page_dirty(). >> */ >> -static void action_result(unsigned long pfn, enum mf_action_page_type type, >> - enum mf_result result) >> +static int action_result(unsigned long pfn, enum mf_action_page_type type, >> + enum mf_result result) >> { >> trace_memory_failure_event(pfn, type, result); >> >> num_poisoned_pages_inc(); >> pr_err("%#lx: recovery action for %s: %s\n", >> pfn, action_page_types[type], action_name[result]); >> + >> + return result == MF_RECOVERED ? 0 : -EBUSY; > I think that MF_DELAYED may be considered as success (returning 0), then > page_action() can be cleaned up a little more (like below?) Yes, MF_DELAYED should be considered as success, > > static int page_action(struct page_state *ps, struct page *p, > unsigned long pfn) > { > int result; > > /* page p should be unlocked after returning from ps->action(). */ > result = ps->action(ps, p); > > /* Could do more checks here if page looks ok */ > /* > * Could adjust zone counters here to correct for the missing page. > */ > > return action_result(pfn, ps->type, result); > } > > Existing direct callers (I mean action_result() called from other than > page_action()) are never called with result==MF_DELAYED, so this change > should not affect them. I will refresh this patch, thanks. > Does it make sense for you? > > Thanks, > Naoya Horiguchi ^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH v2] mm: memory-failure: make action_result() return int 2022-10-21 8:46 ` [PATCH 3/3] mm: memory-failure: make action_result() return int Kefeng Wang 2022-10-23 23:56 ` HORIGUCHI NAOYA(堀口 直也) @ 2022-10-24 3:51 ` Kefeng Wang 2022-10-24 7:35 ` HORIGUCHI NAOYA(堀口 直也) 2022-10-25 3:07 ` Miaohe Lin 1 sibling, 2 replies; 12+ messages in thread From: Kefeng Wang @ 2022-10-24 3:51 UTC (permalink / raw) To: Naoya Horiguchi, Andrew Morton Cc: Miaohe Lin, linux-mm, linux-kernel, Kefeng Wang Check mf_result in action_result(), only return 0 when MF_RECOVERED/MF_DELAYED, or return -EBUSY, which will simplify code a bit. Signed-off-by: Kefeng Wang <wangkefeng.wang@huawei.com> --- v2: - MF_DELAYED is considered as success, suggested by HORIGUCHI - adjust order between unlock_page(head) and action_result() after !hwpoison_user_mappings() to clean code more in try_memory_failure_hugetlb() mm/memory-failure.c | 45 ++++++++++++++++----------------------------- 1 file changed, 16 insertions(+), 29 deletions(-) diff --git a/mm/memory-failure.c b/mm/memory-failure.c index ca0199d0f79d..3cfa1b9ac513 100644 --- a/mm/memory-failure.c +++ b/mm/memory-failure.c @@ -1182,14 +1182,16 @@ static struct page_state error_states[] = { * "Dirty/Clean" indication is not 100% accurate due to the possibility of * setting PG_dirty outside page lock. See also comment above set_page_dirty(). */ -static void action_result(unsigned long pfn, enum mf_action_page_type type, - enum mf_result result) +static int action_result(unsigned long pfn, enum mf_action_page_type type, + enum mf_result result) { trace_memory_failure_event(pfn, type, result); num_poisoned_pages_inc(); pr_err("%#lx: recovery action for %s: %s\n", pfn, action_page_types[type], action_name[result]); + + return (result == MF_RECOVERED || result == MF_DELAYED) ? 0 : -EBUSY; } static int page_action(struct page_state *ps, struct page *p, @@ -1200,14 +1202,12 @@ static int page_action(struct page_state *ps, struct page *p, /* page p should be unlocked after returning from ps->action(). */ result = ps->action(ps, p); - action_result(pfn, ps->type, result); - /* Could do more checks here if page looks ok */ /* * Could adjust zone counters here to correct for the missing page. */ - return (result == MF_RECOVERED || result == MF_DELAYED) ? 0 : -EBUSY; + return action_result(pfn, ps->type, result); } static inline bool PageHWPoisonTakenOff(struct page *page) @@ -1856,8 +1856,7 @@ static int try_memory_failure_hugetlb(unsigned long pfn, int flags, int *hugetlb flags |= MF_NO_RETRY; goto retry; } - action_result(pfn, MF_MSG_UNKNOWN, MF_IGNORED); - return res; + return action_result(pfn, MF_MSG_UNKNOWN, MF_IGNORED); } head = compound_head(p); @@ -1883,22 +1882,17 @@ static int try_memory_failure_hugetlb(unsigned long pfn, int flags, int *hugetlb } else { res = MF_FAILED; } - action_result(pfn, MF_MSG_FREE_HUGE, res); - return res == MF_RECOVERED ? 0 : -EBUSY; + return action_result(pfn, MF_MSG_FREE_HUGE, res); } page_flags = head->flags; if (!hwpoison_user_mappings(p, pfn, flags, head)) { - action_result(pfn, MF_MSG_UNMAP_FAILED, MF_IGNORED); - res = -EBUSY; - goto out; + unlock_page(head); + return action_result(pfn, MF_MSG_UNMAP_FAILED, MF_IGNORED); } return identify_page_state(pfn, p, page_flags); -out: - unlock_page(head); - return res; } #else @@ -2063,16 +2057,13 @@ int memory_failure(unsigned long pfn, int flags) } res = MF_FAILED; } - action_result(pfn, MF_MSG_BUDDY, res); - res = res == MF_RECOVERED ? 0 : -EBUSY; + res = action_result(pfn, MF_MSG_BUDDY, res); } else { - action_result(pfn, MF_MSG_KERNEL_HIGH_ORDER, MF_IGNORED); - res = -EBUSY; + res = action_result(pfn, MF_MSG_KERNEL_HIGH_ORDER, MF_IGNORED); } goto unlock_mutex; } else if (res < 0) { - action_result(pfn, MF_MSG_UNKNOWN, MF_IGNORED); - res = -EBUSY; + res = action_result(pfn, MF_MSG_UNKNOWN, MF_IGNORED); goto unlock_mutex; } } @@ -2093,8 +2084,7 @@ int memory_failure(unsigned long pfn, int flags) */ SetPageHasHWPoisoned(hpage); if (try_to_split_thp_page(p) < 0) { - action_result(pfn, MF_MSG_UNSPLIT_THP, MF_IGNORED); - res = -EBUSY; + res = action_result(pfn, MF_MSG_UNSPLIT_THP, MF_IGNORED); goto unlock_mutex; } VM_BUG_ON_PAGE(!page_count(p), p); @@ -2127,8 +2117,7 @@ int memory_failure(unsigned long pfn, int flags) retry = false; goto try_again; } - action_result(pfn, MF_MSG_DIFFERENT_COMPOUND, MF_IGNORED); - res = -EBUSY; + res = action_result(pfn, MF_MSG_DIFFERENT_COMPOUND, MF_IGNORED); goto unlock_page; } @@ -2168,8 +2157,7 @@ int memory_failure(unsigned long pfn, int flags) * Abort on fail: __filemap_remove_folio() assumes unmapped page. */ if (!hwpoison_user_mappings(p, pfn, flags, p)) { - action_result(pfn, MF_MSG_UNMAP_FAILED, MF_IGNORED); - res = -EBUSY; + res = action_result(pfn, MF_MSG_UNMAP_FAILED, MF_IGNORED); goto unlock_page; } @@ -2177,8 +2165,7 @@ int memory_failure(unsigned long pfn, int flags) * Torn down by someone else? */ if (PageLRU(p) && !PageSwapCache(p) && p->mapping == NULL) { - action_result(pfn, MF_MSG_TRUNCATED_LRU, MF_IGNORED); - res = -EBUSY; + res = action_result(pfn, MF_MSG_TRUNCATED_LRU, MF_IGNORED); goto unlock_page; } -- 2.35.3 ^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH v2] mm: memory-failure: make action_result() return int 2022-10-24 3:51 ` [PATCH v2] " Kefeng Wang @ 2022-10-24 7:35 ` HORIGUCHI NAOYA(堀口 直也) 2022-10-25 3:07 ` Miaohe Lin 1 sibling, 0 replies; 12+ messages in thread From: HORIGUCHI NAOYA(堀口 直也) @ 2022-10-24 7:35 UTC (permalink / raw) To: Kefeng Wang; +Cc: Andrew Morton, Miaohe Lin, linux-mm, linux-kernel On Mon, Oct 24, 2022 at 11:51:38AM +0800, Kefeng Wang wrote: > Check mf_result in action_result(), only return 0 when > MF_RECOVERED/MF_DELAYED, or return -EBUSY, which will > simplify code a bit. > > Signed-off-by: Kefeng Wang <wangkefeng.wang@huawei.com> > --- > v2: > - MF_DELAYED is considered as success, suggested by HORIGUCHI > - adjust order between unlock_page(head) and action_result() > after !hwpoison_user_mappings() to clean code more in > try_memory_failure_hugetlb() > Thank you for the update. The patch looks good to me. Acked-by: Naoya Horiguchi <naoya.horiguchi@nec.com> ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v2] mm: memory-failure: make action_result() return int 2022-10-24 3:51 ` [PATCH v2] " Kefeng Wang 2022-10-24 7:35 ` HORIGUCHI NAOYA(堀口 直也) @ 2022-10-25 3:07 ` Miaohe Lin 1 sibling, 0 replies; 12+ messages in thread From: Miaohe Lin @ 2022-10-25 3:07 UTC (permalink / raw) To: Kefeng Wang; +Cc: linux-mm, linux-kernel, Naoya Horiguchi, Andrew Morton On 2022/10/24 11:51, Kefeng Wang wrote: > Check mf_result in action_result(), only return 0 when > MF_RECOVERED/MF_DELAYED, or return -EBUSY, which will > simplify code a bit. > > Signed-off-by: Kefeng Wang <wangkefeng.wang@huawei.com> Thanks for cleanup. This really makes code cleaner. Reviewed-by: Miaohe Lin <linmiaohe@huawei.com> Thanks, Miaohe Lin ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 1/3] mm: memory-failure: make put_ref_page() more useful 2022-10-21 8:46 [PATCH 1/3] mm: memory-failure: make put_ref_page() more useful Kefeng Wang 2022-10-21 8:46 ` [PATCH 2/3] mm: memory-failure: avoid pfn_valid() twice in soft_offline_page() Kefeng Wang 2022-10-21 8:46 ` [PATCH 3/3] mm: memory-failure: make action_result() return int Kefeng Wang @ 2022-10-23 23:38 ` HORIGUCHI NAOYA(堀口 直也) 2022-10-25 2:52 ` Miaohe Lin 3 siblings, 0 replies; 12+ messages in thread From: HORIGUCHI NAOYA(堀口 直也) @ 2022-10-23 23:38 UTC (permalink / raw) To: Kefeng Wang; +Cc: Andrew Morton, Miaohe Lin, linux-mm, linux-kernel On Fri, Oct 21, 2022 at 04:46:09PM +0800, Kefeng Wang wrote: > Pass pfn/flags to put_ref_page(), then check MF_COUNT_INCREASED > and drop refcount to make the code look cleaner. > > Signed-off-by: Kefeng Wang <wangkefeng.wang@huawei.com> Looks good to me, thank you. Acked-by: Naoya Horiguchi <naoya.horiguchi@nec.com> ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 1/3] mm: memory-failure: make put_ref_page() more useful 2022-10-21 8:46 [PATCH 1/3] mm: memory-failure: make put_ref_page() more useful Kefeng Wang ` (2 preceding siblings ...) 2022-10-23 23:38 ` [PATCH 1/3] mm: memory-failure: make put_ref_page() more useful HORIGUCHI NAOYA(堀口 直也) @ 2022-10-25 2:52 ` Miaohe Lin 3 siblings, 0 replies; 12+ messages in thread From: Miaohe Lin @ 2022-10-25 2:52 UTC (permalink / raw) To: Kefeng Wang; +Cc: linux-mm, linux-kernel, Naoya Horiguchi, Andrew Morton On 2022/10/21 16:46, Kefeng Wang wrote: > Pass pfn/flags to put_ref_page(), then check MF_COUNT_INCREASED > and drop refcount to make the code look cleaner. > > Signed-off-by: Kefeng Wang <wangkefeng.wang@huawei.com> > --- > mm/memory-failure.c | 34 +++++++++++++++++----------------- > 1 file changed, 17 insertions(+), 17 deletions(-) > > diff --git a/mm/memory-failure.c b/mm/memory-failure.c > index bead6bccc7f2..b94152abb1c9 100644 > --- a/mm/memory-failure.c > +++ b/mm/memory-failure.c > @@ -1913,17 +1913,25 @@ static inline unsigned long free_raw_hwp_pages(struct page *hpage, bool flag) > } > #endif /* CONFIG_HUGETLB_PAGE */ > > +/* Drop the extra refcount in case we come from madvise() */ > +static void put_ref_page(unsigned long pfn, int flags) > +{ > + struct page *page; > + > + if (!(flags & MF_COUNT_INCREASED)) > + return; > + > + page = pfn_to_page(pfn); > + if (page) IMO above check is unneeded. Page can't be NULL as pfn is valid. But this is trival and this patch looks good to me. Thanks. Reviewed-by: Miaohe Lin <linmiaohe@huawei.com> Thanks, Miaohe Lin ^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2022-10-25 3:08 UTC | newest] Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2022-10-21 8:46 [PATCH 1/3] mm: memory-failure: make put_ref_page() more useful Kefeng Wang 2022-10-21 8:46 ` [PATCH 2/3] mm: memory-failure: avoid pfn_valid() twice in soft_offline_page() Kefeng Wang 2022-10-23 23:39 ` HORIGUCHI NAOYA(堀口 直也) 2022-10-25 2:55 ` Miaohe Lin 2022-10-21 8:46 ` [PATCH 3/3] mm: memory-failure: make action_result() return int Kefeng Wang 2022-10-23 23:56 ` HORIGUCHI NAOYA(堀口 直也) 2022-10-24 1:39 ` Kefeng Wang 2022-10-24 3:51 ` [PATCH v2] " Kefeng Wang 2022-10-24 7:35 ` HORIGUCHI NAOYA(堀口 直也) 2022-10-25 3:07 ` Miaohe Lin 2022-10-23 23:38 ` [PATCH 1/3] mm: memory-failure: make put_ref_page() more useful HORIGUCHI NAOYA(堀口 直也) 2022-10-25 2:52 ` Miaohe Lin
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.