From owner-freebsd-hackers Mon Oct 7 16:54:15 2002 Delivered-To: freebsd-hackers@freebsd.org Received: from mx1.FreeBSD.org (mx1.freebsd.org [216.136.204.125]) by hub.freebsd.org (Postfix) with ESMTP id 2B95037B401 for ; Mon, 7 Oct 2002 16:54:13 -0700 (PDT) Received: from apollo.backplane.com (apollo.backplane.com [216.240.41.2]) by mx1.FreeBSD.org (Postfix) with ESMTP id A7A3A43E3B for ; Mon, 7 Oct 2002 16:54:11 -0700 (PDT) (envelope-from dillon@apollo.backplane.com) Received: from apollo.backplane.com (localhost [127.0.0.1]) by apollo.backplane.com (8.12.5/8.12.4) with ESMTP id g97Nl4PQ049416; Mon, 7 Oct 2002 16:47:04 -0700 (PDT) (envelope-from dillon@apollo.backplane.com) Received: (from dillon@localhost) by apollo.backplane.com (8.12.5/8.12.4/Submit) id g97Nl3Zo049415; Mon, 7 Oct 2002 16:47:03 -0700 (PDT) (envelope-from dillon) Date: Mon, 7 Oct 2002 16:47:03 -0700 (PDT) From: Matthew Dillon Message-Id: <200210072347.g97Nl3Zo049415@apollo.backplane.com> To: David Schultz Cc: Peter Wemm , Sean Kelly , hackers@FreeBSD.ORG Subject: Re: swapoff? References: <20020713071911.GA1558@HAL9000.wox.org> <20020713073404.9869A3811@overcee.wemm.org> <20020713115746.GA2162@HAL9000.wox.org> <200207131636.g6DGaoqh081285@apollo.backplane.com> <20021007153845.GA371@HAL9000.homeunix.com> Sender: owner-freebsd-hackers@FreeBSD.ORG Precedence: bulk List-ID: List-Archive: (Web Archive) List-Help: (List Instructions) List-Subscribe: List-Unsubscribe: X-Loop: FreeBSD.ORG :I'm resurrecting this thread because I finally got around to :finishing up the patches to implement swapoff. I would appreciate :some review of them, particularly to verify that I have done the :right thing WRT synchronization. I have not optimized it to do :read clustering, but I have ensured that such an optimization :could be made. Other than that, I don't know of any deficiencies. This is great, David. The code is about as clean as it's possible to make in a swapoff implementation. There are some minor inefficiencies.. some shortcuts that can be taken in the blist code for example, but I don't think we have to worry about them for this initial implementation. The SW_CLOSING test is an excellent solution to dealing with the swap bitmap when paging in from the dying swap area. The swap_pager_isswapped() function may not be doing a sufficient test: :+ pswap = swp_pager_hash(object, index); :+ :+ if ((swap = *pswap) != NULL) { :+ for (i = 0; i < SWAP_META_PAGES; ++i) { :+ daddr_t v = swap->swb_pages[i]; :+ if (v != SWAPBLK_NONE && :+ BLK2DEVIDX(v) == devidx && :+ !vm_page_lookup(object, swap->swb_index+i)) :+ return 1; :+ } :+ } It is quite possible for a VM page to be present but invalid, meaning that the swap is still valid. You could incorrectly return that the object is not swapped when in fact it is. BUT, since you only appear to be making this call on the process's UPAGES object, there may not be a problem. Perhapss the best thing to do is to not do the vm_page_lookup() call and instead just unconditionally faultin() the uarea if it looks like there might be a problem. You may need a master lock to ensure that only swapon() or swapoff() is 'in progress' at any given moment. The vm_page_grab() call below may block, I think: :+ :+ if (object->type != OBJT_SWAP) :+ panic("swp_pager_force_pagein: object not backed by swap"); :+ :+ m = vm_page_grab(object, pindex, VM_ALLOC_NORMAL | VM_ALLOC_RETRY); :+ if (m->valid == VM_PAGE_BITS_ALL) { :+ /* :+ * The page is already in memory, but must be :+ * dirtied, since we're taking away its backing store. :+ */ :+ vm_page_lock_queues(); :+ vm_page_activate(m); :+ vm_page_dirty(m); :+ vm_page_wakeup(m); :+ vm_page_unlock_queues(); :+ return 1; :+ } :+ :+ vm_object_pip_add(object, 1); I think you may want to do the pip_add before calling vm_page_grab(). -Matt Matthew Dillon To Unsubscribe: send mail to majordomo@FreeBSD.org with "unsubscribe freebsd-hackers" in the body of the message