On 06/13/2014 04:40 AM, Minchan Kim wrote: > On Thu, Jun 12, 2014 at 04:02:04PM +0200, Vlastimil Babka wrote: >> On 06/12/2014 01:49 AM, Minchan Kim wrote: >> >On Wed, Jun 11, 2014 at 02:22:30PM +0200, Vlastimil Babka wrote: >> >>On 06/11/2014 03:10 AM, Minchan Kim wrote: >> >>>On Mon, Jun 09, 2014 at 11:26:14AM +0200, Vlastimil Babka wrote: >> >>>>Async compaction aborts when it detects zone lock contention or need_resched() >> >>>>is true. David Rientjes has reported that in practice, most direct async >> >>>>compactions for THP allocation abort due to need_resched(). This means that a >> >>>>second direct compaction is never attempted, which might be OK for a page >> >>>>fault, but hugepaged is intended to attempt a sync compaction in such case and >> >>>>in these cases it won't. >> >>>> >> >>>>This patch replaces "bool contended" in compact_control with an enum that >> >>>>distinguieshes between aborting due to need_resched() and aborting due to lock >> >>>>contention. This allows propagating the abort through all compaction functions >> >>>>as before, but declaring the direct compaction as contended only when lock >> >>>>contantion has been detected. >> >>>> >> >>>>As a result, hugepaged will proceed with second sync compaction as intended, >> >>>>when the preceding async compaction aborted due to need_resched(). >> >>> >> >>>You said "second direct compaction is never attempted, which might be OK >> >>>for a page fault" and said "hugepagd is intented to attempt a sync compaction" >> >>>so I feel you want to handle khugepaged so special unlike other direct compact >> >>>(ex, page fault). >> >> >> >>Well khugepaged is my primary concern, but I imagine there are other >> >>direct compaction users besides THP page fault and khugepaged. >> >> >> >>>By this patch, direct compaction take care only lock contention, not rescheduling >> >>>so that pop questions. >> >>> >> >>>Is it okay not to consider need_resched in direct compaction really? >> >> >> >>It still considers need_resched() to back of from async compaction. >> >>It's only about signaling contended_compaction back to >> >>__alloc_pages_slowpath(). There's this code executed after the >> >>first, async compaction fails: >> >> >> >>/* >> >> * It can become very expensive to allocate transparent hugepages at >> >> * fault, so use asynchronous memory compaction for THP unless it is >> >> * khugepaged trying to collapse. >> >> */ >> >>if (!(gfp_mask & __GFP_NO_KSWAPD) || (current->flags & PF_KTHREAD)) >> >> migration_mode = MIGRATE_SYNC_LIGHT; >> >> >> >>/* >> >> * If compaction is deferred for high-order allocations, it is because >> >> * sync compaction recently failed. In this is the case and the caller >> >> * requested a movable allocation that does not heavily disrupt the >> >> * system then fail the allocation instead of entering direct reclaim. >> >> */ >> >>if ((deferred_compaction || contended_compaction) && >> >> (gfp_mask & __GFP_NO_KSWAPD)) >> >> goto nopage; >> >> >> >>Both THP page fault and khugepaged use __GFP_NO_KSWAPD. The first >> >>if() decides whether the second attempt will be sync (for >> >>khugepaged) or async (page fault). The second if() decides that if >> >>compaction was contended, then there won't be any second attempt >> >>(and reclaim) at all. Counting need_resched() as contended in this >> >>case is bad for khugepaged. Even for page fault it means no direct >> > >> >I agree khugepaged shouldn't count on need_resched, even lock contention >> >because it was a result from admin's decision. >> >If it hurts system performance, he should adjust knobs for khugepaged. >> > >> >>reclaim and a second async compaction. David says need_resched() >> >>occurs so often then it is a poor heuristic to decide this. >> > >> >But page fault is a bit different. Inherently, high-order allocation >> >(ie, above PAGE_ALLOC_COSTLY_ORDER) is fragile so all of the caller >> >shoud keep in mind that and prepare second plan(ex, 4K allocation) >> >so direct reclaim/compaction should take care of latency rather than >> >success ratio. >> >> Yes it's a rather delicate balance. But the plan is now to try >> balance this differently than using need_resched. >> >> >If need_resched in second attempt(ie, synchronous compaction) is almost >> >true, it means the process consumed his timeslice so it shouldn't be >> >greedy and gives a CPU resource to others. >> >> Synchronous compaction uses cond_resched() so that's fine I think? > > Sorry for being not clear. I post for the clarification before taking > a rest in holiday. :) > > When THP page fault occurs and found rescheduling while doing async > direct compaction, it goes "nopage" and fall-backed to 4K page. > It's good to me. > > Another topic: I couldn't find any cond_resched. Anyway, it could be > another patch. > Thanks for the explanation. I'll include a cond_resched() at the level of try_to_compact_pages() where it fits better, so it's not necessary in the place you suggested. This should solve the "don't be greedy" problem. I will not yet include the "bail out for latency" part because we are now slowly moving towards removing need_resched() as a condition for stopping compaction, and this would on the contrary extend it to prevent direct reclaim as well. David's data suggests that compaction often bails out due to need_resched(), so this would reduce the amount of direct reclaim and I don't want to touch that area in this series :) -- 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>