From mboxrd@z Thu Jan 1 00:00:00 1970 From: Gu Zheng Subject: Re: [PATCH 4/5] f2fs: optimize restore_node_summary slightly Date: Mon, 10 Mar 2014 13:38:46 +0800 Message-ID: <531D4FE6.7000503@cn.fujitsu.com> References: <5319A2D8.70409@cn.fujitsu.com> <1394426742.3870.89.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-3.v29.ch3.sourceforge.com with esmtp (Exim 4.76) (envelope-from ) id 1WMt3s-0002cw-Hw for linux-f2fs-devel@lists.sourceforge.net; Mon, 10 Mar 2014 05:47:40 +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 1WMt3q-0000Zx-Vt for linux-f2fs-devel@lists.sourceforge.net; Mon, 10 Mar 2014 05:47:40 +0000 In-Reply-To: <1394426742.3870.89.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 SGkgS2ltLApPbiAwMy8xMC8yMDE0IDEyOjQ1IFBNLCBKYWVnZXVrIEtpbSB3cm90ZToKCj4gSGkg R3UsCj4gCj4gMjAxNC0wMy0wNyAo6riIKSwgMTg6NDMgKzA4MDAsIEd1IFpoZW5nOgo+PiBQcmV2 aW91c2x5LCB3ZSByYV9zdW1fcGFnZXMgdG8gcHJlLXJlYWQgY29udGlndW91cyBwYWdlcyBhcyBt b3JlCj4+IGFzIHBvc3NpYmxlLCBhbmQgaWYgd2UgZmFpbCB0byBhbGxvYyBtb3JlIHBhZ2VzLCBh biBFTk9NRU0gZXJyb3IKPj4gd2lsbCBiZSByZXBvcnRlZCB1cHN0cmVhbSwgZXZlbiB0aG91Z2gg d2UgaGF2ZSBhbGxvY2VkIHNvbWUgcGFnZXMKPj4geWV0LiBJbiBmYWN0LCB3ZSBjYW4gdXNlIHRo ZSBhdmFpbGFibGUgcGFnZXMgdG8gZG8gdGhlIGpvYiBwYXJ0bHksCj4+IGFuZCBjb250aW51ZSB0 aGUgcmVzdCBpbiB0aGUgZm9sbG93aW5nIGNpcmNsZS4gT25seSByZXBvcnRpbmcgRU5PTUVNCj4+ IHVwc3RyZWFtIGlmIHdlIHJlYWxseSBjYW4gbm90IGFsbG9jIGFueSBhdmFpbGFibGUgcGFnZS4K Pj4KPj4gQW5kIGFub3RoZXIgZml4IGlzIGlnbm9yaW5nIGRlYWxpbmcgd2l0aCB0aGUgZm9sbG93 aW5nIHBhZ2VzIGlmIGFuCj4+IEVJTyBvY2N1cnMgd2hlbiByZWFkaW5nIHBhZ2UgZnJvbSBwYWdl X2xpc3QuCj4+Cj4+IFNpZ25lZC1vZmYtYnk6IEd1IFpoZW5nIDxndXouZm5zdEBjbi5mdWppdHN1 LmNvbT4KPj4gLS0tCj4+ICBmcy9mMmZzL25vZGUuYyAgICB8ICAgNDQgKysrKysrKysrKysrKysr KysrKystLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0KPj4gIGZzL2YyZnMvc2VnbWVudC5jIHwgICAg NyArKysrKy0tCj4+ICAyIGZpbGVzIGNoYW5nZWQsIDI1IGluc2VydGlvbnMoKyksIDI2IGRlbGV0 aW9ucygtKQo+Pgo+PiBkaWZmIC0tZ2l0IGEvZnMvZjJmcy9ub2RlLmMgYi9mcy9mMmZzL25vZGUu Ywo+PiBpbmRleCA4Nzg3NDY5Li40Yjc4NjFkIDEwMDY0NAo+PiAtLS0gYS9mcy9mMmZzL25vZGUu Ywo+PiArKysgYi9mcy9mMmZzL25vZGUuYwo+PiBAQCAtMTU4OCwxNSArMTU4OCw4IEBAIHN0YXRp YyBpbnQgcmFfc3VtX3BhZ2VzKHN0cnVjdCBmMmZzX3NiX2luZm8gKnNiaSwgc3RydWN0IGxpc3Rf aGVhZCAqcGFnZXMsCj4+ICAJZm9yICg7IHBhZ2VfaWR4IDwgc3RhcnQgKyBucnBhZ2VzOyBwYWdl X2lkeCsrKSB7Cj4+ICAJCS8qIGFsbG9jIHRlbXBvcmFsIHBhZ2UgZm9yIHJlYWQgbm9kZSBzdW1t YXJ5IGluZm8qLwo+PiAgCQlwYWdlID0gYWxsb2NfcGFnZShHRlBfRjJGU19aRVJPKTsKPj4gLQkJ aWYgKCFwYWdlKSB7Cj4+IC0JCQlzdHJ1Y3QgcGFnZSAqdG1wOwo+PiAtCQkJbGlzdF9mb3JfZWFj aF9lbnRyeV9zYWZlKHBhZ2UsIHRtcCwgcGFnZXMsIGxydSkgewo+PiAtCQkJCWxpc3RfZGVsKCZw YWdlLT5scnUpOwo+PiAtCQkJCXVubG9ja19wYWdlKHBhZ2UpOwo+PiAtCQkJCV9fZnJlZV9wYWdl cyhwYWdlLCAwKTsKPj4gLQkJCX0KPj4gLQkJCXJldHVybiAtRU5PTUVNOwo+PiAtCQl9Cj4+ICsJ CWlmICghcGFnZSkKPj4gKwkJCWJyZWFrOwo+PiAgCj4+ICAJCWxvY2tfcGFnZShwYWdlKTsKPj4g IAkJcGFnZS0+aW5kZXggPSBwYWdlX2lkeDsKPj4gQEAgLTE2MDcsNyArMTYwMCw4IEBAIHN0YXRp YyBpbnQgcmFfc3VtX3BhZ2VzKHN0cnVjdCBmMmZzX3NiX2luZm8gKnNiaSwgc3RydWN0IGxpc3Rf aGVhZCAqcGFnZXMsCj4+ICAJCWYyZnNfc3VibWl0X3BhZ2VfbWJpbyhzYmksIHBhZ2UsIHBhZ2Ut PmluZGV4LCAmZmlvKTsKPj4gIAo+PiAgCWYyZnNfc3VibWl0X21lcmdlZF9iaW8oc2JpLCBNRVRB LCBSRUFEKTsKPj4gLQlyZXR1cm4gMDsKPj4gKwo+PiArCXJldHVybiBwYWdlX2lkeCAtIHN0YXJ0 Owo+PiAgfQo+PiAgCj4+ICBpbnQgcmVzdG9yZV9ub2RlX3N1bW1hcnkoc3RydWN0IGYyZnNfc2Jf aW5mbyAqc2JpLAo+PiBAQCAtMTYzMCwyOCArMTYyNCwzMCBAQCBpbnQgcmVzdG9yZV9ub2RlX3N1 bW1hcnkoc3RydWN0IGYyZnNfc2JfaW5mbyAqc2JpLAo+PiAgCQlucnBhZ2VzID0gbWluKGxhc3Rf b2Zmc2V0IC0gaSwgYmlvX2Jsb2Nrcyk7Cj4+ICAKPj4gIAkJLyogcmVhZCBhaGVhZCBub2RlIHBh Z2VzICovCj4+IC0JCWVyciA9IHJhX3N1bV9wYWdlcyhzYmksICZwYWdlX2xpc3QsIGFkZHIsIG5y cGFnZXMpOwo+PiAtCQlpZiAoZXJyKQo+PiAtCQkJcmV0dXJuIGVycjsKPj4gKwkJbnJwYWdlcyA9 IHJhX3N1bV9wYWdlcyhzYmksICZwYWdlX2xpc3QsIGFkZHIsIG5ycGFnZXMpOwo+PiArCQlpZiAo IW5ycGFnZXMpCj4+ICsJCQlyZXR1cm4gLUVOT01FTTsKPj4gIAo+PiAgCQlsaXN0X2Zvcl9lYWNo X2VudHJ5X3NhZmUocGFnZSwgdG1wLCAmcGFnZV9saXN0LCBscnUpIHsKPj4gLQo+IAo+IEhlcmUg d2UgY2FuIGp1c3QgYWRkOgo+IAkJCWlmIChlcnIpCj4gCQkJCWdvdG8gc2tpcDsKPiAJCQlsb2Nr X3BhZ2UoKTsKPiAJCQkuLi4KPiAJCQl1bmxvY2tfcGFnZSgpOwo+IAkJc2tpcDoKPiAJCQlsaXN0 X2RlbCgpOwo+IAkJCV9fZnJlZV9wYWdlcygpOwo+IAo+IElNTywgaXQncyBtb3JlIG5lYXQsIHNv IGlmIHlvdSBoYXZlIGFueSBvYmplY3Rpb24sIGxldCBtZSBrbm93Lgo+IE90aGVyd2lzZSwgSSds bCBoYW5kbGUgdGhpcyBieSBteXNlbGYuIDopCgpUaGFua3MgdmVyeSBtdWNoLgoKUmVnYXJkcywK R3UKCj4gVGhhbmtzLAo+IAo+PiAtCQkJbG9ja19wYWdlKHBhZ2UpOwo+PiAtCQkJaWYgKHVubGlr ZWx5KCFQYWdlVXB0b2RhdGUocGFnZSkpKSB7Cj4+IC0JCQkJZXJyID0gLUVJTzsKPj4gLQkJCX0g ZWxzZSB7Cj4+IC0JCQkJcm4gPSBGMkZTX05PREUocGFnZSk7Cj4+IC0JCQkJc3VtX2VudHJ5LT5u aWQgPSBybi0+Zm9vdGVyLm5pZDsKPj4gLQkJCQlzdW1fZW50cnktPnZlcnNpb24gPSAwOwo+PiAt CQkJCXN1bV9lbnRyeS0+b2ZzX2luX25vZGUgPSAwOwo+PiAtCQkJCXN1bV9lbnRyeSsrOwo+PiAr CQkJaWYgKCFlcnIpIHsKPj4gKwkJCQlsb2NrX3BhZ2UocGFnZSk7Cj4+ICsJCQkJaWYgKHVubGlr ZWx5KCFQYWdlVXB0b2RhdGUocGFnZSkpKSB7Cj4+ICsJCQkJCWVyciA9IC1FSU87Cj4+ICsJCQkJ fSBlbHNlIHsKPj4gKwkJCQkJcm4gPSBGMkZTX05PREUocGFnZSk7Cj4+ICsJCQkJCXN1bV9lbnRy eS0+bmlkID0gcm4tPmZvb3Rlci5uaWQ7Cj4+ICsJCQkJCXN1bV9lbnRyeS0+dmVyc2lvbiA9IDA7 Cj4+ICsJCQkJCXN1bV9lbnRyeS0+b2ZzX2luX25vZGUgPSAwOwo+PiArCQkJCQlzdW1fZW50cnkr KzsKPj4gKwkJCQl9Cj4+ICsJCQkJdW5sb2NrX3BhZ2UocGFnZSk7Cj4+ICAJCQl9Cj4+ICAKPj4g IAkJCWxpc3RfZGVsKCZwYWdlLT5scnUpOwo+PiAtCQkJdW5sb2NrX3BhZ2UocGFnZSk7Cj4+ICAJ CQlfX2ZyZWVfcGFnZXMocGFnZSwgMCk7Cj4+ICAJCX0KPj4gIAl9Cj4+ICsKPj4gIAlyZXR1cm4g ZXJyOwo+PiAgfQo+PiAgCj4+IGRpZmYgLS1naXQgYS9mcy9mMmZzL3NlZ21lbnQuYyBiL2ZzL2Yy ZnMvc2VnbWVudC5jCj4+IGluZGV4IDE5OWM5NjQuLmIzZjg0MzEgMTAwNjQ0Cj4+IC0tLSBhL2Zz L2YyZnMvc2VnbWVudC5jCj4+ICsrKyBiL2ZzL2YyZnMvc2VnbWVudC5jCj4+IEBAIC0xMTYwLDkg KzExNjAsMTIgQEAgc3RhdGljIGludCByZWFkX25vcm1hbF9zdW1tYXJpZXMoc3RydWN0IGYyZnNf c2JfaW5mbyAqc2JpLCBpbnQgdHlwZSkKPj4gIAkJCQlucy0+b2ZzX2luX25vZGUgPSAwOwo+PiAg CQkJfQo+PiAgCQl9IGVsc2Ugewo+PiAtCQkJaWYgKHJlc3RvcmVfbm9kZV9zdW1tYXJ5KHNiaSwg c2Vnbm8sIHN1bSkpIHsKPj4gKwkJCWludCBlcnI7Cj4+ICsKPj4gKwkJCWVyciA9IHJlc3RvcmVf bm9kZV9zdW1tYXJ5KHNiaSwgc2Vnbm8sIHN1bSk7Cj4+ICsJCQlpZiAoZXJyKSB7Cj4+ICAJCQkJ ZjJmc19wdXRfcGFnZShuZXcsIDEpOwo+PiAtCQkJCXJldHVybiAtRUlOVkFMOwo+PiArCQkJCXJl dHVybiBlcnI7Cj4+ICAJCQl9Cj4+ICAJCX0KPj4gIAl9Cj4gCgoKCi0tLS0tLS0tLS0tLS0tLS0t LS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0t LS0tLQpMZWFybiBHcmFwaCBEYXRhYmFzZXMgLSBEb3dubG9hZCBGUkVFIE8nUmVpbGx5IEJvb2sK IkdyYXBoIERhdGFiYXNlcyIgaXMgdGhlIGRlZmluaXRpdmUgbmV3IGd1aWRlIHRvIGdyYXBoIGRh dGFiYXNlcyBhbmQgdGhlaXIKYXBwbGljYXRpb25zLiBXcml0dGVuIGJ5IHRocmVlIGFjY2xhaW1l ZCBsZWFkZXJzIGluIHRoZSBmaWVsZCwKdGhpcyBmaXJzdCBlZGl0aW9uIGlzIG5vdyBhdmFpbGFi bGUuIERvd25sb2FkIHlvdXIgZnJlZSBib29rIHRvZGF5IQpodHRwOi8vcC5zZi5uZXQvc2Z1LzEz NTM0X05lb1RlY2gKX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19f X18KTGludXgtZjJmcy1kZXZlbCBtYWlsaW5nIGxpc3QKTGludXgtZjJmcy1kZXZlbEBsaXN0cy5z b3VyY2Vmb3JnZS5uZXQKaHR0cHM6Ly9saXN0cy5zb3VyY2Vmb3JnZS5uZXQvbGlzdHMvbGlzdGlu Zm8vbGludXgtZjJmcy1kZXZlbAo= From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753309AbaCJFry (ORCPT ); Mon, 10 Mar 2014 01:47:54 -0400 Received: from cn.fujitsu.com ([222.73.24.84]:14979 "EHLO song.cn.fujitsu.com" rhost-flags-OK-FAIL-OK-OK) by vger.kernel.org with ESMTP id S1753286AbaCJFrw convert rfc822-to-8bit (ORCPT ); Mon, 10 Mar 2014 01:47:52 -0400 X-IronPort-AV: E=Sophos;i="4.97,622,1389715200"; d="scan'208";a="9670041" Message-ID: <531D4FE6.7000503@cn.fujitsu.com> Date: Mon, 10 Mar 2014 13:38:46 +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 Subject: Re: [PATCH 4/5] f2fs: optimize restore_node_summary slightly References: <5319A2D8.70409@cn.fujitsu.com> <1394426742.3870.89.camel@kjgkr> In-Reply-To: <1394426742.3870.89.camel@kjgkr> X-MIMETrack: Itemize by SMTP Server on mailserver/fnst(Release 8.5.3|September 15, 2011) at 2014/03/10 13:44:48, Serialize by Router on mailserver/fnst(Release 8.5.3|September 15, 2011) at 2014/03/10 13:44:50 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:45 PM, Jaegeuk Kim wrote: > Hi Gu, > > 2014-03-07 (금), 18:43 +0800, Gu Zheng: >> Previously, we ra_sum_pages to pre-read contiguous pages as more >> as possible, and if we fail to alloc more pages, an ENOMEM error >> will be reported upstream, even though we have alloced some pages >> yet. In fact, we can use the available pages to do the job partly, >> and continue the rest in the following circle. Only reporting ENOMEM >> upstream if we really can not alloc any available page. >> >> And another fix is ignoring dealing with the following pages if an >> EIO occurs when reading page from page_list. >> >> Signed-off-by: Gu Zheng >> --- >> fs/f2fs/node.c | 44 ++++++++++++++++++++------------------------ >> fs/f2fs/segment.c | 7 +++++-- >> 2 files changed, 25 insertions(+), 26 deletions(-) >> >> diff --git a/fs/f2fs/node.c b/fs/f2fs/node.c >> index 8787469..4b7861d 100644 >> --- a/fs/f2fs/node.c >> +++ b/fs/f2fs/node.c >> @@ -1588,15 +1588,8 @@ static int ra_sum_pages(struct f2fs_sb_info *sbi, struct list_head *pages, >> for (; page_idx < start + nrpages; page_idx++) { >> /* alloc temporal page for read node summary info*/ >> page = alloc_page(GFP_F2FS_ZERO); >> - if (!page) { >> - struct page *tmp; >> - list_for_each_entry_safe(page, tmp, pages, lru) { >> - list_del(&page->lru); >> - unlock_page(page); >> - __free_pages(page, 0); >> - } >> - return -ENOMEM; >> - } >> + if (!page) >> + break; >> >> lock_page(page); >> page->index = page_idx; >> @@ -1607,7 +1600,8 @@ static int ra_sum_pages(struct f2fs_sb_info *sbi, struct list_head *pages, >> f2fs_submit_page_mbio(sbi, page, page->index, &fio); >> >> f2fs_submit_merged_bio(sbi, META, READ); >> - return 0; >> + >> + return page_idx - start; >> } >> >> int restore_node_summary(struct f2fs_sb_info *sbi, >> @@ -1630,28 +1624,30 @@ int restore_node_summary(struct f2fs_sb_info *sbi, >> nrpages = min(last_offset - i, bio_blocks); >> >> /* read ahead node pages */ >> - err = ra_sum_pages(sbi, &page_list, addr, nrpages); >> - if (err) >> - return err; >> + nrpages = ra_sum_pages(sbi, &page_list, addr, nrpages); >> + if (!nrpages) >> + return -ENOMEM; >> >> list_for_each_entry_safe(page, tmp, &page_list, lru) { >> - > > Here we can just add: > if (err) > goto skip; > lock_page(); > ... > unlock_page(); > skip: > list_del(); > __free_pages(); > > IMO, it's more neat, so if you have any objection, let me know. > Otherwise, I'll handle this by myself. :) Thanks very much. Regards, Gu > Thanks, > >> - lock_page(page); >> - if (unlikely(!PageUptodate(page))) { >> - err = -EIO; >> - } else { >> - rn = F2FS_NODE(page); >> - sum_entry->nid = rn->footer.nid; >> - sum_entry->version = 0; >> - sum_entry->ofs_in_node = 0; >> - sum_entry++; >> + if (!err) { >> + lock_page(page); >> + if (unlikely(!PageUptodate(page))) { >> + err = -EIO; >> + } else { >> + rn = F2FS_NODE(page); >> + sum_entry->nid = rn->footer.nid; >> + sum_entry->version = 0; >> + sum_entry->ofs_in_node = 0; >> + sum_entry++; >> + } >> + unlock_page(page); >> } >> >> list_del(&page->lru); >> - unlock_page(page); >> __free_pages(page, 0); >> } >> } >> + >> return err; >> } >> >> diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c >> index 199c964..b3f8431 100644 >> --- a/fs/f2fs/segment.c >> +++ b/fs/f2fs/segment.c >> @@ -1160,9 +1160,12 @@ static int read_normal_summaries(struct f2fs_sb_info *sbi, int type) >> ns->ofs_in_node = 0; >> } >> } else { >> - if (restore_node_summary(sbi, segno, sum)) { >> + int err; >> + >> + err = restore_node_summary(sbi, segno, sum); >> + if (err) { >> f2fs_put_page(new, 1); >> - return -EINVAL; >> + return err; >> } >> } >> } >