Date: Fri, 9 Oct 2009 23:58:42 +0300 From: Gleb Kurtsou <gleb.kurtsou@gmail.com> To: Yoshihiro Ota <ota@j.email.ne.jp> Cc: svn-src-head@freebsd.org, svn-src-all@freebsd.org, src-committers@freebsd.org, Xin LI <delphij@FreeBSD.org> Subject: Re: svn commit: r197850 - head/sys/fs/tmpfs Message-ID: <20091009205842.GA3808@tops> In-Reply-To: <20091009154924.689f21e1.ota@j.email.ne.jp> References: <200910072317.n97NHFKL002393@svn.freebsd.org> <20091009154924.689f21e1.ota@j.email.ne.jp>
next in thread | previous in thread | raw e-mail | index | archive | help
On (09/10/2009 15:49), Yoshihiro Ota wrote: > Could someone explain what was the cause and why this fixes the issue? > > When I read it, it looked like that a vm object could be in 2 locations > and one of them looked the cause of the problem. > But I couldn't figure out thereafter. kern_sendfile performs vm_page_grab which allocates a new page if not found. Then it calls VOP_READ with UIO_NOCOPY. The page itself is getting filled by bio routines, because it has no valid bits set (looks like allocbuf handles this case during buffer allocation). Both zfs and tmpfs do not use buffer management routines for io, that's why sendfile should be handled separately. There's also comment about this case in zfs: http://fxr.watson.org/fxr/source/cddl/contrib/opensolaris/uts/common/fs/zfs/zfs_vnops.c?im=excerpts#L451 The patch does "real read" from swap backed storage (tobj) into that page. > > Thanks, > Hiro > > On Wed, 7 Oct 2009 23:17:15 +0000 (UTC) > Xin LI <delphij@FreeBSD.org> wrote: > > > Author: delphij > > Date: Wed Oct 7 23:17:15 2009 > > New Revision: 197850 > > URL: http://svn.freebsd.org/changeset/base/197850 > > > > Log: > > Add a special workaround to handle UIO_NOCOPY case. This fixes data > > corruption observed when sendfile() is being used. > > > > PR: kern/127213 > > Submitted by: gk > > MFC after: 2 weeks > > > > Modified: > > head/sys/fs/tmpfs/tmpfs_vnops.c > > > > Modified: head/sys/fs/tmpfs/tmpfs_vnops.c > > ============================================================================== > > --- head/sys/fs/tmpfs/tmpfs_vnops.c Wed Oct 7 23:01:31 2009 (r197849) > > +++ head/sys/fs/tmpfs/tmpfs_vnops.c Wed Oct 7 23:17:15 2009 (r197850) > > @@ -43,6 +43,8 @@ __FBSDID("$FreeBSD$"); > > #include <sys/priv.h> > > #include <sys/proc.h> > > #include <sys/resourcevar.h> > > +#include <sys/sched.h> > > +#include <sys/sf_buf.h> > > #include <sys/stat.h> > > #include <sys/systm.h> > > #include <sys/unistd.h> > > @@ -428,15 +430,72 @@ tmpfs_setattr(struct vop_setattr_args *v > > } > > > > /* --------------------------------------------------------------------- */ > > +static int > > +tmpfs_nocacheread(vm_object_t tobj, vm_pindex_t idx, > > + vm_offset_t offset, size_t tlen, struct uio *uio) > > +{ > > + vm_page_t m; > > + int error; > > + > > + VM_OBJECT_LOCK(tobj); > > + vm_object_pip_add(tobj, 1); > > + m = vm_page_grab(tobj, idx, VM_ALLOC_WIRED | > > + VM_ALLOC_ZERO | VM_ALLOC_NORMAL | VM_ALLOC_RETRY); > > + if (m->valid != VM_PAGE_BITS_ALL) { > > + if (vm_pager_has_page(tobj, idx, NULL, NULL)) { > > + error = vm_pager_get_pages(tobj, &m, 1, 0); > > + if (error != 0) { > > + printf("tmpfs get pages from pager error [read]\n"); > > + goto out; > > + } > > + } else > > + vm_page_zero_invalid(m, TRUE); > > + } > > + VM_OBJECT_UNLOCK(tobj); > > + error = uiomove_fromphys(&m, offset, tlen, uio); > > + VM_OBJECT_LOCK(tobj); > > +out: > > + vm_page_lock_queues(); > > + vm_page_unwire(m, TRUE); > > + vm_page_unlock_queues(); > > + vm_page_wakeup(m); > > + vm_object_pip_subtract(tobj, 1); > > + VM_OBJECT_UNLOCK(tobj); > > + > > + return (error); > > +} > > + > > +static __inline int > > +tmpfs_nocacheread_buf(vm_object_t tobj, vm_pindex_t idx, > > + vm_offset_t offset, size_t tlen, void *buf) > > +{ > > + struct uio uio; > > + struct iovec iov; > > + > > + uio.uio_iovcnt = 1; > > + uio.uio_iov = &iov; > > + iov.iov_base = buf; > > + iov.iov_len = tlen; > > + > > + uio.uio_offset = 0; > > + uio.uio_resid = tlen; > > + uio.uio_rw = UIO_READ; > > + uio.uio_segflg = UIO_SYSSPACE; > > + uio.uio_td = curthread; > > + > > + return (tmpfs_nocacheread(tobj, idx, offset, tlen, &uio)); > > +} > > > > static int > > tmpfs_mappedread(vm_object_t vobj, vm_object_t tobj, size_t len, struct uio *uio) > > { > > + struct sf_buf *sf; > > vm_pindex_t idx; > > vm_page_t m; > > vm_offset_t offset; > > off_t addr; > > size_t tlen; > > + char *ma; > > int error; > > > > addr = uio->uio_offset; > > @@ -461,33 +520,30 @@ lookupvpg: > > vm_page_wakeup(m); > > VM_OBJECT_UNLOCK(vobj); > > return (error); > > + } else if (m != NULL && uio->uio_segflg == UIO_NOCOPY) { > > + if (vm_page_sleep_if_busy(m, FALSE, "tmfsmr")) > > + goto lookupvpg; > > + vm_page_busy(m); > > + VM_OBJECT_UNLOCK(vobj); > > + sched_pin(); > > + sf = sf_buf_alloc(m, SFB_CPUPRIVATE); > > + ma = (char *)sf_buf_kva(sf); > > + error = tmpfs_nocacheread_buf(tobj, idx, offset, tlen, > > + ma + offset); > > + if (error == 0) { > > + uio->uio_offset += tlen; > > + uio->uio_resid -= tlen; > > + } > > + sf_buf_free(sf); > > + sched_unpin(); > > + VM_OBJECT_LOCK(vobj); > > + vm_page_wakeup(m); > > + VM_OBJECT_UNLOCK(vobj); > > + return (error); > > } > > VM_OBJECT_UNLOCK(vobj); > > nocache: > > - VM_OBJECT_LOCK(tobj); > > - vm_object_pip_add(tobj, 1); > > - m = vm_page_grab(tobj, idx, VM_ALLOC_WIRED | > > - VM_ALLOC_ZERO | VM_ALLOC_NORMAL | VM_ALLOC_RETRY); > > - if (m->valid != VM_PAGE_BITS_ALL) { > > - if (vm_pager_has_page(tobj, idx, NULL, NULL)) { > > - error = vm_pager_get_pages(tobj, &m, 1, 0); > > - if (error != 0) { > > - printf("tmpfs get pages from pager error [read]\n"); > > - goto out; > > - } > > - } else > > - vm_page_zero_invalid(m, TRUE); > > - } > > - VM_OBJECT_UNLOCK(tobj); > > - error = uiomove_fromphys(&m, offset, tlen, uio); > > - VM_OBJECT_LOCK(tobj); > > -out: > > - vm_page_lock_queues(); > > - vm_page_unwire(m, TRUE); > > - vm_page_unlock_queues(); > > - vm_page_wakeup(m); > > - vm_object_pip_subtract(tobj, 1); > > - VM_OBJECT_UNLOCK(tobj); > > + error = tmpfs_nocacheread(tobj, idx, offset, tlen, uio); > > > > return (error); > > } > > _______________________________________________ > > svn-src-all@freebsd.org mailing list > > http://lists.freebsd.org/mailman/listinfo/svn-src-all > > To unsubscribe, send any mail to "svn-src-all-unsubscribe@freebsd.org" > _______________________________________________ > svn-src-all@freebsd.org mailing list > http://lists.freebsd.org/mailman/listinfo/svn-src-all > To unsubscribe, send any mail to "svn-src-all-unsubscribe@freebsd.org"
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20091009205842.GA3808>