[prev in list] [next in list] [prev in thread] [next in thread] 

List:       linux-fsdevel
Subject:    [PATCH] ncp_iget() cleanup
From:       Alexander Viro <viro () math ! psu ! edu>
Date:       1999-10-09 16:27:13
[Download RAW message or body]

	Linus, ncpfs used the same kludge as I've fixed in smbfs back in
June - ncp_iget() grabs the mutex, stores the data in global variable,
calls iget() with fake inumber (guaranteed cache miss) and releases mutex.
read_inode() picks the data from the global variable.
	(a) it's an ugly kludge - after all C is not BASIC.
	(b) it means that iget() for NCPFS works _only_ if called by
ncp_iget() (i.e. is worse than nothing)
	(c) it gives an artificial contention point for no good reason.

Fix: instead of all this kludge-o-rama we should call get_empty_inode(),
fill it with data and call insert_inode_hash(). ->read_inode is set to
NULL (see (b)). Less significant cleanup: ncp_invent_ino() duplicates
iunique(). Replaced and removed.

Patch (against 20-pre2) follows. Please, apply it.
							Cheers,
								Al

diff -urN linux-2.3.20-pre2/fs/ncpfs/dir.c linux-bird.ncpfs/fs/ncpfs/dir.c
--- linux-2.3.20-pre2/fs/ncpfs/dir.c	Fri Oct  8 06:00:32 1999
+++ linux-bird.ncpfs/fs/ncpfs/dir.c	Sat Oct  9 11:24:57 1999
@@ -183,22 +183,6 @@
 	}
 }
 
-/*
- * Generate a unique inode number.
- */
-ino_t ncp_invent_inos(unsigned long n)
-{
-	static ino_t ino = 2;
-
-	if (ino + 2*n < ino)
-	{
-		/* wrap around */
-		ino = 2;
-	}
-	ino += n;
-	return ino;
-}
-
 static inline int
 ncp_single_volume(struct ncp_server *server)
 {
@@ -449,7 +433,7 @@
 	ino = find_inode_number(dentry, &qname);
 
 	if (!ino)
-		ino = ncp_invent_inos(1);
+		ino = iunique(2);
 
 	result = filldir(dirent, name, len, filp->f_pos, ino);
 	if (!result)
@@ -494,7 +478,7 @@
 
 		if (!newdent->d_inode) {
 			entry->opened = 0;
-			entry->ino = ncp_invent_inos(1);
+			entry->ino = iunique(2);
 			newino = ncp_iget(inode->i_sb, entry);
 			if (newino) {
 				newdent->d_op = &ncp_dentry_operations;
@@ -517,7 +501,7 @@
 		ino = find_inode_number(dentry, &qname);
 
 	if (!ino)
-		ino = ncp_invent_inos(1);
+		ino = iunique(2);
 
 	result = filldir(dirent, entry->i.entryName, entry->i.nameLen,
 				filp->f_pos, ino);
@@ -810,7 +794,7 @@
 	 * Create an inode for the entry.
 	 */
 	finfo.opened = 0;
-	finfo.ino = ncp_invent_inos(1);
+	finfo.ino = iunique(2);
 	error = -EACCES;
 	inode = ncp_iget(dir->i_sb, &finfo);
 
@@ -838,7 +822,7 @@
 	struct inode *inode;
 	int error = -EINVAL;
 
-	finfo->ino = ncp_invent_inos(1);
+	finfo->ino = iunique(2);
 	inode = ncp_iget(dir->i_sb, finfo);
 	if (!inode)
 		goto out_close;
diff -urN linux-2.3.20-pre2/fs/ncpfs/inode.c linux-bird.ncpfs/fs/ncpfs/inode.c
--- linux-2.3.20-pre2/fs/ncpfs/inode.c	Fri Oct  8 06:00:32 1999
+++ linux-bird.ncpfs/fs/ncpfs/inode.c	Sat Oct  9 11:23:23 1999
@@ -31,7 +31,6 @@
 
 #include "ncplib_kernel.h"
 
-static void ncp_read_inode(struct inode *);
 static void ncp_put_inode(struct inode *);
 static void ncp_delete_inode(struct inode *);
 static void ncp_put_super(struct super_block *);
@@ -39,7 +38,7 @@
 
 static struct super_operations ncp_sops =
 {
-	ncp_read_inode,		/* read inode */
+	NULL,			/* read inode */
 	NULL,			/* write inode */
 	ncp_put_inode,		/* put inode */
 	ncp_delete_inode,       /* delete inode */
@@ -56,9 +55,6 @@
 extern int ncp_symlink(struct inode*, struct dentry*, const char*);
 #endif
 
-static struct ncp_entry_info *read_nwinfo = NULL;
-static DECLARE_MUTEX(read_sem);
-
 /*
  * Fill in the ncpfs-specific information in the inode.
  */
@@ -216,33 +212,7 @@
 }
 
 /*
- * This is called from iget() with the read semaphore held. 
- * The global ncp_entry_info structure has been set up by ncp_iget.
- */
-static void ncp_read_inode(struct inode *inode)
-{
-	if (read_nwinfo == NULL) {
-		printk(KERN_ERR "ncp_read_inode: invalid call\n");
-		return;
-	}
-
-	ncp_set_attr(inode, read_nwinfo);
-
-	if (S_ISREG(inode->i_mode)) {
-		inode->i_op = &ncp_file_inode_operations;
-	} else if (S_ISDIR(inode->i_mode)) {
-		inode->i_op = &ncp_dir_inode_operations;
-#ifdef CONFIG_NCPFS_EXTRAS
-	} else if (S_ISLNK(inode->i_mode)) {
-		inode->i_op = &ncp_symlink_inode_operations;
-#endif
-	} else {
-		inode->i_op = NULL;
-	}
-}
-
-/*
- * Set up the ncp_entry_info pointer and get a new inode.
+ * Get a new inode.
  */
 struct inode * 
 ncp_iget(struct super_block *sb, struct ncp_entry_info *info)
@@ -254,12 +224,23 @@
 		return NULL;
 	}
 
-	down(&read_sem);
-	read_nwinfo = info;
-	inode = iget(sb, info->ino);
-	read_nwinfo = NULL;
-	up(&read_sem);
-	if (!inode)
+	inode = get_empty_inode();
+	if (inode) {
+		inode->i_sb = sb;
+		inode->i_dev = sb->s_dev;
+		inode->i_ino = info->ino;
+		ncp_set_attr(inode, info);
+		if (S_ISREG(inode->i_mode)) {
+			inode->i_op = &ncp_file_inode_operations;
+		} else if (S_ISDIR(inode->i_mode)) {
+			inode->i_op = &ncp_dir_inode_operations;
+#ifdef CONFIG_NCPFS_EXTRAS
+		} else if (S_ISLNK(inode->i_mode)) {
+			inode->i_op = &ncp_symlink_inode_operations;
+#endif
+		}
+		insert_inode_hash(inode);
+	} else
 		printk(KERN_ERR "ncp_iget: iget failed!\n");
 	return inode;
 }
@@ -709,9 +690,6 @@
 int init_module(void)
 {
 	DPRINTK(KERN_DEBUG "ncpfs: init_module called\n");
-
-	init_MUTEX(&read_sem);
-	read_nwinfo = NULL;
 
 #ifdef DEBUG_NCP_MALLOC
 	ncp_malloced = 0;

[prev in list] [next in list] [prev in thread] [next in thread]