Re: swappiness=0 makes software suspend fail.

[Stuart Young <cef-lkml@optusnet.com.au>]

On Mon, 31 May 2004 05:47, Pavel Machek wrote:
> > Nick Piggin wrote:
> > > Pavel Machek wrote: 
>
> > >Andrew, in 2.6.6 shrink_all_memory() does not work if swappiness ==
> > >0. shrink_all_memory() calls balance_pgdat(), that calls
> > >shrink_zone(), and that calls refill_inactive_zone(), which looks at
> > >swappiness.
> > >
> > >Additional parameter to all these calls neutralizing swappiness would
> > >help, as would temporarily setting swappiness to 100 in
> > >shrink_all_memory. Is there a less ugly solution?
> >
> > I have a cleanup patch that allows this sort of thing to easily
> > be passed into the lower levels of reclaim functions. I don't
> > know if it would be to Andrew's taste though...
> >
> > It basically replaces all function parameters in vmscan.c with
> >
> > struct scan_control {
> > 	unsigned long nr_to_scan;
> > 	unsigned long nr_scanned;
> > 	unsigned long nr_reclaimed;
> > 	unsigned int gfp_mask;
> > 	struct page_state ps;
> > 	int may_writepage;
> > };
> >
> > So you could easily add a field for swsusp.
> >
> > Until something like this goes through, please don't fuglify
> > vmscan.c any more than it is... do the saving and restoring
> > thing that Nigel suggested please.
>
> Okay, this should solve it.
> 							Pavel
>
> --- clean/mm/vmscan.c	2004-05-20 23:08:37.000000000 +0200
> +++ linux/mm/vmscan.c	2004-05-30 21:45:41.000000000 +0200
> @@ -1098,10 +1098,13 @@
>  	pg_data_t *pgdat;
>  	int nr_to_free = nr_pages;
>  	int ret = 0;
> +	int old_swappiness = vm_swappiness;
>  	struct reclaim_state reclaim_state = {
>  		.reclaimed_slab = 0,
>  	};
>
> +	vm_swappiness = 100;
> +
>  	current->reclaim_state = &reclaim_state;
>  	for_each_pgdat(pgdat) {
>  		int freed;
> @@ -1115,6 +1118,8 @@
>  			break;
>  	}
>  	current->reclaim_state = NULL;
> +
> +	vm_swappiness = old_swappiness;
>  	return ret;
>  }
>  #endif

Good stuff. I've cc'ed Con Kolivas in on this as he's just recently posted his 
updated "Autoregulated VM swappiness" patch. In particular, this could also 
cause some issues if it made it into the main tree, as then this code might 
fail/cause issues (eg: as both could end up writing to vm_swappiness at the 
same time). This could possibly be a race condition as per Oliver's earlier 
observation (even if it's non-fatal, it's at least annoying).

Just thinking that Con could check to see somehow wether we're going to 
suspend, and not touch vm_swappiness anymore if that's the case. Yes I know 
the code isn't in the main tree yet, but who knows when another writer to 
vm_swappiness might end up in the main tree. It may also prove a good example 
for other potential writers. Pavel, Nigel, (anyone?) suggestions?

-- 
 Stuart Young (aka Cef)
 cef-lkml@optusnet.com.au is for LKML and related email only