| Message ID | 20240510062602.901510-5-jane.chu@oracle.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | Enhance soft hwpoison handling and injection | expand |
On 2024/5/10 14:26, Jane Chu wrote: > Move hwpoison_filter() higher up as there is no need to spend a lot > cycles only to find out later that the page is supposed to be skipped > for hwpoison handling. > > Signed-off-by: Jane Chu <jane.chu@oracle.com> > --- > mm/memory-failure.c | 15 +++++++-------- > 1 file changed, 7 insertions(+), 8 deletions(-) > > diff --git a/mm/memory-failure.c b/mm/memory-failure.c > index 62133c10fb51..2fa884d8b5a3 100644 > --- a/mm/memory-failure.c > +++ b/mm/memory-failure.c > @@ -2236,6 +2236,13 @@ int memory_failure(unsigned long pfn, int flags) > goto unlock_mutex; > } > > + if (hwpoison_filter(p)) { > + if (flags & MF_COUNT_INCREASED) > + put_page(p); > + res = -EOPNOTSUPP; > + goto unlock_mutex; > + } It might not be a good idea to do hwpoison_filter() here. We don't hold extra page refcnt yet, so the page state will be really unstable. Or am I miss something? Thanks. .
On Fri, May 10, 2024 at 12:26:01AM -0600, Jane Chu wrote: > Move hwpoison_filter() higher up as there is no need to spend a lot > cycles only to find out later that the page is supposed to be skipped > for hwpoison handling. > > Signed-off-by: Jane Chu <jane.chu@oracle.com> > --- > mm/memory-failure.c | 15 +++++++-------- > 1 file changed, 7 insertions(+), 8 deletions(-) > > diff --git a/mm/memory-failure.c b/mm/memory-failure.c > index 62133c10fb51..2fa884d8b5a3 100644 > --- a/mm/memory-failure.c > +++ b/mm/memory-failure.c > @@ -2236,6 +2236,13 @@ int memory_failure(unsigned long pfn, int flags) > goto unlock_mutex; > } > > + if (hwpoison_filter(p)) { > + if (flags & MF_COUNT_INCREASED) > + put_page(p); > + res = -EOPNOTSUPP; > + goto unlock_mutex; > + } Now, it is true that doing this might not be optimal for the reasons explained by Miaohe, but the whole hwpoison_filter() thing is only used by the hwpoison-inject code AFAICS, which is just for testing purposes, so I do not think there is any harm in lifting the check. But no real strong opinion here.
On 5/11/2024 1:29 AM, Miaohe Lin wrote: > On 2024/5/10 14:26, Jane Chu wrote: >> Move hwpoison_filter() higher up as there is no need to spend a lot >> cycles only to find out later that the page is supposed to be skipped >> for hwpoison handling. >> >> Signed-off-by: Jane Chu <jane.chu@oracle.com> >> --- >> mm/memory-failure.c | 15 +++++++-------- >> 1 file changed, 7 insertions(+), 8 deletions(-) >> >> diff --git a/mm/memory-failure.c b/mm/memory-failure.c >> index 62133c10fb51..2fa884d8b5a3 100644 >> --- a/mm/memory-failure.c >> +++ b/mm/memory-failure.c >> @@ -2236,6 +2236,13 @@ int memory_failure(unsigned long pfn, int flags) >> goto unlock_mutex; >> } >> >> + if (hwpoison_filter(p)) { >> + if (flags & MF_COUNT_INCREASED) >> + put_page(p); >> + res = -EOPNOTSUPP; >> + goto unlock_mutex; >> + } > It might not be a good idea to do hwpoison_filter() here. We don't hold extra page refcnt > yet, so the page state will be really unstable. Or am I miss something? I agree with you. It looks like hwpoison_filter_flags() in particular needs a stable page in order to retrieve a wholesome KPF_ flags set that at any time, although the flags could change immediately afterwards, they won't be torn flags. For that, it looks like the folio should be locked as well. thanks! -jane > Thanks. > .
diff --git a/mm/memory-failure.c b/mm/memory-failure.c index 62133c10fb51..2fa884d8b5a3 100644 --- a/mm/memory-failure.c +++ b/mm/memory-failure.c @@ -2236,6 +2236,13 @@ int memory_failure(unsigned long pfn, int flags) goto unlock_mutex; } + if (hwpoison_filter(p)) { + if (flags & MF_COUNT_INCREASED) + put_page(p); + res = -EOPNOTSUPP; + goto unlock_mutex; + } + try_again: res = try_memory_failure_hugetlb(pfn, flags, &hugetlb); if (hugetlb) @@ -2354,14 +2361,6 @@ int memory_failure(unsigned long pfn, int flags) */ page_flags = folio->flags; - if (hwpoison_filter(p)) { - ClearPageHWPoison(p); - folio_unlock(folio); - folio_put(folio); - res = -EOPNOTSUPP; - goto unlock_mutex; - } - /* * __munlock_folio() may clear a writeback folio's LRU flag without * the folio lock. We need to wait for writeback completion for this
Move hwpoison_filter() higher up as there is no need to spend a lot cycles only to find out later that the page is supposed to be skipped for hwpoison handling. Signed-off-by: Jane Chu <jane.chu@oracle.com> --- mm/memory-failure.c | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-)