Date: Sun, 27 Sep 1998 22:40:57 +0000 (GMT) From: Terry Lambert <tlambert@primenet.com> To: current@FreeBSD.ORG Subject: VM mmap file extension bug still exists? Message-ID: <199809272240.PAA02513@usr05.primenet.com>
next in thread | raw e-mail | index | archive | help
I don't think this was committed?
Case 1:
On a system with a coherent VM and buffer cache, the use of msync(2)
is supposed to cause dirty pages in a VM object backed by a file
to be written out.
Case 2:
On a system without a unified VM and buffer cache, the msync is
intended to additionally ensure the coherence between the VM
and buffer cache data for a file being accessed simultaneously,
but serially, via mmap and normal file I/O, or not serially,
if the mapping imposes write-through caching semantics on writes
via the ampping (i.e., the mmap() flag MAP_HASSEMAPHORE is used).
Discussion:
Given this, FreeBSD definitionally should not require an msync(2)
for the INN server to operate properly, since the intent of the
compilation flag for INN, stating that msync(2) is needed, is
intended to address only case 2, above.
FreeBSD isn't supposed to have a case 2 requirement, because it
has a unified VM and buffer cache.
The problem is that the backing object is thought, by the kernel,
to actually be larger than it really is. When an extension occurs
to the file, up to and including the page boundary, the VM
object contents are not updated because the size is less than
the size of the in-core object that the kernel believes to be there.
The cannonically correct fix for this problem is to add a vm_ooffset_t
"actual_size_in_bytes" argument to all of the generic pager_alloc alloc
code and the prototype for the pager alloc function member of the
"struct pagerops" vnode_pager_alloc(), then in vnode_pager_alloc(),
change:
object->un_pager.vnp.vnp_size = (vm_ooffset_t) size * PAGE_SIZE;
to:
object->un_pager.vnp.vnp_size = actual_size_in_bytes;
Here is my patch to work around the problem until someone who can
expend the time to touch all of the pagers for the cannonically
correct fix actually gets around to doing the work.
The race mentioned in the comments is impossible, except under very
low memory conditions, and FreeBSD already barfs there. This will
at least fix the most common case.
Someone please review and commit.
Terry Lambert
terry@lambert.org
---
Any opinions in this posting are my own and not those of my present
or previous employers.
-------------------------------------------------------------------------
*** vm_mmap.c Wed Sep 9 01:39:04 1998
--- vm_mmap.c.new Wed Sep 9 01:39:06 1998
***************
*** 929,934 ****
--- 929,935 ----
vm_ooffset_t objsize;
int docow;
struct proc *p = curproc;
+ struct vattr vat;
if (size == 0)
return (0);
***************
*** 972,978 ****
type = OBJT_DEVICE;
handle = (void *)vp->v_rdev;
} else {
- struct vattr vat;
int error;
error = VOP_GETATTR(vp, &vat, p->p_ucred, p);
--- 973,978 ----
***************
*** 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;
}
/*
*** vnode_pager.c Mon Mar 16 01:56:03 1998
--- vnode_pager.c.new Sun Aug 16 00:05:12 1998
***************
*** 137,142 ****
--- 137,147 ----
object = vm_object_allocate(OBJT_VNODE, size);
object->flags = 0;
+ /*
+ * 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;
object->handle = handle;
-------------------------------------------------------------------------
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?199809272240.PAA02513>