> Am 17.01.2020 um 19:49 schrieb Qian Cai <cai@xxxxxx>: > > > >> On Jan 17, 2020, at 10:46 AM, Michal Hocko <mhocko@xxxxxxxxxx> wrote: >> >>> On Fri 17-01-20 10:05:12, Qian Cai wrote: >>> >>> >>>> On Jan 17, 2020, at 9:39 AM, Michal Hocko <mhocko@xxxxxxxxxx> wrote: >>>> >>>> Thanks a lot. Having it in a separate patch would be great. >>> >>> I was thinking about removing that WARN together in this v5 patch, >>> so there is less churn to touch the same function again. However, I >>> am fine either way, so just shout out if you feel strongly towards a >>> separate patch. >> >> I hope you meant moving rather than removing ;). The warning is useful >> because we shouldn't see unmovable pages in the movable zone. And a >> separate patch makes more sense because the justification is slightly >> different. We do not want to have a way for userspace to trigger the >> warning from userspace - even though it shouldn't be possible, but >> still. Only the offlining path should complain. > > Something like this? > > diff --git a/mm/page_alloc.c b/mm/page_alloc.c > index 621716a25639..32c854851e1f 100644 > --- a/mm/page_alloc.c > +++ b/mm/page_alloc.c > @@ -8307,7 +8307,6 @@ struct page *has_unmovable_pages(struct zone *zone, struct page *page, > } > return NULL; > unmovable: > - WARN_ON_ONCE(zone_idx(zone) == ZONE_MOVABLE); > return pfn_to_page(pfn + iter); > } > > diff --git a/mm/page_isolation.c b/mm/page_isolation.c > index e70586523ca3..08571b515d9f 100644 > --- a/mm/page_isolation.c > +++ b/mm/page_isolation.c > @@ -54,9 +54,11 @@ static int set_migratetype_isolate(struct page *page, int migratetype, int isol_ > > out: > spin_unlock_irqrestore(&zone->lock, flags); > + > if (!ret) > drain_all_pages(zone); > else if ((isol_flags & REPORT_FAILURE) && unmovable) We have a dedicated flag for the offlining part. > + WARN_ON_ONCE(zone_idx(zone) == ZONE_MOVABLE); > /* > * printk() with zone->lock held will guarantee to trigger a > * lockdep splat, so defer it here. > So, are we fine with unmovable data ending up in ZONE_MOVABLE as long as we can offline it? This might make my life in virtio-mem a little easier (I can unplug chunks falling into ZONE_MOVABLE).
