Date: Mon, 7 Oct 2002 16:47:03 -0700 (PDT) From: Matthew Dillon <dillon@apollo.backplane.com> To: David Schultz <dschultz@uclink.Berkeley.EDU> Cc: Peter Wemm <peter@wemm.org>, Sean Kelly <smkelly@zombie.org>, hackers@FreeBSD.ORG Subject: Re: swapoff? Message-ID: <200210072347.g97Nl3Zo049415@apollo.backplane.com> 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>
next in thread | previous in thread | raw e-mail | index | archive | help
: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
<dillon@backplane.com>
To Unsubscribe: send mail to majordomo@FreeBSD.org
with "unsubscribe freebsd-hackers" in the body of the message
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?200210072347.g97Nl3Zo049415>