Re: [RFC PATCH 2/4] mm: Update file times when inodes are written after mmaped writes

[Jan Kara <jack@suse.cz>]

On Fri 21-12-12 12:12:46, Dave Chinner wrote:
> > > > 2. The current behavior is surprising -- the timestamp resulting from
> > > >    an mmaped write will be before the write, not after.  This contradicts
> > > >    the mmap(2) manpage, which says:
> > > > 
> > > >      The st_ctime and st_mtime field for a file mapped with  PROT_WRITE  and
> > > >      MAP_SHARED  will  be  updated  after  a write to the mapped region, and
> > > >      before a subsequent msync(2) with the MS_SYNC or MS_ASYNC flag, if  one
> > > >      occurs.
> > > 
> > > What you propose (time updates in do_writepages()) violates this.
> > > msync(MS_ASYNC) doesn't actually start any IO, therefore the
> > > timestamp wil not be updated.
> > > 
> > > Besides, what POSIX actually says is:
> > > 
> > > | The st_ctime and st_mtime fields of a file that is mapped with
> > > | MAP_SHARED and PROT_WRITE shall be marked for update at some point
> > > | in the interval between a write reference to the mapped region and
> > > | the next call to msync() with MS_ASYNC or MS_SYNC for that portion
> > > | of the file by any process.
> > > 
> > > Which means updating the timestamp during the first write is
> > > perfectly acceptible. Indeed, by definition, we are compliant with
> > > the man page because the update is after the write has occurred.
> > > That is, the write triggered the page fault, so the page fault
> > > processing where we update the timestamps is definitely after the
> > > write occurred. :)
> >   Well, but there can be more writes to the already write faulted page.
> > They can come seconds after we called ->page_mkwrite() and thus updated
> > time stamps. So Andy is correct we violate the spec AFAICT.
> 
> Depends how you read it. It can be updated at *any time* between the
> write and the msync() call, which is exactly what happens right now.
> The fact that second and subsequent writes between the first write
> and the msync call do not change it is irrelevant, as the first one
> is the one that matters... Indeed, if you read to the letter of the
> posix definition, then updating timestamps in the msync call is also
> incorrect, because that is not between the write and the msync()
> call.
> 
> What I'm saying is saying the current behaviour is wrong is
> dependent on a specific intepretation of the standard, and the same
> arguments can be made against this proposal. Hence such arguments
> are not a convincing/compelling reason to change behaviours.
  I have to say I'm not following you :) If I have the program that does:

  fd = open("file", O_RDWR);
  addr = mmap(NULL, 4096, PROT_WRITE, MAP_SHARED, fd, 0);
  addr[0] = 'a';
  sleep(1);
  addr[1] = 'b';
  close(fd);

  Then application of the spec to the second write clearly states that time
stamps should be updated sometime after the write of 'b'. I don't see any
space for other interpretation there... And currently we update time stamps
only at the moment we write 'a'.

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