Skip to content

Commit

Permalink
ufs: remove the BKL
Browse files Browse the repository at this point in the history 
This introduces a new per-superblock mutex in UFS to replace
the big kernel lock. I have been careful to avoid nested
calls to lock_ufs and to get the lock order right with
respect to other mutexes, in particular lock_super.

I did not make any attempt to prove that the big kernel
lock is not needed in a particular place in the code,
which is very possible.

The mutex has a significant performance impact, so it is only
used on SMP or PREEMPT configurations.

As Nick Piggin noticed, any allocation inside of the lock
may end up deadlocking when we get to ufs_getfrag_block
in the reclaim task, so we now use GFP_NOFS.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
Tested-by: Nick Bowler <nbowler@elliptictech.com>
Cc: Evgeniy Dushistov <dushistov@mail.ru>
Cc: Nick Piggin <npiggin@gmail.com>
  • Loading branch information
@arndb
arndb committed Mar 2, 2011
1 parent 9a311b9 commit 788257d
Show file tree
Hide file tree
Showing 7 changed files with 83 additions and 108 deletions.
1 change: 0 additions & 1 deletion fs/ufs/Kconfig
View file
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
config UFS_FS
tristate "UFS file system support (read only)"
depends on BLOCK
depends on BKL # probably fixable
help
BSD and derivate versions of Unix (such as SunOS, FreeBSD, NetBSD,
OpenBSD and NeXTstep) use a file system called UFS. Some System V
Expand Down
78 changes: 21 additions & 57 deletions fs/ufs/inode.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,6 @@
#include <linux/stat.h>
#include <linux/string.h>
#include <linux/mm.h>
#include <linux/smp_lock.h>
#include <linux/buffer_head.h>
#include <linux/writeback.h>

Expand All @@ -43,7 +42,7 @@
#include "swab.h"
#include "util.h"

static u64 ufs_frag_map(struct inode *inode, sector_t frag);
static u64 ufs_frag_map(struct inode *inode, sector_t frag, bool needs_lock);

static int ufs_block_to_path(struct inode *inode, sector_t i_block, sector_t offsets[4])
{
Expand Down Expand Up @@ -82,7 +81,7 @@ static int ufs_block_to_path(struct inode *inode, sector_t i_block, sector_t off
* the begining of the filesystem.
*/

static u64 ufs_frag_map(struct inode *inode, sector_t frag)
static u64 ufs_frag_map(struct inode *inode, sector_t frag, bool needs_lock)
{
struct ufs_inode_info *ufsi = UFS_I(inode);
struct super_block *sb = inode->i_sb;
Expand All @@ -107,7 +106,8 @@ static u64 ufs_frag_map(struct inode *inode, sector_t frag)

p = offsets;

lock_kernel();
if (needs_lock)
lock_ufs(sb);
if ((flags & UFS_TYPE_MASK) == UFS_TYPE_UFS2)
goto ufs2;

Expand Down Expand Up @@ -152,7 +152,8 @@ static u64 ufs_frag_map(struct inode *inode, sector_t frag)
ret = temp + (u64) (frag & uspi->s_fpbmask);

out:
unlock_kernel();
if (needs_lock)
unlock_ufs(sb);
return ret;
}

Expand Down Expand Up @@ -415,14 +416,16 @@ ufs_inode_getblock(struct inode *inode, struct buffer_head *bh,
int ufs_getfrag_block(struct inode *inode, sector_t fragment, struct buffer_head *bh_result, int create)
{
struct super_block * sb = inode->i_sb;
struct ufs_sb_private_info * uspi = UFS_SB(sb)->s_uspi;
struct ufs_sb_info * sbi = UFS_SB(sb);
struct ufs_sb_private_info * uspi = sbi->s_uspi;
struct buffer_head * bh;
int ret, err, new;
unsigned long ptr,phys;
u64 phys64 = 0;
bool needs_lock = (sbi->mutex_owner != current);

if (!create) {
phys64 = ufs_frag_map(inode, fragment);
phys64 = ufs_frag_map(inode, fragment, needs_lock);
UFSD("phys64 = %llu\n", (unsigned long long)phys64);
if (phys64)
map_bh(bh_result, sb, phys64);
Expand All @@ -436,7 +439,8 @@ int ufs_getfrag_block(struct inode *inode, sector_t fragment, struct buffer_head
ret = 0;
bh = NULL;

lock_kernel();
if (needs_lock)
lock_ufs(sb);

UFSD("ENTER, ino %lu, fragment %llu\n", inode->i_ino, (unsigned long long)fragment);
if (fragment >
Expand Down Expand Up @@ -498,56 +502,16 @@ int ufs_getfrag_block(struct inode *inode, sector_t fragment, struct buffer_head
set_buffer_new(bh_result);
map_bh(bh_result, sb, phys);
abort:
unlock_kernel();
if (needs_lock)
unlock_ufs(sb);

return err;

abort_too_big:
ufs_warning(sb, "ufs_get_block", "block > big");
goto abort;
}

static struct buffer_head *ufs_getfrag(struct inode *inode,
unsigned int fragment,
int create, int *err)
{
struct buffer_head dummy;
int error;

dummy.b_state = 0;
dummy.b_blocknr = -1000;
error = ufs_getfrag_block(inode, fragment, &dummy, create);
*err = error;
if (!error && buffer_mapped(&dummy)) {
struct buffer_head *bh;
bh = sb_getblk(inode->i_sb, dummy.b_blocknr);
if (buffer_new(&dummy)) {
memset(bh->b_data, 0, inode->i_sb->s_blocksize);
set_buffer_uptodate(bh);
mark_buffer_dirty(bh);
}
return bh;
}
return NULL;
}

struct buffer_head * ufs_bread (struct inode * inode, unsigned fragment,
int create, int * err)
{
struct buffer_head * bh;

UFSD("ENTER, ino %lu, fragment %u\n", inode->i_ino, fragment);
bh = ufs_getfrag (inode, fragment, create, err);
if (!bh || buffer_uptodate(bh))
return bh;
ll_rw_block (READ, 1, &bh);
wait_on_buffer (bh);
if (buffer_uptodate(bh))
return bh;
brelse (bh);
*err = -EIO;
return NULL;
}

static int ufs_writepage(struct page *page, struct writeback_control *wbc)
{
return block_write_full_page(page,ufs_getfrag_block,wbc);
Expand Down Expand Up @@ -900,9 +864,9 @@ static int ufs_update_inode(struct inode * inode, int do_sync)
int ufs_write_inode(struct inode *inode, struct writeback_control *wbc)
{
int ret;
lock_kernel();
lock_ufs(inode->i_sb);
ret = ufs_update_inode(inode, wbc->sync_mode == WB_SYNC_ALL);
unlock_kernel();
unlock_ufs(inode->i_sb);
return ret;
}

Expand All @@ -922,22 +886,22 @@ void ufs_evict_inode(struct inode * inode)
if (want_delete) {
loff_t old_i_size;
/*UFS_I(inode)->i_dtime = CURRENT_TIME;*/
lock_kernel();
lock_ufs(inode->i_sb);
mark_inode_dirty(inode);
ufs_update_inode(inode, IS_SYNC(inode));
old_i_size = inode->i_size;
inode->i_size = 0;
if (inode->i_blocks && ufs_truncate(inode, old_i_size))
ufs_warning(inode->i_sb, __func__, "ufs_truncate failed\n");
unlock_kernel();
unlock_ufs(inode->i_sb);
}

invalidate_inode_buffers(inode);
end_writeback(inode);

if (want_delete) {
lock_kernel();
lock_ufs(inode->i_sb);
ufs_free_inode (inode);
unlock_kernel();
unlock_ufs(inode->i_sb);
}
}
35 changes: 17 additions & 18 deletions fs/ufs/namei.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,6 @@

#include <linux/time.h>
#include <linux/fs.h>
#include <linux/smp_lock.h>

#include "ufs_fs.h"
#include "ufs.h"
Expand All @@ -55,16 +54,16 @@ static struct dentry *ufs_lookup(struct inode * dir, struct dentry *dentry, stru
if (dentry->d_name.len > UFS_MAXNAMLEN)
return ERR_PTR(-ENAMETOOLONG);

lock_kernel();
lock_ufs(dir->i_sb);
ino = ufs_inode_by_name(dir, &dentry->d_name);
if (ino) {
inode = ufs_iget(dir->i_sb, ino);
if (IS_ERR(inode)) {
unlock_kernel();
unlock_ufs(dir->i_sb);
return ERR_CAST(inode);
}
}
unlock_kernel();
unlock_ufs(dir->i_sb);
d_add(dentry, inode);
return NULL;
}
Expand Down Expand Up @@ -93,9 +92,9 @@ static int ufs_create (struct inode * dir, struct dentry * dentry, int mode,
inode->i_fop = &ufs_file_operations;
inode->i_mapping->a_ops = &ufs_aops;
mark_inode_dirty(inode);
lock_kernel();
lock_ufs(dir->i_sb);
err = ufs_add_nondir(dentry, inode);
unlock_kernel();
unlock_ufs(dir->i_sb);
}
UFSD("END: err=%d\n", err);
return err;
Expand All @@ -115,9 +114,9 @@ static int ufs_mknod (struct inode * dir, struct dentry *dentry, int mode, dev_t
init_special_inode(inode, mode, rdev);
ufs_set_inode_dev(inode->i_sb, UFS_I(inode), rdev);
mark_inode_dirty(inode);
lock_kernel();
lock_ufs(dir->i_sb);
err = ufs_add_nondir(dentry, inode);
unlock_kernel();
unlock_ufs(dir->i_sb);
}
return err;
}
Expand All @@ -133,7 +132,7 @@ static int ufs_symlink (struct inode * dir, struct dentry * dentry,
if (l > sb->s_blocksize)
goto out_notlocked;

lock_kernel();
lock_ufs(dir->i_sb);
inode = ufs_new_inode(dir, S_IFLNK | S_IRWXUGO);
err = PTR_ERR(inode);
if (IS_ERR(inode))
Expand All @@ -156,7 +155,7 @@ static int ufs_symlink (struct inode * dir, struct dentry * dentry,

err = ufs_add_nondir(dentry, inode);
out:
unlock_kernel();
unlock_ufs(dir->i_sb);
out_notlocked:
return err;

Expand All @@ -172,9 +171,9 @@ static int ufs_link (struct dentry * old_dentry, struct inode * dir,
struct inode *inode = old_dentry->d_inode;
int error;

lock_kernel();
lock_ufs(dir->i_sb);
if (inode->i_nlink >= UFS_LINK_MAX) {
unlock_kernel();
unlock_ufs(dir->i_sb);
return -EMLINK;
}

Expand All @@ -183,7 +182,7 @@ static int ufs_link (struct dentry * old_dentry, struct inode * dir,
ihold(inode);

error = ufs_add_nondir(dentry, inode);
unlock_kernel();
unlock_ufs(dir->i_sb);
return error;
}

Expand All @@ -195,7 +194,7 @@ static int ufs_mkdir(struct inode * dir, struct dentry * dentry, int mode)
if (dir->i_nlink >= UFS_LINK_MAX)
goto out;

lock_kernel();
lock_ufs(dir->i_sb);
inode_inc_link_count(dir);

inode = ufs_new_inode(dir, S_IFDIR|mode);
Expand All @@ -216,7 +215,7 @@ static int ufs_mkdir(struct inode * dir, struct dentry * dentry, int mode)
err = ufs_add_link(dentry, inode);
if (err)
goto out_fail;
unlock_kernel();
unlock_ufs(dir->i_sb);

d_instantiate(dentry, inode);
out:
Expand All @@ -228,7 +227,7 @@ static int ufs_mkdir(struct inode * dir, struct dentry * dentry, int mode)
iput (inode);
out_dir:
inode_dec_link_count(dir);
unlock_kernel();
unlock_ufs(dir->i_sb);
goto out;
}

Expand Down Expand Up @@ -259,7 +258,7 @@ static int ufs_rmdir (struct inode * dir, struct dentry *dentry)
struct inode * inode = dentry->d_inode;
int err= -ENOTEMPTY;

lock_kernel();
lock_ufs(dir->i_sb);
if (ufs_empty_dir (inode)) {
err = ufs_unlink(dir, dentry);
if (!err) {
Expand All @@ -268,7 +267,7 @@ static int ufs_rmdir (struct inode * dir, struct dentry *dentry)
inode_dec_link_count(dir);
}
}
unlock_kernel();
unlock_ufs(dir->i_sb);
return err;
}

Expand Down
Loading

0 comments on commit 788257d

Please sign in to comment.