Re: [PATCH -mm] mm/page_isolation: fix potential warning from user

[Michal Hocko <mhocko@kernel.org>]

On Mon 20-01-20 09:50:33, Anshuman Khandual wrote:
> Hello Qian,
> 
> On 01/20/2020 09:12 AM, Qian Cai wrote:
> > It makes sense to call the WARN_ON_ONCE(zone_idx(zone) == ZONE_MOVABLE)
> > from the offlining path, but should avoid triggering it from userspace,
> > i.e, from is_mem_section_removable().
> 
> Could you elaborate why it makes sense not to warn about an unmovable
> ZONE_MOVABLE page when an user tries to query about a memory block
> device's movability through sysfs ?

Because somebody might have panic_on_warn and then this is unlikely (but
not impossible) way to put the system down by arbitrary user. Besides
that it is stupid to warn when we convey the information to the
userspace anyway.
 
[...]
> > +	} else {
> > +		if (isol_flags & MEMORY_OFFLINE)
> > +			WARN_ON_ONCE(zone_idx(zone) == ZONE_MOVABLE);> +
> > +		if ((isol_flags & REPORT_FAILURE) && !IS_ERR(unmovable))
> > +			/*
> > +			 * printk() with zone->lock held will likely trigger a
> > +			 * lockdep splat, so defer it here.
> > +			 */
> > +			dump_page(unmovable, "unmovable page");
> > +	}
> > +
> > +	return !!unmovable;
> >  }
> >  
> >  static void unset_migratetype_isolate(struct page *page, unsigned migratetype)
> 
> set_migratetype_isolate() gets called from CMA as well as HugeTLB
> allocation paths, so its not only during offline. Hence the commit
> message should be changed to reflect this.

We should just report for all those cases I believe.
-- 
Michal Hocko
SUSE Labs