linux-kernel - Re: [patch] mm: don't free swap slots on page deactivation

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <Pine.LNX.4.64.0902252022460.19132@blonde.anvils>
Date:	Wed, 25 Feb 2009 20:40:26 +0000 (GMT)
From:	Hugh Dickins <hugh@...itas.com>
To:	Johannes Weiner <hannes@...xchg.org>
cc:	Andrew Morton <akpm@...ux-foundation.org>,
	Rik van Riel <riel@...hat.com>, linux-mm@...ck.org,
	linux-kernel@...r.kernel.org
Subject: Re: [patch] mm: don't free swap slots on page deactivation

On Wed, 25 Feb 2009, Johannes Weiner wrote:

> The pagevec_swap_free() at the end of shrink_active_list() was
> introduced in 68a22394 "vmscan: free swap space on swap-in/activation"
> when shrink_active_list() was still rotating referenced active pages.
> 
> In 7e9cd48 "vmscan: fix pagecache reclaim referenced bit check" this
> was changed, the rotating removed but the pagevec_swap_free() after
> the rotation loop was forgotten, applying now to the pagevec of the
> deactivation loop instead.
> 
> Now swap space is freed for deactivated pages.  And only for those
> that happen to be on the pagevec after the deactivation loop.
> 
> Complete 7e9cd48 and remove the rest of the swap freeing.
> 
> Signed-off-by: Johannes Weiner <hannes@...xchg.org>
> Cc: Rik van Riel <riel@...hat.com>

Nice observation.  I was going to object that the original code was
indifferent to whether it was freeing swap from active or inactive,
they all got lumped into the same pvec.  But that was just an oversight
in the original code: you're right that if it was our intention to free
swap from inactive pages here (when vm_swap_full), then we'd be freeing
it from them in the loop above (where the buffer_heads_over_limit
pagevec_strip is done).

Once upon a time (early 2007), testing an earlier incarnation of that
code, I did find almost nothing being freed by that pagevec_swap_free
anyway: other vm_swap_full frees were being effective, effective
enough to render this one rather pointless, even when it was operating
as intended.  But I never got around to checking on that in 2008's
splitLRU patches, and a lot changed in between: I may be misleading.

If Rik agrees (I think these do need his Ack), note that there are
no other users of pagevec_swap_free, so you'd do well to remove it
from mm/swap.c and include/linux/pagevec.h - I can well imagine us
wanting to bring it back some time, but can easily look it up when
and if we do need it again in the future.

Hugh

> ---
>  mm/vmscan.c |    3 ---
>  1 file changed, 3 deletions(-)
> 
> --- a/mm/vmscan.c
> +++ b/mm/vmscan.c
> @@ -1303,9 +1303,6 @@ static void shrink_active_list(unsigned 
>  	spin_unlock_irq(&zone->lru_lock);
>  	if (buffer_heads_over_limit)
>  		pagevec_strip(&pvec);
> -	if (vm_swap_full())
> -		pagevec_swap_free(&pvec);
> -
>  	pagevec_release(&pvec);
>  }
>  
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/