On Wed, 2006-12-20 at 14:49 -0800, Linus Torvalds wrote:
>
> On Wed, 20 Dec 2006, Peter Zijlstra wrote:
> >
> > I think this is also needed:
>
> Yeah, that looks about right. Although I think it should go above the
> "try_to_release_page()", because right now we do that "ttrp()" with the
> dirty bit set, and we should let the low-level filesystem just know that
> it's simply not interesting any more (and, indeed, "try_to_free_buffers()"
> too, for that matter).
That makes NFS unhappy, see nfs_release_page().
> Anyway, I think that's a detail. I'd rather know whether this all actually
> makes any difference what-so-ever to the corruption behaviour of Andrei
> &co.
Yeah, I have to tinker with my test setup to make it fail again. Maybe I
have to add more seeds, that seemed to make a difference, it was
impossible to trigger with a single seed.
FWIW I also added some scribble past i_size checks in nobh_writepage()
and block_write_full_page().
FWIW2 I straced rtorrent for a bit and it does an aweful lot of mmap
calls and relatively few msync(MS_ASYNC);munmap(), and no truncate apart
from creating sparse files at the beginning.
> Maybe the UP ARM case is some strange dcache alias issue with PIO IDE, and
> the only reason that started showing up at the same time is the different
> IO loads. Who knows.
>
> [ Although I think you may have been on the right track with that dcache
> flushing stuff in "page_mkclean()".. It might not have been quite
> all there, but I think we should go back and look very closely at
> page_mkclean() regardless of any other issues! ]
current version
Signed-off-by: Peter Zijlstra <[EMAIL PROTECTED]>
---
mm/rmap.c | 23 +++++++++++++----------
1 file changed, 13 insertions(+), 10 deletions(-)
Index: linux-2.6/mm/rmap.c
===================================================================
--- linux-2.6.orig/mm/rmap.c
+++ linux-2.6/mm/rmap.c
@@ -432,7 +432,7 @@ static int page_mkclean_one(struct page
{
struct mm_struct *mm = vma->vm_mm;
unsigned long address;
- pte_t *pte, entry;
+ pte_t *pte;
spinlock_t *ptl;
int ret = 0;
@@ -444,17 +444,18 @@ static int page_mkclean_one(struct page
if (!pte)
goto out;
- if (!pte_dirty(*pte) && !pte_write(*pte))
- goto unlock;
+ while (pte_dirty(*pte) || pte_write(*pte)) {
+ pte_t entry;
- entry = ptep_get_and_clear(mm, address, pte);
- entry = pte_mkclean(entry);
- entry = pte_wrprotect(entry);
- ptep_establish(vma, address, pte, entry);
- lazy_mmu_prot_update(entry);
- ret = 1;
+ flush_cache_page(vma, address, pte_pfn(*pte));
+ entry = ptep_clear_flush(vma, address, pte);
+ entry = pte_wrprotect(entry);
+ entry = pte_mkclean(entry);
+ ptep_establish(vma, address, pte, entry);
+ lazy_mmu_prot_update(entry);
+ ret = 1;
+ }
-unlock:
pte_unmap_unlock(pte, ptl);
out:
return ret;
@@ -489,6 +490,8 @@ int page_mkclean(struct page *page)
if (mapping)
ret = page_mkclean_file(mapping, page);
}
+ if (page_test_and_clear_dirty(page))
+ ret = 1;
return ret;
}
> So far, my whole "cancel_dirty_page/clean_page_dirty_for_io" patch has
> really been just a "try to make the code _look_ sane. I don't think we
> have a single report that the patch actually makes any difference yet.
I failed to compile a kernel with that patch (100% iowait and a bunch of
processes stuck in D state), but sysrq-t was borked (only numbers no
symbols) have yet to retry - I noticed you kicked the unwinder?.
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
