On Fri, Aug 01, 2014 at 10:51:07AM +0200, Vlastimil Babka wrote: > On 07/30/2014 06:22 PM, Vlastimil Babka wrote: > >On 07/29/2014 11:12 AM, Vlastimil Babka wrote: > >>On 07/29/2014 08:38 AM, Joonsoo Kim wrote: > >>> > >>>I still don't understand why defer_compaction() is needed here. > >>>defer_compaction() is intended for not struggling doing compaction on > >>>the zone where we already have tried compaction and found that it > >>>isn't suitable for compaction. Allocation failure doesn't tell us > >>>that we have tried compaction for all the zone range so we shouldn't > >>>make a decision here to defer compaction on this zone carelessly. > >> > >>OK I can remove that, it should make the code nicer anyway. > > > >Weird, that removal of this defer_compaction() call seems ho have > >quadrupled compact_stall and compact_fail counts. The scanner pages > >counters however increased by only 10% so that could indicate the > >problem is occuring only in a small zone such as DMA. Could be another > >case of mismatch between watermark checking in compaction and > >allocation? Perhaps the lack of proper classzone_idx in the compaction > >check? Sigh. > > Yep so it was the DMA zone returning COMPACT_PARTIAL from the > compaction_suitable() check done at the very beginning of > compact_zone(). The meaning of that is "the allocation should > succeed without compaction", so compaction is not done at all. Yet > the COMPACT_PARTIAL return value means it counts as a stall, even > with the patch that doesn't count COMPACT_SKIPPED as stalls. > The watermark check in try_to_compact_pages() also apparently > succeeds as the compaction is not being deferred. With deferral > removed from __alloc_pages_direct_compact(), this zone will be > attempted uselessly each time, and deferred_compaction is > practically never reported back. > > So for now I think it would be best to leave the defer_compaction() > call in __alloc_pages_direct_compact() as it is. Fixing this in a > better way would require more investigation (I guess the lack of > classzone_idx in compaction makes the difference for the watermark > checks here) and another patch(es), which I'll attempt, but I don't > want to further grow this series with new patches right now.
Okay. compaction has many inconsistent watermark check so we cannot easily understand how/why it works. Cleaning it up looks really good topic to investigate. :) Thanks. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
