On Tue, Oct 24, 2017 at 10:12:58AM +0200, Vlastimil Babka wrote: > On 10/24/2017 06:44 AM, Joonsoo Kim wrote: > >>> I'm not sure what is the confusing semantic you mentioned. I think > >>> that set_migratetype_isolate() has confusing semantic and should be > >>> fixed since making the pageblock isolated doesn't need to check if > >>> there is unmovable page or not. Do you think that > >>> set_migratetype_isolate() need to check it? If so, why? > >> > >> My intuitive understanding of set_migratetype_isolate is that it either > >> suceeds and that means that the given pfn range can be isolated for the > >> given type of allocation (be it movable or cma). No new pages will be > >> allocated from this range to allow converging into a free range in a > >> finit amount of time. At least this is how the hotplug code would like > >> to use it and I suppose that the alloc_contig_range would like to > >> guarantee the same to not rely on a fixed amount of migration attempts. > > > > Yes, alloc_contig_range() also want to guarantee the similar thing. > > Major difference between them is 'given pfn range'. memory hotplug > > works by pageblock unit but alloc_contig_range() doesn't. > > alloc_contig_range() works by the page unit. However, there is no easy > > way to isolate individual page so it uses pageblock isolation > > regardless of 'given pfn range'. In this case, checking movability of > > all pages on the pageblock would cause the problem as I mentioned > > before. > > I couldn't look too closely yet, but do I understand correctly that the > *potential* problem (because as you say there are no such > alloc_contig_range callers) you are describing is not newly introduced > by Michal's series? Then his patch fixing the introduced regression This potential problem exists there before Michal's series if the migratetype of the target pageblock isn't MIGRATE_MOVABLE or MIGRATE_CMA. However, his series enlarges this potential problem surface. It would be the problem now even if the migratetype of the target pageblock is MIGRATE_MOVABLE. > should be enough for now, and further improvements could be posted on > top, and not vice versa? Please don't take it wrong, I agree the current > state is a bit of a mess and improvements are welcome. Also it seems to I'm not very sensitive that which patch is applied first. I can do rebase. But, IMHO, correct applying order is my patch first and then Michal's series. Anyway, Michal, feel free to do what you think correct. > me that Michal is right, and there's nothing preventing > alloc_contig_range() to allocate from CMA pageblocks for non-CMA > purposes (likely not movable), and that should be also fixed? I noticed the problem you mentioned now and, yes, it should be fixed. My patch will naturally fixes this issue, too. Thanks. -- To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to majordomo@xxxxxxxxx. For more info on Linux MM, see: http://www.linux-mm.org/ . Don't email: <a href=mailto:"dont@xxxxxxxxx";> email@xxxxxxxxx </a>
