On Mon, Feb 6, 2017 at 8:42 PM, Michal Hocko <mhocko@xxxxxxxxxx> wrote: > On Mon 06-02-17 20:05:21, vinayak menon wrote: > [...] >> By scan I meant pages scanned by shrink_node_memcg/shrink_list >> which is passed as nr_scanned to vmpressure. The calculation of >> pressure for tree is done at the end of vmpressure_win and it is >> that calculation which underflows. With this patch we want only the >> underflow to be avoided. But if we make (reclaimed = scanned) in >> vmpressure(), we change the vmpressure value even when there is no >> underflow right ? >> >> Rewriting the above e.g again. First call to vmpressure with >> nr_scanned=1 and nr_reclaimed=512 (THP) Second call to vmpressure >> with nr_scanned=511 and nr_reclaimed=0 In the second call >> vmpr->tree_scanned becomes equal to vmpressure_win and the work >> is scheduled and it will calculate the vmpressure as 0 because >> tree_reclaimed = 512 >> >> Similarly, if scanned is made equal to reclaimed in vmpressure() >> itself as you had suggested, First call to vmpressure with >> nr_scanned=1 and nr_reclaimed=512 (THP) And in vmpressure, we >> make nr_scanned=1 and nr_reclaimed=1 Second call to vmpressure >> with nr_scanned=511 and nr_reclaimed=0 In the second call >> vmpr->tree_scanned becomes equal to vmpressure_win and the work is >> scheduled and it will calculate the vmpressure as critical, because >> tree_reclaimed = 1 >> >> So it makes a difference, no? > > OK, I see what you meant. Thanks for the clarification. And you are > right that normalizing nr_reclaimed to nr_scanned is a wrong thing to > do because that just doesn't aggregate the real work done. Normalizing > nr_scanned to nr_reclaimed should be better - or it would be even better > to count the scanned pages properly... > With the slab reclaimed issue fixed separately, only the THP case exists AFAIK. In the case of THP, as I understand from one of Minchan's reply, the scan is actually 1. i.e. Only a single huge page is scanned to get 512 reclaimed pages. So the cost involved was scanning a single page. In that case, there is no need to normalize the nr_scanned, no? > My main concern of doing this normalization late on aggregated numbers > is just weird. We are mixing numbers from parallel reclaimers and that > might just add more confusion. It is better to do the fixup as soon as > possible when we still have at least an idea that this was a THP page > scanned and reclaimed. > > If we get back to your example it works as you expect just due to good > luck. Just make your nr_scanned=511 and nr_reclaimed=0 be a separate > event and you have your critical event. You have no real control over > when a new event is fired because parallel reclaimers are basically > unpredictable. > -- > Michal Hocko > SUSE Labs -- 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>