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

[Nick Piggin <npiggin@suse.de>]

On Tue, Nov 13, 2007 at 11:55:45AM +0100, Peter Zijlstra wrote:
> 
> 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.

You can surely support it. You just have to do per-thread preloads if you
want preemption left on.



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