Coda File System

Re: kernel oops

From: Jan Harkes <jaharkes_at_cs.cmu.edu>
Date: Sun, 19 May 2002 17:49:38 -0400
On Sun, May 19, 2002 at 11:16:33AM +0200, Florian Schaefer wrote:
> today I compiled a new kernel for my new RH7.3 system and suddenly I get
> some oops when accessing coda.

Yeah, there is a race in iget4 that only recently became more noticable
and a change that went into the tree earlier broke mmap in some cases.

Some of the needed fixes have already been sent to Marcello, the iget4
race is something that I just finished cleaning up and I have been
running it only for about a week. I will probably send that one soon.

I'll attach the full diff from 2.4.19-pre8 to what I have right now.

> PS: How do I compile a coda-client from cvs source? The coda module seems
> to be for a server, isn't it?

It builds both the client and the server as a lot of the code is shared
(or at least there are a lot of dependencies) between the two. There are
only different install targets 'client-install' and 'server-install'.

Basically the steps to go through are 'cvs co coda', 'cd coda',
'./bootstrap.sh', './configure --prefix=/usr', 'make', 'make client-install'.

Jan


diff -urN orig/fs/coda/cnode.c coda1/fs/coda/cnode.c
--- orig/fs/coda/cnode.c	Fri May  3 13:43:44 2002
+++ coda1/fs/coda/cnode.c	Tue May 14 22:11:50 2002
@@ -57,6 +57,7 @@
         } else if (S_ISLNK(inode->i_mode)) {
 		inode->i_op = &coda_symlink_inode_operations;
 		inode->i_data.a_ops = &coda_symlink_aops;
+		inode->i_mapping = &inode->i_data;
 	} else
                 init_special_inode(inode, inode->i_mode, attr->va_rdev);
 }
@@ -67,11 +68,14 @@
 	struct inode *inode;
 	struct coda_inode_info *cii;
 	ino_t ino = coda_f2i(fid);
+	struct coda_sb_info *sbi = coda_sbp(sb);
 
+	down(&sbi->sbi_iget4_mutex);
 	inode = iget4(sb, ino, coda_inocmp, fid);
 
 	if ( !inode ) { 
 		CDEBUG(D_CNODE, "coda_iget: no inode\n");
+		up(&sbi->sbi_iget4_mutex);
 		return ERR_PTR(-ENOMEM);
 	}
 
@@ -80,9 +84,7 @@
 	if (coda_isnullfid(&cii->c_fid))
 		/* new, empty inode found... initializing */
 		cii->c_fid = *fid;
-
-	/* we shouldnt see inode collisions anymore */
-	if (!coda_fideq(fid, &cii->c_fid)) BUG();
+	up(&sbi->sbi_iget4_mutex);
 
 	/* always replace the attributes, type might have changed */
 	coda_fill_inode(inode, attr);
@@ -147,6 +149,7 @@
 	ino_t nr;
 	struct inode *inode;
 	struct coda_inode_info *cii;
+	struct coda_sb_info *sbi;
 
 	if ( !sb ) {
 		printk("coda_fid_to_inode: no sb!\n");
@@ -155,12 +158,14 @@
 
 	CDEBUG(D_INODE, "%s\n", coda_f2s(fid));
 
+	sbi = coda_sbp(sb);
 	nr = coda_f2i(fid);
+	down(&sbi->sbi_iget4_mutex);
 	inode = iget4(sb, nr, coda_inocmp, fid);
 	if ( !inode ) {
 		printk("coda_fid_to_inode: null from iget, sb %p, nr %ld.\n",
 		       sb, (long)nr);
-		return NULL;
+		goto out_unlock;
 	}
 
 	cii = ITOC(inode);
@@ -169,14 +174,16 @@
 	if (coda_isnullfid(&cii->c_fid)) {
 		inode->i_nlink = 0;
 		iput(inode);
-		return NULL;
+		goto out_unlock;
 	}
 
-	/* we shouldn't see inode collisions anymore */
-	if ( !coda_fideq(fid, &cii->c_fid) ) BUG();
-
         CDEBUG(D_INODE, "found %ld\n", inode->i_ino);
+	up(&sbi->sbi_iget4_mutex);
 	return inode;
+
+out_unlock:
+	up(&sbi->sbi_iget4_mutex);
+	return NULL;
 }
 
 /* the CONTROL inode is made without asking attributes from Venus */
diff -urN orig/fs/coda/dir.c coda1/fs/coda/dir.c
--- orig/fs/coda/dir.c	Fri May  3 13:43:44 2002
+++ coda1/fs/coda/dir.c	Fri May  3 13:50:40 2002
@@ -491,6 +491,7 @@
 	struct dentry *coda_dentry = coda_file->f_dentry;
 	struct coda_file_info *cfi;
 	struct file *host_file;
+	int ret;
 
 	cfi = CODA_FTOC(coda_file);
 	if (!cfi || cfi->cfi_magic != CODA_MAGIC) BUG();
@@ -498,12 +499,21 @@
 
 	coda_vfs_stat.readdir++;
 
-	if ( host_file->f_op->readdir )
+	down(&host_file->f_dentry->d_inode->i_sem);
+	host_file->f_pos = coda_file->f_pos;
+
+	if ( !host_file->f_op->readdir ) {
+		/* Venus: we must read Venus dirents from the file */
+		ret = coda_venus_readdir(host_file, filldir, dirent, coda_dentry);
+	} else {
 		/* potemkin case: we were handed a directory inode */
-		return vfs_readdir(host_file, filldir, dirent);
+		ret = vfs_readdir(host_file, filldir, dirent);
+	}
+
+	coda_file->f_pos = host_file->f_pos;
+	up(&host_file->f_dentry->d_inode->i_sem);
 
-	/* Venus: we must read Venus dirents from the file */
-	return coda_venus_readdir(host_file, filldir, dirent, coda_dentry);
+	return ret;
 }
 
 static inline unsigned int CDT2DT(unsigned char cdt)
@@ -736,7 +746,6 @@
 	return 0;
 
 return_bad_inode:
-	make_bad_inode(inode);
 	unlock_kernel();
 	return -EIO;
 }
diff -urN orig/fs/coda/file.c coda1/fs/coda/file.c
--- orig/fs/coda/file.c	Fri May  3 13:43:44 2002
+++ coda1/fs/coda/file.c	Tue May 14 23:14:56 2002
@@ -48,7 +48,7 @@
 static ssize_t
 coda_file_write(struct file *coda_file, const char *buf, size_t count, loff_t *ppos)
 {
-	struct inode *host_inode, *coda_inode = coda_file->f_dentry->d_inode;
+	struct inode *coda_inode = coda_file->f_dentry->d_inode;
 	struct coda_file_info *cfi;
 	struct file *host_file;
 	ssize_t ret;
@@ -60,12 +60,11 @@
 	if (!host_file->f_op || !host_file->f_op->write)
 		return -EINVAL;
 
-	host_inode = host_file->f_dentry->d_inode;
 	down(&coda_inode->i_sem);
 
 	ret = host_file->f_op->write(host_file, buf, count, ppos);
 
-	coda_inode->i_size = host_inode->i_size;
+	coda_inode->i_size = host_file->f_dentry->d_inode->i_size;
 	coda_inode->i_blocks = (coda_inode->i_size + 511) >> 9;
 	coda_inode->i_mtime = coda_inode->i_ctime = CURRENT_TIME;
 	up(&coda_inode->i_sem);
@@ -77,7 +76,9 @@
 coda_file_mmap(struct file *coda_file, struct vm_area_struct *vma)
 {
 	struct coda_file_info *cfi;
+	struct coda_inode_info *cii;
 	struct file *host_file;
+	struct inode *coda_inode, *host_inode;
 
 	cfi = CODA_FTOC(coda_file);
 	if (!cfi || cfi->cfi_magic != CODA_MAGIC) BUG();
@@ -86,6 +87,21 @@
 	if (!host_file->f_op || !host_file->f_op->mmap)
 		return -ENODEV;
 
+	coda_inode = coda_file->f_dentry->d_inode;
+	host_inode = host_file->f_dentry->d_inode;
+	if (coda_inode->i_mapping == &coda_inode->i_data)
+		coda_inode->i_mapping = host_inode->i_mapping;
+
+	/* only allow additional mmaps as long as userspace isn't changing
+	 * the container file on us! */
+	else if (coda_inode->i_mapping != host_inode->i_mapping)
+		return -EBUSY;
+
+	/* keep track of how often the coda_inode/host_file has been mmapped */
+	cii = ITOC(coda_inode);
+	cii->c_mapcount++;
+	cfi->cfi_mapcount++;
+
 	return host_file->f_op->mmap(host_file, vma);
 }
 
@@ -97,7 +113,6 @@
 	unsigned short coda_flags = coda_flags_to_cflags(flags);
 	struct coda_file_info *cfi;
 
-	lock_kernel();
 	coda_vfs_stat.open++;
 
 	cfi = kmalloc(sizeof(struct coda_file_info), GFP_KERNEL);
@@ -106,6 +121,8 @@
 		return -ENOMEM;
 	}
 
+	lock_kernel();
+
 	error = venus_open(coda_inode->i_sb, coda_i2f(coda_inode), coda_flags,
 			   &host_file); 
 	if (error || !host_file) {
@@ -117,6 +134,7 @@
 	host_file->f_flags |= coda_file->f_flags & (O_APPEND | O_SYNC);
 
 	cfi->cfi_magic = CODA_MAGIC;
+	cfi->cfi_mapcount = 0;
 	cfi->cfi_container = host_file;
 	coda_load_creds(&cfi->cfi_cred);
 
@@ -169,6 +187,8 @@
 	unsigned short flags = (coda_file->f_flags) & (~O_EXCL);
 	unsigned short coda_flags = coda_flags_to_cflags(flags);
 	struct coda_file_info *cfi;
+	struct coda_inode_info *cii;
+	struct inode *host_inode;
 	int err = 0;
 
 	lock_kernel();
@@ -189,6 +209,16 @@
 	if (use_coda_close)
 		err = venus_close(coda_inode->i_sb, coda_i2f(coda_inode),
 				  coda_flags, &cfi->cfi_cred);
+
+	host_inode = cfi->cfi_container->f_dentry->d_inode;
+	cii = ITOC(coda_inode);
+
+	/* did we mmap this file? */
+	if (coda_inode->i_mapping == &host_inode->i_data) {
+		cii->c_mapcount -= cfi->cfi_mapcount;
+		if (!cii->c_mapcount)
+			coda_inode->i_mapping = &coda_inode->i_data;
+	}
 
 	fput(cfi->cfi_container);
 	kfree(coda_file->private_data);
diff -urN orig/fs/coda/inode.c coda1/fs/coda/inode.c
--- orig/fs/coda/inode.c	Fri May  3 13:43:44 2002
+++ coda1/fs/coda/inode.c	Tue May 14 22:22:51 2002
@@ -128,6 +128,7 @@
 	sbi->sbi_sb = sb;
 	sbi->sbi_vcomm = vc;
 	INIT_LIST_HEAD(&sbi->sbi_cihead);
+	init_MUTEX(&sbi->sbi_iget4_mutex);
 
         sb->u.generic_sbp = sbi;
         sb->s_blocksize = 1024;	/* XXXXX  what do we put here?? */
@@ -194,6 +195,7 @@
             return;
         }
 
+	cii->c_mapcount = 0;
 	list_add(&cii->c_cilist, &sbi->sbi_cihead);
 }
 
diff -urN orig/include/linux/coda_fs_i.h coda1/include/linux/coda_fs_i.h
--- orig/include/linux/coda_fs_i.h	Fri May  3 14:23:33 2002
+++ coda1/include/linux/coda_fs_i.h	Tue May 14 23:00:14 2002
@@ -20,6 +20,7 @@
         struct ViceFid     c_fid;	/* Coda identifier */
         u_short	           c_flags;     /* flags (see below) */
 	struct list_head   c_cilist;    /* list of all coda inodes */
+	int		   c_mapcount;	/* how often is this inode mmapped */
         struct coda_cred   c_cached_cred; /* credentials of cached perms */
         unsigned int       c_cached_perm; /* cached access permissions */
 };
@@ -30,6 +31,7 @@
 #define CODA_MAGIC 0xC0DAC0DA
 struct coda_file_info {
 	int		 cfi_magic;	/* magic number */
+	int		 cfi_mapcount;  /* how often this file is mapped */
 	struct file	*cfi_container;	/* container file for this cnode */
 	struct coda_cred cfi_cred;	/* credentials of opener */
 };
diff -urN orig/include/linux/coda_psdev.h coda1/include/linux/coda_psdev.h
--- orig/include/linux/coda_psdev.h	Wed Apr 25 19:18:54 2001
+++ coda1/include/linux/coda_psdev.h	Thu May  9 02:39:34 2002
@@ -11,6 +11,7 @@
 	struct venus_comm * sbi_vcomm;
 	struct super_block *sbi_sb;
 	struct list_head    sbi_cihead;
+	struct semaphore    sbi_iget4_mutex;
 };
 
 /* communication pending/processing queues */
Received on 2002-05-19 17:51:25