From mboxrd@z Thu Jan 1 00:00:00 1970 From: Gu Zheng Subject: Re: [PATCH 5/5] f2fs: move the list_head initialization into the lock protection region Date: Wed, 20 Nov 2013 17:48:09 +0800 Message-ID: <528C8559.6040909@cn.fujitsu.com> References: <528B3783.2040905@cn.fujitsu.com> <1384911101.26319.37.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-1.v29.ch3.sourceforge.com with esmtp (Exim 4.76) (envelope-from ) id 1Vj4Ue-0002FX-Sr for linux-f2fs-devel@lists.sourceforge.net; Wed, 20 Nov 2013 09:54:44 +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 1Vj4Uc-0005Ws-Vn for linux-f2fs-devel@lists.sourceforge.net; Wed, 20 Nov 2013 09:54:44 +0000 In-Reply-To: <1384911101.26319.37.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: fsdevel , linux-kernel , f2fs SGkgS2ltLApPbiAxMS8yMC8yMDEzIDA5OjMxIEFNLCBKYWVnZXVrIEtpbSB3cm90ZToKCj4gSGkg R3UsCj4gCj4gSU1PLCB0aGVyZSBpcyBubyByZWFzb24gdG8gY292ZXIgdGhlIGxpc3QgaGVhZGVy IGJ5IHRoZSBsb2NrLgo+IEluIGFueSBmbG93cywgc2JpIHNob3VsZCBoYXZlIHRoZSBoZWFkZXIg YWxsIHRoZSB0aW1lLgoKWWVzLCB5b3UncmUgcmlnaHQuCgo+IFdoYXQgaXMgeW91ciBvcGluaW9u PwoKTW92aW5nIHRoZSBsaXN0X2hlYWQgaW5pdGlhbGl6YXRpb24gaW50byB0aGUgbG9jayBwcm90 ZWN0aW9uIHJlZ2lvbiwgc28KdGhhdCB3ZSBjYW4gaWdub3JlIHRoZSBsaXN0IGNoYW5nZSBkdXJp bmcgdGhlIGdhcChpbml0aWFsaXphdGlvbjwtLT51c2UpLgpBbmQgb24gdGhlIG90aGVyIHNpZGUs IGl0IGNhbiBrZWVwIHRoZSBjb2RlIHN0eWxlIHVuaWZvcm0gZm9yIGJldHRlcgptYWludGVuYW5j ZS4KClJlZ2FyZHMsCkd1Cgo+IAo+IFRoYW5rcywKPiAKPiAyMDEzLTExLTE5ICjtmZQpLCAxODow MyArMDgwMCwgR3UgWmhlbmc6Cj4+IFNpZ25lZC1vZmYtYnk6IEd1IFpoZW5nIDxndXouZm5zdEBj bi5mdWppdHN1LmNvbT4KPj4gLS0tCj4+ICBmcy9mMmZzL2NoZWNrcG9pbnQuYyB8ICAgMTUgKysr KysrKysrKy0tLS0tCj4+ICAxIGZpbGVzIGNoYW5nZWQsIDEwIGluc2VydGlvbnMoKyksIDUgZGVs ZXRpb25zKC0pCj4+Cj4+IGRpZmYgLS1naXQgYS9mcy9mMmZzL2NoZWNrcG9pbnQuYyBiL2ZzL2Yy ZnMvY2hlY2twb2ludC5jCj4+IGluZGV4IGY4ODQ1ODkuLjFkZTcwY2MgMTAwNjQ0Cj4+IC0tLSBh L2ZzL2YyZnMvY2hlY2twb2ludC5jCj4+ICsrKyBiL2ZzL2YyZnMvY2hlY2twb2ludC5jCj4+IEBA IC01MTEsOCArNTExLDggQEAgdm9pZCBhZGRfZGlydHlfZGlyX2lub2RlKHN0cnVjdCBpbm9kZSAq aW5vZGUpCj4+ICB2b2lkIHJlbW92ZV9kaXJ0eV9kaXJfaW5vZGUoc3RydWN0IGlub2RlICppbm9k ZSkKPj4gIHsKPj4gIAlzdHJ1Y3QgZjJmc19zYl9pbmZvICpzYmkgPSBGMkZTX1NCKGlub2RlLT5p X3NiKTsKPj4gLQlzdHJ1Y3QgbGlzdF9oZWFkICpoZWFkID0gJnNiaS0+ZGlyX2lub2RlX2xpc3Q7 Cj4+IC0Jc3RydWN0IGxpc3RfaGVhZCAqdGhpczsKPj4gKwo+PiArCXN0cnVjdCBsaXN0X2hlYWQg KnRoaXMsICpoZWFkOwo+PiAgCj4+ICAJaWYgKCFTX0lTRElSKGlub2RlLT5pX21vZGUpKQo+PiAg CQlyZXR1cm47Cj4+IEBAIC01MjMsNiArNTIzLDcgQEAgdm9pZCByZW1vdmVfZGlydHlfZGlyX2lu b2RlKHN0cnVjdCBpbm9kZSAqaW5vZGUpCj4+ICAJCXJldHVybjsKPj4gIAl9Cj4+ICAKPj4gKwlo ZWFkID0gJnNiaS0+ZGlyX2lub2RlX2xpc3Q7Cj4+ICAJbGlzdF9mb3JfZWFjaCh0aGlzLCBoZWFk KSB7Cj4+ICAJCXN0cnVjdCBkaXJfaW5vZGVfZW50cnkgKmVudHJ5Owo+PiAgCQllbnRyeSA9IGxp c3RfZW50cnkodGhpcywgc3RydWN0IGRpcl9pbm9kZV9lbnRyeSwgbGlzdCk7Cj4+IEBAIC01NDQs MTEgKzU0NSwxMyBAQCB2b2lkIHJlbW92ZV9kaXJ0eV9kaXJfaW5vZGUoc3RydWN0IGlub2RlICpp bm9kZSkKPj4gIAo+PiAgc3RydWN0IGlub2RlICpjaGVja19kaXJ0eV9kaXJfaW5vZGUoc3RydWN0 IGYyZnNfc2JfaW5mbyAqc2JpLCBuaWRfdCBpbm8pCj4+ICB7Cj4+IC0Jc3RydWN0IGxpc3RfaGVh ZCAqaGVhZCA9ICZzYmktPmRpcl9pbm9kZV9saXN0Owo+PiAtCXN0cnVjdCBsaXN0X2hlYWQgKnRo aXM7Cj4+ICsKPj4gKwlzdHJ1Y3QgbGlzdF9oZWFkICp0aGlzLCAqaGVhZDsKPj4gIAlzdHJ1Y3Qg aW5vZGUgKmlub2RlID0gTlVMTDsKPj4gIAo+PiAgCXNwaW5fbG9jaygmc2JpLT5kaXJfaW5vZGVf bG9jayk7Cj4+ICsKPj4gKwloZWFkID0gJnNiaS0+ZGlyX2lub2RlX2xpc3Q7Cj4+ICAJbGlzdF9m b3JfZWFjaCh0aGlzLCBoZWFkKSB7Cj4+ICAJCXN0cnVjdCBkaXJfaW5vZGVfZW50cnkgKmVudHJ5 Owo+PiAgCQllbnRyeSA9IGxpc3RfZW50cnkodGhpcywgc3RydWN0IGRpcl9pbm9kZV9lbnRyeSwg bGlzdCk7Cj4+IEBAIC01NjMsMTEgKzU2NiwxMyBAQCBzdHJ1Y3QgaW5vZGUgKmNoZWNrX2RpcnR5 X2Rpcl9pbm9kZShzdHJ1Y3QgZjJmc19zYl9pbmZvICpzYmksIG5pZF90IGlubykKPj4gIAo+PiAg dm9pZCBzeW5jX2RpcnR5X2Rpcl9pbm9kZXMoc3RydWN0IGYyZnNfc2JfaW5mbyAqc2JpKQo+PiAg ewo+PiAtCXN0cnVjdCBsaXN0X2hlYWQgKmhlYWQgPSAmc2JpLT5kaXJfaW5vZGVfbGlzdDsKPj4g KwlzdHJ1Y3QgbGlzdF9oZWFkICpoZWFkOwo+PiAgCXN0cnVjdCBkaXJfaW5vZGVfZW50cnkgKmVu dHJ5Owo+PiAgCXN0cnVjdCBpbm9kZSAqaW5vZGU7Cj4+ICByZXRyeToKPj4gIAlzcGluX2xvY2so JnNiaS0+ZGlyX2lub2RlX2xvY2spOwo+PiArCj4+ICsJaGVhZCA9ICZzYmktPmRpcl9pbm9kZV9s aXN0Owo+PiAgCWlmIChsaXN0X2VtcHR5KGhlYWQpKSB7Cj4+ICAJCXNwaW5fdW5sb2NrKCZzYmkt PmRpcl9pbm9kZV9sb2NrKTsKPj4gIAkJcmV0dXJuOwo+IAoKCgotLS0tLS0tLS0tLS0tLS0tLS0t LS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0t LS0KU2hhcGUgdGhlIE1vYmlsZSBFeHBlcmllbmNlOiBGcmVlIFN1YnNjcmlwdGlvbgpTb2Z0d2Fy ZSBleHBlcnRzIGFuZCBkZXZlbG9wZXJzOiBCZSBhdCB0aGUgZm9yZWZyb250IG9mIHRlY2ggaW5u b3ZhdGlvbi4KSW50ZWwoUikgU29mdHdhcmUgQWRyZW5hbGluZSBkZWxpdmVycyBzdHJhdGVnaWMg aW5zaWdodCBhbmQgZ2FtZS1jaGFuZ2luZyAKY29udmVyc2F0aW9ucyB0aGF0IHNoYXBlIHRoZSBy YXBpZGx5IGV2b2x2aW5nIG1vYmlsZSBsYW5kc2NhcGUuIFNpZ24gdXAgbm93LiAKaHR0cDovL3B1 YmFkcy5nLmRvdWJsZWNsaWNrLm5ldC9nYW1wYWQvY2xrP2lkPTYzNDMxMzExJml1PS80MTQwL29z dGcuY2xrdHJrCl9fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19f CkxpbnV4LWYyZnMtZGV2ZWwgbWFpbGluZyBsaXN0CkxpbnV4LWYyZnMtZGV2ZWxAbGlzdHMuc291 cmNlZm9yZ2UubmV0Cmh0dHBzOi8vbGlzdHMuc291cmNlZm9yZ2UubmV0L2xpc3RzL2xpc3RpbmZv L2xpbnV4LWYyZnMtZGV2ZWwK From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752357Ab3KTJyl (ORCPT ); Wed, 20 Nov 2013 04:54:41 -0500 Received: from cn.fujitsu.com ([222.73.24.84]:40178 "EHLO song.cn.fujitsu.com" rhost-flags-OK-FAIL-OK-OK) by vger.kernel.org with ESMTP id S1750834Ab3KTJyh convert rfc822-to-8bit (ORCPT ); Wed, 20 Nov 2013 04:54:37 -0500 X-IronPort-AV: E=Sophos;i="4.93,735,1378828800"; d="scan'208";a="9084664" Message-ID: <528C8559.6040909@cn.fujitsu.com> Date: Wed, 20 Nov 2013 17:48:09 +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 , fsdevel , linux-kernel Subject: Re: [PATCH 5/5] f2fs: move the list_head initialization into the lock protection region References: <528B3783.2040905@cn.fujitsu.com> <1384911101.26319.37.camel@kjgkr> In-Reply-To: <1384911101.26319.37.camel@kjgkr> X-MIMETrack: Itemize by SMTP Server on mailserver/fnst(Release 8.5.3|September 15, 2011) at 2013/11/20 17:52:32, Serialize by Router on mailserver/fnst(Release 8.5.3|September 15, 2011) at 2013/11/20 17:52:40 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 11/20/2013 09:31 AM, Jaegeuk Kim wrote: > Hi Gu, > > IMO, there is no reason to cover the list header by the lock. > In any flows, sbi should have the header all the time. Yes, you're right. > What is your opinion? Moving the list_head initialization into the lock protection region, so that we can ignore the list change during the gap(initialization<-->use). And on the other side, it can keep the code style uniform for better maintenance. Regards, Gu > > Thanks, > > 2013-11-19 (화), 18:03 +0800, Gu Zheng: >> Signed-off-by: Gu Zheng >> --- >> fs/f2fs/checkpoint.c | 15 ++++++++++----- >> 1 files changed, 10 insertions(+), 5 deletions(-) >> >> diff --git a/fs/f2fs/checkpoint.c b/fs/f2fs/checkpoint.c >> index f884589..1de70cc 100644 >> --- a/fs/f2fs/checkpoint.c >> +++ b/fs/f2fs/checkpoint.c >> @@ -511,8 +511,8 @@ void add_dirty_dir_inode(struct inode *inode) >> void remove_dirty_dir_inode(struct inode *inode) >> { >> struct f2fs_sb_info *sbi = F2FS_SB(inode->i_sb); >> - struct list_head *head = &sbi->dir_inode_list; >> - struct list_head *this; >> + >> + struct list_head *this, *head; >> >> if (!S_ISDIR(inode->i_mode)) >> return; >> @@ -523,6 +523,7 @@ void remove_dirty_dir_inode(struct inode *inode) >> return; >> } >> >> + head = &sbi->dir_inode_list; >> list_for_each(this, head) { >> struct dir_inode_entry *entry; >> entry = list_entry(this, struct dir_inode_entry, list); >> @@ -544,11 +545,13 @@ void remove_dirty_dir_inode(struct inode *inode) >> >> struct inode *check_dirty_dir_inode(struct f2fs_sb_info *sbi, nid_t ino) >> { >> - struct list_head *head = &sbi->dir_inode_list; >> - struct list_head *this; >> + >> + struct list_head *this, *head; >> struct inode *inode = NULL; >> >> spin_lock(&sbi->dir_inode_lock); >> + >> + head = &sbi->dir_inode_list; >> list_for_each(this, head) { >> struct dir_inode_entry *entry; >> entry = list_entry(this, struct dir_inode_entry, list); >> @@ -563,11 +566,13 @@ struct inode *check_dirty_dir_inode(struct f2fs_sb_info *sbi, nid_t ino) >> >> void sync_dirty_dir_inodes(struct f2fs_sb_info *sbi) >> { >> - struct list_head *head = &sbi->dir_inode_list; >> + struct list_head *head; >> struct dir_inode_entry *entry; >> struct inode *inode; >> retry: >> spin_lock(&sbi->dir_inode_lock); >> + >> + head = &sbi->dir_inode_list; >> if (list_empty(head)) { >> spin_unlock(&sbi->dir_inode_lock); >> return; >