svn commit: r197850 - head/sys/fs/tmpfs

[Gleb Kurtsou]

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 at 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 at freebsd.org mailing list
> > http://lists.freebsd.org/mailman/listinfo/svn-src-all
> > To unsubscribe, send any mail to "svn-src-all-unsubscribe at freebsd.org"
> _______________________________________________
> svn-src-all at freebsd.org mailing list
> http://lists.freebsd.org/mailman/listinfo/svn-src-all
> To unsubscribe, send any mail to "svn-src-all-unsubscribe at freebsd.org"