| Message ID | 20230708085744.3599311-2-linmiaohe@huawei.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | A few fixup and cleanup patches for memory-failure | expand |
> If hpage isn't Hugetlb page, MF_DELAYED is returned without unlock hpage > leading to hpage permanently locked. But this shouldn't trigger in the > real world because this PageHuge() check is just for potential problems. Please choose an imperative change suggestion. See also: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/submitting-patches.rst?h=v6.4#n94 How do you think about to avoid empty descriptions in recipient lists? Regards, Markus
On Sat, Jul 08, 2023 at 04:57:37PM +0800, Miaohe Lin wrote: > If hpage isn't Hugetlb page, MF_DELAYED is returned without unlock hpage > leading to hpage permanently locked. But this shouldn't trigger in the > real world because this PageHuge() check is just for potential problems. Right, so this if-block is dead code, how about simply removing it? Thanks, Naoya Horiguchi > > Signed-off-by: Miaohe Lin <linmiaohe@huawei.com> > --- > mm/memory-failure.c | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > > diff --git a/mm/memory-failure.c b/mm/memory-failure.c > index a6221a4bc5ea..d21ee27ad412 100644 > --- a/mm/memory-failure.c > +++ b/mm/memory-failure.c > @@ -1187,8 +1187,10 @@ static int me_huge_page(struct page_state *ps, struct page *p) > struct address_space *mapping; > bool extra_pins = false; > > - if (!PageHuge(hpage)) > + if (!PageHuge(hpage)) { > + unlock_page(hpage); > return MF_DELAYED; > + } > > mapping = page_mapping(hpage); > if (mapping) { > -- > 2.33.0 > > >
On 2023/7/10 15:56, Naoya Horiguchi wrote: > On Sat, Jul 08, 2023 at 04:57:37PM +0800, Miaohe Lin wrote: >> If hpage isn't Hugetlb page, MF_DELAYED is returned without unlock hpage >> leading to hpage permanently locked. But this shouldn't trigger in the >> real world because this PageHuge() check is just for potential problems. > > Right, so this if-block is dead code, how about simply removing it? From the current code of view, I think this if-block is actually dead code and won't catch anything too. So removing it makes sense to me. Will do it in v2. Thanks Naoya.
diff --git a/mm/memory-failure.c b/mm/memory-failure.c index a6221a4bc5ea..d21ee27ad412 100644 --- a/mm/memory-failure.c +++ b/mm/memory-failure.c @@ -1187,8 +1187,10 @@ static int me_huge_page(struct page_state *ps, struct page *p) struct address_space *mapping; bool extra_pins = false; - if (!PageHuge(hpage)) + if (!PageHuge(hpage)) { + unlock_page(hpage); return MF_DELAYED; + } mapping = page_mapping(hpage); if (mapping) {
If hpage isn't Hugetlb page, MF_DELAYED is returned without unlock hpage leading to hpage permanently locked. But this shouldn't trigger in the real world because this PageHuge() check is just for potential problems. Signed-off-by: Miaohe Lin <linmiaohe@huawei.com> --- mm/memory-failure.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)