Re: [PATCH -v8 3/4] Enable the MS_ASYNC functionality in sys_msync()

!MAILaRCHIVE_VOTE_RePLACE
Previous message: [thread] [date] [author]
Next message: [thread] [date] [author]
To: Miklos Szeredi <miklos@...>
Cc: <a.p.zijlstra@...>, <salikhmetov@...>, <linux-mm@...>, <jakob@...>, <linux-kernel@...>, <valdis.kletnieks@...>, <riel@...>, <ksm@...>, <staubach@...>, <jesper.juhl@...>, <akpm@...>, <protasnb@...>, <r.e.wolff@...>, <hidave.darkstar@...>, <hch@...>
Date: Wednesday, January 23, 2008 - 5:36 pm

On Wed, 23 Jan 2008, Miklos Szeredi wrote:

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
--
Previous message: [thread] [date] [author]
Next message: [thread] [date] [author]

Messages in current thread:
[PATCH -v8 0/4] Fixing the issue with memory-mapped file times, Anton Salikhmetov, (Tue Jan 22, 7:21 pm)
[PATCH -v8 4/4] The design document for memory-mapped file t..., Anton Salikhmetov, (Tue Jan 22, 7:21 pm)
Re: [PATCH -v8 4/4] The design document for memory-mapped fi..., Anton Salikhmetov, (Fri Jan 25, 12:40 pm)
Re: [PATCH -v8 4/4] The design document for memory-mapped fi..., Anton Salikhmetov, (Wed Jan 23, 6:37 am)
Re: [PATCH -v8 4/4] The design document for memory-mapped fi..., Anton Salikhmetov, (Wed Jan 23, 8:25 am)
[PATCH -v8 1/4] Massive code cleanup of sys_msync(), Anton Salikhmetov, (Tue Jan 22, 7:21 pm)
[PATCH -v8 3/4] Enable the MS_ASYNC functionality in sys_msy..., Anton Salikhmetov, (Tue Jan 22, 7:21 pm)
Re: [PATCH -v8 3/4] Enable the MS_ASYNC functionality in sys..., Linus Torvalds, (Wed Jan 23, 5:36 pm)
Re: [PATCH -v8 3/4] Enable the MS_ASYNC functionality in sys..., Anton Salikhmetov, (Wed Jan 23, 1:26 pm)
Re: [PATCH -v8 3/4] Enable the MS_ASYNC functionality in sys..., Anton Salikhmetov, (Wed Jan 23, 8:53 am)
Re: [PATCH -v8 3/4] Enable the MS_ASYNC functionality in sys..., Anton Salikhmetov, (Wed Jan 23, 9:09 am)
[PATCH -v8 2/4] Update ctime and mtime for memory-mapped files, Anton Salikhmetov, (Tue Jan 22, 7:21 pm)
Re: [PATCH -v8 2/4] Update ctime and mtime for memory-mapped..., Anton Salikhmetov, (Wed Jan 23, 7:14 pm)