From mboxrd@z Thu Jan 1 00:00:00 1970 From: Jaegeuk Kim Subject: Re: [PATCH 1/2] f2fs: reorganize f2fs_vm_page_mkwrite Date: Mon, 29 Apr 2013 11:41:20 +0900 Message-ID: <1367203280.16581.3.camel@kjgkr> References: <1367107458-2439-1-git-send-email-linkinjeon@gmail.com> Reply-To: jaegeuk.kim@samsung.com Mime-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="=-Pgy511eU+ALbmoBN9LLj" Cc: linux-f2fs-devel@lists.sourceforge.net, linux-fsdevel@vger.kernel.org, linux-kernel@vger.kernel.org, Namjae Jeon , Amit Sahrawat To: Namjae Jeon Return-path: Received: from mailout1.samsung.com ([203.254.224.24]:9789 "EHLO mailout1.samsung.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1756629Ab3D2CmV (ORCPT ); Sun, 28 Apr 2013 22:42:21 -0400 In-reply-to: <1367107458-2439-1-git-send-email-linkinjeon@gmail.com> Sender: linux-fsdevel-owner@vger.kernel.org List-ID: --=-Pgy511eU+ALbmoBN9LLj Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable Hi, 2013-04-28 (=EC=9D=BC), 09:04 +0900, Namjae Jeon: > From: Namjae Jeon >=20 > Few things can be changed in the default mkwrite function > 1) Make file_update_time at the start before acquiring any lock > 2) the condition page_offset(page) >=3D i_size_read(inode) should be > changed to page_offset(page) > i_size_read > 3) Move wait_on_page_writeback. >=20 > Signed-off-by: Namjae Jeon > Signed-off-by: Amit Sahrawat > --- > fs/f2fs/file.c | 12 ++++++------ > 1 file changed, 6 insertions(+), 6 deletions(-) >=20 > diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c > index 5cc4dd8..dc76f9b 100644 > --- a/fs/f2fs/file.c > +++ b/fs/f2fs/file.c > @@ -63,9 +63,10 @@ static int f2fs_vm_page_mkwrite(struct vm_area_struct = *vma, > f2fs_put_dnode(&dn); > mutex_unlock_op(sbi, ilock); > =20 > + file_update_time(vma->vm_file); Should we update time even if error is occurred below? > lock_page(page); > if (page->mapping !=3D inode->i_mapping || > - page_offset(page) >=3D i_size_read(inode) || > + page_offset(page) > i_size_read(inode) || Why? IMO, there was no problem. > !PageUptodate(page)) { > unlock_page(page); > err =3D -EFAULT; > @@ -76,10 +77,7 @@ static int f2fs_vm_page_mkwrite(struct vm_area_struct = *vma, > * check to see if the page is mapped already (no holes) > */ > if (PageMappedToDisk(page)) > - goto out; > - > - /* fill the page */ > - wait_on_page_writeback(page); > + goto mapped; > =20 > /* page is wholly or partially inside EOF */ > if (((page->index + 1) << PAGE_CACHE_SHIFT) > i_size_read(inode)) { > @@ -90,7 +88,9 @@ static int f2fs_vm_page_mkwrite(struct vm_area_struct *= vma, > set_page_dirty(page); > SetPageUptodate(page); > =20 > - file_update_time(vma->vm_file); > +mapped: > + /* fill the page */ > + wait_on_page_writeback(page); > out: > sb_end_pagefault(inode->i_sb); > return block_page_mkwrite_return(err); --=20 Jaegeuk Kim Samsung --=-Pgy511eU+ALbmoBN9LLj Content-Type: application/pgp-signature; name="signature.asc" Content-Description: This is a digitally signed message part Content-Transfer-Encoding: 7bit -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.4.11 (GNU/Linux) iQIcBAABAgAGBQJRfd3QAAoJEEAUqH6CSFDSEPgP/032U2PNyWDFz+6FDvOEPaPs KzzxAZbj+Lrpm745+Cv+aNZEiJ3IT2pwuJLirWfB5TSaQ3HWTqkpA8PNeBItMIwx Cu3II8PAZKKeTq89ZCrogqjCMNrMDB4HktCL9Lemscc9hAW5nOdTlD6OZs4rABvP rgYg3gmJI5wX39f8JFIuOqZVNBw+OYsTH92lsV4Vx4G3xOyW1+HsDX89XGOhGASb /MR9XcXT6Cc/kIeGO3iOm2fEhdXbG9g+wbqGw7aeiylsk6brRuOxH4rh3gkKk6Tu x1Va7CR3yD5ocCFBymp1K/tfc3dicl92r9oGLph1rbeLqLQCiNzjKiW0dH2XV6F2 fHcmJx99c/V7TKoJCApERr0mR4yQMoZt95dInSXgQDHNg0ABDcx9Tdv4vr7fUOoy /9aQ3Bp7Decq1j4hZyvn/PASx6n99QubXs8C1HjW6gNZbBfMFih6dviTF1ifsRhY Cz9hHIm9/ohJsc/Jg7r1ffahgcq+mw903/EWeEgDQiocQYA77zhgHNCfgukHZfre 0jQJUQjxwRA8AHNi0agVwSh2TmeAzVGn8pR5L+4e9B1aAheqGMQCdIFhlOhvI9I7 WYDPCsgv+F7AR7lYfHBBTB6KbqLIoEfdXI3UIByOBlCv/U0ATqTjG1IV2G9VB7SA GnzQd1/ixeEHpyvDM/IG =dz9C -----END PGP SIGNATURE----- --=-Pgy511eU+ALbmoBN9LLj--