HPFS: Introduce a global mutex and lock it on every callback from VFS.

Introduce a global mutex and lock it on every callback from VFS. Performance doesn't matter, reviewing the whole code for locking correctness would be too complicated, so simply lock it all. Signed-off-by: Mikulas Patocka <[email protected]> Signed-off-by: Linus Torvalds <[email protected]>
chewitt · May 9, 2011 · 7dd29d8 · 7dd29d8
1 parent 637b424
commit 7dd29d8
Show file tree

Hide file tree

Showing 4 changed files with 51 additions and 18 deletions.
diff --git a/fs/hpfs/buffer.c b/fs/hpfs/buffer.c
@@ -32,6 +32,8 @@ void *hpfs_map_sector(struct super_block *s, unsigned secno, struct buffer_head
 {
 	struct buffer_head *bh;
 
+	hpfs_lock_assert(s);
+
 	cond_resched();
 
 	*bhp = bh = sb_bread(s, secno);
@@ -50,6 +52,8 @@ void *hpfs_get_sector(struct super_block *s, unsigned secno, struct buffer_head
 	struct buffer_head *bh;
 	/*return hpfs_map_sector(s, secno, bhp, 0);*/
 
+	hpfs_lock_assert(s);
+
 	cond_resched();
 
 	if ((*bhp = bh = sb_getblk(s, secno)) != NULL) {
@@ -70,6 +74,8 @@ void *hpfs_map_4sectors(struct super_block *s, unsigned secno, struct quad_buffe
 	struct buffer_head *bh;
 	char *data;
 
+	hpfs_lock_assert(s);
+
 	cond_resched();
 
 	if (secno & 3) {
@@ -125,6 +131,8 @@ void *hpfs_get_4sectors(struct super_block *s, unsigned secno,
 {
 	cond_resched();
 
+	hpfs_lock_assert(s);
+
 	if (secno & 3) {
 		printk("HPFS: hpfs_get_4sectors: unaligned read\n");
 		return NULL;

diff --git a/fs/hpfs/file.c b/fs/hpfs/file.c
@@ -48,38 +48,46 @@ static secno hpfs_bmap(struct inode *inode, unsigned file_secno)
 static void hpfs_truncate(struct inode *i)
 {
 	if (IS_IMMUTABLE(i)) return /*-EPERM*/;
-	hpfs_lock(i->i_sb);
+	hpfs_lock_assert(i->i_sb);
+
 	hpfs_i(i)->i_n_secs = 0;
 	i->i_blocks = 1 + ((i->i_size + 511) >> 9);
 	hpfs_i(i)->mmu_private = i->i_size;
 	hpfs_truncate_btree(i->i_sb, i->i_ino, 1, ((i->i_size + 511) >> 9));
 	hpfs_write_inode(i);
 	hpfs_i(i)->i_n_secs = 0;
-	hpfs_unlock(i->i_sb);
 }
 
 static int hpfs_get_block(struct inode *inode, sector_t iblock, struct buffer_head *bh_result, int create)
 {
+	int r;
 	secno s;
+	hpfs_lock(inode->i_sb);
 	s = hpfs_bmap(inode, iblock);
 	if (s) {
 		map_bh(bh_result, inode->i_sb, s);
-		return 0;
+		goto ret_0;
 	}
-	if (!create) return 0;
+	if (!create) goto ret_0;
 	if (iblock<<9 != hpfs_i(inode)->mmu_private) {
 		BUG();
-		return -EIO;
+		r = -EIO;
+		goto ret_r;
 	}
 	if ((s = hpfs_add_sector_to_btree(inode->i_sb, inode->i_ino, 1, inode->i_blocks - 1)) == -1) {
 		hpfs_truncate_btree(inode->i_sb, inode->i_ino, 1, inode->i_blocks - 1);
-		return -ENOSPC;
+		r = -ENOSPC;
+		goto ret_r;
 	}
 	inode->i_blocks++;
 	hpfs_i(inode)->mmu_private += 512;
 	set_buffer_new(bh_result);
 	map_bh(bh_result, inode->i_sb, s);
-	return 0;
+	ret_0:
+	r = 0;
+	ret_r:
+	hpfs_unlock(inode->i_sb);
+	return r;
 }
 
 static int hpfs_writepage(struct page *page, struct writeback_control *wbc)
@@ -130,8 +138,11 @@ static ssize_t hpfs_file_write(struct file *file, const char __user *buf,
 	ssize_t retval;
 
 	retval = do_sync_write(file, buf, count, ppos);
-	if (retval > 0)
+	if (retval > 0) {
+		hpfs_lock(file->f_path.dentry->d_sb);
 		hpfs_i(file->f_path.dentry->d_inode)->i_dirty = 1;
+		hpfs_unlock(file->f_path.dentry->d_sb);
+	}
 	return retval;
 }
 

diff --git a/fs/hpfs/hpfs_fn.h b/fs/hpfs/hpfs_fn.h
@@ -63,6 +63,7 @@ struct hpfs_inode_info {
 };
 
 struct hpfs_sb_info {
+	struct mutex hpfs_mutex;	/* global hpfs lock */
 	ino_t sb_root;			/* inode number of root dir */
 	unsigned sb_fs_size;		/* file system size, sectors */
 	unsigned sb_bitmaps;		/* sector number of bitmap list */
@@ -346,21 +347,26 @@ static inline time32_t gmt_to_local(struct super_block *s, time_t t)
 /*
  * Locking:
  *
- * hpfs_lock() is a leftover from the big kernel lock.
- * Right now, these functions are empty and only left
- * for documentation purposes. The file system no longer
- * works on SMP systems, so the lock is not needed
- * any more.
+ * hpfs_lock() locks the whole filesystem. It must be taken
+ * on any method called by the VFS.
  *
- * If someone is interested in making it work again, this
- * would be the place to start by adding a per-superblock
- * mutex and fixing all the bugs and performance issues
- * caused by that.
+ * We don't do any per-file locking anymore, it is hard to
+ * review and HPFS is not performance-sensitive anyway.
  */
 static inline void hpfs_lock(struct super_block *s)
 {
+	struct hpfs_sb_info *sbi = hpfs_sb(s);
+	mutex_lock(&sbi->hpfs_mutex);
 }
 
 static inline void hpfs_unlock(struct super_block *s)
 {
+	struct hpfs_sb_info *sbi = hpfs_sb(s);
+	mutex_unlock(&sbi->hpfs_mutex);
+}
+
+static inline void hpfs_lock_assert(struct super_block *s)
+{
+	struct hpfs_sb_info *sbi = hpfs_sb(s);
+	WARN_ON(!mutex_is_locked(&sbi->hpfs_mutex));
 }
diff --git a/fs/hpfs/super.c b/fs/hpfs/super.c
@@ -102,9 +102,12 @@ static void hpfs_put_super(struct super_block *s)
 {
 	struct hpfs_sb_info *sbi = hpfs_sb(s);
 
+	hpfs_lock(s);
+	unmark_dirty(s);
+	hpfs_unlock(s);
+
 	kfree(sbi->sb_cp_table);
 	kfree(sbi->sb_bmp_dir);
-	unmark_dirty(s);
 	s->s_fs_info = NULL;
 	kfree(sbi);
 }
@@ -490,6 +493,9 @@ static int hpfs_fill_super(struct super_block *s, void *options, int silent)
 	sbi->sb_bmp_dir = NULL;
 	sbi->sb_cp_table = NULL;
 
+	mutex_init(&sbi->hpfs_mutex);
+	hpfs_lock(s);
+
 	mutex_init(&sbi->hpfs_creation_de);
 
 	uid = current_uid();
@@ -669,13 +675,15 @@ static int hpfs_fill_super(struct super_block *s, void *options, int silent)
 			root->i_blocks = 5;
 		hpfs_brelse4(&qbh);
 	}
+	hpfs_unlock(s);
 	return 0;
 
 bail4:	brelse(bh2);
 bail3:	brelse(bh1);
 bail2:	brelse(bh0);
 bail1:
 bail0:
+	hpfs_unlock(s);
 	kfree(sbi->sb_bmp_dir);
 	kfree(sbi->sb_cp_table);
 	s->s_fs_info = NULL;