From mboxrd@z Thu Jan 1 00:00:00 1970 From: Gu Zheng Subject: Re: [PATCH] f2fs: optimize fs_lock for better performance Date: Wed, 11 Sep 2013 11:13:07 +0800 Message-ID: <522FDFC3.3010200@cn.fujitsu.com> References: <88.C4.11914.9D4A9225@epcpsbge6.samsung.com> <1378774324.2354.103.camel@kjgkr> Mime-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: base64 Return-path: Received: from sog-mx-4.v43.ch3.sourceforge.com ([172.29.43.194] helo=mx.sourceforge.net) by sfs-ml-2.v29.ch3.sourceforge.com with esmtp (Exim 4.76) (envelope-from ) id 1VJawV-0000DN-N6 for linux-f2fs-devel@lists.sourceforge.net; Wed, 11 Sep 2013 03:18:11 +0000 Received: from [222.73.24.84] (helo=song.cn.fujitsu.com) by sog-mx-4.v43.ch3.sourceforge.com with esmtp (Exim 4.76) id 1VJawT-0002KB-Hk for linux-f2fs-devel@lists.sourceforge.net; Wed, 11 Sep 2013 03:18:11 +0000 In-Reply-To: <1378774324.2354.103.camel@kjgkr> List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: linux-f2fs-devel-bounces@lists.sourceforge.net To: jaegeuk.kim@samsung.com Cc: linux-fsdevel@vger.kernel.org, shu.tan@samsung.com, linux-kernel@vger.kernel.org, linux-f2fs-devel@lists.sourceforge.net SGkgSmFlZ2V1aywKCk9uIDA5LzEwLzIwMTMgMDg6NTIgQU0sIEphZWdldWsgS2ltIHdyb3RlOgoK PiBIaSwKPiAKPiBBdCBmaXJzdCwgdGhhbmsgeW91IGZvciB0aGUgcmVwb3J0IGFuZCBwbGVhc2Ug Zm9sbG93IHRoZSBlbWFpbCB3cml0aW5nCj4gcnVsZXMuIDopCj4gCj4gQW55d2F5LCBJIGFncmVl IHRvIHRoZSBiZWxvdyBpc3N1ZS4KPiBPbmUgdGhpbmcgdGhhdCBJIGNhbiB0aGluayBvZiBpcyB0 aGF0IHdlIGRvbid0IG5lZWQgdG8gdXNlIHRoZQo+IHNwaW5fbG9jaywgc2luY2Ugd2UgZG9uJ3Qg Y2FyZSBhYm91dCB0aGUgZXhhY3QgbG9jayBudW1iZXIsIGJ1dCBqdXN0Cj4gbmVlZCB0byBnZXQg YW55IG5vdC1jb2xsaWRlZCBudW1iZXIuCgpBZ3JlZSwgYnV0IGlmIGFsbCB0aGUgbG9ja3MgYXJl IGhlbGQsIElNTywgd2UgbmVlZCB0byBiYWxhbmNlIHRoZSBmb2xsb3dpbmcKdGhyZWFkcyB0byB3 YWl0IGZvciBlYWNoIG5vdC1jb2xsaWRlZCBudW1iZXIgbG9jaywgdGhvdWdoIGNvbXBsZXRlIGJh bGFuY2UgaXMgdW5yZWFjaGFibGUuCgo+IAo+IFNvLCBob3cgYWJvdXQgcmVtb3ZpbmcgdGhlIHNw aW5fbG9jaz8KClllYWgsIGluIHRoaXMgY2FzZSwgc3Bpbl9sb2NrIGlzIGEgYml0IGhlYXZ5IGNv c3QuIAoKPiBBbmQgaG93IGFib3V0IHVzaW5nIGEgcmFuZG9tIG51bWJlcj8KCk5vdyBOUl9HTE9C QUxfTE9DS1MgaXMgOCwgaXQgc2VlbXMgdGhhdCByYW5kb20gY2FuIG5vdCBvZmZlciBhbiBiYWxh bmNlIG51bWJlciBhcyB3ZSBleHBlY3RlZC4KClJlZ2FyZHMsCkd1IAoKPiBUaGFua3MsCj4gCj4g MjAxMy0wOS0wNiAo6riIKSwgMDk6NDggKzAwMDAsIENoYW8gWXU6Cj4+IEhpIEtpbToKPj4KPj4g ICAgICBJIHRoaW5rIHRoZXJlIGlzIGEgcGVyZm9ybWFuY2UgcHJvYmxlbTogd2hlbiBhbGwgc2Jp LT5mc19sb2NrIGlzCj4+IGhvbGRlZCwgCj4+Cj4+IHRoZW4gYWxsIG90aGVyIHRocmVhZHMgbWF5 IGdldCB0aGUgc2FtZSBuZXh0X2xvY2sgdmFsdWUgZnJvbQo+PiBzYmktPm5leHRfbG9ja19udW0g aW4gZnVuY3Rpb24gbXV0ZXhfbG9ja19vcCwgCj4+Cj4+IGFuZCB3YWl0IHRvIGdldCB0aGUgc2Ft ZSBsb2NrIGF0IHBvc2l0aW9uIGZzX2xvY2tbbmV4dF9sb2NrXSwgaXQKPj4gdW5iYWxhbmNlIHRo ZSBmc19sb2NrIHVzYWdlLiAKPj4KPj4gSXQgbWF5IGxvc3QgcGVyZm9ybWFuY2Ugd2hlbiB3ZSBk byB0aGUgbXVsdGl0aHJlYWQgdGVzdC4KPj4KPj4gIAo+Pgo+PiBIZXJlIGlzIHRoZSBwYXRjaCB0 byBmaXggdGhpcyBwcm9ibGVtOgo+Pgo+PiAgCj4+Cj4+IFNpZ25lZC1vZmYtYnk6IFl1IENoYW8g PGNoYW8yLnl1QHNhbXN1bmcuY29tPgo+Pgo+PiBkaWZmIC0tZ2l0IGEvZnMvZjJmcy9mMmZzLmgg Yi9mcy9mMmZzL2YyZnMuaAo+Pgo+PiBvbGQgbW9kZSAxMDA2NDQKPj4KPj4gbmV3IG1vZGUgMTAw NzU1Cj4+Cj4+IGluZGV4IDQ2N2Q0MmQuLjk4M2JiNDUKPj4KPj4gLS0tIGEvZnMvZjJmcy9mMmZz LmgKPj4KPj4gKysrIGIvZnMvZjJmcy9mMmZzLmgKPj4KPj4gQEAgLTM3MSw2ICszNzEsNyBAQCBz dHJ1Y3QgZjJmc19zYl9pbmZvIHsKPj4KPj4gICAgICAgICBzdHJ1Y3QgbXV0ZXggZnNfbG9ja1tO Ul9HTE9CQUxfTE9DS1NdOyAgLyogYmxvY2tpbmcgRlMKPj4gb3BlcmF0aW9ucyAqLwo+Pgo+PiAg ICAgICAgIHN0cnVjdCBtdXRleCBub2RlX3dyaXRlOyAgICAgICAgICAgICAgICAvKiBsb2NraW5n IG5vZGUgd3JpdGVzCj4+ICovCj4+Cj4+ICAgICAgICAgc3RydWN0IG11dGV4IHdyaXRlcGFnZXM7 ICAgICAgICAgICAgICAgIC8qIG11dGV4IGZvcgo+PiB3cml0ZXBhZ2VzKCkgKi8KPj4KPj4gKyAg ICAgICBzcGlubG9ja190IHNwaW5fbG9jazsgICAgICAgICAgICAgICAgICAgLyogbG9jayBmb3IK Pj4gbmV4dF9sb2NrX251bSAqLwo+Pgo+PiAgICAgICAgIHVuc2lnbmVkIGNoYXIgbmV4dF9sb2Nr X251bTsgICAgICAgICAgICAvKiByb3VuZC1yb2JpbiBnbG9iYWwKPj4gbG9ja3MgKi8KPj4KPj4g ICAgICAgICBpbnQgcG9yX2RvaW5nOyAgICAgICAgICAgICAgICAgICAgICAgICAgLyogcmVjb3Zl cnkgaXMgZG9pbmcKPj4gb3Igbm90ICovCj4+Cj4+ICAgICAgICAgaW50IG9uX2J1aWxkX2ZyZWVf bmlkczsgICAgICAgICAgICAgICAgIC8qIGJ1aWxkX2ZyZWVfbmlkcyBpcwo+PiBkb2luZyAqLwo+ Pgo+PiBAQCAtNTMzLDE1ICs1MzQsMTkgQEAgc3RhdGljIGlubGluZSB2b2lkIG11dGV4X3VubG9j a19hbGwoc3RydWN0Cj4+IGYyZnNfc2JfaW5mbyAqc2JpKQo+Pgo+PiAgCj4+Cj4+ICBzdGF0aWMg aW5saW5lIGludCBtdXRleF9sb2NrX29wKHN0cnVjdCBmMmZzX3NiX2luZm8gKnNiaSkKPj4KPj4g IHsKPj4KPj4gLSAgICAgICB1bnNpZ25lZCBjaGFyIG5leHRfbG9jayA9IHNiaS0+bmV4dF9sb2Nr X251bSAlCj4+IE5SX0dMT0JBTF9MT0NLUzsKPj4KPj4gKyAgICAgICB1bnNpZ25lZCBjaGFyIG5l eHRfbG9jazsKPj4KPj4gICAgICAgICBpbnQgaSA9IDA7Cj4+Cj4+ICAKPj4KPj4gICAgICAgICBm b3IgKDsgaSA8IE5SX0dMT0JBTF9MT0NLUzsgaSsrKQo+Pgo+PiAgICAgICAgICAgICAgICAgaWYg KG11dGV4X3RyeWxvY2soJnNiaS0+ZnNfbG9ja1tpXSkpCj4+Cj4+ICAgICAgICAgICAgICAgICAg ICAgICAgIHJldHVybiBpOwo+Pgo+PiAgCj4+Cj4+IC0gICAgICAgbXV0ZXhfbG9jaygmc2JpLT5m c19sb2NrW25leHRfbG9ja10pOwo+Pgo+PiArICAgICAgIHNwaW5fbG9jaygmc2JpLT5zcGluX2xv Y2spOwo+Pgo+PiArICAgICAgIG5leHRfbG9jayA9IHNiaS0+bmV4dF9sb2NrX251bSAlIE5SX0dM T0JBTF9MT0NLUzsKPj4KPj4gICAgICAgICBzYmktPm5leHRfbG9ja19udW0rKzsKPj4KPj4gKyAg ICAgICBzcGluX3VubG9jaygmc2JpLT5zcGluX2xvY2spOwo+Pgo+PiArCj4+Cj4+ICsgICAgICAg bXV0ZXhfbG9jaygmc2JpLT5mc19sb2NrW25leHRfbG9ja10pOwo+Pgo+PiAgICAgICAgIHJldHVy biBuZXh0X2xvY2s7Cj4+Cj4+ICB9Cj4+Cj4+ICAKPj4KPj4gZGlmZiAtLWdpdCBhL2ZzL2YyZnMv c3VwZXIuYyBiL2ZzL2YyZnMvc3VwZXIuYwo+Pgo+PiBvbGQgbW9kZSAxMDA2NDQKPj4KPj4gbmV3 IG1vZGUgMTAwNzU1Cj4+Cj4+IGluZGV4IDc1YzdkYzMuLjRmMjc1OTYKPj4KPj4gLS0tIGEvZnMv ZjJmcy9zdXBlci5jCj4+Cj4+ICsrKyBiL2ZzL2YyZnMvc3VwZXIuYwo+Pgo+PiBAQCAtNjU3LDYg KzY1Nyw3IEBAIHN0YXRpYyBpbnQgZjJmc19maWxsX3N1cGVyKHN0cnVjdCBzdXBlcl9ibG9jayAq c2IsCj4+IHZvaWQgKmRhdGEsIGludCBzaWxlbnQpCj4+Cj4+ICAgICAgICAgbXV0ZXhfaW5pdCgm c2JpLT5jcF9tdXRleCk7Cj4+Cj4+ICAgICAgICAgZm9yIChpID0gMDsgaSA8IE5SX0dMT0JBTF9M T0NLUzsgaSsrKQo+Pgo+PiAgICAgICAgICAgICAgICAgbXV0ZXhfaW5pdCgmc2JpLT5mc19sb2Nr W2ldKTsKPj4KPj4gKyAgICAgICBzcGluX2xvY2tfaW5pdCgmc2JpLT5zcGluX2xvY2spOwo+Pgo+ PiAgICAgICAgIG11dGV4X2luaXQoJnNiaS0+bm9kZV93cml0ZSk7Cj4+Cj4+ICAgICAgICAgc2Jp LT5wb3JfZG9pbmcgPSAwOwo+Pgo+PiAgICAgICAgIHNwaW5fbG9ja19pbml0KCZzYmktPnN0YXRf bG9jayk7Cj4+Cj4+IChFTkQpCj4+Cj4+ICAKPj4KPj4KPj4KPj4KPiAKCgoKLS0tLS0tLS0tLS0t LS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0t LS0tLS0tLS0tCkhvdyBTZXJ2aWNlTm93IGhlbHBzIElUIHBlb3BsZSB0cmFuc2Zvcm0gSVQgZGVw YXJ0bWVudHM6CjEuIENvbnNvbGlkYXRlIGxlZ2FjeSBJVCBzeXN0ZW1zIHRvIGEgc2luZ2xlIHN5 c3RlbSBvZiByZWNvcmQgZm9yIElUCjIuIFN0YW5kYXJkaXplIGFuZCBnbG9iYWxpemUgc2Vydmlj ZSBwcm9jZXNzZXMgYWNyb3NzIElUCjMuIEltcGxlbWVudCB6ZXJvLXRvdWNoIGF1dG9tYXRpb24g dG8gcmVwbGFjZSBtYW51YWwsIHJlZHVuZGFudCB0YXNrcwpodHRwOi8vcHViYWRzLmcuZG91Ymxl Y2xpY2submV0L2dhbXBhZC9jbGs/aWQ9NTEyNzExMTEmaXU9LzQxNDAvb3N0Zy5jbGt0cmsKX19f X19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX18KTGludXgtZjJmcy1k ZXZlbCBtYWlsaW5nIGxpc3QKTGludXgtZjJmcy1kZXZlbEBsaXN0cy5zb3VyY2Vmb3JnZS5uZXQK aHR0cHM6Ly9saXN0cy5zb3VyY2Vmb3JnZS5uZXQvbGlzdHMvbGlzdGluZm8vbGludXgtZjJmcy1k ZXZlbAo= From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753882Ab3IKD3W (ORCPT ); Tue, 10 Sep 2013 23:29:22 -0400 Received: from cn.fujitsu.com ([222.73.24.84]:50871 "EHLO song.cn.fujitsu.com" rhost-flags-OK-FAIL-OK-OK) by vger.kernel.org with ESMTP id S1753793Ab3IKD3S convert rfc822-to-8bit (ORCPT ); Tue, 10 Sep 2013 23:29:18 -0400 X-IronPort-AV: E=Sophos;i="4.90,881,1371052800"; d="scan'208";a="8483952" Message-ID: <522FDFC3.3010200@cn.fujitsu.com> Date: Wed, 11 Sep 2013 11:13:07 +0800 From: Gu Zheng User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:7.0.1) Gecko/20110930 Thunderbird/7.0.1 MIME-Version: 1.0 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 References: <88.C4.11914.9D4A9225@epcpsbge6.samsung.com> <1378774324.2354.103.camel@kjgkr> In-Reply-To: <1378774324.2354.103.camel@kjgkr> X-MIMETrack: Itemize by SMTP Server on mailserver/fnst(Release 8.5.3|September 15, 2011) at 2013/09/11 11:15:34, Serialize by Router on mailserver/fnst(Release 8.5.3|September 15, 2011) at 2013/09/11 11:15:41 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8BIT Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 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 >> >> 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) >> >> >> >> >> >> >