Messages in this thread | | | Date | Mon, 12 Jul 2010 07:28:09 +0900 | Subject | Re: [PATCH] vmscan: stop meaningless loop iteration when no reclaimable slab | From | Minchan Kim <> |
| |
On Fri, Jul 9, 2010 at 8:04 PM, KOSAKI Motohiro <kosaki.motohiro@jp.fujitsu.com> wrote: >> On Fri, Jul 9, 2010 at 7:13 PM, KOSAKI Motohiro >> <kosaki.motohiro@jp.fujitsu.com> wrote: >> > If number of reclaimable slabs are zero, shrink_icache_memory() and >> > shrink_dcache_memory() return 0. but strangely shrink_slab() ignore >> > it and continue meaningless loop iteration. >> > >> > This patch fixes it. >> > >> > Signed-off-by: KOSAKI Motohiro <kosaki.motohiro@jp.fujitsu.com> >> > --- >> > mm/vmscan.c | 5 +++++ >> > 1 files changed, 5 insertions(+), 0 deletions(-) >> > >> > diff --git a/mm/vmscan.c b/mm/vmscan.c >> > index 0f9f624..8f61adb 100644 >> > --- a/mm/vmscan.c >> > +++ b/mm/vmscan.c >> > @@ -243,6 +243,11 @@ unsigned long shrink_slab(unsigned long scanned, gfp_t gfp_mask, >> > int nr_before; >> > >> > nr_before = (*shrinker->shrink)(0, gfp_mask); >> > + /* no slab objects, no more reclaim. */ >> > + if (nr_before == 0) { >> > + total_scan = 0; >> >> Why do you reset totoal_scan to 0? > > If shab objects are zero, we don't need more reclaim. > >> I don't know exact meaning of shrinker->nr. > > similar meaning of reclaim_stat->nr_saved_scan. > If total_scan can't divide SHRINK_BATCH(128), saving remainder and using at next shrink_slab(). > >> AFAIU, it can affect next shrinker's total_scan. >> Isn't it harmful? > > No. This loop is > > total_scan = shrinker->nr; /* Reset and init total_scan */ > shrinker->nr = 0; > > while (total_scan >= SHRINK_BATCH) { > nr_before = (*shrinker->shrink)(0, gfp_mask); > /* no slab objects, no more reclaim. */ > if (nr_before == 0) { > total_scan = 0; > break; > } > shrink_ret = (*shrinker->shrink)(this_scan, gfp_mask); > if (shrink_ret == -1) > break; > if (shrink_ret < nr_before) > ret += nr_before - shrink_ret; > total_scan -= this_scan; > } > > shrinker->nr += total_scan; /* save remainder #of-scan */ > > I can't understand your point.
old shrink_slab
shrinker->nr += delta; /* Add delta to previous shrinker's remained count */ total_scan = shrinker->nr;
while(total_scan >= SHRINK_BATCH) { nr_before = shrink(xxx); total_scan =- this_scan; }
shrinker->nr += total_scan;
The total_scan can always be the number < SHRINK_BATCH. So, when next shrinker calcuates loop count, the number can affect.
new shrink_slab
shrinker->nr += delta; /* nr is always zero by your patch */ total_scan = shrinker->nr;
while(total_scan >= SHRINK_BATCH) { nr_before = shrink(xxx); if (nr_before == 0) { total_scan = 0; break; } }
shrinker->nr += 0;
But after your patch, total_scan is always zero. It never affect next shrinker's loop count.
Am I missing something? -- Kind regards, Minchan Kim -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
| |