Date: Sun, 16 Aug 1998 06:15:30 -0700 From: David Greenman <dg@root.com> To: Terry Lambert <tlambert@primenet.com> Cc: current@FreeBSD.ORG, karl@mcs.net Subject: Re: Better VM patches (was Tentative fix for VM bug) Message-ID: <199808161315.GAA00184@implode.root.com> In-Reply-To: Your message of "Sun, 16 Aug 1998 01:13:33 -0000." <199808160113.SAA17303@usr07.primenet.com>
next in thread | previous in thread | raw e-mail | index | archive | help
>The problem with vm_object_page_remove() is that in the clean_only != 0 >case, the page is marked invalid. > >This will result in orphaned pages. I don't think "orphaned" pages will be created, but I can imagine that changes might be lost in the wired && clean_only case. I agree that this appears to be a bug. It might be a bug in the wired && !clean_only case, too - in other words, setting valid=0 might never be correct for wired pages. >This may be the source of the: > > * XXX: sometimes we get pages which aren't wired down or on any queue - > * we need to put them on the inactive queue also, otherwise we lose > * track of them. Paul Mackerras (paulus@cs.anu.edu.au) 9-Jan-93. > >comment in vm_page.c, and it may be the source of the zero page >phenomenon. That comment simply means what it says and must be taken in the context of the function where it appears (vm_page_deactivate). The system sometimes needs to put a page on a queue - any queue, and vm_page_deactivate needs to be able to handle the case of not-on-any-queue. Really, the comment is obsolete and dates back to a time when the VM system didn't manage the page queues properly. >Another problem in the mmap code is that when the vnode_pager object >is created, it is set to be the number of pages times the page size >in length; but also in vnode_pager, the setsize on the object sets >the length to the actual file length. > >This discrepancy opens a window in which some changes may be lost. My reading of the code shows that the object size is set to the rounded-up index. vnp_size is set to the unrounded new size, but that's expected and I don't see any bugs related to doing that; rounding it will likely break the proper reading of partial pages. Am I missing something? vnp_size is mostly private to the vnode_pager. The rounded-up object->size is used outside of the vnode pager. >John also suggested another change for detecting a case where page >orphans can be created. in vm_page.c, it's possible that an object >entry in the page insert function will overwrite an existing entry; >I added a DIAGNOSTIC panic to catch this when it happens. You mean that there can be multiple pages at the same offset? It would be bad if that happend, but I'm skeptical that it actually does. >*************** >*** 990,995 **** >--- 990,1002 ---- > handle, OFF_TO_IDX(objsize), prot, foff); > if (object == NULL) > return (type == OBJT_DEVICE ? EINVAL : ENOMEM); >+ /* >+ * XXX we are in a race here. If the file is extended >+ * between the time we get the object and the time >+ * we set the size, then we lose... >+ */ >+ if (!(flags & MAP_ANON) && vp->v_type != VCHR) >+ object->un_pager.vnp.vnp_size = vat.va_size; Hmmm. >+ /* >+ * The object is allocated to a page boundary. This is >+ * incorrect if page is only partially backed by the vp; >+ * we must fix this up in the caller. >+ */ > object->un_pager.vnp.vnp_size = (vm_ooffset_t) size * PAGE_SIZE; I think the above change is wrong. -DG David Greenman Co-founder/Principal Architect, The FreeBSD Project To Unsubscribe: send mail to majordomo@FreeBSD.org with "unsubscribe freebsd-current" in the body of the message
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?199808161315.GAA00184>