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'

Thanks a lot!
/bz


>  Reviewed by: attilio, kib
> 
> Modified:
>  head/sys/vm/vm_fault.c
>  head/sys/vm/vm_page.c
>  head/sys/vm/vm_page.h
> 
> 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
>               }
>       }
>       if (pmap_failed) {
> 
> 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)
> @@ -729,7 +729,12 @@ vm_page_sleep(vm_page_t m, const char *m
> /*
>  *    vm_page_dirty:
>  *
> - *   make page all dirty
> + *   Set all bits in the page's dirty field.
> + *
> + *   The object containing the specified page must be locked if the call is
> + *   made from the machine-independent layer.  If, however, the call is
> + *   made from the pmap layer, then the page queues lock may be required.
> + *   See vm_page_clear_dirty_mask().
>  */
> void
> vm_page_dirty(vm_page_t m)
> @@ -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);
> +#elif PAGE_SIZE == 8192
> +             atomic_clear_short(&m->dirty, pagebits);
> +#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)
> @@ -89,10 +89,26 @@
>  *    and offset to which this page belongs (for pageout),
>  *    and sundry status bits.
>  *
> - *   Fields in this structure are locked either by the lock on the
> - *   object that the page belongs to (O), its corresponding page lock (P),
> - *   or by the lock on the page queues (Q).
> - *   
> + *   In general, operations on this structure's mutable fields are
> + *   synchronized using either one of or a combination of the lock on the
> + *   object that the page belongs to (O), the pool lock for the page (P),
> + *   or the lock for either the free or paging queues (Q).  If a field is
> + *   annotated below with two of these locks, then holding either lock is
> + *   sufficient for read access, but both locks are required for write 
> + *   access.
> + *
> + *   In contrast, the synchronization of accesses to the page's dirty field
> + *   is machine dependent (M).  In the machine-independent layer, the lock
> + *   on the object that the page belongs to must be held in order to
> + *   operate on the field.  However, the pmap layer is permitted to set
> + *   all bits within the field without holding that lock.  Therefore, if
> + *   the underlying architecture does not support atomic read-modify-write
> + *   operations on the field's type, then the machine-independent layer
> + *   must also hold the page queues lock when performing read-modify-write
> + *   operations and the pmap layer must hold the page queues lock when
> + *   setting the field.  In the machine-independent layer, the
> + *   implementation of read-modify-write operations on the field is
> + *   encapsulated in vm_page_clear_dirty_mask().
>  */
> 
> TAILQ_HEAD(pglist, vm_page);
> @@ -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) */
> #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) */
> #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) */
> #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) */
> #endif
> };
> 

-- 
Bjoern A. Zeeb                                 You have to have visions!
         Stop bit received. Insert coin for new address family.

_______________________________________________
svn-src-all@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"

Reply via email to