Prev: Documentation/kbuild: fixed typos and wording
Next: [PATCH v3] Documentation/sysctl/vm.txt typo
From: Minchan Kim on 26 Jun 2010 21:10
On Fri, Jun 25, 2010 at 11:07 PM, Christoph Lameter
<cl(a)linux-foundation.org> wrote:
> On Fri, 25 Jun 2010, KOSAKI Motohiro wrote:
>
>> Fix simple argument error. Usually 'order' is very small value than
>> lru_pages. then it can makes unnecessary icache dropping.
>
> This is going to reduce the delta that is added to shrinker->nr
> significantly thereby increasing the number of times that shrink_slab() is
> called.
>
> What does the "lru_pages" parameter do in shrink_slab()? Looks
> like its only role is as a divison factor in a complex calculation of
> pages to be scanned.

Yes. But I think it can make others confuse like this.

Except zone_reclaim, lru_pages had been used for balancing slab
reclaim VS page reclaim.
So lru_page naming is a good.

But in 0ff38490, you observed rather corner case.
AFAIU with your description, you wanted to shrink slabs until
unsuccessful or reached the limit.
So you intentionally passed order instead of the number of lru pages
for shrinking many slabs as possible as.

So at least, we need some comment to prevent confusion.

diff --git a/mm/vmscan.c b/mm/vmscan.c
index 9c7e57c..5523eae 100644
--- a/mm/vmscan.c
+++ b/mm/vmscan.c
@@ -2626,6 +2626,9 @@ static int __zone_reclaim(struct zone *zone,
gfp_t gfp_mask, unsigned int order)
*
* Note that shrink_slab will free memory on all zones and may
* take a long time.
+ *
+ * We pass order instead of lru_pages for shrinking slab
+ * as much as possible.
*/
while (shrink_slab(sc.nr_scanned, gfp_mask, order) &&
zone_page_state(zone, NR_SLAB_RECLAIMABLE) >


>
> do_try_to_free_pages passes 0 as "lru_pages" to shrink_slab() when trying
> to do cgroup lru scans. Why is that?
>

memcg doesn't call shrink_slab.


--
Kind regards,
Minchan Kim
--
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: Minchan Kim on 9 Jul 2010 04:40
On Thu, Jul 8, 2010 at 4:40 PM, KOSAKI Motohiro
<kosaki.motohiro(a)jp.fujitsu.com> wrote:
> Fix simple argument error. Usually 'order' is very small value than
> lru_pages. then it can makes unnecessary icache dropping.
>
> Signed-off-by: KOSAKI Motohiro <kosaki.motohiro(a)jp.fujitsu.com>
Reviewed-by: Minchan Kim <minchan.kim(a)gmail.com>

With your test result, This patch makes sense to me.
Please, include your test result in description.

--
Kind regards,
Minchan Kim
--
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: Minchan Kim on 15 Jul 2010 21:50
On Fri, Jul 16, 2010 at 10:39 AM, KOSAKI Motohiro
<kosaki.motohiro(a)jp.fujitsu.com> wrote:
>> > � � nr_slab_pages0 = zone_page_state(zone, NR_SLAB_RECLAIMABLE);
>> > � � if (nr_slab_pages0 > zone->min_slab_pages) {
>> > + � � � � � unsigned long lru_pages = zone_reclaimable_pages(zone);
>> > +
>> > � � � � � � /*
>> > � � � � � � �* shrink_slab() does not currently allow us to determine how
>> > � � � � � � �* many pages were freed in this zone. So we take the current
>> > @@ -2622,7 +2624,7 @@ static int __zone_reclaim(struct zone *zone, gfp_t gfp_mask, unsigned int order)
>> > � � � � � � �* Note that shrink_slab will free memory on all zones and may
>> > � � � � � � �* take a long time.
>> > � � � � � � �*/
>> > - � � � � � while (shrink_slab(sc.nr_scanned, gfp_mask, order) &&
>> > + � � � � � while (shrink_slab(sc.nr_scanned, gfp_mask, lru_pages) &&
>> > � � � � � � � � � �(zone_page_state(zone, NR_SLAB_RECLAIMABLE) + nr_pages >
>> > � � � � � � � � � � nr_slab_pages0))
>> > � � � � � � � � � � ;
>>
>> Wouldn't it be better to recalculate zone_reclaimable_pages() each time
>> around the loop? �For example, shrink_icache_memory()->prune_icache()
>> will remove pagecache from an inode if it hits the tail of the list.
>> This can change the number of reclaimable pages by squigabytes, but
>> this code thinks nothing changed?
>
> Ah, I missed this. incrementa patch is here.
>
> thank you!
>
>
>
> From 8f7c70cfb4a25f8292a59564db6c3ff425a69b53 Mon Sep 17 00:00:00 2001
> From: KOSAKI Motohiro <kosaki.motohiro(a)jp.fujitsu.com>
> Date: Fri, 16 Jul 2010 08:40:01 +0900
> Subject: [PATCH] vmscan: recalculate lru_pages on each shrink_slab()
>
> Andrew Morton pointed out shrink_slab() may change number of reclaimable
> pages (e.g. shrink_icache_memory()->prune_icache() will remove unmapped
> pagecache).
>
> So, we need to recalculate lru_pages on each shrink_slab() calling.
> This patch fixes it.
>
> Signed-off-by: KOSAKI Motohiro <kosaki.motohiro(a)jp.fujitsu.com>
Reviewed-by: Minchan Kim <minchan.kim(a)gmail.com>

It does make sense.

--
Kind regards,
Minchan Kim
--
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/
 | 
Pages: 1
Prev: Documentation/kbuild: fixed typos and wording
Next: [PATCH v3] Documentation/sysctl/vm.txt typo