I encountered this same issue recently and solved it in much the same way. Can we rename "spin_lock" to something more meaningful?
This race actually exposed a potential deadlock between f2fs_create() and
f2fs_initxattrs():
- vfs_create()
- f2fs_create() - takes an fs_lock
- f2fs_add_link()
- __f2fs_add_link()
- init_inode_metadata()
- f2fs_init_security()
- security_inode_init_security()
- f2fs_initxattrs()
- f2fs_setxattr() - also takes an fs_lock
If another CPU happens to have the same lock that f2fs_setxattr() was
trying to take because of the race around next_lock_num, we can get into a
deadlock situation if the two threads are also contending over another
resource (like bdi).
Another scenario is if the above happens while another thread is in the
middle of grabbing all of the locks via mutex_lock_all(). f2fs_create() is
holding a lock that mutex_lock_all() is waiting for and mutex_lock_all() is
holding a lock that f2fs_setxattr() is waiting for.
Russ
On Fri, Sep 6, 2013 at 4:48 AM, Chao Yu <[email protected]> wrote:
> Hi Kim:****
>
> ** **I think there is a performance problem: when all sbi->fs_lock is
> holded,
>
> then all other threads may get the same next_lock value from
> sbi->next_lock_num in function mutex_lock_op,
>
> and wait to get the same lock at position fs_lock[next_lock], it unbalance
> the fs_lock usage.
>
> It may lost performance when we do the multithread test.****
>
> ** **
>
> Here is the patch to fix this problem:****
>
> ** **
>
> Signed-off-by: Yu Chao <[email protected]>****
>
> diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h****
>
> old mode 100644****
>
> new mode 100755****
>
> index 467d42d..983bb45****
>
> --- a/fs/f2fs/f2fs.h****
>
> +++ b/fs/f2fs/f2fs.h****
>
> @@ -371,6 +371,7 @@ struct f2fs_sb_info {****
>
> struct mutex fs_lock[NR_GLOBAL_LOCKS]; /* blocking FS operations
> */****
>
> struct mutex node_write; /* locking node writes */*
> ***
>
> struct mutex writepages; /* mutex for writepages()
> */****
>
> + spinlock_t spin_lock; /* lock for next_lock_num
> */****
>
> unsigned char next_lock_num; /* round-robin global
> locks */****
>
> int por_doing; /* recovery is doing or
> not */****
>
> int on_build_free_nids; /* build_free_nids is
> doing */****
>
> @@ -533,15 +534,19 @@ static inline void mutex_unlock_all(struct
> f2fs_sb_info *sbi)****
>
> ****
>
> static inline int mutex_lock_op(struct f2fs_sb_info *sbi)****
>
> {****
>
> - unsigned char next_lock = sbi->next_lock_num % NR_GLOBAL_LOCKS;***
> *
>
> + unsigned char next_lock;****
>
> int i = 0;****
>
> ****
>
> for (; i < NR_GLOBAL_LOCKS; i++)****
>
> if (mutex_trylock(&sbi->fs_lock[i]))****
>
> return i;****
>
> ****
>
> - mutex_lock(&sbi->fs_lock[next_lock]);****
>
> + spin_lock(&sbi->spin_lock);****
>
> + next_lock = sbi->next_lock_num % NR_GLOBAL_LOCKS;****
>
> sbi->next_lock_num++;****
>
> + spin_unlock(&sbi->spin_lock);****
>
> +****
>
> + mutex_lock(&sbi->fs_lock[next_lock]);****
>
> return next_lock;****
>
> }****
>
> ****
>
> diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c****
>
> old mode 100644****
>
> new mode 100755****
>
> index 75c7dc3..4f27596****
>
> --- a/fs/f2fs/super.c****
>
> +++ b/fs/f2fs/super.c****
>
> @@ -657,6 +657,7 @@ static int f2fs_fill_super(struct super_block *sb,
> void *data, int silent)****
>
> mutex_init(&sbi->cp_mutex);****
>
> for (i = 0; i < NR_GLOBAL_LOCKS; i++)****
>
> mutex_init(&sbi->fs_lock[i]);****
>
> + spin_lock_init(&sbi->spin_lock);****
>
> mutex_init(&sbi->node_write);****
>
> sbi->por_doing = 0;****
>
> spin_lock_init(&sbi->stat_lock);****
>
> (END)****
>
> ** **
>
>
>
> ------------------------------------------------------------------------------
> Learn the latest--Visual Studio 2012, SharePoint 2013, SQL 2012, more!
> Discover the easy way to master current and previous Microsoft technologies
> and advance your career. Get an incredible 1,500+ hours of step-by-step
> tutorial videos with LearnDevNow. Subscribe today and save!
> http://pubads.g.doubleclick.net/gampad/clk?id=58041391&iu=/4140/ostg.clktrk
> _______________________________________________
> Linux-f2fs-devel mailing list
> [email protected]
> https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
>
>
<<201309061748313_TIC2ESYT.gif>>
------------------------------------------------------------------------------ Learn the latest--Visual Studio 2012, SharePoint 2013, SQL 2012, more! Discover the easy way to master current and previous Microsoft technologies and advance your career. Get an incredible 1,500+ hours of step-by-step tutorial videos with LearnDevNow. Subscribe today and save! http://pubads.g.doubleclick.net/gampad/clk?id=58041391&iu=/4140/ostg.clktrk
_______________________________________________ Linux-f2fs-devel mailing list [email protected] https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
