Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752213Ab0H3GQh (ORCPT ); Mon, 30 Aug 2010 02:16:37 -0400 Received: from mail-iw0-f174.google.com ([209.85.214.174]:59204 "EHLO mail-iw0-f174.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751139Ab0H3GQg convert rfc822-to-8bit (ORCPT ); Mon, 30 Aug 2010 02:16:36 -0400 DomainKey-Signature: a=rsa-sha1; c=nofws; d=gmail.com; s=gamma; h=mime-version:in-reply-to:references:date:message-id:subject:from:to :cc:content-type:content-transfer-encoding; b=JeOCSVG747cg/PeuCfJdRN6+SN6xLILRNT4f+6mK0eHFCgPTCqYAihSD22SQkbd5p3 9IDTTA3AxDV71/2cM49XeJA032wNPMqk8FmUTyhgDmDdjrDkDrQLdyG50tEtMQyZPamU 4wVT5pnTm2iyR4XN15oqQ34Axd+QvBbAIVTZc= MIME-Version: 1.0 In-Reply-To: References: <1283096628-4450-1-git-send-email-minchan.kim@gmail.com> <4C7ABD14.9050207@redhat.com> Date: Mon, 30 Aug 2010 15:16:35 +0900 Message-ID: Subject: Re: [PATCH] vmscan: prevent background aging of anon page in no swap system From: Minchan Kim To: Ying Han Cc: Rik van Riel , Andrew Morton , linux-mm , LKML , Venkatesh Pallipadi , KOSAKI Motohiro , Johannes Weiner Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 8BIT Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 3988 Lines: 102 On Mon, Aug 30, 2010 at 2:40 PM, Ying Han wrote: > On Sun, Aug 29, 2010 at 5:18 PM, Minchan Kim wrote: >> Hi Ying, >> >> On Mon, Aug 30, 2010 at 6:23 AM, Ying Han wrote: >>> On Sun, Aug 29, 2010 at 1:03 PM, Rik van Riel wrote: >>>> On 08/29/2010 01:45 PM, Ying Han wrote: >>>> >>>>> There are few other places in vmscan where we check nr_swap_pages and >>>>> inactive_anon_is_low. Are we planning to change them to use >>>>> total_swap_pages >>>>> to be consistent ? >>>> >>>> If that makes sense, maybe the check can just be moved into >>>> inactive_anon_is_low itself? >>> >>> That was the initial patch posted, instead we changed to use >>> total_swap_pages instead. How this patch looks: >>> >>> @@ -1605,6 +1605,9 @@ static int inactive_anon_is_low(struct zone >>> *zone, struct scan_control *sc) >>> ?{ >>> ? ? ? ?int low; >>> >>> + ? ? ? if (total_swap_pages <= 0) >>> + ? ? ? ? ? ? ? return 0; >>> + >>> ? ? ? ?if (scanning_global_lru(sc)) >>> ? ? ? ? ? ? ? ?low = inactive_anon_is_low_global(zone); >>> ? ? ? ?else >>> @@ -1856,7 +1859,7 @@ static void shrink_zone(int priority, struct zone *zone, >>> ? ? ? ? * Even if we did not try to evict anon pages at all, we want to >>> ? ? ? ? * rebalance the anon lru active/inactive ratio. >>> ? ? ? ? */ >>> - ? ? ? if (inactive_anon_is_low(zone, sc) && nr_swap_pages > 0) >>> + ? ? ? if (inactive_anon_is_low(zone, sc)) >>> ? ? ? ? ? ? ? ?shrink_active_list(SWAP_CLUSTER_MAX, zone, sc, priority, 0); >>> >>> ? ? ? ?throttle_vm_writeout(sc->gfp_mask); >>> >>> --Ying >>> >>>> >> >> I did it intentionally since inactive_anon_is_low have been used both >> direct reclaim and background path. In this point, your patch could >> make side effect in swap enabled system when swap is full. >> >> I think we need aging in only background if system is swap full. >> That's because if the swap space is full, we don't reclaim anon pages >> in direct reclaim path with (nr_swap_pages < 0) ?and even have been >> not rebalance it until now. >> I think direct reclaim path is important about latency as well as >> reclaim's effectiveness. >> So if you don't mind, I hope direct reclaim patch would be left just as it is. > > Minchan, I would prefer to make kswapd as well as direct reclaim to be > consistent if possible. > They both try to reclaim pages when system is under memory pressure, > and also do not make > much sense to look at anon lru if no swap space available. Either > because of no swapon or run > out of swap space. In out of swap space, The few swap space would become more precious. So I think we still need background aging to protect hot page swap out. But I admit it's hard to measure it so I can't insist on. But I wanted to maintain it as it is to avoid _unexpected_ side effect. And your patch can't compile out inactive_anon_is_low call in non swap configurable system. It makes unnecessary call. So I want to use nr_swap_pages && inactive_anon_is_low. For it, I sended following patch at last version diff --git a/mm/vmscan.c b/mm/vmscan.c index 1b145e6..0b8a3ce 100644 --- a/mm/vmscan.c +++ b/mm/vmscan.c @@ -1747,7 +1747,7 @@ static void shrink_zone(int priority, struct zone *zone, * Even if we did not try to evict anon pages at all, we want to * rebalance the anon lru active/inactive ratio. */ - if (inactive_anon_is_low(zone, sc) && nr_swap_pages > 0) + if (nr_swap_pges > 0 && inactive_anon_is_low(zone, sc)) shrink_active_list(SWAP_CLUSTER_MAX, zone, sc, priority, 0); throttle_vm_writeout(sc->gfp_mask); But Andrew merged middle version. I will send this patch again. Thanks. -- 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/