Date: Mon, 12 Sep 2005 09:43:47 GMT From: soc-chenk <soc-chenk@FreeBSD.org> To: Perforce Change Reviews <perforce@freebsd.org> Subject: PERFORCE change 83446 for review Message-ID: <200509120943.j8C9hlmo030268@repoman.freebsd.org>
next in thread | raw e-mail | index | archive | help
http://perforce.freebsd.org/chv.cgi?CH=83446 Change 83446 by soc-chenk@soc-chenk_leavemealone on 2005/09/12 09:43:28 refined handling of cache and unbound filehandles Submitted by: soc-chenk Affected files ... .. //depot/projects/soc2005/fuse4bsd2/Changelog#3 edit .. //depot/projects/soc2005/fuse4bsd2/IMPLEMENTATION_NOTES#2 edit .. //depot/projects/soc2005/fuse4bsd2/fuse_module/fuse.c#4 edit Differences ... ==== //depot/projects/soc2005/fuse4bsd2/Changelog#3 (text+ko) ==== @@ -1,3 +1,12 @@ +Mon Sep 12 11:37:04 CEST 2005 at node: creo.hu, nick: csaba + * refined handling of cache and unbound filehandles + + fuse_open always asks for a filehandle (to see if to keep cache) + + filehandle gc done upon each open + + IMPLEMENTATION_NOTES updated to reflect this + Sun Sep 11 18:45:52 CEST 2005 at node: creo.hu, nick: csaba * Sanitized fuse.c's look ==== //depot/projects/soc2005/fuse4bsd2/IMPLEMENTATION_NOTES#2 (text+ko) ==== @@ -299,8 +299,12 @@ But what to do with the filehandle when the strategy has done its job? +Releasing it immediately (that is, strategy releases it before +return) is pretty unefficient: the file should be re-opened at each +turn of a lengty read-in. + Just simply forgetting about it and polluting the daemon with worn-out -filehandles is not a good idea. Some kind of resource management should +filehandles is neither a good idea. Some kind of resource management should be used for these "unbound" filehandles. As the first step of that, a list of filehandles is maintained by each @@ -314,27 +318,25 @@ question then: what event should trigger gc and on what thread should gc run? -For a while, I considered running a dedicated kernel daemon to do the -gc, which is activated periodically upon some filesystem usage counter -reaching a certain value, and which would decide about the fate of -unbound filehandles based upon complex filesystem usage statistics. But -in the end, a pretty much simple mechanism was implemented. +There is a neat built-in gc mechanism: it's invoked when vnodes become +unused. The usual effect of this method is disassociating the vnode +from its file (node), and putting it back to the pool of free vnodes. +We don't do that, as then we would lose the number of lookups (which +is needed for Fuse to operate correctly). Yet it's a pretty fine time +to gc unbound filehandles. + +But if a file is kept open for a longer while, that will block this +mechanism: the respective vnode remains in use during this time. +Then the filehandles created by subsequent internal (fileless) opens +will persist. (Imagine that one opens /mnt/fuse/bin/ls as a regular +file, and keeps it open. In this case each execution of this file +will create a new unbound filehandle which won't go away upon the +termination of the process.) -When a file (node) is opened, either the buffer cache of the respective -vnode has to be flushed, or not, depending on whether the daemon -requests its preservation (in fact, this won't happen on a case-by-case -basis, it's determined globally, depending on whether the "kernel_cache" -option has been passed to the daemon). If we are told to flush the -buffer cache (or we are open it for writing), we also walk over the list -of filehandles, and throw away those ones which are not in use at the -moment. That's it. The point is that unbound filehandles won't -proliferate while we keep the buffer cache: as long as we have the -cache, a read-like operation will get its data from the cache, and the -strategy won't be called. (In fact, we could as well just throw away all -unbound filehandles [not in use] upon each open; keeping them when the -cache is not flushed and no write takes place is just a slight -optimization, which has a chance to have impact on anything only when -reading relatively large files.) +So one more gc entry point is needed. Using the open handler +for this purpose is good enough. This will stop the proliferation of +unbound filehandles in the above scenario, yet won't occur with an +unbearable frequency. 3) Syncing ==== //depot/projects/soc2005/fuse4bsd2/fuse_module/fuse.c#4 (text+ko) ==== @@ -176,8 +176,11 @@ fuse_iov_adjust(struct fuse_iov *fiov, size_t size) { /* XXX limits should be settable via sysctl */ - if (fiov->allocated_size < size || (FUSE_MAX_PERMANENT_IOBUF && fiov->allocated_size - size > FUSE_MAX_PERMANENT_IOBUF)) { - fiov->base = realloc(fiov->base, FU_AT_LEAST(size), M_FUSEMSG, M_WAITOK | M_ZERO); + if (fiov->allocated_size < size || + (FUSE_MAX_PERMANENT_IOBUF && + fiov->allocated_size - size > FUSE_MAX_PERMANENT_IOBUF)) { + fiov->base = realloc(fiov->base, FU_AT_LEAST(size), M_FUSEMSG, + M_WAITOK | M_ZERO); fiov->allocated_size = FU_AT_LEAST(size); } @@ -1978,7 +1981,9 @@ int mode, struct fuse_filehandle *fufh, void *param) { KASSERT(fufh->useco >= 0, ("negative use count for fuse filehandle")); - DEBUG("vnode #%d, fufh owner %p, useco %d\n", VTOI(vp), fufh->fp, fufh->useco); + KASSERT(! fufh->fp || fufh->useco > 0, + ("filehandle bound with 0 use counter")); + DEBUG2G("vnode #%d, fufh owner %p, useco %d\n", VTOI(vp), fufh->fp, fufh->useco); if (! fufh->fp && fufh->useco == 0) { LIST_REMOVE(fufh, fh_link); fuse_send_release(vp, td, cred, fufh, fufh->mode); @@ -2734,7 +2739,7 @@ struct ucred *cred = ap->a_cred; int mode = ap->a_mode; - struct fuse_filehandle *fufh; + struct fuse_filehandle *fufh = NULL; struct fuse_vnode_data *fvdat = vp->v_data; #if DIRECTIO_FOR_DIRS struct ucred *clonecrp; @@ -2757,24 +2762,28 @@ } #endif - if (fdidx < 0) - return (0); - if (vp->v_type == VDIR) mode = FREAD; else mode &= ~(O_CREAT | O_EXCL | O_NOCTTY | O_TRUNC); /* - * That certain "pretty disgustingly long chain" - * by which we can put our hands of the file struct - * assinged to this call. + * Contrary to that how it used to be, now we go on even if it's an + * internal (fileless) open: to match the "specs", we must get the + * keep_cache information (and act according to it). */ - fp = td->td_proc->p_fd->fd_ofiles[fdidx]; - if (! fp) - panic("nonneg file desc passed to us but no file there"); + if (fdidx >= 0) { + /* + * That certain "pretty disgustingly long chain" + * by which we can put our hands of the file struct + * assinged to this call. + */ + fp = td->td_proc->p_fd->fd_ofiles[fdidx]; + if (! fp) + panic("nonneg file desc passed to us but no file there"); - DEBUG2G("fp->f_flag 0x%x, open mode %d\n", fp->f_flag, mode); + DEBUG2G("fp->f_flag 0x%x, open mode %d\n", fp->f_flag, mode); + } fdi.iosize = sizeof(*foi); if ((err = fdisp_prepare_all(&fdi, (vp->v_type == VDIR) ? FUSE_OPENDIR : FUSE_OPEN, vp, td, cred))) @@ -2792,9 +2801,12 @@ fufh->fh_id = foo->fh; - fp->f_flag = mode; + if (fp) + fp->f_flag = mode; if (foo->open_flags & FOPEN_DIRECT_IO) { - fp->f_flag |= O_DIRECT; /* This seems not to be permanent */ + if (fp) + /* This seems not to be permanent */ + fp->f_flag |= O_DIRECT; fufh->flags |= FUSEFH_DIRECTIO; } @@ -2862,13 +2874,15 @@ fp->f_ops = &fuse_fileops; #endif - fp->f_ops = &fuse_fileops; fufh->mode = mode; fufh->cred = crhold(cred); fufh->pid = td->td_proc->p_pid; fufh->useco = 1; - fufh->fp = fp; - fp->f_data = fufh; + if (fp) { + fp->f_ops = &fuse_fileops; + fufh->fp = fp; + fp->f_data = fufh; + } #if _DEBUG DEBUG2G("before gc...\n"); @@ -2880,7 +2894,7 @@ /* * The neat idea is that a cache cleanup is the best time for a gc -- * gc'ing unbound filehandles at inactive/reclaim time is not enough, - * they can Just accumulate while something happens with the file, + * they can just accumulate while something happens with the file, * given the cache is flushed frequently (otherwise there is no reason * for the system to spawn them, as the data can be read from core). * So then, let's just tie their gc to cache flush... @@ -2888,10 +2902,14 @@ * is get kept until there is reference to it, because that's done * via the lookup/forget mechanism, and not via filehandles. */ - if ((! keep_cache) || (mode & FWRITE)) { - DEBUG2G("gc'ing...\n"); - iterate_filehandles(vp, td, cred, 0, release_filehandle, NULL); - } + /* + * Now we use unbound filehandles pretty rarely (as read/write of + * regular files doesn't rely on them anymore), only during exec + * and reading directories. It's pretty much unlikely that one + * can be reused in any way, so we drop them unconditionally. + */ + DEBUG2G("gc'ing...\n"); + iterate_filehandles(vp, td, cred, 0, release_filehandle, NULL); sx_xunlock(&fvdat->fh_lock); fvdat->fh_counter++; #if _DEBUG2G @@ -2899,6 +2917,9 @@ vn_printf(vp, " "); #endif + if (! fp) + fufh->useco--; + if (! keep_cache) { DEBUG2G("flushing cache\n"); /* @@ -4765,10 +4786,7 @@ struct vnode *vp; int rc; -#if _DEBUG2G - DEBUG2G("vnode #%d\n", VTOI((struct vnode *)bo->bo_private)); - kdb_backtrace(); -#endif + DEBUG("vnode #%d\n", VTOI((struct vnode *)bo->bo_private)); vp = bo->bo_private; KASSERT(bo == &vp->v_bufobj, ("BO/VP mismatch: vp %p #(%d) bo %p != %p", vp, VTOI(vp), &vp->v_bufobj, bo));
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?200509120943.j8C9hlmo030268>