On Fri, Apr 29, 2022 at 10:22:05PM +0800, zhenwei pi wrote:
> In the memory failure procedure, hwpoison_filter has higher priority,
> if memory_filter() filters the error event, there is no need to do
> the further work.
Could you clarify what problem you are trying to solve (what does
"optimize" mean in this context or what is the benefit)?
Now hwpoison_filter() can be called both with *and* without taking page refcount.
It's mainly called *with* taking page refcount in order to make sure that the
actual handling process is executed only for pages that meet a given condition.
IOW, it's important to prevent pages which do not meet the condition from going
ahead to further steps (false-positive is not permitted). So this type of
callsite should not be omittable.
As for the other case, hwpoison_filter() is also called in hwpoison_inject()
*without* taking page refcount. This actually has a different nuance and
intended to speculatively filter the injection events before setting
PageHWPoison flag to reduce the noise due to setting PG_hwpoison temporary.
The point is that it's not intended here to filter precisely and this callsite
is omittable.
So in my understanding, we need keep hwpoison_filter() after taking page
refcount as we do now. Maybe optionally and additionally calling
hwpoison_filter() at the beginning of memory_failure() might be possible,
but I'm not sure yet how helpful...
Thanks,
Naoya Horiguchi
>
> Cc: Wu Fengguang <fengguang.wu@xxxxxxxxx>
> Signed-off-by: zhenwei pi <pizhenwei@xxxxxxxxxxxxx>
> ---
> mm/memory-failure.c | 14 +++++---------
> 1 file changed, 5 insertions(+), 9 deletions(-)
>
> diff --git a/mm/memory-failure.c b/mm/memory-failure.c
> index ece05858568f..a6a27c8b800f 100644
> --- a/mm/memory-failure.c
> +++ b/mm/memory-failure.c
> @@ -1800,6 +1800,11 @@ int memory_failure(unsigned long pfn, int flags)
> goto unlock_mutex;
> }
>
> + if (hwpoison_filter(p)) {
> + res = -EOPNOTSUPP;
> + goto unlock_mutex;
> + }
> +
> try_again:
> res = try_memory_failure_hugetlb(pfn, flags, &hugetlb);
> if (hugetlb)
> @@ -1937,15 +1942,6 @@ int memory_failure(unsigned long pfn, int flags)
> */
> page_flags = p->flags;
>
> - if (hwpoison_filter(p)) {
> - if (TestClearPageHWPoison(p))
> - num_poisoned_pages_dec();
> - unlock_page(p);
> - put_page(p);
> - res = -EOPNOTSUPP;
> - goto unlock_mutex;
> - }
> -
> /*
> * __munlock_pagevec may clear a writeback page's LRU flag without
> * page_lock. We need wait writeback completion for this page or it
> --
> 2.20.1
>
