That sounds like a good idea, but it doesn't work.
The thing is, we need to hold the page-table lock over the whole sequence
of
if (page_mkclean(page))
set_page_dirty(page);
if (TestClearPageDirty(page))
..
and there's a big comment about why in clear_page_dirty_for_io().
So if you split it up, so that the first phase is that
if (page_mkclean(page))
set_page_dirty(page);
and the second phase is the one that just does a
if (TestClearPageDirty(page))
writeback(..)
and having dropped the page lock in between, then you lose: because
another thread migth have faulted in and re-dirtied the page table entry,
and you MUST NOT do that "TestClearPageDirty()" in that case!
That dirty bit handling is really really important, and it's sadly also
really really easy to get wrong (usually in ways that are hard to even
notice: things still work 99% of the time, and you might just be leaking
memory slowly, and fsync/msync() might not write back memory mapped data
to disk at all etc).
Well, the plain added "file_update_time()" call addition looked like a
trivial fix, and if there are actually *customers* that have bad backups
due to this, then I think that part was worth doing. At least a "sync"
will then sync the file times...
Linus
--
