From mboxrd@z Thu Jan 1 00:00:00 1970 From: Gu Zheng Subject: Re: [PATCH 5/5] f2fs: add a wait queue to avoid unnecessary, build_free_nid Date: Mon, 10 Mar 2014 13:37:34 +0800 Message-ID: <531D4F9E.9070804@cn.fujitsu.com> References: <5319A2DB.8040705@cn.fujitsu.com> <1394427024.3870.94.camel@kjgkr> Mime-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: base64 Return-path: Received: from sog-mx-3.v43.ch3.sourceforge.com ([172.29.43.193] helo=mx.sourceforge.net) by sfs-ml-4.v29.ch3.sourceforge.com with esmtp (Exim 4.76) (envelope-from ) id 1WMt2q-0000np-SY for linux-f2fs-devel@lists.sourceforge.net; Mon, 10 Mar 2014 05:46:36 +0000 Received: from [222.73.24.84] (helo=song.cn.fujitsu.com) by sog-mx-3.v43.ch3.sourceforge.com with esmtp (Exim 4.76) id 1WMt2o-0007AH-MK for linux-f2fs-devel@lists.sourceforge.net; Mon, 10 Mar 2014 05:46:36 +0000 In-Reply-To: <1394427024.3870.94.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-kernel , f2fs SGkgS2ltLApPbiAwMy8xMC8yMDE0IDEyOjUwIFBNLCBKYWVnZXVrIEtpbSB3cm90ZToKCj4gSGkg R3UsCj4gCj4gMjAxNC0wMy0wNyAo6riIKSwgMTg6NDMgKzA4MDAsIEd1IFpoZW5nOgo+PiBQcmV2 aW91c2x5LCB3aGVuIHdlIHRyeSB0byBhbGxvYyBmcmVlIG5pZCB3aGlsZSB0aGUgYnVpbGQgZnJl ZSBuaWQKPj4gaXMgZ29pbmcsIHRoZSBhbGxvY2VyIHdpbGwgYmUgcnVuIGludG8gdGhlIGZsb3cg dGhhdCB3YWl0aW5nIGZvcgo+PiAibm1faS0+YnVpbGRfbG9jayIsIHNlZSBmb2xsb3dpbmc6Cj4+ IAkvKiBXZSBzaG91bGQgbm90IHVzZSBzdGFsZSBmcmVlIG5pZHMgY3JlYXRlZCBieSBidWlsZF9m cmVlX25pZHMgKi8KPj4gLS0tLT4JaWYgKG5tX2ktPmZjbnQgJiYgIW9uX2J1aWxkX2ZyZWVfbmlk cyhubV9pKSkgewo+PiAJCWYyZnNfYnVnX29uKGxpc3RfZW1wdHkoJm5tX2ktPmZyZWVfbmlkX2xp c3QpKTsKPj4gCQlsaXN0X2Zvcl9lYWNoKHRoaXMsICZubV9pLT5mcmVlX25pZF9saXN0KSB7Cj4+ IAkJCWkgPSBsaXN0X2VudHJ5KHRoaXMsIHN0cnVjdCBmcmVlX25pZCwgbGlzdCk7Cj4+IAkJCWlm IChpLT5zdGF0ZSA9PSBOSURfTkVXKQo+PiAJCQkJYnJlYWs7Cj4+IAkJfQo+Pgo+PiAJCWYyZnNf YnVnX29uKGktPnN0YXRlICE9IE5JRF9ORVcpOwo+PiAJCSpuaWQgPSBpLT5uaWQ7Cj4+IAkJaS0+ c3RhdGUgPSBOSURfQUxMT0M7Cj4+IAkJbm1faS0+ZmNudC0tOwo+PiAJCXNwaW5fdW5sb2NrKCZu bV9pLT5mcmVlX25pZF9saXN0X2xvY2spOwo+PiAJCXJldHVybiB0cnVlOwo+PiAJfQo+PiAJc3Bp bl91bmxvY2soJm5tX2ktPmZyZWVfbmlkX2xpc3RfbG9jayk7Cj4+Cj4+IAkvKiBMZXQncyBzY2Fu IG5hdCBwYWdlcyBhbmQgaXRzIGNhY2hlcyB0byBnZXQgZnJlZSBuaWRzICovCj4+IC0tLS0+CW11 dGV4X2xvY2soJm5tX2ktPmJ1aWxkX2xvY2spOwo+PiAJYnVpbGRfZnJlZV9uaWRzKHNiaSk7Cj4+ IAltdXRleF91bmxvY2soJm5tX2ktPmJ1aWxkX2xvY2spOwo+PiBhbmQgdGhpcyB3aWxsIGNhdXNl IGFub3RoZXIgdW5uZWNlc3NhcnkgYnVpbGRpbmcgZnJlZSBuaWQgaWYgdGhlIGN1cnJlbnQKPj4g YnVpbGRpbmcgZnJlZSBuaWQgam9iIGlzIGRvbmUuCj4gCj4gQ291bGQgeW91IHN1cHBvcnQgYW55 IHBlcmZvcm1hbmNlIG51bWJlciBmb3IgdGhpcz8KCkkganVzdCBydW4gc29tZSBjb21tb24gdGVz dCB2aWEgZmlvIHdpdGggc2ltdWxhdGVkIHNzZCh2aWEgbG9vcCkuCgo+IFNpbmNlLCBJTU8sIHRo ZSBjb250ZW5kZWQgYnVpbGRpbmcgcHJvY2Vzc2VzIHdpbGwgYmUgcmVsZWFzZWQgcmlnaHQgYXdh eQo+IGJlY2F1c2Ugb2YgdGhlIGZvbGxvd2luZyBjb25kaXRpb24gY2hlY2sgaW5zaWRlIGJ1aWxk X2ZyZWVfbmlkcygpLgo+IAo+IGlmIChubV9pLT5mY250ID4gTkFUX0VOVFJZX1BFUl9CTE9DSykK PiAJcmV0dXJuOwoKSXQgZG9lcy4gQnV0LCBJTU8sIHdlIGNhbiBub3QgcHJvbWlzZSBubV9pLT5m Y250ID4gTkFUX0VOVFJZX1BFUl9CTE9DSyB3aGVuIHRoZQpjb250ZW5kZWQgYnVpbGRpbmcgcHJv Y2VzcyBlbnRlcmluZywgZXNwZWNpYWxseSBpbiBoaWdoIGNvbmN1cnJlbmN5IGNvbmRpdGlvbi4K Cj4gCj4gU28sIEkgZG9uJ3QgdGhpbmsgdGhpcyBnaXZlcyB1cyBhbnkgaGlnaCBsYXRlbmN5Lgo+ IENhbiB0aGUgd2FrZXVwX2FsbCgpIGJlY29tZSBhbm90aGVyIG92ZXJoZWFkIGFsbCB0aGUgdGlt ZT8KClllYWgsIG1heWJlIHdlIG11c3QgdGVzdCB3aGV0aGVyIGl0IGNhbiBhbHNvIGNhdXNlIHRo ZSBwZXJmb3JtYW5jZSByZWdyZXNzaW9uLApiZWNhdXNlIHRoZSB3YWtldXBfYWxsIGFsc28gaW50 cm9kdWNlIG92ZXJoYW5kIGFzIHlvdSBzYWlkLgpCdXQgd2hhdCBpcyBiYWQgaXMgdGhhdCBJIGRv IG5vdCBoYXZlIGEgcHJvZHVjdGlvbiBlbnZpcm9ubWVudCB0byB0ZXN0IGl0LCBhcyB5b3UKa25v dyB0aGUgc2ltdWxhdGVkIGVudmlyb25tZW50IGlzIG5vdCBzdHJpY3QuCgpjYyBZdSwKQ291bGQg eW91IHBsZWFzZSBoZWxwIHRvIHRlc3QgaXQ/CgpSZWdhcmRzLApHdQoKPiBUaGFua3MsCj4gCj4+ IFNvIGhlcmUgd2UgaW50cm9kdWNlIGEgd2FpdF9xdWV1ZSB0byBhdm9pZCB0aGlzIGlzc3VlLgo+ Pgo+PiBTaWduZWQtb2ZmLWJ5OiBHdSBaaGVuZyA8Z3V6LmZuc3RAY24uZnVqaXRzdS5jb20+Cj4+ IC0tLQo+PiAgZnMvZjJmcy9mMmZzLmggfCAgICAxICsKPj4gIGZzL2YyZnMvbm9kZS5jIHwgICAx MCArKysrKysrKystCj4+ICAyIGZpbGVzIGNoYW5nZWQsIDEwIGluc2VydGlvbnMoKyksIDEgZGVs ZXRpb25zKC0pCj4+Cj4+IGRpZmYgLS1naXQgYS9mcy9mMmZzL2YyZnMuaCBiL2ZzL2YyZnMvZjJm cy5oCj4+IGluZGV4IGY4NDVlOTIuLjdhZTE5M2UgMTAwNjQ0Cj4+IC0tLSBhL2ZzL2YyZnMvZjJm cy5oCj4+ICsrKyBiL2ZzL2YyZnMvZjJmcy5oCj4+IEBAIC0yNTYsNiArMjU2LDcgQEAgc3RydWN0 IGYyZnNfbm1faW5mbyB7Cj4+ICAJc3BpbmxvY2tfdCBmcmVlX25pZF9saXN0X2xvY2s7CS8qIHBy b3RlY3QgZnJlZSBuaWQgbGlzdCAqLwo+PiAgCXVuc2lnbmVkIGludCBmY250OwkJLyogdGhlIG51 bWJlciBvZiBmcmVlIG5vZGUgaWQgKi8KPj4gIAlzdHJ1Y3QgbXV0ZXggYnVpbGRfbG9jazsJLyog bG9jayBmb3IgYnVpbGQgZnJlZSBuaWRzICovCj4+ICsJd2FpdF9xdWV1ZV9oZWFkX3QgYnVpbGRf d3E7CS8qIHdhaXQgcXVldWUgZm9yIGJ1aWxkIGZyZWUgbmlkcyAqLwo+PiAgCj4+ICAJLyogZm9y IGNoZWNrcG9pbnQgKi8KPj4gIAljaGFyICpuYXRfYml0bWFwOwkJLyogTkFUIGJpdG1hcCBwb2lu dGVyICovCj4+IGRpZmYgLS1naXQgYS9mcy9mMmZzL25vZGUuYyBiL2ZzL2YyZnMvbm9kZS5jCj4+ IGluZGV4IDRiNzg2MWQuLmFiNDQ3MTEgMTAwNjQ0Cj4+IC0tLSBhL2ZzL2YyZnMvbm9kZS5jCj4+ ICsrKyBiL2ZzL2YyZnMvbm9kZS5jCj4+IEBAIC0xNDIyLDcgKzE0MjIsMTMgQEAgcmV0cnk6Cj4+ ICAJc3Bpbl9sb2NrKCZubV9pLT5mcmVlX25pZF9saXN0X2xvY2spOwo+PiAgCj4+ICAJLyogV2Ug c2hvdWxkIG5vdCB1c2Ugc3RhbGUgZnJlZSBuaWRzIGNyZWF0ZWQgYnkgYnVpbGRfZnJlZV9uaWRz ICovCj4+IC0JaWYgKG5tX2ktPmZjbnQgJiYgIW9uX2J1aWxkX2ZyZWVfbmlkcyhubV9pKSkgewo+ PiArCWlmIChvbl9idWlsZF9mcmVlX25pZHMobm1faSkpIHsKPj4gKwkJc3Bpbl91bmxvY2soJm5t X2ktPmZyZWVfbmlkX2xpc3RfbG9jayk7Cj4+ICsJCXdhaXRfZXZlbnQobm1faS0+YnVpbGRfd3Es ICFvbl9idWlsZF9mcmVlX25pZHMobm1faSkpOwo+PiArCQlnb3RvIHJldHJ5Owo+PiArCX0KPj4g Kwo+PiArCWlmIChubV9pLT5mY250KSB7Cj4+ICAJCWYyZnNfYnVnX29uKGxpc3RfZW1wdHkoJm5t X2ktPmZyZWVfbmlkX2xpc3QpKTsKPj4gIAkJbGlzdF9mb3JfZWFjaCh0aGlzLCAmbm1faS0+ZnJl ZV9uaWRfbGlzdCkgewo+PiAgCQkJaSA9IGxpc3RfZW50cnkodGhpcywgc3RydWN0IGZyZWVfbmlk LCBsaXN0KTsKPj4gQEAgLTE0NDMsNiArMTQ0OSw3IEBAIHJldHJ5Ogo+PiAgCW11dGV4X2xvY2so Jm5tX2ktPmJ1aWxkX2xvY2spOwo+PiAgCWJ1aWxkX2ZyZWVfbmlkcyhzYmkpOwo+PiAgCW11dGV4 X3VubG9jaygmbm1faS0+YnVpbGRfbG9jayk7Cj4+ICsJd2FrZV91cF9hbGwoJm5tX2ktPmJ1aWxk X3dxKTsKPj4gIAlnb3RvIHJldHJ5Owo+PiAgfQo+PiAgCj4+IEBAIC0xODEzLDYgKzE4MjAsNyBA QCBzdGF0aWMgaW50IGluaXRfbm9kZV9tYW5hZ2VyKHN0cnVjdCBmMmZzX3NiX2luZm8gKnNiaSkK Pj4gIAlJTklUX0xJU1RfSEVBRCgmbm1faS0+ZGlydHlfbmF0X2VudHJpZXMpOwo+PiAgCj4+ICAJ bXV0ZXhfaW5pdCgmbm1faS0+YnVpbGRfbG9jayk7Cj4+ICsJaW5pdF93YWl0cXVldWVfaGVhZCgm bm1faS0+YnVpbGRfd3EpOwo+PiAgCXNwaW5fbG9ja19pbml0KCZubV9pLT5mcmVlX25pZF9saXN0 X2xvY2spOwo+PiAgCXJ3bG9ja19pbml0KCZubV9pLT5uYXRfdHJlZV9sb2NrKTsKPj4gIAo+IAoK CgotLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0t LS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0KTGVhcm4gR3JhcGggRGF0YWJhc2VzIC0gRG93bmxvYWQg RlJFRSBPJ1JlaWxseSBCb29rCiJHcmFwaCBEYXRhYmFzZXMiIGlzIHRoZSBkZWZpbml0aXZlIG5l dyBndWlkZSB0byBncmFwaCBkYXRhYmFzZXMgYW5kIHRoZWlyCmFwcGxpY2F0aW9ucy4gV3JpdHRl biBieSB0aHJlZSBhY2NsYWltZWQgbGVhZGVycyBpbiB0aGUgZmllbGQsCnRoaXMgZmlyc3QgZWRp dGlvbiBpcyBub3cgYXZhaWxhYmxlLiBEb3dubG9hZCB5b3VyIGZyZWUgYm9vayB0b2RheSEKaHR0 cDovL3Auc2YubmV0L3NmdS8xMzUzNF9OZW9UZWNoCl9fX19fX19fX19fX19fX19fX19fX19fX19f X19fX19fX19fX19fX19fX19fX19fCkxpbnV4LWYyZnMtZGV2ZWwgbWFpbGluZyBsaXN0CkxpbnV4 LWYyZnMtZGV2ZWxAbGlzdHMuc291cmNlZm9yZ2UubmV0Cmh0dHBzOi8vbGlzdHMuc291cmNlZm9y Z2UubmV0L2xpc3RzL2xpc3RpbmZvL2xpbnV4LWYyZnMtZGV2ZWwK From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752651AbaCJFqn (ORCPT ); Mon, 10 Mar 2014 01:46:43 -0400 Received: from cn.fujitsu.com ([222.73.24.84]:1460 "EHLO song.cn.fujitsu.com" rhost-flags-OK-FAIL-OK-OK) by vger.kernel.org with ESMTP id S1751183AbaCJFqm convert rfc822-to-8bit (ORCPT ); Mon, 10 Mar 2014 01:46:42 -0400 X-IronPort-AV: E=Sophos;i="4.97,622,1389715200"; d="scan'208";a="9670036" Message-ID: <531D4F9E.9070804@cn.fujitsu.com> Date: Mon, 10 Mar 2014 13:37:34 +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: f2fs , linux-kernel , =?UTF-8?B?5L+e6LaF?= Subject: Re: [PATCH 5/5] f2fs: add a wait queue to avoid unnecessary, build_free_nid References: <5319A2DB.8040705@cn.fujitsu.com> <1394427024.3870.94.camel@kjgkr> In-Reply-To: <1394427024.3870.94.camel@kjgkr> X-MIMETrack: Itemize by SMTP Server on mailserver/fnst(Release 8.5.3|September 15, 2011) at 2014/03/10 13:43:39, Serialize by Router on mailserver/fnst(Release 8.5.3|September 15, 2011) at 2014/03/10 13:43:46 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 Kim, On 03/10/2014 12:50 PM, Jaegeuk Kim wrote: > Hi Gu, > > 2014-03-07 (금), 18:43 +0800, Gu Zheng: >> Previously, when we try to alloc free nid while the build free nid >> is going, the allocer will be run into the flow that waiting for >> "nm_i->build_lock", see following: >> /* We should not use stale free nids created by build_free_nids */ >> ----> if (nm_i->fcnt && !on_build_free_nids(nm_i)) { >> f2fs_bug_on(list_empty(&nm_i->free_nid_list)); >> list_for_each(this, &nm_i->free_nid_list) { >> i = list_entry(this, struct free_nid, list); >> if (i->state == NID_NEW) >> break; >> } >> >> f2fs_bug_on(i->state != NID_NEW); >> *nid = i->nid; >> i->state = NID_ALLOC; >> nm_i->fcnt--; >> spin_unlock(&nm_i->free_nid_list_lock); >> return true; >> } >> spin_unlock(&nm_i->free_nid_list_lock); >> >> /* Let's scan nat pages and its caches to get free nids */ >> ----> mutex_lock(&nm_i->build_lock); >> build_free_nids(sbi); >> mutex_unlock(&nm_i->build_lock); >> and this will cause another unnecessary building free nid if the current >> building free nid job is done. > > Could you support any performance number for this? I just run some common test via fio with simulated ssd(via loop). > Since, IMO, the contended building processes will be released right away > because of the following condition check inside build_free_nids(). > > if (nm_i->fcnt > NAT_ENTRY_PER_BLOCK) > return; It does. But, IMO, we can not promise nm_i->fcnt > NAT_ENTRY_PER_BLOCK when the contended building process entering, especially in high concurrency condition. > > So, I don't think this gives us any high latency. > Can the wakeup_all() become another overhead all the time? Yeah, maybe we must test whether it can also cause the performance regression, because the wakeup_all also introduce overhand as you said. But what is bad is that I do not have a production environment to test it, as you know the simulated environment is not strict. cc Yu, Could you please help to test it? Regards, Gu > Thanks, > >> So here we introduce a wait_queue to avoid this issue. >> >> Signed-off-by: Gu Zheng >> --- >> fs/f2fs/f2fs.h | 1 + >> fs/f2fs/node.c | 10 +++++++++- >> 2 files changed, 10 insertions(+), 1 deletions(-) >> >> diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h >> index f845e92..7ae193e 100644 >> --- a/fs/f2fs/f2fs.h >> +++ b/fs/f2fs/f2fs.h >> @@ -256,6 +256,7 @@ struct f2fs_nm_info { >> spinlock_t free_nid_list_lock; /* protect free nid list */ >> unsigned int fcnt; /* the number of free node id */ >> struct mutex build_lock; /* lock for build free nids */ >> + wait_queue_head_t build_wq; /* wait queue for build free nids */ >> >> /* for checkpoint */ >> char *nat_bitmap; /* NAT bitmap pointer */ >> diff --git a/fs/f2fs/node.c b/fs/f2fs/node.c >> index 4b7861d..ab44711 100644 >> --- a/fs/f2fs/node.c >> +++ b/fs/f2fs/node.c >> @@ -1422,7 +1422,13 @@ retry: >> spin_lock(&nm_i->free_nid_list_lock); >> >> /* We should not use stale free nids created by build_free_nids */ >> - if (nm_i->fcnt && !on_build_free_nids(nm_i)) { >> + if (on_build_free_nids(nm_i)) { >> + spin_unlock(&nm_i->free_nid_list_lock); >> + wait_event(nm_i->build_wq, !on_build_free_nids(nm_i)); >> + goto retry; >> + } >> + >> + if (nm_i->fcnt) { >> f2fs_bug_on(list_empty(&nm_i->free_nid_list)); >> list_for_each(this, &nm_i->free_nid_list) { >> i = list_entry(this, struct free_nid, list); >> @@ -1443,6 +1449,7 @@ retry: >> mutex_lock(&nm_i->build_lock); >> build_free_nids(sbi); >> mutex_unlock(&nm_i->build_lock); >> + wake_up_all(&nm_i->build_wq); >> goto retry; >> } >> >> @@ -1813,6 +1820,7 @@ static int init_node_manager(struct f2fs_sb_info *sbi) >> INIT_LIST_HEAD(&nm_i->dirty_nat_entries); >> >> mutex_init(&nm_i->build_lock); >> + init_waitqueue_head(&nm_i->build_wq); >> spin_lock_init(&nm_i->free_nid_list_lock); >> rwlock_init(&nm_i->nat_tree_lock); >> >