* Re: [f2fs-dev][PATCH] f2fs: optimize fs_lock for better performance [not found] <88.C4.11914.9D4A9225@epcpsbge6.samsung.com> @ 2013-09-10 0:52 ` Jaegeuk Kim 2013-09-11 3:13 ` [PATCH] " Gu Zheng 2013-09-11 5:37 ` Gu Zheng 0 siblings, 2 replies; 13+ messages in thread From: Jaegeuk Kim @ 2013-09-10 0:52 UTC (permalink / raw) To: chao2.yu; +Cc: shu.tan, linux-fsdevel, linux-kernel, linux-f2fs-devel Hi, At first, thank you for the report and please follow the email writing rules. :) Anyway, I agree to the below issue. One thing that I can think of is that we don't need to use the spin_lock, since we don't care about the exact lock number, but just need to get any not-collided number. So, how about removing the spin_lock? And how about using a random number? Thanks, 2013-09-06 (금), 09:48 +0000, Chao Yu: > 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 <chao2.yu@samsung.com> > > 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) > > > > > > -- Jaegeuk Kim Samsung ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] f2fs: optimize fs_lock for better performance 2013-09-10 0:52 ` [f2fs-dev][PATCH] f2fs: optimize fs_lock for better performance Jaegeuk Kim @ 2013-09-11 3:13 ` Gu Zheng 2013-09-11 5:37 ` Gu Zheng 1 sibling, 0 replies; 13+ messages in thread From: Gu Zheng @ 2013-09-11 3:13 UTC (permalink / raw) To: jaegeuk.kim; +Cc: linux-fsdevel, shu.tan, linux-kernel, linux-f2fs-devel Hi Jaegeuk, On 09/10/2013 08:52 AM, Jaegeuk Kim wrote: > Hi, > > At first, thank you for the report and please follow the email writing > rules. :) > > Anyway, I agree to the below issue. > One thing that I can think of is that we don't need to use the > spin_lock, since we don't care about the exact lock number, but just > need to get any not-collided number. Agree, but if all the locks are held, IMO, we need to balance the following threads to wait for each not-collided number lock, though complete balance is unreachable. > > So, how about removing the spin_lock? Yeah, in this case, spin_lock is a bit heavy cost. > And how about using a random number? Now NR_GLOBAL_LOCKS is 8, it seems that random can not offer an balance number as we expected. Regards, Gu > Thanks, > > 2013-09-06 (금), 09:48 +0000, Chao Yu: >> 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 <chao2.yu@samsung.com> >> >> 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) >> >> >> >> >> >> > ------------------------------------------------------------------------------ How ServiceNow helps IT people transform IT departments: 1. Consolidate legacy IT systems to a single system of record for IT 2. Standardize and globalize service processes across IT 3. Implement zero-touch automation to replace manual, redundant tasks http://pubads.g.doubleclick.net/gampad/clk?id=51271111&iu=/4140/ostg.clktrk _______________________________________________ Linux-f2fs-devel mailing list Linux-f2fs-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] f2fs: optimize fs_lock for better performance 2013-09-10 0:52 ` [f2fs-dev][PATCH] f2fs: optimize fs_lock for better performance Jaegeuk Kim 2013-09-11 3:13 ` [PATCH] " Gu Zheng @ 2013-09-11 5:37 ` Gu Zheng 2013-09-11 13:22 ` [f2fs-dev] " Kim Jaegeuk 2013-09-12 2:40 ` 俞超 1 sibling, 2 replies; 13+ messages in thread From: Gu Zheng @ 2013-09-11 5:37 UTC (permalink / raw) To: jaegeuk.kim; +Cc: linux-fsdevel, shu.tan, linux-kernel, linux-f2fs-devel Hi Jaegeuk, Chao, On 09/10/2013 08:52 AM, Jaegeuk Kim wrote: > Hi, > > At first, thank you for the report and please follow the email writing > rules. :) > > Anyway, I agree to the below issue. > One thing that I can think of is that we don't need to use the > spin_lock, since we don't care about the exact lock number, but just > need to get any not-collided number. IMHO, just moving sbi->next_lock_num++ before mutex_lock(&sbi->fs_lock[next_lock]) can avoid unbalance issue mostly. IMO, the case two or more threads increase sbi->next_lock_num in the same time is really very very little. If you think it is not rigorous, change next_lock_num to atomic one can fix it. What's your opinion? Regards, Gu > > So, how about removing the spin_lock? > And how about using a random number? > Thanks, > > 2013-09-06 (금), 09:48 +0000, Chao Yu: >> 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 <chao2.yu@samsung.com> >> >> 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) >> >> >> >> >> >> > ------------------------------------------------------------------------------ How ServiceNow helps IT people transform IT departments: 1. Consolidate legacy IT systems to a single system of record for IT 2. Standardize and globalize service processes across IT 3. Implement zero-touch automation to replace manual, redundant tasks http://pubads.g.doubleclick.net/gampad/clk?id=51271111&iu=/4140/ostg.clktrk _______________________________________________ Linux-f2fs-devel mailing list Linux-f2fs-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [f2fs-dev] [PATCH] f2fs: optimize fs_lock for better performance 2013-09-11 5:37 ` Gu Zheng @ 2013-09-11 13:22 ` Kim Jaegeuk 2013-09-12 2:40 ` 俞超 1 sibling, 0 replies; 13+ messages in thread From: Kim Jaegeuk @ 2013-09-11 13:22 UTC (permalink / raw) To: Gu Zheng Cc: Jaegeuk Kim, linux-fsdevel@vger.kernel.org, 谭姝, linux-kernel@vger.kernel.org, linux-f2fs-devel@lists.sourceforge.net Hi Gu, 2013/9/11 Gu Zheng <guz.fnst@cn.fujitsu.com>: > Hi Jaegeuk, Chao, > > On 09/10/2013 08:52 AM, Jaegeuk Kim wrote: > >> Hi, >> >> At first, thank you for the report and please follow the email writing >> rules. :) >> >> Anyway, I agree to the below issue. >> One thing that I can think of is that we don't need to use the >> spin_lock, since we don't care about the exact lock number, but just >> need to get any not-collided number. > > IMHO, just moving sbi->next_lock_num++ before mutex_lock(&sbi->fs_lock[next_lock]) > can avoid unbalance issue mostly. > IMO, the case two or more threads increase sbi->next_lock_num in the same time is > really very very little. If you think it is not rigorous, change next_lock_num to > atomic one can fix it. > What's your opinion? As your opinion, I think it is enough to replace it with simple sbi->next_lock_num++. Thanks, > > Regards, > Gu > >> >> So, how about removing the spin_lock? >> And how about using a random number? > >> Thanks, >> >> 2013-09-06 (금), 09:48 +0000, Chao Yu: >>> 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 <chao2.yu@samsung.com> >>> >>> 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) >>> >>> >>> >>> >>> >>> >> > > > > ------------------------------------------------------------------------------ > How ServiceNow helps IT people transform IT departments: > 1. Consolidate legacy IT systems to a single system of record for IT > 2. Standardize and globalize service processes across IT > 3. Implement zero-touch automation to replace manual, redundant tasks > http://pubads.g.doubleclick.net/gampad/clk?id=51271111&iu=/4140/ostg.clktrk > _______________________________________________ > Linux-f2fs-devel mailing list > Linux-f2fs-devel@lists.sourceforge.net > https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel -- To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] f2fs: optimize fs_lock for better performance 2013-09-11 5:37 ` Gu Zheng 2013-09-11 13:22 ` [f2fs-dev] " Kim Jaegeuk @ 2013-09-12 2:40 ` 俞超 2013-09-12 3:17 ` [PATCH V2] " Gu Zheng 2013-09-12 3:18 ` [PATCH] " Gu Zheng 1 sibling, 2 replies; 13+ messages in thread From: 俞超 @ 2013-09-12 2:40 UTC (permalink / raw) To: 'Gu Zheng', jaegeuk.kim Cc: linux-fsdevel, shu.tan, linux-kernel, linux-f2fs-devel Hi Gu > -----Original Message----- > From: Gu Zheng [mailto:guz.fnst@cn.fujitsu.com] > Sent: Wednesday, September 11, 2013 1:38 PM > To: jaegeuk.kim@samsung.com > Cc: chao2.yu@samsung.com; shu.tan@samsung.com; > linux-fsdevel@vger.kernel.org; linux-kernel@vger.kernel.org; > linux-f2fs-devel@lists.sourceforge.net > Subject: Re: [f2fs-dev][PATCH] f2fs: optimize fs_lock for better performance > > Hi Jaegeuk, Chao, > > On 09/10/2013 08:52 AM, Jaegeuk Kim wrote: > > > Hi, > > > > At first, thank you for the report and please follow the email writing > > rules. :) > > > > Anyway, I agree to the below issue. > > One thing that I can think of is that we don't need to use the > > spin_lock, since we don't care about the exact lock number, but just > > need to get any not-collided number. > > IMHO, just moving sbi->next_lock_num++ before > mutex_lock(&sbi->fs_lock[next_lock]) > can avoid unbalance issue mostly. > IMO, the case two or more threads increase sbi->next_lock_num in the same > time is really very very little. If you think it is not rigorous, change > next_lock_num to atomic one can fix it. > What's your opinion? > > Regards, > Gu I did the test sbi->next_lock_num++ compare with the atomic one, And I found performance of them is almost the same under a small number thread racing. So as your and Kim's opinion, it's enough to use "sbi->next_lock_num++" to fix this issue. Thanks for the advice. > > > > > So, how about removing the spin_lock? > > And how about using a random number? > > > Thanks, > > > > 2013-09-06 (금), 09:48 +0000, Chao Yu: > >> 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 <chao2.yu@samsung.com> > >> > >> 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) > >> > >> > >> > >> > >> > >> > > > > > = ------------------------------------------------------------------------------ How ServiceNow helps IT people transform IT departments: 1. Consolidate legacy IT systems to a single system of record for IT 2. Standardize and globalize service processes across IT 3. Implement zero-touch automation to replace manual, redundant tasks http://pubads.g.doubleclick.net/gampad/clk?id=51271111&iu=/4140/ostg.clktrk _______________________________________________ Linux-f2fs-devel mailing list Linux-f2fs-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel ^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH V2] f2fs: optimize fs_lock for better performance 2013-09-12 2:40 ` 俞超 @ 2013-09-12 3:17 ` Gu Zheng 2013-09-12 3:18 ` [PATCH] " Gu Zheng 1 sibling, 0 replies; 13+ messages in thread From: Gu Zheng @ 2013-09-12 3:17 UTC (permalink / raw) To: jaegeuk.kim; +Cc: linux-fsdevel, shu.tan, linux-kernel, linux-f2fs-devel From: Yu Chao <chao2.yu@samsung.com> There is a performance problem: when all sbi->fs_lock are holded, then all the following threads may get the same next_lock value from sbi->next_lock_num in function mutex_lock_op, and wait for the same lock(fs_lock[next_lock]), it may cause performance reduce. So we move the sbi->next_lock_num++ before getting lock, this will average the following threads if all sbi->fs_lock are holded. v1-->v2: Drop the needless spin_lock as Jaegeuk suggested. Suggested-by: Jaegeuk Kim <jaegeuk.kim@samsung.com> Signed-off-by: Yu Chao <chao2.yu@samsung.com> Signed-off-by: Gu Zheng <guz.fnst@cn.fujitsu.com> --- fs/f2fs/f2fs.h | 4 ++-- 1 files changed, 2 insertions(+), 2 deletions(-) diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h index 608f0df..7fd99d8 100644 --- a/fs/f2fs/f2fs.h +++ b/fs/f2fs/f2fs.h @@ -544,15 +544,15 @@ 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; + next_lock = sbi->next_lock_num++ % NR_GLOBAL_LOCKS; mutex_lock(&sbi->fs_lock[next_lock]); - sbi->next_lock_num++; return next_lock; } -- 1.7.7 ------------------------------------------------------------------------------ How ServiceNow helps IT people transform IT departments: 1. Consolidate legacy IT systems to a single system of record for IT 2. Standardize and globalize service processes across IT 3. Implement zero-touch automation to replace manual, redundant tasks http://pubads.g.doubleclick.net/gampad/clk?id=51271111&iu=/4140/ostg.clktrk ^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH] f2fs: optimize fs_lock for better performance 2013-09-12 2:40 ` 俞超 2013-09-12 3:17 ` [PATCH V2] " Gu Zheng @ 2013-09-12 3:18 ` Gu Zheng 1 sibling, 0 replies; 13+ messages in thread From: Gu Zheng @ 2013-09-12 3:18 UTC (permalink / raw) To: 俞超; +Cc: linux-fsdevel, shu.tan, linux-kernel, linux-f2fs-devel Hi Chao, On 09/12/2013 10:40 AM, 俞超 wrote: > Hi Gu > >> -----Original Message----- >> From: Gu Zheng [mailto:guz.fnst@cn.fujitsu.com] >> Sent: Wednesday, September 11, 2013 1:38 PM >> To: jaegeuk.kim@samsung.com >> Cc: chao2.yu@samsung.com; shu.tan@samsung.com; >> linux-fsdevel@vger.kernel.org; linux-kernel@vger.kernel.org; >> linux-f2fs-devel@lists.sourceforge.net >> Subject: Re: [f2fs-dev][PATCH] f2fs: optimize fs_lock for better performance >> >> Hi Jaegeuk, Chao, >> >> On 09/10/2013 08:52 AM, Jaegeuk Kim wrote: >> >>> Hi, >>> >>> At first, thank you for the report and please follow the email writing >>> rules. :) >>> >>> Anyway, I agree to the below issue. >>> One thing that I can think of is that we don't need to use the >>> spin_lock, since we don't care about the exact lock number, but just >>> need to get any not-collided number. >> >> IMHO, just moving sbi->next_lock_num++ before >> mutex_lock(&sbi->fs_lock[next_lock]) >> can avoid unbalance issue mostly. >> IMO, the case two or more threads increase sbi->next_lock_num in the same >> time is really very very little. If you think it is not rigorous, change >> next_lock_num to atomic one can fix it. >> What's your opinion? >> >> Regards, >> Gu > > I did the test sbi->next_lock_num++ compare with the atomic one, > And I found performance of them is almost the same under a small number thread racing. > So as your and Kim's opinion, it's enough to use "sbi->next_lock_num++" to fix this issue. Good, but it seems that your replay patch is out of format, and it's hard for Jaegeuk to merge. I'll format it, see the following thread. Thanks, Gu > > Thanks for the advice. >> >>> >>> So, how about removing the spin_lock? >>> And how about using a random number? >> >>> Thanks, >>> >>> 2013-09-06 (금), 09:48 +0000, Chao Yu: >>>> 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 <chao2.yu@samsung.com> >>>> >>>> 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) >>>> >>>> >>>> >>>> >>>> >>>> >>> >> >> >> = > > ------------------------------------------------------------------------------ How ServiceNow helps IT people transform IT departments: 1. Consolidate legacy IT systems to a single system of record for IT 2. Standardize and globalize service processes across IT 3. Implement zero-touch automation to replace manual, redundant tasks http://pubads.g.doubleclick.net/gampad/clk?id=51271111&iu=/4140/ostg.clktrk _______________________________________________ Linux-f2fs-devel mailing list Linux-f2fs-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel ^ permalink raw reply [flat|nested] 13+ messages in thread
[parent not found: <04.C0.13361.61DDA225@epcpsbge5.samsung.com>]
* Re: Re: [f2fs-dev] [PATCH] f2fs: optimize fs_lock for better performance [not found] <04.C0.13361.61DDA225@epcpsbge5.samsung.com> @ 2013-09-10 0:59 ` Jaegeuk Kim 2013-09-10 4:17 ` Russ Knize 2013-09-11 3:22 ` Gu Zheng 0 siblings, 2 replies; 13+ messages in thread From: Jaegeuk Kim @ 2013-09-10 0:59 UTC (permalink / raw) To: chao2.yu Cc: Russ Knize, linux-fsdevel@vger.kernel.org, 谭姝, linux-kernel@vger.kernel.org, linux-f2fs-devel@lists.sourceforge.net Hi, 2013-09-07 (토), 08:00 +0000, Chao Yu: > Hi Knize, > > Thanks for your reply, I think it's actually meaningless that it's > being named after "spin_lock", > it's better to rename this spinlock to "round_robin_lock". > > This patch can only resolve the issue of unbalanced fs_lock usage, > it can not fix the deadlock issue. > can we fix deadlock issue through this method: > > - vfs_create() > - f2fs_create() - takes an fs_lock and save current thread info into > thread_info[NR_GLOBAL_LOCKS] > - f2fs_add_link() > - __f2fs_add_link() > - init_inode_metadata() > - f2fs_init_security() > - security_inode_init_security() > - f2fs_initxattrs() > - f2fs_setxattr() - get fs_lock only if there is no current > thread info in thread_info > > So it keeps one thread can only hold one fs_lock to avoid deadlock. > Can we use this solution? It could be. But, I think we can avoid to grab the fs_lock at the f2fs_initxattrs() level, since this case only happens when f2fs_initxattrs() is called. Let's think about ut in more detail. Thanks, > > > > thanks again! > > > > ------- Original Message ------- > > Sender : Russ Knize<Russ.Knize@motorola.com> > > Date : 九月 07, 2013 04:25 (GMT+09:00) > > Title : Re: [f2fs-dev] [PATCH] f2fs: optimize fs_lock for better > performance > > > > 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 <chao2.yu@samsung.com> 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 <chao2.yu@samsung.com> > > 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 > Linux-f2fs-devel@lists.sourceforge.net > https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel > > > > > > > > > > > > -- Jaegeuk Kim Samsung ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] f2fs: optimize fs_lock for better performance 2013-09-10 0:59 ` Re: [f2fs-dev] " Jaegeuk Kim @ 2013-09-10 4:17 ` Russ Knize 2013-09-11 3:22 ` Gu Zheng 1 sibling, 0 replies; 13+ messages in thread From: Russ Knize @ 2013-09-10 4:17 UTC (permalink / raw) To: jaegeuk.kim Cc: linux-fsdevel@vger.kernel.org, 谭姝, linux-kernel@vger.kernel.org, linux-f2fs-devel@lists.sourceforge.net [-- Attachment #1.1: Type: text/plain, Size: 7517 bytes --] I wasn't sure if f2fs_setxattr() needed the lock or not, since the call path from f2fs_xattr_set_acl() doesn't take one anywhere. My first thought was to move the lock over to the acl code and remove it from here. Russ On Mon, Sep 9, 2013 at 7:59 PM, Jaegeuk Kim <jaegeuk.kim@samsung.com> wrote: > Hi, > > 2013-09-07 (토), 08:00 +0000, Chao Yu: > > Hi Knize, > > > > Thanks for your reply, I think it's actually meaningless that it's > > being named after "spin_lock", > > it's better to rename this spinlock to "round_robin_lock". > > > > This patch can only resolve the issue of unbalanced fs_lock usage, > > it can not fix the deadlock issue. > > can we fix deadlock issue through this method: > > > > - vfs_create() > > - f2fs_create() - takes an fs_lock and save current thread info into > > thread_info[NR_GLOBAL_LOCKS] > > - f2fs_add_link() > > - __f2fs_add_link() > > - init_inode_metadata() > > - f2fs_init_security() > > - security_inode_init_security() > > - f2fs_initxattrs() > > - f2fs_setxattr() - get fs_lock only if there is no current > > thread info in thread_info > > > > So it keeps one thread can only hold one fs_lock to avoid deadlock. > > Can we use this solution? > > It could be. > But, I think we can avoid to grab the fs_lock at the f2fs_initxattrs() > level, since this case only happens when f2fs_initxattrs() is called. > Let's think about ut in more detail. > Thanks, > > > > > > > > > thanks again! > > > > > > > > ------- Original Message ------- > > > > Sender : Russ Knize<Russ.Knize@motorola.com> > > > > Date : 九月 07, 2013 04:25 (GMT+09:00) > > > > Title : Re: [f2fs-dev] [PATCH] f2fs: optimize fs_lock for better > > performance > > > > > > > > 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 <chao2.yu@samsung.com> 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 <chao2.yu@samsung.com> > > > > 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 > > Linux-f2fs-devel@lists.sourceforge.net > > https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel > > > > > > > > > > > > > > > > > > > > > > > > > > -- > Jaegeuk Kim > Samsung > > [-- Attachment #1.2: Type: text/html, Size: 10417 bytes --] [-- Attachment #2: Type: text/plain, Size: 407 bytes --] ------------------------------------------------------------------------------ How ServiceNow helps IT people transform IT departments: 1. Consolidate legacy IT systems to a single system of record for IT 2. Standardize and globalize service processes across IT 3. Implement zero-touch automation to replace manual, redundant tasks http://pubads.g.doubleclick.net/gampad/clk?id=51271111&iu=/4140/ostg.clktrk [-- Attachment #3: Type: text/plain, Size: 179 bytes --] _______________________________________________ Linux-f2fs-devel mailing list Linux-f2fs-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] f2fs: optimize fs_lock for better performance 2013-09-10 0:59 ` Re: [f2fs-dev] " Jaegeuk Kim 2013-09-10 4:17 ` Russ Knize @ 2013-09-11 3:22 ` Gu Zheng 2013-09-11 3:47 ` Russ Knize 1 sibling, 1 reply; 13+ messages in thread From: Gu Zheng @ 2013-09-11 3:22 UTC (permalink / raw) To: jaegeuk.kim Cc: 谭姝, Russ Knize, linux-kernel@vger.kernel.org, linux-f2fs-devel@lists.sourceforge.net, linux-fsdevel@vger.kernel.org Hi Jaegeuk, On 09/10/2013 08:59 AM, Jaegeuk Kim wrote: > Hi, > > 2013-09-07 (토), 08:00 +0000, Chao Yu: >> Hi Knize, >> >> Thanks for your reply, I think it's actually meaningless that it's >> being named after "spin_lock", >> it's better to rename this spinlock to "round_robin_lock". >> >> This patch can only resolve the issue of unbalanced fs_lock usage, >> it can not fix the deadlock issue. >> can we fix deadlock issue through this method: >> >> - vfs_create() >> - f2fs_create() - takes an fs_lock and save current thread info into >> thread_info[NR_GLOBAL_LOCKS] >> - f2fs_add_link() >> - __f2fs_add_link() >> - init_inode_metadata() >> - f2fs_init_security() >> - security_inode_init_security() >> - f2fs_initxattrs() >> - f2fs_setxattr() - get fs_lock only if there is no current >> thread info in thread_info >> >> So it keeps one thread can only hold one fs_lock to avoid deadlock. >> Can we use this solution? > > It could be. > But, I think we can avoid to grab the fs_lock at the f2fs_initxattrs() Agree. This fs_lock here is used to protect the xattr from parallel modification, but here is in the initxattrs routine, parallel modification can not happen. And in the normal setxattr routine the inode->i_mutex (vfs layer) is used to avoid parallel modification. So I think this fs_lock is needless. Am I missing something? Regards, Gu > level, since this case only happens when f2fs_initxattrs() is called. > Let's think about ut in more detail. > Thanks, > >> >> >> >> thanks again! >> >> >> >> ------- Original Message ------- >> >> Sender : Russ Knize<Russ.Knize@motorola.com> >> >> Date : 九月 07, 2013 04:25 (GMT+09:00) >> >> Title : Re: [f2fs-dev] [PATCH] f2fs: optimize fs_lock for better >> performance >> >> >> >> 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 <chao2.yu@samsung.com> 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 <chao2.yu@samsung.com> >> >> 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 >> Linux-f2fs-devel@lists.sourceforge.net >> https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel >> >> >> >> >> >> >> >> >> >> >> >> > ------------------------------------------------------------------------------ How ServiceNow helps IT people transform IT departments: 1. Consolidate legacy IT systems to a single system of record for IT 2. Standardize and globalize service processes across IT 3. Implement zero-touch automation to replace manual, redundant tasks http://pubads.g.doubleclick.net/gampad/clk?id=51271111&iu=/4140/ostg.clktrk _______________________________________________ Linux-f2fs-devel mailing list Linux-f2fs-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] f2fs: optimize fs_lock for better performance 2013-09-11 3:22 ` Gu Zheng @ 2013-09-11 3:47 ` Russ Knize 0 siblings, 0 replies; 13+ messages in thread From: Russ Knize @ 2013-09-11 3:47 UTC (permalink / raw) To: Gu Zheng Cc: 谭姝, linux-kernel@vger.kernel.org, linux-f2fs-devel@lists.sourceforge.net, linux-fsdevel@vger.kernel.org [-- Attachment #1.1: Type: text/plain, Size: 8318 bytes --] Hi Jaegeuk/Gu, I've removed the lock and have been stress-testing with SELinux and some additional xattr torture for 24+ hours. I have not encountered any issues yet. My previous suggestion about moving the lock is probably not a good idea without some significant code rework (thanks to the f2fs_balance_fs call in f2fs_setxattr). Russ On Tue, Sep 10, 2013 at 10:22 PM, Gu Zheng <guz.fnst@cn.fujitsu.com> wrote: > Hi Jaegeuk, > On 09/10/2013 08:59 AM, Jaegeuk Kim wrote: > > > Hi, > > > > 2013-09-07 (토), 08:00 +0000, Chao Yu: > >> Hi Knize, > >> > >> Thanks for your reply, I think it's actually meaningless that it's > >> being named after "spin_lock", > >> it's better to rename this spinlock to "round_robin_lock". > >> > >> This patch can only resolve the issue of unbalanced fs_lock usage, > >> it can not fix the deadlock issue. > >> can we fix deadlock issue through this method: > >> > >> - vfs_create() > >> - f2fs_create() - takes an fs_lock and save current thread info into > >> thread_info[NR_GLOBAL_LOCKS] > >> - f2fs_add_link() > >> - __f2fs_add_link() > >> - init_inode_metadata() > >> - f2fs_init_security() > >> - security_inode_init_security() > >> - f2fs_initxattrs() > >> - f2fs_setxattr() - get fs_lock only if there is no current > >> thread info in thread_info > >> > >> So it keeps one thread can only hold one fs_lock to avoid deadlock. > >> Can we use this solution? > > > > It could be. > > But, I think we can avoid to grab the fs_lock at the f2fs_initxattrs() > > Agree. This fs_lock here is used to protect the xattr from parallel > modification, > but here is in the initxattrs routine, parallel modification can not > happen. > And in the normal setxattr routine the inode->i_mutex (vfs layer) is used > to > avoid parallel modification. So I think this fs_lock is needless. > Am I missing something? > > Regards, > Gu > > > level, since this case only happens when f2fs_initxattrs() is called. > > Let's think about ut in more detail. > > Thanks, > > > >> > >> > >> > >> thanks again! > >> > >> > >> > >> ------- Original Message ------- > >> > >> Sender : Russ Knize<Russ.Knize@motorola.com> > >> > >> Date : 九月 07, 2013 04:25 (GMT+09:00) > >> > >> Title : Re: [f2fs-dev] [PATCH] f2fs: optimize fs_lock for better > >> performance > >> > >> > >> > >> 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 <chao2.yu@samsung.com> 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 <chao2.yu@samsung.com> > >> > >> 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 > >> Linux-f2fs-devel@lists.sourceforge.net > >> https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel > >> > >> > >> > >> > >> > >> > >> > >> > >> > >> > >> > >> > > > > > [-- Attachment #1.2: Type: text/html, Size: 11941 bytes --] [-- Attachment #2: Type: text/plain, Size: 407 bytes --] ------------------------------------------------------------------------------ How ServiceNow helps IT people transform IT departments: 1. Consolidate legacy IT systems to a single system of record for IT 2. Standardize and globalize service processes across IT 3. Implement zero-touch automation to replace manual, redundant tasks http://pubads.g.doubleclick.net/gampad/clk?id=51271111&iu=/4140/ostg.clktrk [-- Attachment #3: Type: text/plain, Size: 179 bytes --] _______________________________________________ Linux-f2fs-devel mailing list Linux-f2fs-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] f2fs: optimize fs_lock for better performance @ 2013-09-07 8:00 Chao Yu 0 siblings, 0 replies; 13+ messages in thread From: Chao Yu @ 2013-09-07 8:00 UTC (permalink / raw) To: Russ Knize Cc: linux-fsdevel@vger.kernel.org, 谭姝, linux-kernel@vger.kernel.org, linux-f2fs-devel@lists.sourceforge.net [-- Attachment #1.1: Type: text/html, Size: 12962 bytes --] [-- Attachment #1.2: 201309071600221_BEI0XT4N.gif --] [-- Type: image/gif, Size: 14036 bytes --] [-- Attachment #1.3: 201309071600271_2LL5XOK0.gif --] [-- Type: image/gif, Size: 14036 bytes --] [-- Attachment #2: Type: text/plain, Size: 433 bytes --] ------------------------------------------------------------------------------ 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 [-- Attachment #3: Type: text/plain, Size: 179 bytes --] _______________________________________________ Linux-f2fs-devel mailing list Linux-f2fs-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel ^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH] f2fs: optimize fs_lock for better performance @ 2013-09-06 9:48 Chao Yu 2013-09-06 19:25 ` Russ Knize 0 siblings, 1 reply; 13+ messages in thread From: Chao Yu @ 2013-09-06 9:48 UTC (permalink / raw) To: jaegeuk.kim; +Cc: linux-fsdevel, shu.tan, linux-kernel, linux-f2fs-devel [-- Attachment #1.1: Type: text/html, Size: 17017 bytes --] [-- Attachment #1.2: 201309061748313_TIC2ESYT.gif --] [-- Type: image/gif, Size: 14036 bytes --] [-- Attachment #2: Type: text/plain, Size: 433 bytes --] ------------------------------------------------------------------------------ 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 [-- Attachment #3: Type: text/plain, Size: 179 bytes --] _______________________________________________ Linux-f2fs-devel mailing list Linux-f2fs-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] f2fs: optimize fs_lock for better performance 2013-09-06 9:48 Chao Yu @ 2013-09-06 19:25 ` Russ Knize 0 siblings, 0 replies; 13+ messages in thread From: Russ Knize @ 2013-09-06 19:25 UTC (permalink / raw) To: chao2.yu; +Cc: linux-fsdevel, shu.tan, linux-kernel, linux-f2fs-devel [-- Attachment #1.1.1: Type: text/plain, Size: 4612 bytes --] 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 <chao2.yu@samsung.com> 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 <chao2.yu@samsung.com>**** > > 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 > Linux-f2fs-devel@lists.sourceforge.net > https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel > > [-- Attachment #1.1.2: Type: text/html, Size: 8803 bytes --] [-- Attachment #1.2: 201309061748313_TIC2ESYT.gif --] [-- Type: image/gif, Size: 14036 bytes --] [-- Attachment #2: Type: text/plain, Size: 433 bytes --] ------------------------------------------------------------------------------ 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 [-- Attachment #3: Type: text/plain, Size: 179 bytes --] _______________________________________________ Linux-f2fs-devel mailing list Linux-f2fs-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel ^ permalink raw reply [flat|nested] 13+ messages in thread
![[-- Attachment #1.2: 201309061748313_TIC2ESYT.gif --] [-- Type: image/gif, Size: 14036 bytes --]](../../CAGfvh62EGuQfGBRhasNEXAVEvS2BwKCijnAS67f+GL2fHS-fTA@mail.gmail.com/1.2-201309061748313_TIC2ESYT.gif){kind=link}
end of thread, other threads:[~2013-09-12 3:18 UTC | newest]
Thread overview: 13+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <88.C4.11914.9D4A9225@epcpsbge6.samsung.com>
2013-09-10 0:52 ` [f2fs-dev][PATCH] f2fs: optimize fs_lock for better performance Jaegeuk Kim
2013-09-11 3:13 ` [PATCH] " Gu Zheng
2013-09-11 5:37 ` Gu Zheng
2013-09-11 13:22 ` [f2fs-dev] " Kim Jaegeuk
2013-09-12 2:40 ` 俞超
2013-09-12 3:17 ` [PATCH V2] " Gu Zheng
2013-09-12 3:18 ` [PATCH] " Gu Zheng
[not found] <04.C0.13361.61DDA225@epcpsbge5.samsung.com>
2013-09-10 0:59 ` Re: [f2fs-dev] " Jaegeuk Kim
2013-09-10 4:17 ` Russ Knize
2013-09-11 3:22 ` Gu Zheng
2013-09-11 3:47 ` Russ Knize
2013-09-07 8:00 Chao Yu
-- strict thread matches above, loose matches on Subject: below --
2013-09-06 9:48 Chao Yu
2013-09-06 19:25 ` Russ Knize
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).