| Message ID | 20210930215311.240774-2-shy828301@gmail.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | Solve silent data loss caused by poisoned page cache (shmem/tmpfs) | expand |
Hi Naoya, Any comment on this patch and patch #3? I'd prefer to fix more comments for a new version. Thanks, Yang On Thu, Sep 30, 2021 at 2:53 PM Yang Shi <shy828301@gmail.com> wrote: > > When handling THP hwpoison checked if the THP is in allocation or free > stage since hwpoison may mistreat it as hugetlb page. After > commit 415c64c1453a ("mm/memory-failure: split thp earlier in memory error > handling") the problem has been fixed, so this check is no longer > needed. Remove it. The side effect of the removal is hwpoison may > report unsplit THP instead of unknown error for shmem THP. It seems not > like a big deal. > > The following patch depends on this, which fixes shmem THP with > hwpoisoned subpage(s) are mapped PMD wrongly. So this patch needs to be > backported to -stable as well. > > Cc: <stable@vger.kernel.org> > Suggested-by: Naoya Horiguchi <naoya.horiguchi@nec.com> > Signed-off-by: Yang Shi <shy828301@gmail.com> > --- > mm/memory-failure.c | 14 -------------- > 1 file changed, 14 deletions(-) > > diff --git a/mm/memory-failure.c b/mm/memory-failure.c > index 54879c339024..ed28eba50f98 100644 > --- a/mm/memory-failure.c > +++ b/mm/memory-failure.c > @@ -1147,20 +1147,6 @@ static int __get_hwpoison_page(struct page *page) > if (!HWPoisonHandlable(head)) > return -EBUSY; > > - if (PageTransHuge(head)) { > - /* > - * Non anonymous thp exists only in allocation/free time. We > - * can't handle such a case correctly, so let's give it up. > - * This should be better than triggering BUG_ON when kernel > - * tries to touch the "partially handled" page. > - */ > - if (!PageAnon(head)) { > - pr_err("Memory failure: %#lx: non anonymous thp\n", > - page_to_pfn(page)); > - return 0; > - } > - } > - > if (get_page_unless_zero(head)) { > if (head == compound_head(page)) > return 1; > -- > 2.26.2 >
> Any comment on this patch and patch #3? I'd prefer to fix more > comments for a new version. No. Both 1/5 and 3/5 look fine to me. So ... > On Thu, Sep 30, 2021 at 2:53 PM Yang Shi <shy828301@gmail.com> wrote: > > > > When handling THP hwpoison checked if the THP is in allocation or free > > stage since hwpoison may mistreat it as hugetlb page. After > > commit 415c64c1453a ("mm/memory-failure: split thp earlier in memory error > > handling") the problem has been fixed, so this check is no longer > > needed. Remove it. The side effect of the removal is hwpoison may > > report unsplit THP instead of unknown error for shmem THP. It seems not > > like a big deal. > > > > The following patch depends on this, which fixes shmem THP with > > hwpoisoned subpage(s) are mapped PMD wrongly. So this patch needs to be > > backported to -stable as well. > > > > Cc: <stable@vger.kernel.org> > > Suggested-by: Naoya Horiguchi <naoya.horiguchi@nec.com> > > Signed-off-by: Yang Shi <shy828301@gmail.com> Acked-by: Naoya Horiguchi <naoya.horiguchi@nec.com> Patch 3/5 already has my Signed-off-by, so I think it can be considered as acked by me. Thanks, Naoya Horiguchi
On Tue, Oct 5, 2021 at 9:00 PM Naoya Horiguchi <naoya.horiguchi@linux.dev> wrote: > > > Any comment on this patch and patch #3? I'd prefer to fix more > > comments for a new version. > > No. Both 1/5 and 3/5 look fine to me. So ... > > > On Thu, Sep 30, 2021 at 2:53 PM Yang Shi <shy828301@gmail.com> wrote: > > > > > > When handling THP hwpoison checked if the THP is in allocation or free > > > stage since hwpoison may mistreat it as hugetlb page. After > > > commit 415c64c1453a ("mm/memory-failure: split thp earlier in memory error > > > handling") the problem has been fixed, so this check is no longer > > > needed. Remove it. The side effect of the removal is hwpoison may > > > report unsplit THP instead of unknown error for shmem THP. It seems not > > > like a big deal. > > > > > > The following patch depends on this, which fixes shmem THP with > > > hwpoisoned subpage(s) are mapped PMD wrongly. So this patch needs to be > > > backported to -stable as well. > > > > > > Cc: <stable@vger.kernel.org> > > > Suggested-by: Naoya Horiguchi <naoya.horiguchi@nec.com> > > > Signed-off-by: Yang Shi <shy828301@gmail.com> > > Acked-by: Naoya Horiguchi <naoya.horiguchi@nec.com> > > Patch 3/5 already has my Signed-off-by, so I think it can be considered > as acked by me. Aha, thanks. I will add your ack to patch 3/5 too. > > Thanks, > Naoya Horiguchi
diff --git a/mm/memory-failure.c b/mm/memory-failure.c index 54879c339024..ed28eba50f98 100644 --- a/mm/memory-failure.c +++ b/mm/memory-failure.c @@ -1147,20 +1147,6 @@ static int __get_hwpoison_page(struct page *page) if (!HWPoisonHandlable(head)) return -EBUSY; - if (PageTransHuge(head)) { - /* - * Non anonymous thp exists only in allocation/free time. We - * can't handle such a case correctly, so let's give it up. - * This should be better than triggering BUG_ON when kernel - * tries to touch the "partially handled" page. - */ - if (!PageAnon(head)) { - pr_err("Memory failure: %#lx: non anonymous thp\n", - page_to_pfn(page)); - return 0; - } - } - if (get_page_unless_zero(head)) { if (head == compound_head(page)) return 1;
When handling THP hwpoison checked if the THP is in allocation or free stage since hwpoison may mistreat it as hugetlb page. After commit 415c64c1453a ("mm/memory-failure: split thp earlier in memory error handling") the problem has been fixed, so this check is no longer needed. Remove it. The side effect of the removal is hwpoison may report unsplit THP instead of unknown error for shmem THP. It seems not like a big deal. The following patch depends on this, which fixes shmem THP with hwpoisoned subpage(s) are mapped PMD wrongly. So this patch needs to be backported to -stable as well. Cc: <stable@vger.kernel.org> Suggested-by: Naoya Horiguchi <naoya.horiguchi@nec.com> Signed-off-by: Yang Shi <shy828301@gmail.com> --- mm/memory-failure.c | 14 -------------- 1 file changed, 14 deletions(-)