Re: [patch-2.5.8-pre] swapinfo accounting

[Hugh Dickins]

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]) {