Re: [patch] nfs: use GFP_NOFS preloads for radix-tree insertion

[Peter Zijlstra <peterz@infradead.org>]

On Thu, 2007-11-08 at 07:56 +0100, Nick Piggin wrote:
> Here is the NFS version. I guess Trond should ack it before you pick it
> up.
> 
> --
> 
> NFS should use GFP_NOFS mode radix tree preloads rather than GFP_ATOMIC
> allocations at radix-tree insertion-time. This is important to reduce the
> atomic memory requirement.

In another mail you said:

> Anyway we can also simplify the code because the insertion can't fail with a
> preload.

Can we please avoid adding strict dependencies on that as the preload
API is unsupportable in -rt.

> Signed-off-by: Nick Piggin <npiggin@suse.de>
> ---
> Index: linux-2.6/fs/nfs/write.c
> ===================================================================
> --- linux-2.6.orig/fs/nfs/write.c
> +++ linux-2.6/fs/nfs/write.c
> @@ -363,15 +363,13 @@ int nfs_writepages(struct address_space 
>  /*
>   * Insert a write request into an inode
>   */
> -static int nfs_inode_add_request(struct inode *inode, struct nfs_page *req)
> +static void nfs_inode_add_request(struct inode *inode, struct nfs_page *req)
>  {
>  	struct nfs_inode *nfsi = NFS_I(inode);
>  	int error;
>  
>  	error = radix_tree_insert(&nfsi->nfs_page_tree, req->wb_index, req);
> -	BUG_ON(error == -EEXIST);
> -	if (error)
> -		return error;
> +	BUG_ON(error);
>  	if (!nfsi->npages) {
>  		igrab(inode);
>  		if (nfs_have_delegation(inode, FMODE_WRITE))
> @@ -381,7 +379,6 @@ static int nfs_inode_add_request(struct 
>  	set_page_private(req->wb_page, (unsigned long)req);
>  	nfsi->npages++;
>  	kref_get(&req->wb_kref);
> -	return 0;
>  }
>  
>  /*
> @@ -593,6 +590,13 @@ static struct nfs_page * nfs_update_requ
>  		/* Loop over all inode entries and see if we find
>  		 * A request for the page we wish to update
>  		 */
> +		if (new) {
> +			if (radix_tree_preload(GFP_NOFS)) {
> +				nfs_release_request(new);
> +				return ERR_PTR(-ENOMEM);
> +			}
> +		}
> +
>  		spin_lock(&inode->i_lock);
>  		req = nfs_page_find_request_locked(page);
>  		if (req) {
> @@ -603,28 +607,27 @@ static struct nfs_page * nfs_update_requ
>  				error = nfs_wait_on_request(req);
>  				nfs_release_request(req);
>  				if (error < 0) {
> -					if (new)
> +					if (new) {
> +						radix_tree_preload_end();
>  						nfs_release_request(new);
> +					}
>  					return ERR_PTR(error);
>  				}
>  				continue;
>  			}
>  			spin_unlock(&inode->i_lock);
> -			if (new)
> +			if (new) {
> +				radix_tree_preload_end();
>  				nfs_release_request(new);
> +			}
>  			break;
>  		}
>  
>  		if (new) {
> -			int error;
>  			nfs_lock_request_dontget(new);
> -			error = nfs_inode_add_request(inode, new);
> -			if (error) {
> -				spin_unlock(&inode->i_lock);
> -				nfs_unlock_request(new);
> -				return ERR_PTR(error);
> -			}
> +			nfs_inode_add_request(inode, new);
>  			spin_unlock(&inode->i_lock);
> +			radix_tree_preload_end();
>  			req = new;
>  			goto zero_page;
>  		}
> 
> --
> To unsubscribe, send a message with 'unsubscribe linux-mm' in
> the body to majordomo@kvack.org.  For more info on Linux MM,
> see: http://www.linux-mm.org/ .
> Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>