From 3280870dca4cc17ef27025b99f6ecfeb093136af Mon Sep 17 00:00:00 2001 From: Konstantin Belousov Date: Wed, 29 Dec 2010 12:53:53 +0000 Subject: [PATCH] Move the increment of vm object generation count into vm_object_set_writeable_dirty(). Fix an issue where restart of the scan in vm_object_page_clean() did not removed write permissions for newly added pages or, if the mapping for some already scanned page changed to writeable due to fault. Merge the two loops in vm_object_page_clean(), doing the remove of write permission and cleaning in the same loop. The restart of the loop then correctly downgrade writeable mappings. Fix an issue where a second caller to msync() might actually return before the first caller had actually completed flushing the pages. Clear the OBJ_MIGHTBEDIRTY flag after the cleaning loop, not before. Calls to pmap_is_modified() are not needed after pmap_remove_write() there. Proposed, reviewed and tested by: alc MFC after: 1 week --- sys/vm/vm_object.c | 65 ++++++++++++++++++++++++---------------------- sys/vm/vm_page.c | 1 - 2 files changed, 34 insertions(+), 32 deletions(-) diff --git a/sys/vm/vm_object.c b/sys/vm/vm_object.c index 9e5d15918d0c..b7ddf601797f 100644 --- a/sys/vm/vm_object.c +++ b/sys/vm/vm_object.c @@ -101,7 +101,9 @@ SYSCTL_INT(_vm, OID_AUTO, old_msync, CTLFLAG_RW, &old_msync, 0, "Use old (insecure) msync behavior"); static int vm_object_page_collect_flush(vm_object_t object, vm_page_t p, - int pagerflags); + int pagerflags, int flags, int *clearobjflags); +static boolean_t vm_object_page_remove_write(vm_page_t p, int flags, + int *clearobjflags); static void vm_object_qcollapse(vm_object_t object); static void vm_object_vndeallocate(vm_object_t object); @@ -753,6 +755,24 @@ vm_object_terminate(vm_object_t object) vm_object_destroy(object); } +static boolean_t +vm_object_page_remove_write(vm_page_t p, int flags, int *clearobjflags) +{ + + /* + * If we have been asked to skip nosync pages and this is a + * nosync page, skip it. Note that the object flags were not + * cleared in this case so we do not have to set them. + */ + if ((flags & OBJPC_NOSYNC) != 0 && (p->oflags & VPO_NOSYNC) != 0) { + *clearobjflags = 0; + return (FALSE); + } else { + pmap_remove_write(p); + return (p->dirty != 0); + } +} + /* * vm_object_page_clean * @@ -799,17 +819,6 @@ vm_object_page_clean(vm_object_t object, vm_pindex_t start, vm_pindex_t end, * object flags. */ clearobjflags = 1; - for (p = vm_page_find_least(object, start); - p != NULL && p->pindex < tend; p = TAILQ_NEXT(p, listq)) { - if ((flags & OBJPC_NOSYNC) != 0 && - (p->oflags & VPO_NOSYNC) != 0) - clearobjflags = 0; - else - pmap_remove_write(p); - } - - if (clearobjflags && (start == 0) && (tend == object->size)) - vm_object_clear_flag(object, OBJ_MIGHTBEDIRTY); rescan: curgeneration = object->generation; @@ -827,20 +836,11 @@ vm_object_page_clean(vm_object_t object, vm_pindex_t start, vm_pindex_t end, np = vm_page_find_least(object, pi); continue; } - vm_page_test_dirty(p); - if (p->dirty == 0) + if (!vm_object_page_remove_write(p, flags, &clearobjflags)) continue; - /* - * If we have been asked to skip nosync pages and this is a - * nosync page, skip it. Note that the object flags were - * not cleared in this case so we do not have to set them. - */ - if ((flags & OBJPC_NOSYNC) != 0 && - (p->oflags & VPO_NOSYNC) != 0) - continue; - - n = vm_object_page_collect_flush(object, p, pagerflags); + n = vm_object_page_collect_flush(object, p, pagerflags, + flags, &clearobjflags); if (object->generation != curgeneration) goto rescan; np = vm_page_find_least(object, pi + n); @@ -850,10 +850,13 @@ vm_object_page_clean(vm_object_t object, vm_pindex_t start, vm_pindex_t end, #endif vm_object_clear_flag(object, OBJ_CLEANING); + if (clearobjflags && start == 0 && tend == object->size) + vm_object_clear_flag(object, OBJ_MIGHTBEDIRTY); } static int -vm_object_page_collect_flush(vm_object_t object, vm_page_t p, int pagerflags) +vm_object_page_collect_flush(vm_object_t object, vm_page_t p, int pagerflags, + int flags, int *clearobjflags) { vm_page_t ma[vm_pageout_page_count], p_first, tp; int count, i, mreq, runlen; @@ -869,8 +872,7 @@ vm_object_page_collect_flush(vm_object_t object, vm_page_t p, int pagerflags) tp = vm_page_next(tp); if (tp == NULL || tp->busy != 0 || (tp->oflags & VPO_BUSY) != 0) break; - vm_page_test_dirty(tp); - if (tp->dirty == 0) + if (!vm_object_page_remove_write(tp, flags, clearobjflags)) break; } @@ -878,8 +880,7 @@ vm_object_page_collect_flush(vm_object_t object, vm_page_t p, int pagerflags) tp = vm_page_prev(p_first); if (tp == NULL || tp->busy != 0 || (tp->oflags & VPO_BUSY) != 0) break; - vm_page_test_dirty(tp); - if (tp->dirty == 0) + if (!vm_object_page_remove_write(tp, flags, clearobjflags)) break; p_first = tp; mreq++; @@ -1985,8 +1986,10 @@ vm_object_set_writeable_dirty(vm_object_t object) { VM_OBJECT_LOCK_ASSERT(object, MA_OWNED); - if (object->type != OBJT_VNODE || - (object->flags & OBJ_MIGHTBEDIRTY) != 0) + if (object->type != OBJT_VNODE) + return; + object->generation++; + if ((object->flags & OBJ_MIGHTBEDIRTY) != 0) return; vm_object_set_flag(object, OBJ_MIGHTBEDIRTY); } diff --git a/sys/vm/vm_page.c b/sys/vm/vm_page.c index e708a5706fcd..5afa622b46f2 100644 --- a/sys/vm/vm_page.c +++ b/sys/vm/vm_page.c @@ -811,7 +811,6 @@ vm_page_insert(vm_page_t m, vm_object_t object, vm_pindex_t pindex) } } object->root = m; - object->generation++; /* * show that the object has one more resident page.