[PATCH 00/11] Push file_update_time() into .page_mkwrite

[PATCH 00/11] Push file_update_time() into .page_mkwrite

[Jan Kara]

  Hello,

  to provide reliable support for filesystem freezing, filesystems need to have
complete control over when metadata is changed. In particular,
file_update_time() calls from page fault code make it impossible for
filesystems to prevent inodes from being dirtied while the filesystem is
frozen.

To fix the issue, this patch set changes page fault code to call
file_update_time() only when ->page_mkwrite() callback is not provided. If the
callback is provided, it is the responsibility of the filesystem to perform
update of i_mtime / i_ctime if needed. We also push file_update_time() call
to all existing ->page_mkwrite() implementations if the time update does not
obviously happen by other means. If you know your filesystem does not need
update of modification times in ->page_mkwrite() handler, please speak up and
I'll drop the patch for your filesystem.

As a side note, an alternative would be to remove call of file_update_time()
from page fault code altogether and require all filesystems needing it to do
that in their ->page_mkwrite() implementation. That is certainly possible
although maybe slightly inefficient and would require auditting 100+
vm_operations_structs *shake*.

If I get acks on these patches, Andrew, would you be willing to take these
patches?

								Honza

CC: Peter Zijlstra <a.p.zijlstra@chello.nl>
CC: Ingo Molnar <mingo@elte.hu>
CC: Paul Mackerras <paulus@samba.org>
CC: Arnaldo Carvalho de Melo <acme@ghostprotocols.net>
CC: Jaya Kumar <jayalk@intworks.biz>
CC: Sage Weil <sage@newdream.net>
CC: ceph-devel@vger.kernel.org
CC: Steve French <sfrench@samba.org>
CC: linux-cifs@vger.kernel.org
CC: Eric Van Hensbergen <ericvh@gmail.com>
CC: Ron Minnich <rminnich@sandia.gov>
CC: Latchesar Ionkov <lucho@ionkov.net>
CC: v9fs-developer@lists.sourceforge.net
CC: Miklos Szeredi <miklos@szeredi.hu>
CC: fuse-devel@lists.sourceforge.net
CC: Steven Whitehouse <swhiteho@redhat.com>
CC: cluster-devel@redhat.com
CC: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
CC: Trond Myklebust <Trond.Myklebust@netapp.com>
CC: linux-nfs@vger.kernel.org

[PATCH 10/11] nfs: Push file_update_time() into nfs_vm_page_mkwrite()

[Jan Kara]

CC: Trond Myklebust <Trond.Myklebust@netapp.com>
CC: linux-nfs@vger.kernel.org
Signed-off-by: Jan Kara <jack@suse.cz>
---
 fs/nfs/file.c |    3 +++
 1 files changed, 3 insertions(+), 0 deletions(-)

diff --git a/fs/nfs/file.c b/fs/nfs/file.c
index c43a452..2407922 100644
--- a/fs/nfs/file.c
+++ b/fs/nfs/file.c
@@ -525,6 +525,9 @@ static int nfs_vm_page_mkwrite(struct vm_area_struct *vma, struct vm_fault *vmf)
 	/* make sure the cache has finished storing the page */
 	nfs_fscache_wait_on_page_write(NFS_I(dentry->d_inode), page);
 
+	/* Update file times before taking page lock */
+	file_update_time(filp);
+
 	lock_page(page);
 	mapping = page->mapping;
 	if (mapping != dentry->d_inode->i_mapping)
-- 
1.7.1

Re: [PATCH 10/11] nfs: Push file_update_time() into nfs_vm_page_mkwrite()

[Myklebust, Trond]

T24gVGh1LCAyMDEyLTAyLTE2IGF0IDE0OjQ2ICswMTAwLCBKYW4gS2FyYSB3cm90ZToNCj4gQ0M6
IFRyb25kIE15a2xlYnVzdCA8VHJvbmQuTXlrbGVidXN0QG5ldGFwcC5jb20+DQo+IENDOiBsaW51
eC1uZnNAdmdlci5rZXJuZWwub3JnDQo+IFNpZ25lZC1vZmYtYnk6IEphbiBLYXJhIDxqYWNrQHN1
c2UuY3o+DQo+IC0tLQ0KPiAgZnMvbmZzL2ZpbGUuYyB8ICAgIDMgKysrDQo+ICAxIGZpbGVzIGNo
YW5nZWQsIDMgaW5zZXJ0aW9ucygrKSwgMCBkZWxldGlvbnMoLSkNCj4gDQo+IGRpZmYgLS1naXQg
YS9mcy9uZnMvZmlsZS5jIGIvZnMvbmZzL2ZpbGUuYw0KPiBpbmRleCBjNDNhNDUyLi4yNDA3OTIy
IDEwMDY0NA0KPiAtLS0gYS9mcy9uZnMvZmlsZS5jDQo+ICsrKyBiL2ZzL25mcy9maWxlLmMNCj4g
QEAgLTUyNSw2ICs1MjUsOSBAQCBzdGF0aWMgaW50IG5mc192bV9wYWdlX21rd3JpdGUoc3RydWN0
IHZtX2FyZWFfc3RydWN0ICp2bWEsIHN0cnVjdCB2bV9mYXVsdCAqdm1mKQ0KPiAgCS8qIG1ha2Ug
c3VyZSB0aGUgY2FjaGUgaGFzIGZpbmlzaGVkIHN0b3JpbmcgdGhlIHBhZ2UgKi8NCj4gIAluZnNf
ZnNjYWNoZV93YWl0X29uX3BhZ2Vfd3JpdGUoTkZTX0koZGVudHJ5LT5kX2lub2RlKSwgcGFnZSk7
DQo+ICANCj4gKwkvKiBVcGRhdGUgZmlsZSB0aW1lcyBiZWZvcmUgdGFraW5nIHBhZ2UgbG9jayAq
Lw0KPiArCWZpbGVfdXBkYXRlX3RpbWUoZmlscCk7DQo+ICsNCj4gIAlsb2NrX3BhZ2UocGFnZSk7
DQo+ICAJbWFwcGluZyA9IHBhZ2UtPm1hcHBpbmc7DQo+ICAJaWYgKG1hcHBpbmcgIT0gZGVudHJ5
LT5kX2lub2RlLT5pX21hcHBpbmcpDQoNCkhpIEphbiwNCg0KZmlsZV91cGRhdGVfdGltZSgpIGlz
IGEgbm8tb3AgaW4gTkZTLCBzaW5jZSB3ZSBzZXQgU19OT0FUSU1FfFNfTk9DTVRJTUUNCm9uIGFs
bCBpbm9kZXMuDQoNCkNoZWVycw0KICBUcm9uZA0KDQotLSANClRyb25kIE15a2xlYnVzdA0KTGlu
dXggTkZTIGNsaWVudCBtYWludGFpbmVyDQoNCk5ldEFwcA0KVHJvbmQuTXlrbGVidXN0QG5ldGFw
cC5jb20NCnd3dy5uZXRhcHAuY29tDQoNCg==

Re: [PATCH 10/11] nfs: Push file_update_time() into nfs_vm_page_mkwrite()

[Jan Kara]

On Thu 16-02-12 13:50:19, Myklebust, Trond wrote:
> On Thu, 2012-02-16 at 14:46 +0100, Jan Kara wrote:
> > CC: Trond Myklebust <Trond.Myklebust@netapp.com>
> > CC: linux-nfs@vger.kernel.org
> > Signed-off-by: Jan Kara <jack@suse.cz>
> > ---
> >  fs/nfs/file.c |    3 +++
> >  1 files changed, 3 insertions(+), 0 deletions(-)
> > 
> > diff --git a/fs/nfs/file.c b/fs/nfs/file.c
> > index c43a452..2407922 100644
> > --- a/fs/nfs/file.c
> > +++ b/fs/nfs/file.c
> > @@ -525,6 +525,9 @@ static int nfs_vm_page_mkwrite(struct vm_area_struct *vma, struct vm_fault *vmf)
> >  	/* make sure the cache has finished storing the page */
> >  	nfs_fscache_wait_on_page_write(NFS_I(dentry->d_inode), page);
> >  
> > +	/* Update file times before taking page lock */
> > +	file_update_time(filp);
> > +
> >  	lock_page(page);
> >  	mapping = page->mapping;
> >  	if (mapping != dentry->d_inode->i_mapping)
> 
> Hi Jan,
> 
> file_update_time() is a no-op in NFS, since we set S_NOATIME|S_NOCMTIME
> on all inodes.
  Thanks. I've discarded the patch.

								Honza
-- 
Jan Kara <jack@suse.cz>
SUSE Labs, CR