| Message ID | 20230428100846.95535-1-yajun.deng@linux.dev (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | mm/memory_hotplug: fix dead loop in offline_pages() | expand |
On Fri 28-04-23 18:08:46, Yajun Deng wrote: > When migration failure in do_migrate_range() and then the > scan_movable_pages() will most likely return the same movable pfn. > In this case, there is no condition to bail out, they will > always run like this: > > ... > [878020.623959] migrating pfn 1727813 failed ret:1 > [878020.623960] page:00000000faa9673c refcount:3 mapcount:0 mapping:00000000144ccd79 index:0x14280025 pfn:0x1727813 > [878020.623962] memcg:ffffa0ff82d5a000 > [878020.623962] aops:def_blk_aops ino:fd00001 > [878020.623964] flags: 0x17ffffc000206a(referenced|dirty|active|workingset|private|node=0|zone=2|lastcpupid=0x1fffff) > [878020.623966] raw: 0017ffffc000206a ffffb0d14f50fbd8 ffffb0d14f50fbd8 ffffa0ff9c155018 > [878020.623967] raw: 0000000014280025 ffffa10327d702d8 00000003ffffffff ffffa0ff82d5a000 > [878020.623968] page dumped because: migration failure > [878020.626196] migrating pfn 1727813 failed ret:1 > [878020.626198] page:00000000faa9673c refcount:3 mapcount:0 mapping:00000000144ccd79 index:0x14280025 pfn:0x1727813 > [878020.626200] memcg:ffffa0ff82d5a000 > [878020.626200] aops:def_blk_aops ino:fd00001 > [878020.626202] flags: 0x17ffffc000206a(referenced|dirty|active|workingset|private|node=0|zone=2|lastcpupid=0x1fffff) > [878020.626204] raw: 0017ffffc000206a ffffb0d14f50fbd8 ffffb0d14f50fbd8 ffffa0ff9c155018 > [878020.626205] raw: 0000000014280025 ffffa10327d702d8 00000003ffffffff ffffa0ff82d5a000 > [878020.626206] page dumped because: migration failure > ... > > Bail out when migration failures reach 3 times. > > Fixes: bb8965bd82fd ("mm, memory_hotplug: deobfuscate migration part of offlining") > Signed-off-by: Yajun Deng <yajun.deng@linux.dev> Any hard coded failure retry limit will lead to premature failures. Have a look at 72b39cfc4d75 ("mm, memory_hotplug: do not fail offlining too early"). A proper way to deal with this is to implement termination from the userspace (e.g. timeout $TIMEOUT /bin/echo 0 > $PATH_TO_MEM/online) Nacked-by: Michal Hocko <mhocko@suse.com> > --- > mm/memory_hotplug.c | 19 ++++++++++++++----- > 1 file changed, 14 insertions(+), 5 deletions(-) > > diff --git a/mm/memory_hotplug.c b/mm/memory_hotplug.c > index 8e0fa209d533..72dd385b8892 100644 > --- a/mm/memory_hotplug.c > +++ b/mm/memory_hotplug.c > @@ -1800,11 +1800,12 @@ int __ref offline_pages(unsigned long start_pfn, unsigned long nr_pages, > struct zone *zone, struct memory_group *group) > { > const unsigned long end_pfn = start_pfn + nr_pages; > - unsigned long pfn, system_ram_pages = 0; > + unsigned long pfn, tmp_pfn, system_ram_pages = 0; > const int node = zone_to_nid(zone); > unsigned long flags; > struct memory_notify arg; > char *reason; > + int count = 0; > int ret; > > /* > @@ -1887,12 +1888,20 @@ int __ref offline_pages(unsigned long start_pfn, unsigned long nr_pages, > > cond_resched(); > > + tmp_pfn = pfn; > ret = scan_movable_pages(pfn, end_pfn, &pfn); > if (!ret) { > - /* > - * TODO: fatal migration failures should bail > - * out > - */ > + if (pfn == tmp_pfn) > + count++; > + else > + count = 0; > + > + if (unlikely(count == 3)) { > + ret = -EBUSY; > + reason = "migration failure"; > + goto failed_removal_isolated; > + } > + > do_migrate_range(pfn, end_pfn); > } > } while (!ret); > -- > 2.25.1
On 28.04.23 13:07, Michal Hocko wrote: > On Fri 28-04-23 18:08:46, Yajun Deng wrote: >> When migration failure in do_migrate_range() and then the >> scan_movable_pages() will most likely return the same movable pfn. >> In this case, there is no condition to bail out, they will >> always run like this: >> >> ... >> [878020.623959] migrating pfn 1727813 failed ret:1 >> [878020.623960] page:00000000faa9673c refcount:3 mapcount:0 mapping:00000000144ccd79 index:0x14280025 pfn:0x1727813 >> [878020.623962] memcg:ffffa0ff82d5a000 >> [878020.623962] aops:def_blk_aops ino:fd00001 >> [878020.623964] flags: 0x17ffffc000206a(referenced|dirty|active|workingset|private|node=0|zone=2|lastcpupid=0x1fffff) >> [878020.623966] raw: 0017ffffc000206a ffffb0d14f50fbd8 ffffb0d14f50fbd8 ffffa0ff9c155018 >> [878020.623967] raw: 0000000014280025 ffffa10327d702d8 00000003ffffffff ffffa0ff82d5a000 >> [878020.623968] page dumped because: migration failure >> [878020.626196] migrating pfn 1727813 failed ret:1 >> [878020.626198] page:00000000faa9673c refcount:3 mapcount:0 mapping:00000000144ccd79 index:0x14280025 pfn:0x1727813 >> [878020.626200] memcg:ffffa0ff82d5a000 >> [878020.626200] aops:def_blk_aops ino:fd00001 >> [878020.626202] flags: 0x17ffffc000206a(referenced|dirty|active|workingset|private|node=0|zone=2|lastcpupid=0x1fffff) >> [878020.626204] raw: 0017ffffc000206a ffffb0d14f50fbd8 ffffb0d14f50fbd8 ffffa0ff9c155018 >> [878020.626205] raw: 0000000014280025 ffffa10327d702d8 00000003ffffffff ffffa0ff82d5a000 >> [878020.626206] page dumped because: migration failure >> ... >> >> Bail out when migration failures reach 3 times. >> >> Fixes: bb8965bd82fd ("mm, memory_hotplug: deobfuscate migration part of offlining") >> Signed-off-by: Yajun Deng <yajun.deng@linux.dev> > > Any hard coded failure retry limit will lead to premature failures. Have > a look at 72b39cfc4d75 ("mm, memory_hotplug: do not fail offlining too > early"). A proper way to deal with this is to implement termination from > the userspace (e.g. timeout $TIMEOUT /bin/echo 0 > $PATH_TO_MEM/online) > Nacked-by: Michal Hocko <mhocko@suse.com> Agreed; it's a bit more complicated when offlining is triggered by ACPI code :(
diff --git a/mm/memory_hotplug.c b/mm/memory_hotplug.c index 8e0fa209d533..72dd385b8892 100644 --- a/mm/memory_hotplug.c +++ b/mm/memory_hotplug.c @@ -1800,11 +1800,12 @@ int __ref offline_pages(unsigned long start_pfn, unsigned long nr_pages, struct zone *zone, struct memory_group *group) { const unsigned long end_pfn = start_pfn + nr_pages; - unsigned long pfn, system_ram_pages = 0; + unsigned long pfn, tmp_pfn, system_ram_pages = 0; const int node = zone_to_nid(zone); unsigned long flags; struct memory_notify arg; char *reason; + int count = 0; int ret; /* @@ -1887,12 +1888,20 @@ int __ref offline_pages(unsigned long start_pfn, unsigned long nr_pages, cond_resched(); + tmp_pfn = pfn; ret = scan_movable_pages(pfn, end_pfn, &pfn); if (!ret) { - /* - * TODO: fatal migration failures should bail - * out - */ + if (pfn == tmp_pfn) + count++; + else + count = 0; + + if (unlikely(count == 3)) { + ret = -EBUSY; + reason = "migration failure"; + goto failed_removal_isolated; + } + do_migrate_range(pfn, end_pfn); } } while (!ret);
When migration failure in do_migrate_range() and then the scan_movable_pages() will most likely return the same movable pfn. In this case, there is no condition to bail out, they will always run like this: ... [878020.623959] migrating pfn 1727813 failed ret:1 [878020.623960] page:00000000faa9673c refcount:3 mapcount:0 mapping:00000000144ccd79 index:0x14280025 pfn:0x1727813 [878020.623962] memcg:ffffa0ff82d5a000 [878020.623962] aops:def_blk_aops ino:fd00001 [878020.623964] flags: 0x17ffffc000206a(referenced|dirty|active|workingset|private|node=0|zone=2|lastcpupid=0x1fffff) [878020.623966] raw: 0017ffffc000206a ffffb0d14f50fbd8 ffffb0d14f50fbd8 ffffa0ff9c155018 [878020.623967] raw: 0000000014280025 ffffa10327d702d8 00000003ffffffff ffffa0ff82d5a000 [878020.623968] page dumped because: migration failure [878020.626196] migrating pfn 1727813 failed ret:1 [878020.626198] page:00000000faa9673c refcount:3 mapcount:0 mapping:00000000144ccd79 index:0x14280025 pfn:0x1727813 [878020.626200] memcg:ffffa0ff82d5a000 [878020.626200] aops:def_blk_aops ino:fd00001 [878020.626202] flags: 0x17ffffc000206a(referenced|dirty|active|workingset|private|node=0|zone=2|lastcpupid=0x1fffff) [878020.626204] raw: 0017ffffc000206a ffffb0d14f50fbd8 ffffb0d14f50fbd8 ffffa0ff9c155018 [878020.626205] raw: 0000000014280025 ffffa10327d702d8 00000003ffffffff ffffa0ff82d5a000 [878020.626206] page dumped because: migration failure ... Bail out when migration failures reach 3 times. Fixes: bb8965bd82fd ("mm, memory_hotplug: deobfuscate migration part of offlining") Signed-off-by: Yajun Deng <yajun.deng@linux.dev> --- mm/memory_hotplug.c | 19 ++++++++++++++----- 1 file changed, 14 insertions(+), 5 deletions(-)