lkml.org 
[lkml]   [2000]   [Oct]   [20]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
Date
From
SubjectRe: oopses in test10-pre4 (was Re: [RFC] atomic pte updates and pae changes, take 3)
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/

\
 
 \ /
  Last update: 2005-03-22 12:41    [W:0.076 / U:1.564 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site