Hi Nick,
I'm still working on the bug in
http://bugzilla.kernel.org/show_bug.cgi?id=12913 . One other source of
grief appears to be munmap(), which is calling set_page_dirty() on a
number of pages without locking them or first calling page_mkwrite().
Currently, this means that we either ignore that dirty bit (since
nfs_page_async_flush() won't find a corresponding write request) or it
too can end up triggering the PG_CLEAN BUG() in fs/nfs/write.c:252 if
the timing is right.
So what is the reason why zap_pte_range() calls set_page_dirty()
directly?
Cheers
Trond
On Thu, 23 Apr 2009, Trond Myklebust wrote:
> On Thu, 2009-04-23 at 21:52 +0200, Miklos Szeredi wrote:
> > Now this is mostly done at page fault time, and the pte's are always
> > being re-protected whenever the PG_dirty flag is cleared (see
> > page_mkclean()).
> >
> > But in some cases (shmfs being the example I know) pages are not write
> > protected and so zap_pte_range(), and other functions, still need to
> > transfer the pte dirtyness to the page flag.
>
> My main worry is that this is all happening at munmap() time. There
> shouldn't be any more page faults after that completes (am I right?), so
> what other mechanism would transfer the pte dirtyness?
After munmap() a page fault will result in SIGSEGV. A write access
during munmap(), when the vma has been removed but the page table is
still intact is more interesting. But in that case the write fault
should also result in a SEGV, because it won't be able to find the
matching VMA.
Now lets see what happens if writeback is started against the page
during this limbo period. page_mkclean() is called, which doesn't
find the vma, so it doesn't re-protect the pte. But the PG_dirty will
be cleared regadless. So AFAICS it can happen that the pte remains
dirty but the page is clean.
And in that case that set_page_dirty() in zap_pte_range() is
important, since the page could have been dirtied through the mapping
after the writeback finished.
> > Not sure how this matters to NFS though. If the above is correct,
> > then the set_page_dirty() call in zap_pte_range() should always result
> > in a no-op, since the PG_dirty flag would already have been set by the
> > page fault...
>
> If I can ignore the dirty flag on these occasions, then that would be
> great. That would enable me to get rid of that BUG_ON(PG_CLEAN) in
> write.c, and close the bug...
I don't think you can ignore the dirty flag...
Hmm, I guess this is a bit nasty: the VM promises filesystems that
->page_mkwrite() will be called when the page is dirtied through a
mapping, _almost_ all of the time. Except when munmap happens to race
with clear_page_dirty_for_io().
I don't have any ideas how this could be fixed, CC-ing linux-mm...
Miklos
On Thu, 23 Apr 2009, Trond Myklebust wrote:
> I'm still working on the bug in
> http://bugzilla.kernel.org/show_bug.cgi?id=12913 . One other source of
> grief appears to be munmap(), which is calling set_page_dirty() on a
> number of pages without locking them or first calling page_mkwrite().
>
> Currently, this means that we either ignore that dirty bit (since
> nfs_page_async_flush() won't find a corresponding write request) or it
> too can end up triggering the PG_CLEAN BUG() in fs/nfs/write.c:252 if
> the timing is right.
>
> So what is the reason why zap_pte_range() calls set_page_dirty()
> directly?
In the old times this was one of the main ways of transferring the pte
dirtyness to the PG_dirty page flag.
Now this is mostly done at page fault time, and the pte's are always
being re-protected whenever the PG_dirty flag is cleared (see
page_mkclean()).
But in some cases (shmfs being the example I know) pages are not write
protected and so zap_pte_range(), and other functions, still need to
transfer the pte dirtyness to the page flag.
Not sure how this matters to NFS though. If the above is correct,
then the set_page_dirty() call in zap_pte_range() should always result
in a no-op, since the PG_dirty flag would already have been set by the
page fault...
Miklos
On Thu, 2009-04-23 at 21:52 +0200, Miklos Szeredi wrote:
> On Thu, 23 Apr 2009, Trond Myklebust wrote:
> > I'm still working on the bug in
> > http://bugzilla.kernel.org/show_bug.cgi?id=12913 . One other source of
> > grief appears to be munmap(), which is calling set_page_dirty() on a
> > number of pages without locking them or first calling page_mkwrite().
> >
> > Currently, this means that we either ignore that dirty bit (since
> > nfs_page_async_flush() won't find a corresponding write request) or it
> > too can end up triggering the PG_CLEAN BUG() in fs/nfs/write.c:252 if
> > the timing is right.
> >
> > So what is the reason why zap_pte_range() calls set_page_dirty()
> > directly?
>
> In the old times this was one of the main ways of transferring the pte
> dirtyness to the PG_dirty page flag.
>
> Now this is mostly done at page fault time, and the pte's are always
> being re-protected whenever the PG_dirty flag is cleared (see
> page_mkclean()).
>
> But in some cases (shmfs being the example I know) pages are not write
> protected and so zap_pte_range(), and other functions, still need to
> transfer the pte dirtyness to the page flag.
My main worry is that this is all happening at munmap() time. There
shouldn't be any more page faults after that completes (am I right?), so
what other mechanism would transfer the pte dirtyness?
> Not sure how this matters to NFS though. If the above is correct,
> then the set_page_dirty() call in zap_pte_range() should always result
> in a no-op, since the PG_dirty flag would already have been set by the
> page fault...
If I can ignore the dirty flag on these occasions, then that would be
great. That would enable me to get rid of that BUG_ON(PG_CLEAN) in
write.c, and close the bug...
Cheers
Trond