reduce NFS stack usage

David Mansfield spotted a bug in a memset() call, in my initial patch.

Attached is an updated version. The only change is s/sizeof(my_entry)/sizeof(*my_entry)/ in the memset.

Jeff

------------------------------------------------------------------------

===== fs/nfs/dir.c 1.12 vs edited =====

fs/nfs/dir.c +51 -20

@@ -349,14 +349,25 @@
 {
 	struct dentry	*dentry = filp->f_dentry;
 	struct inode	*inode = dentry->d_inode;
-	nfs_readdir_descriptor_t my_desc,
-			*desc = &my_desc;
-	struct nfs_entry my_entry;
+	nfs_readdir_descriptor_t *desc;
+	struct nfs_entry *my_entry;
 	long		res;
+	int		rc = -EINVAL;
+	void		*mem;
+
+	mem = kmalloc(sizeof(struct nfs_entry) +
+		      sizeof(nfs_readdir_descriptor_t), GFP_KERNEL);
+	if (!mem)
+		return -ENOMEM;
+
+	my_entry = mem;
+	desc = mem + sizeof(struct nfs_entry);
 
 	res = nfs_revalidate(dentry);
-	if (res < 0)
-		return res;
+	if (res < 0) {
+		rc = (int) res;
+		goto out;
+	}
 
 	/*
 	 * filp->f_pos points to the file offset in the page cache.

@@ -365,11 +376,11 @@
 	 * itself.
 	 */
 	memset(desc, 0, sizeof(*desc));
-	memset(&my_entry, 0, sizeof(my_entry));
+	memset(my_entry, 0, sizeof(*my_entry));
 
 	desc->file = filp;
 	desc->target = filp->f_pos;
-	desc->entry = &my_entry;
+	desc->entry = my_entry;
 	desc->decode = NFS_PROTO(inode)->decode_dirent;
 
 	while(!desc->entry->eof) {

@@ -393,11 +404,16 @@
 			break;
 		}
 	}
+
 	if (desc->error < 0)
-		return desc->error;
-	if (res < 0)
-		return res;
-	return 0;
+		rc = desc->error;
+	else if (res < 0)
+		rc = res;
+	/* fall through */
+
+out:
+	kfree(mem);
+	return rc;
 }
 
 /*

@@ -476,13 +492,22 @@
 	struct inode *dir;
 	struct inode *inode;
 	int error;
-	struct nfs_fh fhandle;
-	struct nfs_fattr fattr;
+	struct nfs_fh *fhandle;
+	struct nfs_fattr *fattr;
+	void *mem;
+	int rc = 0;
 
 	lock_kernel();
 	dir = dentry->d_parent->d_inode;
 	inode = dentry->d_inode;
 
+	mem = kmalloc(sizeof(struct nfs_fh) + sizeof(struct nfs_fattr),
+		      GFP_KERNEL);
+	if (!mem)
+		goto out_bad;
+	fhandle = mem;
+	fattr = mem + sizeof(struct nfs_fh);
+
 	if (!inode) {
 		if (nfs_neg_need_reval(dir, dentry))
 			goto out_bad;

@@ -505,18 +530,19 @@
 	if (NFS_STALE(inode))
 		goto out_bad;
 
-	error = NFS_PROTO(dir)->lookup(dir, &dentry->d_name, &fhandle, &fattr);
+	error = NFS_PROTO(dir)->lookup(dir, &dentry->d_name, fhandle, fattr);
 	if (error)
 		goto out_bad;
-	if (memcmp(NFS_FH(inode), &fhandle, sizeof(struct nfs_fh))!= 0)
+	if (memcmp(NFS_FH(inode), fhandle, sizeof(struct nfs_fh))!= 0)
 		goto out_bad;
-	if ((error = nfs_refresh_inode(inode, &fattr)) != 0)
+	if ((error = nfs_refresh_inode(inode, fattr)) != 0)
 		goto out_bad;
 
 	nfs_renew_times(dentry);
  out_valid:
-	unlock_kernel();
-	return 1;
+ 	rc = 1;
+	goto out;
+
  out_bad:
 	NFS_CACHEINV(dir);
 	if (inode && S_ISDIR(inode->i_mode)) {

@@ -528,8 +554,14 @@
 		shrink_dcache_parent(dentry);
 	}
 	d_drop(dentry);
+	rc = 0;
+	/* fall through */
+
+out:
 	unlock_kernel();
-	return 0;
+	if (mem)
+		kfree(mem);
+	return rc;
 }
 
 /*
-

To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

Quoted 1 line

Quoted 1 line

Quoted 1 line

Quoted 1 line

" " == Steve Dickson <[email protected]> writes:

There are always alternatives...

If really this is a problem, how about slabifying the structs nfs_fattr and/or nfs_fh?

Also, since nfs_entry is only large due to the fh and fattr fields which are unused unless you have READDIRPLUS (in which case they are converted to pointers anyhow), how about kicking them out of nfs_entry altogether?

There are always alternatives...

If really this is a problem, how about slabifying the structs nfs_fattr and/or nfs_fh?

Also, since nfs_entry is only large due to the fh and fattr fields which are unused unless you have READDIRPLUS (in which case they are converted to pointers anyhow), how about kicking them out of nfs_entry altogether?

Hey Jeff,

Question:
Why are only nfs_lookup_revalidate() and nfs_readdir() a problem and not the other 4 ops (like nfs_lookup())? Is the case only those two showed up in the stack overflow oops trace?

Also, not like there much choice in matter, but I wonder what type of performance hit (if any) there will be by making these routines call kmalloc()... lookups and readdirs are pretty popular ops...