Remove BKL from FAT/VFAT/MSDOS (v1) (was Re: Fw: Regression caused by bf726e "semaphore: fix,")

Remove BKL from FAT/VFAT/MSDOS (v1) (was Re: Fw: Regression caused by bf726e "semaphore: fix,")

[Linus Torvalds]

On Fri, 16 May 2008, Linus Torvalds wrote:
> 
> In fact, some of the lock_kernel() calls looked like they could even be 
> dropped:
> 
>  - fat_clear_inode already gets the real spinlocks (inode_hash_lock and 
>    cache_lru_lock), and the kernel lock looks pointless.

Ok, this was also the only one that was called under the superblock lock 
(sys_umount->deactivate_super->invalidate_inodes->clear_inode), and 
everything else could be directly converted to use the superblock lock, so 
here's a trial balloon patch that does exactly that: just blindly convert 
all the "lock_kernel()" calls to "lock_super(sb)", except for the one in 
fat_clear_inode(), which is dropped entirely.

NOTE! There's no way in hell I'll actually commit this, but I did test it 
a bit with a vfat filesystem. I did *not* test extensively, but I did 
various "generate a directory tree, read it, write it, an remove it", so 
the basics do work. But I didn't test an old 8+3 plaim MSDOS filesystem, 
nor did I run any real filesystem stress test, but I do think this is at 
least an interesting starting point, and it's at least not "totally 
obviously broken".

Linux-kernel added to Cc list in case some random person is interested in 
looking at FAT/VFAT/MSDOS lock_kernel removal. 

ANOTHER NOTE! I didn't change this to use .unlocked_ioctl(), and I didn't 
even really look at it. It still takes the BKL, but obviously doesn't help 
any. It didn't look like it should really need it, because it seems to do 
the right locking as-is (ie it gets the inode mutex, and it calls 
__fat_readdir() which now does the lock_super()).

So afaik, those things should just be changed to ".unlocked_ioctl" 
instead, but that also drops the inode as an argument, so then you need to 
also add that "struct inode *inode = filp->f_dentry->d_inode" thing. And 
I'm hoping that other people are interested and will look at this more 
anyway!

		Linus

---
 fs/fat/cache.c   |    2 +-
 fs/fat/dir.c     |    4 ++--
 fs/fat/file.c    |   12 +++++++-----
 fs/fat/inode.c   |   26 ++++++++++++++++----------
 fs/msdos/namei.c |   35 +++++++++++++++++++----------------
 fs/vfat/namei.c  |   35 +++++++++++++++++++----------------
 6 files changed, 64 insertions(+), 50 deletions(-)

diff --git a/fs/fat/cache.c b/fs/fat/cache.c
index fda2547..3a9ecac 100644
--- a/fs/fat/cache.c
+++ b/fs/fat/cache.c
@@ -61,7 +61,7 @@ void fat_cache_destroy(void)
 
 static inline struct fat_cache *fat_cache_alloc(struct inode *inode)
 {
-	return kmem_cache_alloc(fat_cache_cachep, GFP_KERNEL);
+	return kmem_cache_alloc(fat_cache_cachep, GFP_NOFS);
 }
 
 static inline void fat_cache_free(struct fat_cache *cache)
diff --git a/fs/fat/dir.c b/fs/fat/dir.c
index 486725e..34541d0 100644
--- a/fs/fat/dir.c
+++ b/fs/fat/dir.c
@@ -472,7 +472,7 @@ static int __fat_readdir(struct inode *inode, struct file *filp, void *dirent,
 	loff_t cpos;
 	int ret = 0;
 
-	lock_kernel();
+	lock_super(sb);
 
 	cpos = filp->f_pos;
 	/* Fake . and .. for the root directory. */
@@ -654,7 +654,7 @@ FillFailed:
 	if (unicode)
 		__putname(unicode);
 out:
-	unlock_kernel();
+	unlock_super(sb);
 	return ret;
 }
 
diff --git a/fs/fat/file.c b/fs/fat/file.c
index 27cc116..7059928 100644
--- a/fs/fat/file.c
+++ b/fs/fat/file.c
@@ -229,7 +229,8 @@ static int fat_free(struct inode *inode, int skip)
 
 void fat_truncate(struct inode *inode)
 {
-	struct msdos_sb_info *sbi = MSDOS_SB(inode->i_sb);
+	struct super_block *sb = inode->i_sb;
+	struct msdos_sb_info *sbi = MSDOS_SB(sb);
 	const unsigned int cluster_size = sbi->cluster_size;
 	int nr_clusters;
 
@@ -242,9 +243,9 @@ void fat_truncate(struct inode *inode)
 
 	nr_clusters = (inode->i_size + (cluster_size - 1)) >> sbi->cluster_bits;
 
-	lock_kernel();
+	lock_super(sb);
 	fat_free(inode, nr_clusters);
-	unlock_kernel();
+	unlock_super(sb);
 	fat_flush_inodes(inode->i_sb, inode, NULL);
 }
 
@@ -297,12 +298,13 @@ static int fat_allow_set_time(struct msdos_sb_info *sbi, struct inode *inode)
 
 int fat_setattr(struct dentry *dentry, struct iattr *attr)
 {
+	struct super_block *sb = dentry->d_sb;
 	struct msdos_sb_info *sbi = MSDOS_SB(dentry->d_sb);
 	struct inode *inode = dentry->d_inode;
 	int mask, error = 0;
 	unsigned int ia_valid;
 
-	lock_kernel();
+	lock_super(sb);
 
 	/*
 	 * Expand the file. Since inode_setattr() updates ->i_size
@@ -356,7 +358,7 @@ int fat_setattr(struct dentry *dentry, struct iattr *attr)
 		mask = sbi->options.fs_fmask;
 	inode->i_mode &= S_IFMT | (S_IRWXUGO & ~mask);
 out:
-	unlock_kernel();
+	unlock_super(sb);
 	return error;
 }
 EXPORT_SYMBOL_GPL(fat_setattr);
diff --git a/fs/fat/inode.c b/fs/fat/inode.c
index 4e0a3dd..46a4508 100644
--- a/fs/fat/inode.c
+++ b/fs/fat/inode.c
@@ -440,14 +440,13 @@ static void fat_delete_inode(struct inode *inode)
 
 static void fat_clear_inode(struct inode *inode)
 {
-	struct msdos_sb_info *sbi = MSDOS_SB(inode->i_sb);
+	struct super_block *sb = inode->i_sb;
+	struct msdos_sb_info *sbi = MSDOS_SB(sb);
 
-	lock_kernel();
 	spin_lock(&sbi->inode_hash_lock);
 	fat_cache_inval_inode(inode);
 	hlist_del_init(&MSDOS_I(inode)->i_fat_hash);
 	spin_unlock(&sbi->inode_hash_lock);
-	unlock_kernel();
 }
 
 static void fat_write_super(struct super_block *sb)
@@ -485,7 +484,7 @@ static struct kmem_cache *fat_inode_cachep;
 static struct inode *fat_alloc_inode(struct super_block *sb)
 {
 	struct msdos_inode_info *ei;
-	ei = kmem_cache_alloc(fat_inode_cachep, GFP_KERNEL);
+	ei = kmem_cache_alloc(fat_inode_cachep, GFP_NOFS);
 	if (!ei)
 		return NULL;
 	return &ei->vfs_inode;
@@ -567,7 +566,7 @@ retry:
 	if (inode->i_ino == MSDOS_ROOT_INO || !i_pos)
 		return 0;
 
-	lock_kernel();
+	lock_super(sb);
 	bh = sb_bread(sb, i_pos >> sbi->dir_per_block_bits);
 	if (!bh) {
 		printk(KERN_ERR "FAT: unable to read inode block "
@@ -579,7 +578,7 @@ retry:
 	if (i_pos != MSDOS_I(inode)->i_pos) {
 		spin_unlock(&sbi->inode_hash_lock);
 		brelse(bh);
-		unlock_kernel();
+		unlock_super(sb);
 		goto retry;
 	}
 
@@ -606,7 +605,7 @@ retry:
 		err = sync_dirty_buffer(bh);
 	brelse(bh);
 out:
-	unlock_kernel();
+	unlock_super(sb);
 	return err;
 }
 
@@ -736,6 +735,7 @@ fat_encode_fh(struct dentry *de, __u32 *fh, int *lenp, int connectable)
 
 static struct dentry *fat_get_parent(struct dentry *child)
 {
+	struct super_block *sb = child->d_sb;
 	struct buffer_head *bh;
 	struct msdos_dir_entry *de;
 	loff_t i_pos;
@@ -743,14 +743,14 @@ static struct dentry *fat_get_parent(struct dentry *child)
 	struct inode *inode;
 	int err;
 
-	lock_kernel();
+	lock_super(sb);
 
 	err = fat_get_dotdot_entry(child->d_inode, &bh, &de, &i_pos);
 	if (err) {
 		parent = ERR_PTR(err);
 		goto out;
 	}
-	inode = fat_build_inode(child->d_sb, de, i_pos);
+	inode = fat_build_inode(sb, de, i_pos);
 	brelse(bh);
 	if (IS_ERR(inode)) {
 		parent = ERR_CAST(inode);
@@ -762,7 +762,7 @@ static struct dentry *fat_get_parent(struct dentry *child)
 		parent = ERR_PTR(-ENOMEM);
 	}
 out:
-	unlock_kernel();
+	unlock_super(sb);
 
 	return parent;
 }
@@ -1172,6 +1172,12 @@ int fat_fill_super(struct super_block *sb, void *data, int silent,
 	long error;
 	char buf[50];
 
+	/*
+	 * GFP_KERNEL is ok here, because while we do hold the
+	 * supeblock lock, memory pressure can't call back into
+	 * the filesystem, since we're only just about to mount
+	 * it and have no inodes etc active!
+	 */
 	sbi = kzalloc(sizeof(struct msdos_sb_info), GFP_KERNEL);
 	if (!sbi)
 		return -ENOMEM;
diff --git a/fs/msdos/namei.c b/fs/msdos/namei.c
index 05ff4f1..1f7f295 100644
--- a/fs/msdos/namei.c
+++ b/fs/msdos/namei.c
@@ -214,7 +214,7 @@ static struct dentry *msdos_lookup(struct inode *dir, struct dentry *dentry,
 
 	dentry->d_op = &msdos_dentry_operations;
 
-	lock_kernel();
+	lock_super(sb);
 	res = msdos_find(dir, dentry->d_name.name, dentry->d_name.len, &sinfo);
 	if (res == -ENOENT)
 		goto add;
@@ -232,7 +232,7 @@ add:
 	if (dentry)
 		dentry->d_op = &msdos_dentry_operations;
 out:
-	unlock_kernel();
+	unlock_super(sb);
 	if (!res)
 		return dentry;
 	return ERR_PTR(res);
@@ -286,7 +286,7 @@ static int msdos_create(struct inode *dir, struct dentry *dentry, int mode,
 	unsigned char msdos_name[MSDOS_NAME];
 	int err, is_hid;
 
-	lock_kernel();
+	lock_super(sb);
 
 	err = msdos_format_name(dentry->d_name.name, dentry->d_name.len,
 				msdos_name, &MSDOS_SB(sb)->options);
@@ -315,7 +315,7 @@ static int msdos_create(struct inode *dir, struct dentry *dentry, int mode,
 
 	d_instantiate(dentry, inode);
 out:
-	unlock_kernel();
+	unlock_super(sb);
 	if (!err)
 		err = fat_flush_inodes(sb, dir, inode);
 	return err;
@@ -324,11 +324,12 @@ out:
 /***** Remove a directory */
 static int msdos_rmdir(struct inode *dir, struct dentry *dentry)
 {
+	struct super_block *sb = dir->i_sb;
 	struct inode *inode = dentry->d_inode;
 	struct fat_slot_info sinfo;
 	int err;
 
-	lock_kernel();
+	lock_super(sb);
 	/*
 	 * Check whether the directory is not in use, then check
 	 * whether it is empty.
@@ -349,9 +350,9 @@ static int msdos_rmdir(struct inode *dir, struct dentry *dentry)
 	inode->i_ctime = CURRENT_TIME_SEC;
 	fat_detach(inode);
 out:
-	unlock_kernel();
+	unlock_super(sb);
 	if (!err)
-		err = fat_flush_inodes(inode->i_sb, dir, inode);
+		err = fat_flush_inodes(sb, dir, inode);
 
 	return err;
 }
@@ -366,7 +367,7 @@ static int msdos_mkdir(struct inode *dir, struct dentry *dentry, int mode)
 	struct timespec ts;
 	int err, is_hid, cluster;
 
-	lock_kernel();
+	lock_super(sb);
 
 	err = msdos_format_name(dentry->d_name.name, dentry->d_name.len,
 				msdos_name, &MSDOS_SB(sb)->options);
@@ -404,14 +405,14 @@ static int msdos_mkdir(struct inode *dir, struct dentry *dentry, int mode)
 
 	d_instantiate(dentry, inode);
 
-	unlock_kernel();
+	unlock_super(sb);
 	fat_flush_inodes(sb, dir, inode);
 	return 0;
 
 out_free:
 	fat_free_clusters(dir, cluster);
 out:
-	unlock_kernel();
+	unlock_super(sb);
 	return err;
 }
 
@@ -419,10 +420,11 @@ out:
 static int msdos_unlink(struct inode *dir, struct dentry *dentry)
 {
 	struct inode *inode = dentry->d_inode;
+	struct super_block *sb= inode->i_sb;
 	struct fat_slot_info sinfo;
 	int err;
 
-	lock_kernel();
+	lock_super(sb);
 	err = msdos_find(dir, dentry->d_name.name, dentry->d_name.len, &sinfo);
 	if (err)
 		goto out;
@@ -434,9 +436,9 @@ static int msdos_unlink(struct inode *dir, struct dentry *dentry)
 	inode->i_ctime = CURRENT_TIME_SEC;
 	fat_detach(inode);
 out:
-	unlock_kernel();
+	unlock_super(sb);
 	if (!err)
-		err = fat_flush_inodes(inode->i_sb, dir, inode);
+		err = fat_flush_inodes(sb, dir, inode);
 
 	return err;
 }
@@ -618,10 +620,11 @@ error_inode:
 static int msdos_rename(struct inode *old_dir, struct dentry *old_dentry,
 			struct inode *new_dir, struct dentry *new_dentry)
 {
+	struct super_block *sb = old_dir->i_sb;
 	unsigned char old_msdos_name[MSDOS_NAME], new_msdos_name[MSDOS_NAME];
 	int err, is_hid;
 
-	lock_kernel();
+	lock_super(sb);
 
 	err = msdos_format_name(old_dentry->d_name.name,
 				old_dentry->d_name.len, old_msdos_name,
@@ -640,9 +643,9 @@ static int msdos_rename(struct inode *old_dir, struct dentry *old_dentry,
 	err = do_msdos_rename(old_dir, old_msdos_name, old_dentry,
 			      new_dir, new_msdos_name, new_dentry, is_hid);
 out:
-	unlock_kernel();
+	unlock_super(sb);
 	if (!err)
-		err = fat_flush_inodes(old_dir->i_sb, old_dir, new_dir);
+		err = fat_flush_inodes(sb, old_dir, new_dir);
 	return err;
 }
 
diff --git a/fs/vfat/namei.c b/fs/vfat/namei.c
index a352272..b546ba6 100644
--- a/fs/vfat/namei.c
+++ b/fs/vfat/namei.c
@@ -645,7 +645,7 @@ static int vfat_add_entry(struct inode *dir, struct qstr *qname, int is_dir,
 	if (len == 0)
 		return -ENOENT;
 
-	slots = kmalloc(sizeof(*slots) * MSDOS_SLOTS, GFP_KERNEL);
+	slots = kmalloc(sizeof(*slots) * MSDOS_SLOTS, GFP_NOFS);
 	if (slots == NULL)
 		return -ENOMEM;
 
@@ -687,7 +687,7 @@ static struct dentry *vfat_lookup(struct inode *dir, struct dentry *dentry,
 	struct dentry *alias;
 	int err, table;
 
-	lock_kernel();
+	lock_super(sb);
 	table = (MSDOS_SB(sb)->options.name_check == 's') ? 2 : 0;
 	dentry->d_op = &vfat_dentry_ops[table];
 
@@ -699,7 +699,7 @@ static struct dentry *vfat_lookup(struct inode *dir, struct dentry *dentry,
 	inode = fat_build_inode(sb, sinfo.de, sinfo.i_pos);
 	brelse(sinfo.bh);
 	if (IS_ERR(inode)) {
-		unlock_kernel();
+		unlock_super(sb);
 		return ERR_CAST(inode);
 	}
 	alias = d_find_alias(inode);
@@ -708,13 +708,13 @@ static struct dentry *vfat_lookup(struct inode *dir, struct dentry *dentry,
 			dput(alias);
 		else {
 			iput(inode);
-			unlock_kernel();
+			unlock_super(sb);
 			return alias;
 		}
 
 	}
 error:
-	unlock_kernel();
+	unlock_super(sb);
 	dentry->d_op = &vfat_dentry_ops[table];
 	dentry->d_time = dentry->d_parent->d_inode->i_version;
 	dentry = d_splice_alias(inode, dentry);
@@ -734,7 +734,7 @@ static int vfat_create(struct inode *dir, struct dentry *dentry, int mode,
 	struct timespec ts;
 	int err;
 
-	lock_kernel();
+	lock_super(sb);
 
 	ts = CURRENT_TIME_SEC;
 	err = vfat_add_entry(dir, &dentry->d_name, 0, 0, &ts, &sinfo);
@@ -755,17 +755,18 @@ static int vfat_create(struct inode *dir, struct dentry *dentry, int mode,
 	dentry->d_time = dentry->d_parent->d_inode->i_version;
 	d_instantiate(dentry, inode);
 out:
-	unlock_kernel();
+	unlock_super(sb);
 	return err;
 }
 
 static int vfat_rmdir(struct inode *dir, struct dentry *dentry)
 {
 	struct inode *inode = dentry->d_inode;
+	struct super_block *sb = dir->i_sb;
 	struct fat_slot_info sinfo;
 	int err;
 
-	lock_kernel();
+	lock_super(sb);
 
 	err = fat_dir_empty(inode);
 	if (err)
@@ -783,7 +784,7 @@ static int vfat_rmdir(struct inode *dir, struct dentry *dentry)
 	inode->i_mtime = inode->i_atime = CURRENT_TIME_SEC;
 	fat_detach(inode);
 out:
-	unlock_kernel();
+	unlock_super(sb);
 
 	return err;
 }
@@ -791,10 +792,11 @@ out:
 static int vfat_unlink(struct inode *dir, struct dentry *dentry)
 {
 	struct inode *inode = dentry->d_inode;
+	struct super_block *sb = dir->i_sb;
 	struct fat_slot_info sinfo;
 	int err;
 
-	lock_kernel();
+	lock_super(sb);
 
 	err = vfat_find(dir, &dentry->d_name, &sinfo);
 	if (err)
@@ -807,7 +809,7 @@ static int vfat_unlink(struct inode *dir, struct dentry *dentry)
 	inode->i_mtime = inode->i_atime = CURRENT_TIME_SEC;
 	fat_detach(inode);
 out:
-	unlock_kernel();
+	unlock_super(sb);
 
 	return err;
 }
@@ -820,7 +822,7 @@ static int vfat_mkdir(struct inode *dir, struct dentry *dentry, int mode)
 	struct timespec ts;
 	int err, cluster;
 
-	lock_kernel();
+	lock_super(sb);
 
 	ts = CURRENT_TIME_SEC;
 	cluster = fat_alloc_new_dir(dir, &ts);
@@ -849,13 +851,13 @@ static int vfat_mkdir(struct inode *dir, struct dentry *dentry, int mode)
 	dentry->d_time = dentry->d_parent->d_inode->i_version;
 	d_instantiate(dentry, inode);
 
-	unlock_kernel();
+	unlock_super(sb);
 	return 0;
 
 out_free:
 	fat_free_clusters(dir, cluster);
 out:
-	unlock_kernel();
+	unlock_super(sb);
 	return err;
 }
 
@@ -869,11 +871,12 @@ static int vfat_rename(struct inode *old_dir, struct dentry *old_dentry,
 	struct timespec ts;
 	loff_t dotdot_i_pos, new_i_pos;
 	int err, is_dir, update_dotdot, corrupt = 0;
+	struct super_block *sb = old_dir->i_sb;
 
 	old_sinfo.bh = sinfo.bh = dotdot_bh = NULL;
 	old_inode = old_dentry->d_inode;
 	new_inode = new_dentry->d_inode;
-	lock_kernel();
+	lock_super(sb);
 	err = vfat_find(old_dir, &old_dentry->d_name, &old_sinfo);
 	if (err)
 		goto out;
@@ -951,7 +954,7 @@ out:
 	brelse(sinfo.bh);
 	brelse(dotdot_bh);
 	brelse(old_sinfo.bh);
-	unlock_kernel();
+	unlock_super(sb);
 
 	return err;
 

Re: Remove BKL from FAT/VFAT/MSDOS (v1) (was Re: Fw: Regression caused by bf726e "semaphore: fix,")

[Linus Torvalds]

On Fri, 16 May 2008, Linus Torvalds wrote:
>
> 				[...] but I do think this is at 
> least an interesting starting point, and it's at least not "totally 
> obviously broken".

To clarify: lock_super() is actually a much *stronger* lock than 
lock_kernel(), which is why testing is actually interesting: the locking 
hasn't been weakened, it's actually become much stricter (because unlike 
the kernel lock, the superblock lock is kept over scheduling events, and 
doesn't silently nest).

So the most likely failure case is not that some locking went away and 
you'd race on the data structures, it's that something deadlocks on itself 
if the locking isn't right.

There could be some implicit kernel lock I missed (ie something that acts 
like the "ioctl" callback I already mentioned), of course, so I'm not 
going to swear myself blue in the face that there isn't a new race, but on 
the whole this patch should be pretty safe from the angle that at least 
the obvious locking bugs should cause deadlocks, not race conditions.

And if you enable lockdep, the deadlocks aren't silent deaths, but clear 
"this is where you took it, and here is where you deadlocked due to 
nesting" reports in dmesg. So this should be eminently debuggable even by 
almost total newbies that are just interested in BKL removal.

		Linus

Re: Remove BKL from FAT/VFAT/MSDOS (v1) (was Re: Fw: Regression caused by bf726e "semaphore: fix,")

[OGAWA Hirofumi]

Linus Torvalds <torvalds@linux-foundation.org> writes:

> On Fri, 16 May 2008, Linus Torvalds wrote:
>> 
>> In fact, some of the lock_kernel() calls looked like they could even be 
>> dropped:
>> 
>>  - fat_clear_inode already gets the real spinlocks (inode_hash_lock and 
>>    cache_lru_lock), and the kernel lock looks pointless.
>
> Ok, this was also the only one that was called under the superblock lock 
> (sys_umount->deactivate_super->invalidate_inodes->clear_inode), and 
> everything else could be directly converted to use the superblock lock, so 
> here's a trial balloon patch that does exactly that: just blindly convert 
> all the "lock_kernel()" calls to "lock_super(sb)", except for the one in 
> fat_clear_inode(), which is dropped entirely.

IIRC, when I looked this lastly, almost BKLs was just tossed from VFS,
and FAT taked the needed locks anymore, but I'm not sure NFS related
stuff.  Basically, we can just drop BKLs and I tested it repeatedly.

I attached the tested patch by me (but nfs stuff is not tested, it
triggers -ESTALE easily). But, I can't say, "I'm very sure, this patch
is safe", so, I didn't submit the patch yet...

> ANOTHER NOTE! I didn't change this to use .unlocked_ioctl(), and I didn't 
> even really look at it. It still takes the BKL, but obviously doesn't help 
> any. It didn't look like it should really need it, because it seems to do 
> the right locking as-is (ie it gets the inode mutex, and it calls 
> __fat_readdir() which now does the lock_super()).

Ah, I didn't notice about this. But we should be able to just use it.

>  static inline struct fat_cache *fat_cache_alloc(struct inode *inode)
>  {
> -	return kmem_cache_alloc(fat_cache_cachep, GFP_KERNEL);
> +	return kmem_cache_alloc(fat_cache_cachep, GFP_NOFS);
>  }

Yes.

>  static struct inode *fat_alloc_inode(struct super_block *sb)
>  {
>  	struct msdos_inode_info *ei;
> -	ei = kmem_cache_alloc(fat_inode_cachep, GFP_KERNEL);
> +	ei = kmem_cache_alloc(fat_inode_cachep, GFP_NOFS);
>  	if (!ei)
>  		return NULL;
>  	return &ei->vfs_inode;

Um... do we need this? I think this path is not called from memory
allocation path...
-- 
OGAWA Hirofumi <hirofumi@mail.parknet.co.jp>



Signed-off-by: OGAWA Hirofumi <hirofumi@mail.parknet.co.jp>
---

 fs/fat/dir.c     |    6 +++---
 fs/fat/file.c    |   10 +++++-----
 fs/fat/inode.c   |   16 ++++++++--------
 fs/msdos/namei.c |   28 ++++++++++++++--------------
 fs/vfat/namei.c  |   32 ++++++++++++++++----------------
 5 files changed, 46 insertions(+), 46 deletions(-)

diff -puN fs/fat/dir.c~fat_kill-bkl fs/fat/dir.c
--- linux-2.6/fs/fat/dir.c~fat_kill-bkl	2008-04-23 04:40:10.000000000 +0900
+++ linux-2.6-hirofumi/fs/fat/dir.c	2008-04-23 04:40:10.000000000 +0900
@@ -18,7 +18,7 @@
 #include <linux/time.h>
 #include <linux/msdos_fs.h>
 #include <linux/dirent.h>
-#include <linux/smp_lock.h>
+//#include <linux/smp_lock.h>
 #include <linux/buffer_head.h>
 #include <linux/compat.h>
 #include <asm/uaccess.h>
@@ -472,7 +472,7 @@ static int __fat_readdir(struct inode *i
 	loff_t cpos;
 	int ret = 0;
 
-	lock_kernel();
+//	lock_kernel();
 
 	cpos = filp->f_pos;
 	/* Fake . and .. for the root directory. */
@@ -654,7 +654,7 @@ FillFailed:
 	if (unicode)
 		__putname(unicode);
 out:
-	unlock_kernel();
+//	unlock_kernel();
 	return ret;
 }
 
diff -puN fs/fat/file.c~fat_kill-bkl fs/fat/file.c
--- linux-2.6/fs/fat/file.c~fat_kill-bkl	2008-04-23 04:40:10.000000000 +0900
+++ linux-2.6-hirofumi/fs/fat/file.c	2008-04-23 04:40:10.000000000 +0900
@@ -11,7 +11,7 @@
 #include <linux/mount.h>
 #include <linux/time.h>
 #include <linux/msdos_fs.h>
-#include <linux/smp_lock.h>
+//#include <linux/smp_lock.h>
 #include <linux/buffer_head.h>
 #include <linux/writeback.h>
 #include <linux/backing-dev.h>
@@ -244,9 +244,9 @@ void fat_truncate(struct inode *inode)
 
 	nr_clusters = (inode->i_size + (cluster_size - 1)) >> sbi->cluster_bits;
 
-	lock_kernel();
+//	lock_kernel();
 	fat_free(inode, nr_clusters);
-	unlock_kernel();
+//	unlock_kernel();
 	fat_flush_inodes(inode->i_sb, inode, NULL);
 	fs_mark_flush(sb);
 }
@@ -305,7 +305,7 @@ int fat_setattr(struct dentry *dentry, s
 	int mask, error = 0;
 	unsigned int ia_valid;
 
-	lock_kernel();
+//	lock_kernel();
 
 	/*
 	 * Expand the file. Since inode_setattr() updates ->i_size
@@ -359,7 +359,7 @@ int fat_setattr(struct dentry *dentry, s
 		mask = sbi->options.fs_fmask;
 	inode->i_mode &= S_IFMT | (S_IRWXUGO & ~mask);
 out:
-	unlock_kernel();
+//	unlock_kernel();
 	return error;
 }
 EXPORT_SYMBOL_GPL(fat_setattr);
diff -puN fs/fat/inode.c~fat_kill-bkl fs/fat/inode.c
--- linux-2.6/fs/fat/inode.c~fat_kill-bkl	2008-04-23 04:40:10.000000000 +0900
+++ linux-2.6-hirofumi/fs/fat/inode.c	2008-04-23 04:40:10.000000000 +0900
@@ -14,7 +14,7 @@
 #include <linux/init.h>
 #include <linux/time.h>
 #include <linux/slab.h>
-#include <linux/smp_lock.h>
+//#include <linux/smp_lock.h>
 #include <linux/seq_file.h>
 #include <linux/msdos_fs.h>
 #include <linux/pagemap.h>
@@ -442,12 +442,12 @@ static void fat_clear_inode(struct inode
 {
 	struct msdos_sb_info *sbi = MSDOS_SB(inode->i_sb);
 
-	lock_kernel();
+//	lock_kernel();
 	spin_lock(&sbi->inode_hash_lock);
 	fat_cache_inval_inode(inode);
 	hlist_del_init(&MSDOS_I(inode)->i_fat_hash);
 	spin_unlock(&sbi->inode_hash_lock);
-	unlock_kernel();
+//	unlock_kernel();
 }
 
 static void fat_write_super(struct super_block *sb)
@@ -567,7 +567,7 @@ retry:
 	if (inode->i_ino == MSDOS_ROOT_INO || !i_pos)
 		return 0;
 
-	lock_kernel();
+//	lock_kernel();
 	bh = sb_bread(sb, i_pos >> sbi->dir_per_block_bits);
 	if (!bh) {
 		printk(KERN_ERR "FAT: unable to read inode block "
@@ -579,7 +579,7 @@ retry:
 	if (i_pos != MSDOS_I(inode)->i_pos) {
 		spin_unlock(&sbi->inode_hash_lock);
 		brelse(bh);
-		unlock_kernel();
+//		unlock_kernel();
 		goto retry;
 	}
 
@@ -606,7 +606,7 @@ retry:
 		err = sync_dirty_buffer(bh);
 	brelse(bh);
 out:
-	unlock_kernel();
+//	unlock_kernel();
 	return err;
 }
 
@@ -743,7 +743,7 @@ static struct dentry *fat_get_parent(str
 	struct inode *inode;
 	int err;
 
-	lock_kernel();
+//	lock_kernel();
 
 	err = fat_get_dotdot_entry(child->d_inode, &bh, &de, &i_pos);
 	if (err) {
@@ -762,7 +762,7 @@ static struct dentry *fat_get_parent(str
 		parent = ERR_PTR(-ENOMEM);
 	}
 out:
-	unlock_kernel();
+//	unlock_kernel();
 
 	return parent;
 }
diff -puN fs/msdos/namei.c~fat_kill-bkl fs/msdos/namei.c
--- linux-2.6/fs/msdos/namei.c~fat_kill-bkl	2008-04-23 04:40:10.000000000 +0900
+++ linux-2.6-hirofumi/fs/msdos/namei.c	2008-04-23 04:40:10.000000000 +0900
@@ -10,7 +10,7 @@
 #include <linux/time.h>
 #include <linux/buffer_head.h>
 #include <linux/msdos_fs.h>
-#include <linux/smp_lock.h>
+//#include <linux/smp_lock.h>
 
 /* Characters that are undesirable in an MS-DOS file name */
 static unsigned char bad_chars[] = "*?<>|\"";
@@ -214,7 +214,7 @@ static struct dentry *msdos_lookup(struc
 
 	dentry->d_op = &msdos_dentry_operations;
 
-	lock_kernel();
+//	lock_kernel();
 	res = msdos_find(dir, dentry->d_name.name, dentry->d_name.len, &sinfo);
 	if (res == -ENOENT)
 		goto add;
@@ -232,7 +232,7 @@ add:
 	if (dentry)
 		dentry->d_op = &msdos_dentry_operations;
 out:
-	unlock_kernel();
+//	unlock_kernel();
 	if (!res)
 		return dentry;
 	return ERR_PTR(res);
@@ -286,7 +286,7 @@ static int msdos_create(struct inode *di
 	unsigned char msdos_name[MSDOS_NAME];
 	int err, is_hid;
 
-	lock_kernel();
+//	lock_kernel();
 
 	err = msdos_format_name(dentry->d_name.name, dentry->d_name.len,
 				msdos_name, &MSDOS_SB(sb)->options);
@@ -315,7 +315,7 @@ static int msdos_create(struct inode *di
 
 	d_instantiate(dentry, inode);
 out:
-	unlock_kernel();
+//	unlock_kernel();
 	if (!err)
 		err = fat_flush_inodes(sb, dir, inode);
 	return err;
@@ -328,7 +328,7 @@ static int msdos_rmdir(struct inode *dir
 	struct fat_slot_info sinfo;
 	int err;
 
-	lock_kernel();
+//	lock_kernel();
 	/*
 	 * Check whether the directory is not in use, then check
 	 * whether it is empty.
@@ -349,7 +349,7 @@ static int msdos_rmdir(struct inode *dir
 	inode->i_ctime = CURRENT_TIME_SEC;
 	fat_detach(inode);
 out:
-	unlock_kernel();
+//	unlock_kernel();
 	if (!err)
 		err = fat_flush_inodes(inode->i_sb, dir, inode);
 
@@ -366,7 +366,7 @@ static int msdos_mkdir(struct inode *dir
 	struct timespec ts;
 	int err, is_hid, cluster;
 
-	lock_kernel();
+//	lock_kernel();
 
 	err = msdos_format_name(dentry->d_name.name, dentry->d_name.len,
 				msdos_name, &MSDOS_SB(sb)->options);
@@ -404,14 +404,14 @@ static int msdos_mkdir(struct inode *dir
 
 	d_instantiate(dentry, inode);
 
-	unlock_kernel();
+//	unlock_kernel();
 	fat_flush_inodes(sb, dir, inode);
 	return 0;
 
 out_free:
 	fat_free_clusters(dir, cluster);
 out:
-	unlock_kernel();
+//	unlock_kernel();
 	return err;
 }
 
@@ -422,7 +422,7 @@ static int msdos_unlink(struct inode *di
 	struct fat_slot_info sinfo;
 	int err;
 
-	lock_kernel();
+//	lock_kernel();
 	err = msdos_find(dir, dentry->d_name.name, dentry->d_name.len, &sinfo);
 	if (err)
 		goto out;
@@ -434,7 +434,7 @@ static int msdos_unlink(struct inode *di
 	inode->i_ctime = CURRENT_TIME_SEC;
 	fat_detach(inode);
 out:
-	unlock_kernel();
+//	unlock_kernel();
 	if (!err)
 		err = fat_flush_inodes(inode->i_sb, dir, inode);
 
@@ -621,7 +621,7 @@ static int msdos_rename(struct inode *ol
 	unsigned char old_msdos_name[MSDOS_NAME], new_msdos_name[MSDOS_NAME];
 	int err, is_hid;
 
-	lock_kernel();
+//	lock_kernel();
 
 	err = msdos_format_name(old_dentry->d_name.name,
 				old_dentry->d_name.len, old_msdos_name,
@@ -640,7 +640,7 @@ static int msdos_rename(struct inode *ol
 	err = do_msdos_rename(old_dir, old_msdos_name, old_dentry,
 			      new_dir, new_msdos_name, new_dentry, is_hid);
 out:
-	unlock_kernel();
+//	unlock_kernel();
 	if (!err)
 		err = fat_flush_inodes(old_dir->i_sb, old_dir, new_dir);
 	return err;
diff -puN fs/vfat/namei.c~fat_kill-bkl fs/vfat/namei.c
--- linux-2.6/fs/vfat/namei.c~fat_kill-bkl	2008-04-23 04:40:10.000000000 +0900
+++ linux-2.6-hirofumi/fs/vfat/namei.c	2008-04-23 04:40:10.000000000 +0900
@@ -21,7 +21,7 @@
 #include <linux/msdos_fs.h>
 #include <linux/ctype.h>
 #include <linux/slab.h>
-#include <linux/smp_lock.h>
+//#include <linux/smp_lock.h>
 #include <linux/buffer_head.h>
 #include <linux/namei.h>
 
@@ -687,7 +687,7 @@ static struct dentry *vfat_lookup(struct
 	struct dentry *alias;
 	int err, table;
 
-	lock_kernel();
+//	lock_kernel();
 	table = (MSDOS_SB(sb)->options.name_check == 's') ? 2 : 0;
 	dentry->d_op = &vfat_dentry_ops[table];
 
@@ -699,7 +699,7 @@ static struct dentry *vfat_lookup(struct
 	inode = fat_build_inode(sb, sinfo.de, sinfo.i_pos);
 	brelse(sinfo.bh);
 	if (IS_ERR(inode)) {
-		unlock_kernel();
+//		unlock_kernel();
 		return ERR_CAST(inode);
 	}
 	alias = d_find_alias(inode);
@@ -708,13 +708,13 @@ static struct dentry *vfat_lookup(struct
 			dput(alias);
 		else {
 			iput(inode);
-			unlock_kernel();
+//			unlock_kernel();
 			return alias;
 		}
 
 	}
 error:
-	unlock_kernel();
+//	unlock_kernel();
 	dentry->d_op = &vfat_dentry_ops[table];
 	dentry->d_time = dentry->d_parent->d_inode->i_version;
 	dentry = d_splice_alias(inode, dentry);
@@ -734,7 +734,7 @@ static int vfat_create(struct inode *dir
 	struct timespec ts;
 	int err;
 
-	lock_kernel();
+//	lock_kernel();
 
 	ts = CURRENT_TIME_SEC;
 	err = vfat_add_entry(dir, &dentry->d_name, 0, 0, &ts, &sinfo);
@@ -755,7 +755,7 @@ static int vfat_create(struct inode *dir
 	dentry->d_time = dentry->d_parent->d_inode->i_version;
 	d_instantiate(dentry, inode);
 out:
-	unlock_kernel();
+//	unlock_kernel();
 	return err;
 }
 
@@ -765,7 +765,7 @@ static int vfat_rmdir(struct inode *dir,
 	struct fat_slot_info sinfo;
 	int err;
 
-	lock_kernel();
+//	lock_kernel();
 
 	err = fat_dir_empty(inode);
 	if (err)
@@ -783,7 +783,7 @@ static int vfat_rmdir(struct inode *dir,
 	inode->i_mtime = inode->i_atime = CURRENT_TIME_SEC;
 	fat_detach(inode);
 out:
-	unlock_kernel();
+//	unlock_kernel();
 
 	return err;
 }
@@ -794,7 +794,7 @@ static int vfat_unlink(struct inode *dir
 	struct fat_slot_info sinfo;
 	int err;
 
-	lock_kernel();
+//	lock_kernel();
 
 	err = vfat_find(dir, &dentry->d_name, &sinfo);
 	if (err)
@@ -807,7 +807,7 @@ static int vfat_unlink(struct inode *dir
 	inode->i_mtime = inode->i_atime = CURRENT_TIME_SEC;
 	fat_detach(inode);
 out:
-	unlock_kernel();
+//	unlock_kernel();
 
 	return err;
 }
@@ -820,7 +820,7 @@ static int vfat_mkdir(struct inode *dir,
 	struct timespec ts;
 	int err, cluster;
 
-	lock_kernel();
+//	lock_kernel();
 
 	ts = CURRENT_TIME_SEC;
 	cluster = fat_alloc_new_dir(dir, &ts);
@@ -849,13 +849,13 @@ static int vfat_mkdir(struct inode *dir,
 	dentry->d_time = dentry->d_parent->d_inode->i_version;
 	d_instantiate(dentry, inode);
 
-	unlock_kernel();
+//	unlock_kernel();
 	return 0;
 
 out_free:
 	fat_free_clusters(dir, cluster);
 out:
-	unlock_kernel();
+//	unlock_kernel();
 	return err;
 }
 
@@ -873,7 +873,7 @@ static int vfat_rename(struct inode *old
 	old_sinfo.bh = sinfo.bh = dotdot_bh = NULL;
 	old_inode = old_dentry->d_inode;
 	new_inode = new_dentry->d_inode;
-	lock_kernel();
+//	lock_kernel();
 	err = vfat_find(old_dir, &old_dentry->d_name, &old_sinfo);
 	if (err)
 		goto out;
@@ -951,7 +951,7 @@ out:
 	brelse(sinfo.bh);
 	brelse(dotdot_bh);
 	brelse(old_sinfo.bh);
-	unlock_kernel();
+//	unlock_kernel();
 
 	return err;
 
_

Re: Remove BKL from FAT/VFAT/MSDOS (v1) (was Re: Fw: Regression caused by bf726e "semaphore: fix,")

[Linus Torvalds]

On Sat, 17 May 2008, OGAWA Hirofumi wrote:
> >  static struct inode *fat_alloc_inode(struct super_block *sb)
> >  {
> >  	struct msdos_inode_info *ei;
> > -	ei = kmem_cache_alloc(fat_inode_cachep, GFP_KERNEL);
> > +	ei = kmem_cache_alloc(fat_inode_cachep, GFP_NOFS);
> >  	if (!ei)
> >  		return NULL;
> >  	return &ei->vfs_inode;
> 
> Um... do we need this? I think this path is not called from memory
> allocation path...

The issue isn't that *this* is called by memory allocation paths, but 
whether this can hold the lock and then some memory allocation path calls 
back to the filesystem two write something out - and deadlock.

In other words, the chain is something like this:

	msdos_lookup
	  lock_super()			****
	  fat_build_inode ->
	    new_inode() ->
	      s->s_ops->alloc_inode = fat_alloc_inode ->
	        kmem_cache_alloc() ->
	          .. pageout ..
	            sync_inodes ->
	            (or any other writeout)
	              lock_super()	****

and now it deadlocks.

In other words, the kmem_cache_alloc() must _not_ be allowed to actually 
cause a filesystem writeout, and that's what GFP_NOFS is all about.

That said, just removing the lock-kernel entirely obviously would not have 
that problem either, but I feel safer at least keeping the same locking it 
had, rather than removing locking entirely.

		Linus

Re: Remove BKL from FAT/VFAT/MSDOS (v1) (was Re: Fw: Regression caused by bf726e "semaphore: fix,")

[Tony Luck]

I'm seeing a process hang running a kernel built from the linux-next tree
with tag next-20080529 The problem commit looks to be:
35e447fc2fa408df8af2c8735f83a90cafe651ff

"Replace BKL with superblock lock in fat/msdos/vfat"

This causes a lockup when overwriting an existing file
on a vfat filesystem.  E.g. for me (where there already
exists a vmlinux.gz file in the target directory):

# cp --force vmlinux.gz /boot/efi/efi/redhat/

Stack trace looks like this:
Call Trace:
 [<a000000100712550>] schedule+0x11f0/0x1380
                                sp=e0000001bd55fc30 bsp=e0000001bd5510b8
 [<a000000100714cb0>] __mutex_lock_slowpath+0x2d0/0x520
                                sp=e0000001bd55fc50 bsp=e0000001bd551060
 [<a000000100714f20>] mutex_lock+0x20/0x40
                                sp=e0000001bd55fc80 bsp=e0000001bd551040
 [<a0000001001367d0>] lock_super+0x30/0x60
                                sp=e0000001bd55fc80 bsp=e0000001bd551020
 [<a00000010028ca30>] fat_truncate+0xb0/0x640^M
                                sp=e0000001bd55fc80 bsp=e0000001bd550fa0
 [<a0000001000fd930>] vmtruncate+0x1f0/0x260
                                sp=e0000001bd55fce0 bsp=e0000001bd550f70
 [<a000000100164250>] inode_setattr+0x50/0x320
                                sp=e0000001bd55fcf0 bsp=e0000001bd550f38
 [<a00000010028d520>] fat_setattr+0x4c0/0x580
                                sp=e0000001bd55fcf0 bsp=e0000001bd550ed0
 [<a0000001001648e0>] notify_change+0x3c0/0x620
                                sp=e0000001bd55fd10 bsp=e0000001bd550e70
 [<a00000010012fe00>] do_truncate+0xc0/0x120
                                sp=e0000001bd55fd30 bsp=e0000001bd550e30
 [<a0000001001494c0>] may_open+0x340/0x3a0
                                sp=e0000001bd55fd80 bsp=e0000001bd550de0
 [<a000000100149c60>] do_filp_open+0x740/0x11c0
                                sp=e0000001bd55fd80 bsp=e0000001bd550d18
 [<a000000100132470>] do_sys_open+0x90/0x1c0
                                sp=e0000001bd55fe30 bsp=e0000001bd550cc8
 [<a0000001001325f0>] sys_open+0x50/0x80
                                sp=e0000001bd55fe30 bsp=e0000001bd550c70

Looking at fat_setattr() I see it calls lock_super() at the start and releases
it at the end.  In between is the call to inode_setattr() ... which calls
down through inode_setattr() and vmtruncate() to fat_truncate() ...
which calls lock_super() again. Deadlock.

-Tony

Re: Remove BKL from FAT/VFAT/MSDOS (v1) (was Re: Fw: Regression caused by bf726e "semaphore: fix,")

[Linus Torvalds]

On Thu, 29 May 2008, Tony Luck wrote:
> 
> This causes a lockup when overwriting an existing file
> on a vfat filesystem.  E.g. for me (where there already
> exists a vmlinux.gz file in the target directory):

Thanks, that was something I didn't test. I just tested various simple 
file create/delete/read/write.

> Looking at fat_setattr() I see it calls lock_super() at the start and releases
> it at the end.  In between is the call to inode_setattr() ... which calls
> down through inode_setattr() and vmtruncate() to fat_truncate() ...
> which calls lock_super() again. Deadlock.

Yup.

And as far as I can tell there is absolutely nothing in fat_setattr() that 
we actually want to protect - it calls things like fat_cont_expand(), but 
that just calls down to the generic VFS layer that uses the pagecache 
functions, and those need to handle the locking correctly for other 
reasons (totally unrelated to setattr - they get called without locking 
for regular writes, after all).

So it looks like the correct fix is to just remove the lock_super() in 
fat_setattr() entirely (along with the "sb" variable that is then no 
longer used).

It *also* turns out that we should remove the lock_super() from 
fat_truncate: all the truncate paths already hold the inode mutex from the 
VFS layer, so the inode data structures themselves would be serialized for 
other reasons. And it only protects the call to "fat_free()", which in 
turn calls the FAT cluster routines ("fatent") that already are protected 
by the fatent spinlock.

More importantly, if it's a filesystem marked DIRSYNC, it will call 
"fat_sync_inode()", which takes the superblock lock (and needs it, because 
it's called frm the VFS layer too) and would deadlock there.

So the end result of that is all the "lock_kernel()" calls in 
fs/fat/file.c should actually just go away - not be replaced by 
lock_super() at alL!

Jonathan, do you want an updated replacement patch, or an incremental one, 
or will you just do that trivial fix yourself?

			Linus

Re: Remove BKL from FAT/VFAT/MSDOS (v1) (was Re: Fw: Regression caused by bf726e "semaphore: fix,")

[Jonathan Corbet]

Linus Torvalds <torvalds@linux-foundation.org> wrote:

> Jonathan, do you want an updated replacement patch, or an incremental one, 
> or will you just do that trivial fix yourself?

Something like the attached?  Seems to work for me.  If nobody gripes
I'll stick it into the bkl-removal tree.

jon

Remove deadlocking lock_super() calls from FAT

Linus sez:

	So the end result of that is all the "lock_kernel()" calls in
	fs/fat/file.c should actually just go away - not be replaced by
	lock_super() at alL!

This patch causes that to be.

Signed-off-by: Jonathan Corbet <corbet@lwn.net>


diff --git a/fs/fat/file.c b/fs/fat/file.c
index 7059928..bdf91e9 100644
--- a/fs/fat/file.c
+++ b/fs/fat/file.c
@@ -11,7 +11,6 @@
 #include <linux/mount.h>
 #include <linux/time.h>
 #include <linux/msdos_fs.h>
-#include <linux/smp_lock.h>
 #include <linux/buffer_head.h>
 #include <linux/writeback.h>
 #include <linux/backing-dev.h>
@@ -229,8 +228,7 @@ static int fat_free(struct inode *inode, int skip)
 
 void fat_truncate(struct inode *inode)
 {
-	struct super_block *sb = inode->i_sb;
-	struct msdos_sb_info *sbi = MSDOS_SB(sb);
+	struct msdos_sb_info *sbi = MSDOS_SB(inode->i_sb);
 	const unsigned int cluster_size = sbi->cluster_size;
 	int nr_clusters;
 
@@ -243,9 +241,7 @@ void fat_truncate(struct inode *inode)
 
 	nr_clusters = (inode->i_size + (cluster_size - 1)) >> sbi->cluster_bits;
 
-	lock_super(sb);
 	fat_free(inode, nr_clusters);
-	unlock_super(sb);
 	fat_flush_inodes(inode->i_sb, inode, NULL);
 }
 
@@ -298,14 +294,11 @@ static int fat_allow_set_time(struct msdos_sb_info *sbi, struct inode *inode)
 
 int fat_setattr(struct dentry *dentry, struct iattr *attr)
 {
-	struct super_block *sb = dentry->d_sb;
 	struct msdos_sb_info *sbi = MSDOS_SB(dentry->d_sb);
 	struct inode *inode = dentry->d_inode;
 	int mask, error = 0;
 	unsigned int ia_valid;
 
-	lock_super(sb);
-
 	/*
 	 * Expand the file. Since inode_setattr() updates ->i_size
 	 * before calling the ->truncate(), but FAT needs to fill the
@@ -358,7 +351,6 @@ int fat_setattr(struct dentry *dentry, struct iattr *attr)
 		mask = sbi->options.fs_fmask;
 	inode->i_mode &= S_IFMT | (S_IRWXUGO & ~mask);
 out:
-	unlock_super(sb);
 	return error;
 }
 EXPORT_SYMBOL_GPL(fat_setattr);

Re: Remove BKL from FAT/VFAT/MSDOS (v1) (was Re: Fw: Regression caused by bf726e "semaphore: fix,")

[Linus Torvalds]

On Thu, 29 May 2008, Jonathan Corbet wrote:
> 
> Something like the attached?

Ack. And good of you to notice the <linux/smp_lock.h> include. Although 
there's a few others that should go away too.

		Linus