Re: filecache/swapcache questions [RFC] [RFT] [PATCH] kanoj-mm12-2.3.8

[kanoj@google.engr.sgi.com (Kanoj Sarcar)]

> Here it is the alternate fix:
> 
> Index: mm/swap_state.c
> ===================================================================
> RCS file: /var/cvs/linux/mm/swap_state.c,v
> retrieving revision 1.1.1.3
> diff -u -r1.1.1.3 swap_state.c
> --- mm/swap_state.c	1999/06/14 15:30:09	1.1.1.3
> +++ mm/swap_state.c	1999/06/28 10:15:15
> @@ -125,7 +125,7 @@
>  		"swap_duplicate: entry %08lx, offset exceeds max\n", entry);
>  	goto out;
>  bad_unused:
> -	printk(KERN_ERR
> +	printk(KERN_WARNING
>  		"swap_duplicate at %8p: entry %08lx, unused page\n", 
>  	       __builtin_return_address(0), entry);
>  	goto out;
> @@ -291,20 +291,15 @@
>  	       entry, wait ? ", wait" : "");
>  #endif
>  	/*
> -	 * Make sure the swap entry is still in use.
> -	 */
> -	if (!swap_duplicate(entry))	/* Account for the swap cache */
> -		goto out;
> -	/*
>  	 * Look for the page in the swap cache.
>  	 */
>  	found_page = lookup_swap_cache(entry);
>  	if (found_page)
> -		goto out_free_swap;
> +		goto out;
>  
>  	new_page_addr = __get_free_page(GFP_USER);
>  	if (!new_page_addr)
> -		goto out_free_swap;	/* Out of memory */
> +		goto out;	/* Out of memory */
>  	new_page = mem_map + MAP_NR(new_page_addr);
>  
>  	/*
> @@ -313,6 +308,11 @@
>  	found_page = lookup_swap_cache(entry);
>  	if (found_page)
>  		goto out_free_page;
> +	/*
> +	 * Make sure the swap entry is still in use.
> +	 */
> +	if (!swap_duplicate(entry))	/* Account for the swap cache */
> +		goto out_free_page;
>  	/* 
>  	 * Add it to the swap cache and read its contents.
>  	 */
> @@ -330,8 +330,6 @@
>  
>  out_free_page:
>  	__free_page(new_page);
> -out_free_swap:
> -	swap_free(entry);
>  out:
>  	return found_page;
>  }
> 
> 
> 
> NOTE: this will cause swap_duplicate to generate some warning message but

Or not, depending on whether the swap id has already been allocated 
to a newly added swap device. In which case, the worst we will do is
read-ahead in some unneeded swap pages. Not too bad ... I thought 
about this solution, and figured that a better idea is probably to
give up on read-ahead in swapin_readahead() if the faulting pte
had already been swapped in.

Anyway, it seems to prevent fork/exit races, grabbing mmap_sem is
needed in the swapoff path. Best to use that synchronization, since
the fault path also grabs mmap_sem.

Thanks.

Kanoj

> everything will work fine then, exactly because the swapin code just check
> if the pte is changed (swapped in from swapoff) before looking if
> read_swap_cache returned a NULL pointer. (also the shm.c swap-cache code
> checks if the pte is changed before to go oom).
> 
> But probably the right thing to do is to grab the mm semaphore in swapoff
> as you did since we don't risk to deadlock there :).
> 
> Comments?
> 
> Andrea
> 
> --
> To unsubscribe, send a message with 'unsubscribe linux-mm' in
> the body to majordomo@kvack.org.  For more info on Linux MM,
> see: http://humbolt.geo.uu.nl/Linux-MM/
> 

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://humbolt.geo.uu.nl/Linux-MM/