From 4259905d3171a4cdd087436ff1c17eed74830b60 Mon Sep 17 00:00:00 2001 From: kib Date: Sat, 4 Aug 2012 18:16:43 +0000 Subject: Reduce code duplication and exposure of direct access to struct vm_page oflags by providing helper function vm_page_readahead_finish(), which handles completed reads for pages with indexes other then the requested one, for VOP_GETPAGES(). Reviewed by: alc MFC after: 1 week --- sys/fs/nfsclient/nfs_clbio.c | 32 ++------------------------------ sys/fs/nwfs/nwfs_io.c | 32 ++------------------------------ sys/fs/smbfs/smbfs_io.c | 32 ++------------------------------ sys/nfsclient/nfs_bio.c | 32 ++------------------------------ sys/vm/vm_page.c | 33 +++++++++++++++++++++++++++++++++ sys/vm/vm_page.h | 1 + sys/vm/vnode_pager.c | 33 ++------------------------------- 7 files changed, 44 insertions(+), 151 deletions(-) diff --git a/sys/fs/nfsclient/nfs_clbio.c b/sys/fs/nfsclient/nfs_clbio.c index f7af6fb..d71aeea 100644 --- a/sys/fs/nfsclient/nfs_clbio.c +++ b/sys/fs/nfsclient/nfs_clbio.c @@ -223,36 +223,8 @@ ncl_getpages(struct vop_getpages_args *ap) */ ; } - if (i != ap->a_reqpage) { - /* - * Whether or not to leave the page activated is up in - * the air, but we should put the page on a page queue - * somewhere (it already is in the object). Result: - * It appears that emperical results show that - * deactivating pages is best. - */ - - /* - * Just in case someone was asking for this page we - * now tell them that it is ok to use. - */ - if (!error) { - if (m->oflags & VPO_WANTED) { - vm_page_lock(m); - vm_page_activate(m); - vm_page_unlock(m); - } else { - vm_page_lock(m); - vm_page_deactivate(m); - vm_page_unlock(m); - } - vm_page_wakeup(m); - } else { - vm_page_lock(m); - vm_page_free(m); - vm_page_unlock(m); - } - } + if (i != ap->a_reqpage) + vm_page_readahead_finish(m, error); } VM_OBJECT_UNLOCK(object); return (0); diff --git a/sys/fs/nwfs/nwfs_io.c b/sys/fs/nwfs/nwfs_io.c index a7844b3..220fb9e 100644 --- a/sys/fs/nwfs/nwfs_io.c +++ b/sys/fs/nwfs/nwfs_io.c @@ -457,36 +457,8 @@ nwfs_getpages(ap) ("nwfs_getpages: page %p is dirty", m)); } - if (i != ap->a_reqpage) { - /* - * Whether or not to leave the page activated is up in - * the air, but we should put the page on a page queue - * somewhere (it already is in the object). Result: - * It appears that emperical results show that - * deactivating pages is best. - */ - - /* - * Just in case someone was asking for this page we - * now tell them that it is ok to use. - */ - if (!error) { - if (m->oflags & VPO_WANTED) { - vm_page_lock(m); - vm_page_activate(m); - vm_page_unlock(m); - } else { - vm_page_lock(m); - vm_page_deactivate(m); - vm_page_unlock(m); - } - vm_page_wakeup(m); - } else { - vm_page_lock(m); - vm_page_free(m); - vm_page_unlock(m); - } - } + if (i != ap->a_reqpage) + vm_page_readahead_finish(m, error); } VM_OBJECT_UNLOCK(object); return 0; diff --git a/sys/fs/smbfs/smbfs_io.c b/sys/fs/smbfs/smbfs_io.c index d4a4262..b581a98 100644 --- a/sys/fs/smbfs/smbfs_io.c +++ b/sys/fs/smbfs/smbfs_io.c @@ -521,36 +521,8 @@ smbfs_getpages(ap) ; } - if (i != reqpage) { - /* - * Whether or not to leave the page activated is up in - * the air, but we should put the page on a page queue - * somewhere (it already is in the object). Result: - * It appears that emperical results show that - * deactivating pages is best. - */ - - /* - * Just in case someone was asking for this page we - * now tell them that it is ok to use. - */ - if (!error) { - if (m->oflags & VPO_WANTED) { - vm_page_lock(m); - vm_page_activate(m); - vm_page_unlock(m); - } else { - vm_page_lock(m); - vm_page_deactivate(m); - vm_page_unlock(m); - } - vm_page_wakeup(m); - } else { - vm_page_lock(m); - vm_page_free(m); - vm_page_unlock(m); - } - } + if (i != reqpage) + vm_page_readahead_finish(m, error); } VM_OBJECT_UNLOCK(object); return 0; diff --git a/sys/nfsclient/nfs_bio.c b/sys/nfsclient/nfs_bio.c index 2ffd822..568f990 100644 --- a/sys/nfsclient/nfs_bio.c +++ b/sys/nfsclient/nfs_bio.c @@ -217,36 +217,8 @@ nfs_getpages(struct vop_getpages_args *ap) */ ; } - if (i != ap->a_reqpage) { - /* - * Whether or not to leave the page activated is up in - * the air, but we should put the page on a page queue - * somewhere (it already is in the object). Result: - * It appears that emperical results show that - * deactivating pages is best. - */ - - /* - * Just in case someone was asking for this page we - * now tell them that it is ok to use. - */ - if (!error) { - if (m->oflags & VPO_WANTED) { - vm_page_lock(m); - vm_page_activate(m); - vm_page_unlock(m); - } else { - vm_page_lock(m); - vm_page_deactivate(m); - vm_page_unlock(m); - } - vm_page_wakeup(m); - } else { - vm_page_lock(m); - vm_page_free(m); - vm_page_unlock(m); - } - } + if (i != ap->a_reqpage) + vm_page_readahead_finish(m, error); } VM_OBJECT_UNLOCK(object); return (0); diff --git a/sys/vm/vm_page.c b/sys/vm/vm_page.c index 53353e9..b679290 100644 --- a/sys/vm/vm_page.c +++ b/sys/vm/vm_page.c @@ -695,6 +695,39 @@ vm_page_free_zero(vm_page_t m) } /* + * Unbusy and handle the page queueing for a page from the VOP_GETPAGES() + * array which is not the request page. + */ +void +vm_page_readahead_finish(vm_page_t m, int error) +{ + + if (error == 0) { + /* + * Since the page is not the requested page, whether + * it should be activated or deactivated is not + * obvious. Empirical results have shown that + * deactivating the page is usually the best choice, + * unless the page is wanted by another thread. + */ + if (m->oflags & VPO_WANTED) { + vm_page_lock(m); + vm_page_activate(m); + vm_page_unlock(m); + } else { + vm_page_lock(m); + vm_page_deactivate(m); + vm_page_unlock(m); + } + vm_page_wakeup(m); + } else { + vm_page_lock(m); + vm_page_free(m); + vm_page_unlock(m); + } +} + +/* * vm_page_sleep: * * Sleep and release the page and page queues locks. diff --git a/sys/vm/vm_page.h b/sys/vm/vm_page.h index 5b68e06..f9d1e97 100644 --- a/sys/vm/vm_page.h +++ b/sys/vm/vm_page.h @@ -390,6 +390,7 @@ vm_page_t vm_page_next(vm_page_t m); int vm_page_pa_tryrelock(pmap_t, vm_paddr_t, vm_paddr_t *); vm_page_t vm_page_prev(vm_page_t m); void vm_page_putfake(vm_page_t m); +void vm_page_readahead_finish(vm_page_t m, int error); void vm_page_reference(vm_page_t m); void vm_page_remove (vm_page_t); void vm_page_rename (vm_page_t, vm_object_t, vm_pindex_t); diff --git a/sys/vm/vnode_pager.c b/sys/vm/vnode_pager.c index 9c12be1..b51a500 100644 --- a/sys/vm/vnode_pager.c +++ b/sys/vm/vnode_pager.c @@ -985,37 +985,8 @@ vnode_pager_generic_getpages(vp, m, bytecount, reqpage) mt)); } - if (i != reqpage) { - - /* - * whether or not to leave the page activated is up in - * the air, but we should put the page on a page queue - * somewhere. (it already is in the object). Result: - * It appears that empirical results show that - * deactivating pages is best. - */ - - /* - * just in case someone was asking for this page we - * now tell them that it is ok to use - */ - if (!error) { - if (mt->oflags & VPO_WANTED) { - vm_page_lock(mt); - vm_page_activate(mt); - vm_page_unlock(mt); - } else { - vm_page_lock(mt); - vm_page_deactivate(mt); - vm_page_unlock(mt); - } - vm_page_wakeup(mt); - } else { - vm_page_lock(mt); - vm_page_free(mt); - vm_page_unlock(mt); - } - } + if (i != reqpage) + vm_page_readahead_finish(mt, error); } VM_OBJECT_UNLOCK(object); if (error) { -- cgit v1.1