Re: Thoughts on TMPFS no longer being considered "highly experimental"

On Fri, Jun 24, 2011 at 02:06:27PM +0300, Kostik Belousov wrote:
> On Fri, Jun 24, 2011 at 12:30:16PM +0200, Peter Holm wrote:
> > On Thu, Jun 23, 2011 at 11:21:53PM +0300, Kostik Belousov wrote:
> > > On Thu, Jun 23, 2011 at 09:31:09AM -0700, David O'Brien wrote:
> > > > Does anyone object to this patch?
> > > > 
> > > > David Wolfskill and I have run TMPFS on a number of machines for two
> > > > years with no problems.
> > > > 
> > > > I may have missed something, but I'm not aware of any serious PRs on
> > > > TMPFS either.
> > > > 
> > > > 
> > > > Index: tmpfs_vfsops.c
> > > > ===================================================================
> > > > --- tmpfs_vfsops.c	(revision 221113)
> > > > +++ tmpfs_vfsops.c	(working copy)
> > > > _at__at_ -155,9 +155,6 _at__at_ tmpfs_mount(struct mount *mp)
> > > >  		return EOPNOTSUPP;
> > > >  	}
> > > >  
> > > > -	printf("WARNING: TMPFS is considered to be a highly experimental "
> > > > -	    "feature in FreeBSD.\n");
> > > > -
> > > >  	vn_lock(mp->mnt_vnodecovered, LK_SHARED | LK_RETRY);
> > > >  	error = VOP_GETATTR(mp->mnt_vnodecovered, &va, mp->mnt_cred);
> > > >  	VOP_UNLOCK(mp->mnt_vnodecovered, 0);
> > > 
> > > The things I am aware of:
> > > - there is a races on the lookup. They were papered over in r212305,
> > > but the bug was not really fixed, AFAIR.
> > > 
> > > - the tmpfs does double-buffering for the mapped vnodes. This is quite
> > > insulting for the memory-backed fs, isn't it ? I have a patch, but it is
> > > still under review.
> > > 
> > > - I believe Peter Holm has more test cases that fails with tmpfs. He
> > > would have more details. I somewhat remember some panic on execve(2) the
> > > binary located on tmpfs.
> > > 
> > 
> > I ran the TMPFS tests I have and so far I only spotted the mmap(2)
> > problem:
> > 
> > http://people.freebsd.org/~pho/stress/log/tmpfs/
> It would be indeed good if the issue was the only remaining problem.

Well, more testing is needed for sure.

> The deadlock in tmpfs6.txt is caused by doing copyin() while having
> a page busied. This should be fixed indirectly by the patch to
> avoid double-buffering, I uploaded the latest version at
> http://people.freebsd.org/~kib/misc/tmpfs.5.patch
> 
> > 
> > > Removing the warning will not make the issues coming away.
> > 

This doesn't compile:

===> tmpfs (all)
cc -O2 -pipe -fno-strict-aliasing -Werror -D_KERNEL -DKLD_MODULE -nostdinc   -DHAVE_KERNEL_OPTION_HEADERS -include /usr/src/sys/i386/compile/PHO/opt_global.h -I. -I_at_ -I_at_/contrib/altq
-finline-limit=8000 --param inline-unit-growth=100 --param large-function-growth=1000 -fno-common -g -I/usr/src/sys/i386/compile/PHO  -mno-align-long-strings -mpreferred-stack-boundary=2 -mno-sse
-mno-mmx -msoft-float -ffreestanding -fstack-protector -std=iso9899:1999 -fstack-protector -Wall -Wredundant-decls -Wnested-externs -Wstrict-prototypes  -Wmissing-prototypes -Wpointer-arith -Winline
-Wcast-qual  -Wundef -Wno-pointer-sign -fformat-extensions  -Wmissing-include-dirs -fdiagnostics-show-option -c /usr/src/sys/modules/tmpfs/../../fs/tmpfs/tmpfs_subr.c
cc1: warnings being treated as errors
/usr/src/sys/modules/tmpfs/../../fs/tmpfs/tmpfs_subr.c: In function 'tmpfs_reg_resize':
/usr/src/sys/modules/tmpfs/../../fs/tmpfs/tmpfs_subr.c:911: warning: 'uobj' is used uninitialized in this function
*** Error code 1

 886 int
 887 tmpfs_reg_resize(struct vnode *vp, off_t newsize)
 888 {
 889         struct tmpfs_mount *tmp;
 890         struct tmpfs_node *node;
 891         vm_object_t uobj;
 892         vm_page_t m;
 893         vm_pindex_t newpages, oldpages;
 894         off_t oldsize;
 895         size_t zerolen;
 896 
 897         MPASS(vp->v_type == VREG);
 898         MPASS(newsize >= 0);
 899 
 900         node = VP_TO_TMPFS_NODE(vp);
 901         tmp = VFS_TO_TMPFS(vp->v_mount);
 902 
 903         /*
 904          * Convert the old and new sizes to the number of pages needed to
 905          * store them.  It may happen that we do not need to do anything
 906          * because the last allocated page can accommodate the change on
 907          * its own.
 908          */
 909         oldsize = node->tn_size;
 910         oldpages = OFF_TO_IDX(oldsize + PAGE_MASK);
 911         MPASS(oldpages == uobj->size);
 912         newpages = OFF_TO_IDX(newsize + PAGE_MASK);
 913         if (newpages > oldpages &&
 914             newpages - oldpages > TMPFS_PAGES_AVAIL(tmp))
 915                 return (ENOSPC);
 916 
 917         TMPFS_LOCK(tmp);
 918         tmp->tm_pages_used += (newpages - oldpages);
 919         TMPFS_UNLOCK(tmp);
 920 
 921         node->tn_size = newsize;
 922         VM_OBJECT_LOCK(uobj);

- Peter