Print this page
6975745 xattr directories should be more transparent

@@ -52,19 +52,13 @@
         xattr_view_t    xattr_view;
 } xattr_file_t;
 
 typedef struct {
         gfs_dir_t       xattr_gfs_private;
-        vnode_t         *xattr_realvp;  /* Only used for VOP_REALVP */
+        vnode_t         *xattr_realvp;
 } xattr_dir_t;
 
-/*
- * xattr_realvp is only used for VOP_REALVP, this is so we don't
- * keep an unnecessary hold on the *real* xattr dir unless we have
- * no other choice.
- */
-
 /* ARGSUSED */
 static int
 xattr_file_open(vnode_t **vpp, int flags, cred_t *cr, caller_context_t *ct)
 {
         xattr_file_t *np = (*vpp)->v_data;

@@ -877,53 +871,76 @@
 
 static int
 xattr_dir_realdir(vnode_t *dvp, vnode_t **realdvp, int lookup_flags,
     cred_t *cr, caller_context_t *ct)
 {
-        vnode_t *pvp;
+        xattr_dir_t *xattr_dir;
         int error;
-        struct pathname pn;
-        char *startnm = "";
 
         *realdvp = NULL;
 
-        pvp = gfs_file_parent(dvp);
+        if (dvp->v_type != VDIR)
+                return (EINVAL);
 
-        error = pn_get(startnm, UIO_SYSSPACE, &pn);
-        if (error) {
-                VN_RELE(pvp);
-                return (error);
-        }
+        mutex_enter(&dvp->v_lock);
+        xattr_dir = dvp->v_data;
+        *realdvp = xattr_dir->xattr_realvp;
+        mutex_exit(&dvp->v_lock);
 
-        /*
-         * Set the LOOKUP_HAVE_SYSATTR_DIR flag so that we don't get into an
-         * infinite loop with fop_lookup calling back to xattr_dir_lookup.
-         */
-        lookup_flags |= LOOKUP_HAVE_SYSATTR_DIR;
-        error = VOP_LOOKUP(pvp, startnm, realdvp, &pn, lookup_flags,
-            rootvp, cr, ct, NULL, NULL);
-        pn_free(&pn);
+        if (*realdvp != NULL) {
+                VN_HOLD(*realdvp);
+                error = 0;
+        } else
+                error = ENOENT;
 
         return (error);
 }
 
 /* ARGSUSED */
 static int
 xattr_dir_open(vnode_t **vpp, int flags, cred_t *cr, caller_context_t *ct)
 {
+        vnode_t *realvp;
+        int error;
+
         if (flags & FWRITE) {
                 return (EACCES);
         }
 
+        /*
+         * The underlying FS may need this VOP call.
+         */
+        error = xattr_dir_realdir(*vpp, &realvp, LOOKUP_XATTR, cr, ct);
+        if (error == 0) {
+                error = VOP_OPEN(&realvp, flags, cr, ct);
+                VN_RELE(realvp);
+                if (error)
+                        return (error);
+        } /* else ignore this error */
+
         return (0);
 }
 
 /* ARGSUSED */
 static int
-xattr_dir_close(vnode_t *vpp, int flags, int count, offset_t off, cred_t *cr,
+xattr_dir_close(vnode_t *vp, int flags, int count, offset_t off, cred_t *cr,
     caller_context_t *ct)
 {
+        vnode_t *realvp;
+        int error;
+
+        /*
+         * The underlying FS may need this VOP call.
+         */
+        error = xattr_dir_realdir(vp, &realvp, LOOKUP_XATTR, cr, ct);
+        if (error == 0) {
+                error = VOP_CLOSE(realvp, flags, count, off, cr, ct);
+                VN_RELE(realvp);
+                if (error)
+                        return (error);
+        } /* else ignore this error */
+
         return (0);
 }
 
 /*
  * Retrieve the attributes on an xattr directory.  If there is a "real"

@@ -1354,42 +1371,15 @@
 
 /* ARGSUSED */
 static int
 xattr_dir_realvp(vnode_t *vp, vnode_t **realvp, caller_context_t *ct)
 {
-        xattr_dir_t *xattr_dir;
-
-        mutex_enter(&vp->v_lock);
-        xattr_dir = vp->v_data;
-        if (xattr_dir->xattr_realvp) {
-                *realvp = xattr_dir->xattr_realvp;
-                mutex_exit(&vp->v_lock);
-                return (0);
-        } else {
-                vnode_t *xdvp;
                 int error;
 
-                mutex_exit(&vp->v_lock);
-                if ((error = xattr_dir_realdir(vp, &xdvp,
-                    LOOKUP_XATTR, kcred, NULL)) == 0) {
-                        /*
-                         * verify we aren't racing with another thread
-                         * to find the xattr_realvp
-                         */
-                        mutex_enter(&vp->v_lock);
-                        if (xattr_dir->xattr_realvp == NULL) {
-                                xattr_dir->xattr_realvp = xdvp;
-                                *realvp = xdvp;
-                                mutex_exit(&vp->v_lock);
-                        } else {
-                                *realvp = xattr_dir->xattr_realvp;
-                                mutex_exit(&vp->v_lock);
-                                VN_RELE(xdvp);
-                        }
-                }
+        error = xattr_dir_realdir(vp, realvp, LOOKUP_XATTR, kcred, NULL);
                 return (error);
-        }
+
 }
 
 static const fs_operation_def_t xattr_dir_tops[] = {
         { VOPNAME_OPEN,         { .vop_open = xattr_dir_open }          },
         { VOPNAME_CLOSE,        { .vop_close = xattr_dir_close }        },

@@ -1468,14 +1458,20 @@
 xattr_init(void)
 {
         VERIFY(gfs_make_opsvec(xattr_opsvec) == 0);
 }
 
+/* See vnode.c: fop_lookup() */
 int
 xattr_dir_lookup(vnode_t *dvp, vnode_t **vpp, int flags, cred_t *cr)
 {
         int error = 0;
+        vnode_t *gfs_vp = NULL;
+        vnode_t *real_vp = NULL;
+        xattr_dir_t *xattr_dir;
+        struct pathname pn;
+        char *nm = "";
 
         *vpp = NULL;
 
         if (dvp->v_type != VDIR && dvp->v_type != VREG)
                 return (EINVAL);

@@ -1490,12 +1486,12 @@
                 mutex_exit(&dvp->v_lock);
                 return (EINVAL);
         }
 
         if (dvp->v_xattrdir != NULL) {
-                *vpp = dvp->v_xattrdir;
-                VN_HOLD(*vpp);
+                gfs_vp = dvp->v_xattrdir;
+                VN_HOLD(gfs_vp);
         } else {
                 ulong_t val;
                 int xattrs_allowed = dvp->v_vfsp->vfs_flag & VFS_XATTR;
                 int sysattrs_allowed = 1;
 

@@ -1517,13 +1513,10 @@
 
                 if (!xattrs_allowed && !sysattrs_allowed)
                         return (EINVAL);
 
                 if (!sysattrs_allowed) {
-                        struct pathname pn;
-                        char *nm = "";
-
                         error = pn_get(nm, UIO_SYSSPACE, &pn);
                         if (error)
                                 return (error);
                         error = VOP_LOOKUP(dvp, nm, vpp, &pn,
                             flags|LOOKUP_HAVE_SYSATTR_DIR, rootvp, cr, NULL,

@@ -1534,11 +1527,11 @@
 
                 /*
                  * Note that we act as if we were given CREATE_XATTR_DIR,
                  * but only for creation of the GFS directory.
                  */
-                *vpp = gfs_dir_create(
+                gfs_vp = gfs_dir_create(
                     sizeof (xattr_dir_t), dvp, xattr_dir_ops, xattr_dirents,
                     xattrdir_do_ino, MAXNAMELEN, NULL, xattr_lookup_cb);
                 mutex_enter(&dvp->v_lock);
                 if (dvp->v_xattrdir != NULL) {
                         /*

@@ -1545,37 +1538,66 @@
                          * We lost the race to create the xattr dir.
                          * Destroy this one, use the winner.  We can't
                          * just call VN_RELE(*vpp), because the vnode
                          * is only partially initialized.
                          */
-                        gfs_dir_t *dp = (*vpp)->v_data;
+                        gfs_dir_t *dp = gfs_vp->v_data;
 
-                        ASSERT((*vpp)->v_count == 1);
-                        vn_free(*vpp);
+                        ASSERT(gfs_vp->v_count == 1);
+                        vn_free(gfs_vp);
 
                         mutex_destroy(&dp->gfsd_lock);
                         kmem_free(dp->gfsd_static,
                             dp->gfsd_nstatic * sizeof (gfs_dirent_t));
                         kmem_free(dp, dp->gfsd_file.gfs_size);
 
                         /*
                          * There is an implied VN_HOLD(dvp) here.  We should
                          * be doing a VN_RELE(dvp) to clean up the reference
-                         * from *vpp, and then a VN_HOLD(dvp) for the new
+                         * from gfs_vp, and then a VN_HOLD(dvp) for the new
                          * reference.  Instead, we just leave the count alone.
                          */
 
-                        *vpp = dvp->v_xattrdir;
-                        VN_HOLD(*vpp);
+                        gfs_vp = dvp->v_xattrdir;
+                        VN_HOLD(gfs_vp);
                 } else {
-                        (*vpp)->v_flag |= (V_XATTRDIR|V_SYSATTR);
-                        dvp->v_xattrdir = *vpp;
+                        gfs_vp->v_flag |= (V_XATTRDIR|V_SYSATTR);
+                        dvp->v_xattrdir = gfs_vp;
                 }
         }
         mutex_exit(&dvp->v_lock);
 
-        return (error);
+        /*
+         * In order to make this module relatively transparent
+         * to the underlying filesystem, we need to lookup the
+         * xattr dir in the lower filesystem and (if found)
+         * keep a hold on it for as long as there is a hold
+         * on the gfs_vp we're about to return.  This hold is
+         * released in xattr_dir_inactive.
+         */
+        xattr_dir = gfs_vp->v_data;
+        if ((dvp->v_vfsp->vfs_flag & VFS_XATTR) &&
+            (xattr_dir->xattr_realvp == NULL)) {
+                error = pn_get(nm, UIO_SYSSPACE, &pn);
+                if (error == 0) {
+                        error = VOP_LOOKUP(dvp, nm, &real_vp, &pn,
+                            flags|LOOKUP_HAVE_SYSATTR_DIR, rootvp, cr, NULL,
+                            NULL, NULL);
+                        pn_free(&pn);
+                }
+                if (error == 0) {
+                        mutex_enter(&gfs_vp->v_lock);
+                        if (xattr_dir->xattr_realvp == NULL)
+                                xattr_dir->xattr_realvp = real_vp;
+                        else
+                                VN_RELE(real_vp);
+                        mutex_exit(&gfs_vp->v_lock);
+                }
+        }
+
+        *vpp = gfs_vp;
+        return (0);
 }
 
 int
 xattr_dir_vget(vfs_t *vfsp, vnode_t **vpp, fid_t *fidp)
 {