Hi Mel, On Fri, Aug 10, 2012 at 09:34:38AM +0100, Mel Gorman wrote: > On Fri, Aug 10, 2012 at 08:27:33AM +0900, Minchan Kim wrote: > > > <SNIP> > > > > > > The intention is that an allocation can fail but each subsequent attempt > > > will > > > try harder until there is success. Each allocation request does a portion > > > of the necessary work to spread the cost between multiple requests. Take > > > THP for example where there is a constant request for THP allocations > > > for whatever reason (heavy fork workload, large buffer allocation being > > > populated etc.). Some of those allocations fail but if they do, future > > > THP requests will reclaim more pages. When compaction resumes again, it > > > will be more likely to succeed and compact_defer_shift gets reset. In the > > > specific case of THP there will be allocations that fail but khugepaged > > > will promote them later if the process is long-lived. > > > > You assume high-order allocation are *constant* and I guess your test > > enviroment > > is optimal for it. > > Ok, my example stated they were constant because it was the easiest to > illustrate but it does not necessarily have to be the case. The high-order > allocation requests can be separated by any length of time with a read or > write stream running in the background applying a small amount of memory > pressure and the same scenario applies. > > > I agree your patch if we can make sure such high-order > > allocation are always constant. But, is it true? Otherwise, your patch > > could reclaim > > too many pages unnecessary and it could reduce system performance by > > eviction > > The "too many pages unnecessarily" is unlikely. For compact_defer_shift to be > elevated there has to have been recent failures by try_to_compact_pages(). If > compact_defer_shift is elevated and a large process exited then > try_to_compact_pages() may succeed and reset compact_defer_shift without > calling direct reclaim and entering this path at all. > > > of page cache and swap out of workingset part. That's a concern to me. > > In summary, I think your patch is rather agressive so how about this? > > > > diff --git a/mm/vmscan.c b/mm/vmscan.c > > index 66e4310..0cb2593 100644 > > --- a/mm/vmscan.c > > +++ b/mm/vmscan.c > > @@ -1708,6 +1708,7 @@ static inline bool should_continue_reclaim(struct > > lruvec *lruvec, > > { > > unsigned long pages_for_compaction; > > unsigned long inactive_lru_pages; > > + struct zone *zone; > > > > /* If not in reclaim/compaction mode, stop */ > > if (!in_reclaim_compaction(sc)) > > @@ -1741,6 +1742,15 @@ static inline bool should_continue_reclaim(struct > > lruvec *lruvec, > > * inactive lists are large enough, continue reclaiming > > */ > > pages_for_compaction = (2UL << sc->order); > > + > > + /* > > + * If compaction is deferred for this order then scale the number of > > + * pages reclaimed based on the number of consecutive allocation > > + * failures > > + */ > > + zone = lruvec_zone(lruvec); > > + if (zone->compact_order_failed <= sc->order) { > > + if (zone->compact_defer_shift) > > + /* > > + * We can't make sure deferred requests will come > > again > > + * The probability is 50:50. > > + */ > > + pages_for_compaction <<= (zone->compact_defer_shift > > - 1); > > This patch is not doing anything radically different to my own patch. > compact_defer_shift == 0 if allocations succeeded recently using > reclaim/compaction at its normal level. Functionally the only difference > is that you delay when more pages get reclaim by one failure. > > Was that what you intended? If so, it's not clear why you think this patch > is better or how you concluded that the probability of another failure was > "50:50".
Please ignore my comment about this patch. I got confused between compat_considered and compact_defer_shift. compact_defer_shift is indication of constant high order page allocationfailing so I have no objection any more. Sorry for the noise. :( -- Kind regards, Minchan Kim -- 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/