vmscan: handle underflow for get_scan_ratio
in [Kernel]
|
Prev: [PATCH]vmscan: handle underflow for get_scan_ratio
Next: fs/cifs: Neaten cERROR and cFYI macros, reduce text space ~2.5K
From: KOSAKI Motohiro on 12 Apr 2010 21:40 > On 04/09/2010 05:20 PM, Andrew Morton wrote: > > > Come to that, it's not obvious that we need this in 2.6.34 either. What > > is the user-visible impact here? > > I suspect very little impact, especially during workloads > where we can just reclaim clean page cache at DEF_PRIORITY. > FWIW, the patch looks good to me, so: > > Acked-by: Rik van Riel <riel(a)redhat.com> > I'm surprised this ack a bit. Rik, do you have any improvement plan about streaming io detection logic? I think the patch have a slightly marginal benefit, it help to <1% scan ratio case. but it have big regression, it cause streaming io (e.g. backup operation) makes tons swap. So, I thought we sould do either, 1) drop this one 2) merge to change stream io detection logic improvement at first, and merge this one at second. Am i missing something? -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
From: Rik van Riel on 12 Apr 2010 22:50 On 04/12/2010 09:30 PM, KOSAKI Motohiro wrote: >> On 04/09/2010 05:20 PM, Andrew Morton wrote: >> >>> Come to that, it's not obvious that we need this in 2.6.34 either. What >>> is the user-visible impact here? >> >> I suspect very little impact, especially during workloads >> where we can just reclaim clean page cache at DEF_PRIORITY. >> FWIW, the patch looks good to me, so: >> >> Acked-by: Rik van Riel<riel(a)redhat.com> >> > > I'm surprised this ack a bit. Rik, do you have any improvement plan about > streaming io detection logic? > I think the patch have a slightly marginal benefit, it help to<1% scan > ratio case. but it have big regression, it cause streaming io (e.g. backup > operation) makes tons swap. How? From the description I believe it took 16GB in a zone before we start scanning anon pages when reclaiming at DEF_PRIORITY? Would that casue a problem? > So, I thought we sould do either, > 1) drop this one > 2) merge to change stream io detection logic improvement at first, and > merge this one at second. We may need better streaming IO detection, anyway. I have noticed that while heavy sequential reads are fine, the virtual machines on my desktop system do a lot of whole block writes. Presumably, a lot of those writes are to the same blocks, over and over again. This causes the blocks to be promoted to the active file list, which ends up growing the active file list to the point where things from the working set get evicted. All for file pages that may only get WRITTEN to by the guests, because the guests cache their own copy whenever they need to read them! I'll have to check the page cache code to see if it keeps frequently written pages as accessed. We may be better off evicting frequently written pages, and keeping our cache space for data that is read... -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
From: KOSAKI Motohiro on 13 Apr 2010 04:00 > On 04/12/2010 09:30 PM, KOSAKI Motohiro wrote: > >> On 04/09/2010 05:20 PM, Andrew Morton wrote: > >> > >>> Come to that, it's not obvious that we need this in 2.6.34 either. What > >>> is the user-visible impact here? > >> > >> I suspect very little impact, especially during workloads > >> where we can just reclaim clean page cache at DEF_PRIORITY. > >> FWIW, the patch looks good to me, so: > >> > >> Acked-by: Rik van Riel<riel(a)redhat.com> > >> > > > > I'm surprised this ack a bit. Rik, do you have any improvement plan about > > streaming io detection logic? > > I think the patch have a slightly marginal benefit, it help to<1% scan > > ratio case. but it have big regression, it cause streaming io (e.g. backup > > operation) makes tons swap. > > How? From the description I believe it took 16GB in > a zone before we start scanning anon pages when > reclaiming at DEF_PRIORITY? > > Would that casue a problem? Please remember, 2.6.27 has following +1 scanning modifier. zone->nr_scan_active += (zone_page_state(zone, NR_ACTIVE) >> priority) + 1; ^^^^ and, early (ano not yet merged) SplitLRU VM has similar +1. likes scan = zone_nr_lru_pages(zone, sc, l); scan >>= priority; scan = (scan * percent[file]) / 100 + 1; ^^^ We didn't think only one page scanning is not big matter. but it was not correct. we got streaming io bug report. the above +1 makes annoying swap io. because some server need big backup operation rather much much than physical memory size. example, If vm are dropping 1TB use once pages, 0.1% anon scanning makes 1GB scan. and almost server only have some gigabyte swap although it has >1TB memory. If my memory is not correct, please correct me. My point is, greater or smaller than 16GB isn't essential. all patches should have big worth than the downside. The description said "the impact sounds not a big deal", nobody disagree it. but it's worth is more little. I don't imagine this patch improve anything. > > > So, I thought we sould do either, > > 1) drop this one > > 2) merge to change stream io detection logic improvement at first, and > > merge this one at second. > > We may need better streaming IO detection, anyway. agreed. that's no doubt. > I have noticed that while heavy sequential reads are fine, > the virtual machines on my desktop system do a lot of whole > block writes. Presumably, a lot of those writes are to the > same blocks, over and over again. > > This causes the blocks to be promoted to the active file > list, which ends up growing the active file list to the > point where things from the working set get evicted. > > All for file pages that may only get WRITTEN to by the > guests, because the guests cache their own copy whenever > they need to read them! > > I'll have to check the page cache code to see if it > keeps frequently written pages as accessed. We may be > better off evicting frequently written pages, and > keeping our cache space for data that is read... One question, In such case your guest don't use DirectIO? Or do you talk about guest VM behabior? I guess inactive_file_is_low_global() can be improvement a lot. but I'm not sure. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
From: KOSAKI Motohiro on 13 Apr 2010 05:00 > > > I'm surprised this ack a bit. Rik, do you have any improvement plan about > > > streaming io detection logic? > > > I think the patch have a slightly marginal benefit, it help to<1% scan > > > ratio case. but it have big regression, it cause streaming io (e.g. backup > > > operation) makes tons swap. > > > > How? From the description I believe it took 16GB in > > a zone before we start scanning anon pages when > > reclaiming at DEF_PRIORITY? > > > > Would that casue a problem? > > Please remember, 2.6.27 has following +1 scanning modifier. > > zone->nr_scan_active += (zone_page_state(zone, NR_ACTIVE) >> priority) + 1; > ^^^^ > > and, early (ano not yet merged) SplitLRU VM has similar +1. likes > > scan = zone_nr_lru_pages(zone, sc, l); > scan >>= priority; > scan = (scan * percent[file]) / 100 + 1; > ^^^ > > We didn't think only one page scanning is not big matter. but it was not > correct. we got streaming io bug report. the above +1 makes annoying swap > io. because some server need big backup operation rather much much than > physical memory size. > > example, If vm are dropping 1TB use once pages, 0.1% anon scanning makes > 1GB scan. and almost server only have some gigabyte swap although it > has >1TB memory. > > If my memory is not correct, please correct me. > > My point is, greater or smaller than 16GB isn't essential. all patches > should have big worth than the downside. The description said "the impact > sounds not a big deal", nobody disagree it. but it's worth is more little. > I don't imagine this patch improve anything. And now I've merged this patch into my local vmscan patch queue. After solving streaming io issue, I'll put it to mainline. Thanks. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
From: Shaohua Li on 13 Apr 2010 21:30
On Tue, Apr 13, 2010 at 04:55:52PM +0800, KOSAKI Motohiro wrote: > > > > I'm surprised this ack a bit. Rik, do you have any improvement plan about > > > > streaming io detection logic? > > > > I think the patch have a slightly marginal benefit, it help to<1% scan > > > > ratio case. but it have big regression, it cause streaming io (e.g. backup > > > > operation) makes tons swap. > > > > > > How? From the description I believe it took 16GB in > > > a zone before we start scanning anon pages when > > > reclaiming at DEF_PRIORITY? > > > > > > Would that casue a problem? > > > > Please remember, 2.6.27 has following +1 scanning modifier. > > > > zone->nr_scan_active += (zone_page_state(zone, NR_ACTIVE) >> priority) + 1; > > ^^^^ > > > > and, early (ano not yet merged) SplitLRU VM has similar +1. likes > > > > scan = zone_nr_lru_pages(zone, sc, l); > > scan >>= priority; > > scan = (scan * percent[file]) / 100 + 1; > > ^^^ > > > > We didn't think only one page scanning is not big matter. but it was not > > correct. we got streaming io bug report. the above +1 makes annoying swap > > io. because some server need big backup operation rather much much than > > physical memory size. > > > > example, If vm are dropping 1TB use once pages, 0.1% anon scanning makes > > 1GB scan. and almost server only have some gigabyte swap although it > > has >1TB memory. > > > > If my memory is not correct, please correct me. > > > > My point is, greater or smaller than 16GB isn't essential. all patches > > should have big worth than the downside. The description said "the impact > > sounds not a big deal", nobody disagree it. but it's worth is more little. > > I don't imagine this patch improve anything. > > And now I've merged this patch into my local vmscan patch queue. > After solving streaming io issue, I'll put it to mainline. if the streaming io issue is popular, how about below patch against my last one? we take priority == DEF_PRIORITY an exception. Index: linux/mm/vmscan.c =================================================================== --- linux.orig/mm/vmscan.c 2010-04-14 09:03:28.000000000 +0800 +++ linux/mm/vmscan.c 2010-04-14 09:19:56.000000000 +0800 @@ -1629,6 +1629,22 @@ static void get_scan_count(struct zone * fraction[0] = ap; fraction[1] = fp; denominator = ap + fp + 1; + + /* + * memory pressure isn't high, we allow percentage underflow. This + * avoids swap in stream io case. + */ + if (priority == DEF_PRIORITY) { + if (fraction[0] * 99 < fraction[1]) { + fraction[0] = 0; + fraction[1] = 1; + denominator = 1; + } else if (fraction[1] * 99 < fraction[0]) { + fraction[0] = 1; + fraction[1] = 0; + denominator = 1; + } + } out: for_each_evictable_lru(l) { int file = is_file_lru(l); -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/ |