Return-Path: <linux-kernel-owner@vger.kernel.org>
Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand
	id <S314090AbSDKPNs>; Thu, 11 Apr 2002 11:13:48 -0400
Received: (majordomo@vger.kernel.org) by vger.kernel.org
	id <S314089AbSDKPNr>; Thu, 11 Apr 2002 11:13:47 -0400
Received: from bay-bridge.veritas.com ([143.127.3.10]:18901 "EHLO
	svldns02.veritas.com") by vger.kernel.org with ESMTP
	id <S314088AbSDKPNq>; Thu, 11 Apr 2002 11:13:46 -0400
Date: Thu, 11 Apr 2002 16:15:50 +0100 (BST)
From: Hugh Dickins <hugh@veritas.com>
To: Andrew Morton <akpm@zip.com.au>
cc: "Randy.Dunlap" <rddunlap@osdl.org>, linux-kernel@vger.kernel.org
Subject: Re: [patch-2.5.8-pre] swapinfo accounting
In-Reply-To: <3CB4DD71.DED82F57@zip.com.au>
Message-ID: <Pine.LNX.4.21.0204111543340.1231-100000@localhost.localdomain>
MIME-Version: 1.0
Content-Type: TEXT/PLAIN; charset=US-ASCII
Sender: linux-kernel-owner@vger.kernel.org
X-Mailing-List: linux-kernel@vger.kernel.org

On Wed, 10 Apr 2002, Andrew Morton wrote:
> "Randy.Dunlap" wrote:
> > 
> > It looks to me like mm/swapfile.c::si_swapinfo()
> > shouldn't be adding nr_to_be_unused to total_swap_pages
> > or nr_swap_pages for return in val->freeswap and
> > val->totalswap.

Good observation, thanks Randy, but wrong fix.

> whee, an si_swapinfo() maintainer.

That might be me?  I rewrote that code for 2.4.10
(when you called attention to si_swapinfo slowness).

> Your function sucks :)  I'm spending 15 CPU-seconds
> in there during a kernel build.  The problem appears
> to be that a fix from 2.4 hasn't been propagated
> forward.

Not a new fix needing propagation: it was right in 2.5.0.

> 2.4 has:
> 
>                 if (swap_info[i].flags != SWP_USED)
> 
> and 2.5 has:
> 
>                 if (!(swap_info[i].flags & SWP_USED))

It was mistakenly changed to this in 2.5.4, when an
additional SWP_BLOCKDEV flag was added (gone in 2.5.6).

> and I think the 2.4 version will fix the accounting
> problem you're seeing?

Yes, it does.

> (I haven't checked whather it's the _right_ fix, but
> it looks like it'll make it go away?)

It is the right fix; but since that condition was misunderstood,
and some other flag might be added in future, the safer patch
would be this more explicit one (which I'll now send to Linus
with a briefer description).

But I hope nobody backports this to 2.4, where it would be
wrong: 2.5.4 confusingly changed the nature of SWP_WRITEOK.

Hugh

--- 2.5.8-pre3/mm/swapfile.c	Mon Mar 11 12:30:56 2002
+++ linux/mm/swapfile.c	Thu Apr 11 15:26:51 2002
@@ -1095,7 +1095,8 @@
 	swap_list_lock();
 	for (i = 0; i < nr_swapfiles; i++) {
 		unsigned int j;
-		if (!(swap_info[i].flags & SWP_USED))
+		if (!(swap_info[i].flags & SWP_USED) ||
+		     (swap_info[i].flags & SWP_WRITEOK))
 			continue;
 		for (j = 0; j < swap_info[i].max; ++j) {
 			switch (swap_info[i].swap_map[j]) {

-
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/