On Sat, 12 Jun 2010, Kostik Belousov wrote:

On Sat, Jun 12, 2010 at 11:56:10AM +0300, Mikolaj Golub wrote:

On Sun, 6 Jun 2010 16:44:43 +0200 Leon Me??ner wrote:

 LM> Hi,
 LM> I hope this is not the wrong list to ask. Didn't get any answers on
 LM> -questions.

 LM> When you try to do the following inside a nullfs mounted directory,
 LM> where the nullfs origin is itself mounted via nfs you get an error:

 LM> # foo
 LM> # tail -f foo&
 LM> # rm -f foo
 LM> tail: foo: Stale NFS file handle
 LM> # fg

 LM> This is really a problem when running services inside jails and using
 LM> NFS as storage. As of [2] it looks like this problem is known for a
 LM> while. On a normal NFS mount this does not happen as "silly renaming"
 LM> [1] works there (producing nasty little .nfsXXXX files).

nfs_sillyrename() is called when vnode's usecount is more then 1. It is
expected that unlink() syscall increases vnode's usecount in namei() and if
the file has been already opened usecount will be more then 1.

But with nullfs layer present the reference counts are held by the upper node,
not the lower (nfs) one, so when unlink() is called it increases usecount of
the upper vnode, not nfs vnode and nfs_sillyrename() is never called.

The strightforward solution looks like to implement null_remove() that will
increase lower vnode's refcount before calling null_bypass() and then
decrement it after the call. See the attached patch (it works for me on both
8-STABLE and CURRENT).

The upper vnode holds a reference to the lower vnode, as you noted.
Now, with your patch, I believe that _all_ calls to the nfs_remove()
are happen with refcount > 1.

I'm not familiar with the nullfs so this might be way off, but would
this patch be ok by any chance?

Index: sys/fs/nullfs/null_vnops.c
===================================================================
--- sys/fs/nullfs/null_vnops.c  (revision 208960)
+++ sys/fs/nullfs/null_vnops.c  (working copy)
@@ -499,6 +499,23 @@
 }

 /*
+ * Increasing refcount of lower vnode is needed at least for the case
+ * when lower FS is NFS to do sillyrename if the file is in use.
+ */
+static int
+null_remove(struct vop_remove_args *ap)
+{
+       int retval;
+       struct vnode *lvp;
+
+       if (ap->a_vp->v_usecount > 1) {
+               lvp = NULLVPTOLOWERVP(ap->a_vp);
+               VREF(lvp);
+       } else
+               lvp = NULL;
+       retval = null_bypass(&ap->a_gen);
+       if (lvp != NULL)
+               vrele(lvp);
+       return (retval);
+}
+
+/*
  * We handle this to eliminate null FS to lower FS
  * file moving. Don't know why we don't allow this,
  * possibly we should.
@@ -809,6 +826,7 @@
        .vop_open =             null_open,
        .vop_print =            null_print,
        .vop_reclaim =          null_reclaim,
+       .vop_remove =           null_remove,
        .vop_rename =           null_rename,
        .vop_setattr =          null_setattr,
        .vop_strategy =         VOP_EOPNOTSUPP,
_______________________________________________
freebsd-stable@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-stable
To unsubscribe, send any mail to "freebsd-stable-unsubscr...@freebsd.org"

Reply via email to