On Tue, 21 Jun 2011, Bjoern A. Zeeb wrote:
On Jun 19, 2011, at 7:13 PM, Alan Cox wrote:
Hi Alan,
Author: alc
Date: Sun Jun 19 19:13:24 2011
New Revision: 223307
URL: http://svn.freebsd.org/changeset/base/223307
Log:
Precisely document the synchronization rules for the page's dirty field.
(Saying that the lock on the object that the page belongs to must be held
only represents one aspect of the rules.)
Eliminate the use of the page queues lock for atomically performing read-
modify-write operations on the dirty field when the underlying architecture
supports atomic operations on char and short types.
Document the fact that 32KB pages aren't really supported.
contrary to the tinderbox I'd like to point out that all mips kernels built by
universe are broken with a SVN HEAD from earlier today. Could you please check
and see if you can fix it? The errors I get are:
vm_page.o: In function `vm_page_clear_dirty':
/sys/vm/vm_page.c:(.text+0x18d0): undefined reference to `atomic_clear_8'
/sys/vm/vm_page.c:(.text+0x18d0): relocation truncated to fit: R_MIPS_26
against `atomic_clear_8'
vm_page.o: In function `vm_page_set_validclean':
/sys/vm/vm_page.c:(.text+0x38f0): undefined reference to `atomic_clear_8'
/sys/vm/vm_page.c:(.text+0x38f0): relocation truncated to fit: R_MIPS_26
against `atomic_clear_8'
Atomic types shorter than int cannot be used in MI code, since they might
not exist. Apparently they don't exist on mips. jake@ fixed all their
old uses for sparc4 in ~Y2K.
Modified: head/sys/vm/vm_fault.c
==============================================================================
--- head/sys/vm/vm_fault.c Sun Jun 19 18:34:49 2011 (r223306)
+++ head/sys/vm/vm_fault.c Sun Jun 19 19:13:24 2011 (r223307)
@@ -1089,10 +1089,20 @@ vm_fault_quick_hold_pages(vm_map_t map,
* caller's changes may go unnoticed because they are
* performed through an unmanaged mapping or by a DMA
* operation.
+ *
+ * The object lock is not held here. Therefore, like
+ * a pmap operation, the page queues lock may be
+ * required in order to call vm_page_dirty(). See
+ * vm_page_clear_dirty_mask().
*/
+#if defined(__amd64__) || defined(__i386__) || defined(__ia64__) || \
+ defined(__mips__)
+ vm_page_dirty(*mp);
+#else
vm_page_lock_queues();
vm_page_dirty(*mp);
vm_page_unlock_queues();
+#endif
Apparently, it used to work by using a global lock, so it didn't need to
use atomic ops that don't exist because the data sizes are too small.
Modified: head/sys/vm/vm_page.c
==============================================================================
--- head/sys/vm/vm_page.c Sun Jun 19 18:34:49 2011 (r223306)
+++ head/sys/vm/vm_page.c Sun Jun 19 19:13:24 2011 (r223307)
...
@@ -2325,15 +2330,41 @@ vm_page_clear_dirty_mask(vm_page_t m, in
/*
* If the object is locked and the page is neither VPO_BUSY nor
* PG_WRITEABLE, then the page's dirty field cannot possibly be
- * modified by a concurrent pmap operation.
+ * set by a concurrent pmap operation.
*/
VM_OBJECT_LOCK_ASSERT(m->object, MA_OWNED);
if ((m->oflags & VPO_BUSY) == 0 && (m->flags & PG_WRITEABLE) == 0)
m->dirty &= ~pagebits;
else {
+#if defined(__amd64__) || defined(__i386__) || defined(__ia64__) || \
+ defined(__mips__)
+ /*
+ * On the aforementioned architectures, the page queues lock
+ * is not required by the following read-modify-write
+ * operation. The combination of the object's lock and an
+ * atomic operation suffice. Moreover, the pmap layer on
+ * these architectures can call vm_page_dirty() without
+ * holding the page queues lock.
+ */
+#if PAGE_SIZE == 4096
+ atomic_clear_char(&m->dirty, pagebits);
Cannot do that in MI code, though it might work accidentally because all
archies with PAGE_SIZE == 4096 might support it.
+#elif PAGE_SIZE == 8192
+ atomic_clear_short(&m->dirty, pagebits);
Cannot do that...
+#elif PAGE_SIZE == 16384
+ atomic_clear_int(&m->dirty, pagebits);
+#else
+#error "PAGE_SIZE is not supported."
+#endif
+#else
+ /*
+ * Otherwise, the page queues lock is required to ensure that
+ * a concurrent pmap operation does not set the page's dirty
+ * field during the following read-modify-write operation.
+ */
vm_page_lock_queues();
m->dirty &= ~pagebits;
vm_page_unlock_queues();
+#endif
}
}
Modified: head/sys/vm/vm_page.h
==============================================================================
--- head/sys/vm/vm_page.h Sun Jun 19 18:34:49 2011 (r223306)
+++ head/sys/vm/vm_page.h Sun Jun 19 19:13:24 2011 (r223307)
@@ -120,18 +136,19 @@ struct vm_page {
u_char busy; /* page busy count (O) */
/* NOTE that these must support one bit per DEV_BSIZE in a page!!! */
/* so, on normal X86 kernels, they must be at least 8 bits wide */
+ /* In reality, support for 32KB pages is not fully implemented. */
#if PAGE_SIZE == 4096
u_char valid; /* map of valid DEV_BSIZE chunks (O) */
- u_char dirty; /* map of dirty DEV_BSIZE chunks (O) */
+ u_char dirty; /* map of dirty DEV_BSIZE chunks (M) */
A small size may be good for space efficiency, but the struct is not very
well packed, so perhaps the size can be increased without further loss.
Must be >= 8 bits, and u_char guarantees this. u_char might be larger
than 8 bits anyway, but POSIX requires 8-bit chars.
#elif PAGE_SIZE == 8192
u_short valid; /* map of valid DEV_BSIZE chunks (O) */
- u_short dirty; /* map of dirty DEV_BSIZE chunks (O) */
+ u_short dirty; /* map of dirty DEV_BSIZE chunks (M) */
Must be >= 16 bits. I'm not sure if POSIX requires exactly 16.
#elif PAGE_SIZE == 16384
u_int valid; /* map of valid DEV_BSIZE chunks (O) */
- u_int dirty; /* map of dirty DEV_BSIZE chunks (O) */
+ u_int dirty; /* map of dirty DEV_BSIZE chunks (M) */
Must be >= 32 bits. POSIX requires >= 32, but C doesn't. I'm not
sure if POSIX requires exactly 16.
#elif PAGE_SIZE == 32768
u_long valid; /* map of valid DEV_BSIZE chunks (O) */
- u_long dirty; /* map of dirty DEV_BSIZE chunks (O) */
+ u_long dirty; /* map of dirty DEV_BSIZE chunks (M) */
Must be >= 64 bits. u_long certainly doesn't guarantees this, but does
in practice on all arches that support this page size, since no arches
support this page size :-). Should use uintN_t for this if not the others.
uintN_t is more clearly unportable. It only exists for all the usual N
because arches are not very variant, and then even then only for non-atomic
ops.
#endif
};
Bruce
_______________________________________________
svn-src-head@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"
