On Thu, 19 Oct 2000, Linus Torvalds wrote:
....
> I think you overlooked the fact that SHM mappings use the page cache, and
> it's ok if such pages are dirty and writable - they will get written out
> by the shm_swap() logic once there are no mappings active any more.
>
> I like the test per se, because I think it's correct for the "normal"
> case of a private page, but I really think those two BUG()'s are not bugs
> at all in general, and we should just remove the two tests.
>
> Comments? Anything I've overlooked?
The primary reason I added the BUG was that if this is valid, it means
that the pte has to be removed from the page tables first with
pte_get_and_clear since it can be modified by the other CPU. Although
this may be safe for shm, I think it's very ugly and inconsistent. I'd
rather make the code transfer the dirty bit to the page struct so that we
*know* there is no information loss.
If the above is correct, then the following patch should do (untested).
Oh, I think I missed adding pte_same in the generic pgtable.h macros, too.
<doh!> I'm willing to take a closer look if you think it's needed.
-ben
diff -urN v2.4.0-test10-pre4/include/asm-generic/pgtable.h work-foo/include/asm-generic/pgtable.h
--- v2.4.0-test10-pre4/include/asm-generic/pgtable.h Fri Oct 20 00:58:03 2000
+++ work-foo/include/asm-generic/pgtable.h Fri Oct 20 01:42:24 2000
@@ -38,4 +38,6 @@
set_pte(ptep, pte_mkdirty(old_pte));
}
+#define pte_same(left,right) (pte_val(left) == pte_val(right))
+
#endif /* _ASM_GENERIC_PGTABLE_H */
diff -urN v2.4.0-test10-pre4/mm/vmscan.c work-foo/mm/vmscan.c
--- v2.4.0-test10-pre4/mm/vmscan.c Fri Oct 20 00:58:04 2000
+++ work-foo/mm/vmscan.c Fri Oct 20 01:43:54 2000
@@ -87,6 +87,13 @@
if (TryLockPage(page))
goto out_failed;
+ /* From this point on, the odds are that we're going to
+ * nuke this pte, so read and clear the pte. This hook
+ * is needed on CPUs which update the accessed and dirty
+ * bits in hardware.
+ */
+ pte = ptep_get_and_clear(page_table);
+
/*
* Is the page already in the swap cache? If so, then
* we can just drop our reference to it without doing
@@ -98,10 +105,6 @@
if (PageSwapCache(page)) {
entry.val = page->index;
swap_duplicate(entry);
- if (pte_dirty(pte))
- BUG();
- if (pte_write(pte))
- BUG();
set_pte(page_table, swp_entry_to_pte(entry));
drop_pte:
UnlockPage(page);
@@ -111,13 +114,6 @@
page_cache_release(page);
goto out_failed;
}
-
- /* From this point on, the odds are that we're going to
- * nuke this pte, so read and clear the pte. This hook
- * is needed on CPUs which update the accessed and dirty
- * bits in hardware.
- */
- pte = ptep_get_and_clear(page_table);
/*
* Is it a clean page? Then it must be recoverable
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
Please read the FAQ at http://www.tux.org/lkml/
This archive was generated by hypermail 2b29 : Mon Oct 23 2000 - 21:00:17 EST