Re: [PATCH] update ctime and mtime for mmaped write

From: Peter Staubach
Date: Thu Feb 22 2007 - 15:12:13 EST


Miklos Szeredi wrote:
+int set_page_dirty_mapping(struct page *page);
This aspect of the design seems intrusive to me. I didn't see a strong
reason to introduce new versions of many of the routines just to handle
these semantics. What motivated this part of your design? Why the new
_mapping versions of routines?
Because there's no way to know inside the set_page_dirty() functions
if the dirtying comes from a memory mapping or from a modification
through a normal write(). And they have different semantics, for
write() the modification times are updated immediately.
Perhaps I didn't understand what page_mapped() does, but it does seem to
have the right semantics as far as I could see.
The problems will start, when you have a file that is both mapped and
modified with write(). Then the dirying from the write() will set the
flag, and that will have undesirable consequences.
I don't think that I quite follow the logic. The dirtying from write()
will set the flag, but then the mtime will get updated and the flag will
be cleared by the hook in file_update_time(). Right?

Take this example:

fd = open()
addr = mmap(.., fd)
write(fd, ...)
close(fd)
sleep(100)
msync(addr,...)
munmap(addr)

The file times will be updated in write(), but with your patch, the
bit in the mapping will also be set.

Then in msync() the file times will be updated again, which is wrong,
since the memory was _not_ modified through the mapping.

This is correct. I have updated my proposed patch to include the clearing
of AS_MCTIME in the routine which updates the mtime field. I haven't
reposted it yet until I complete testing of the new resulting system. I
anticipate doing this later today.

Thanx..

ps
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/