fs/xattr.c should be more transparent
The implementation of Extensible Attributes (PSARC/2007/315)
uses what could be described as a layered file system module
to handle all access to the special extended attribute files
SUNWattr_ro and SUNWattr_rw. This layered module is found in
$SRC/uts/common/fs/xattr.c, referred to hereafter as fs/xattr.
Filesystems that indicate support for (PSARC/2007/315) have
fs/xattr layered above them by code in vnode.c:fop_lookup.
When the filesystem below fs/xattr supports extended attributes,
fs/xattr implements vnode operations (VOP) for objects other than
SUNWattr_ro or SUNWattr_rw by passing the VOP calls through to
the underlying filesystem. These "pass-through" VOP calls look
to the underlying filesystem much the same as when the fs/xattr
module layer is not in use, so this layer is mostly transparent
to the underlying filesystem.
The pass-through VOP calls in fs/xattr lack transparency in
two respects: (1) open and close are not passed through,
and (2) vnode hold lifetimes are much shorter when fs/xattr
is in use.
Problem 1: Missing open/close calls (and solution)
In some filesystems such as nfs(v4) and smbfs, the open/close
VOP calls need to maintain some state (i.e. remote handles).
While both nfs(v4) and smbfs use open/close calls, only smbfs
is affected by the missign open/close because nfs does not
currently implement the Extensible Attributes feature.
Adding logic to pass through open/close calls is trivial,
and ignored by file systems that don't need them.
Problem 2: Shortened hold times (and solution)
The fs/xattr module has some misguided comments at the top
about avoiding vnode hold times. That makes no sense.
Short hold times are advantageous for mutex locks etc,
but holding a vnode reference through an operation is
actually preferred (and more efficient) than droppping
that hold only to have to lookup the vnode again soon.
This enhancement lets fs/xattr keep the vnode hold on
the lower file system vnode for the same duration as
the hold on the xattr directory vnode. Not only is this
more efficient (avoiding extra vnode lookup/reclaims)
but simplifies many operations in the fs/xattr code.
Updated by Electric Monk about 3 years ago
- Status changed from New to Closed
- % Done changed from 0 to 100
commit 9b24702912cd165acd5682b2c0c853496d320536 Author: Gordon Ross <firstname.lastname@example.org> Date: 2016-10-28T22:34:19.000Z 7444 fs/xattr.c should be more transparent Reviewed by: Piotr Jasiukajtis <email@example.com> Reviewed by: Jerry Jelinek <firstname.lastname@example.org> Approved by: Dan McDonald <email@example.com>
Updated by Electric Monk over 2 years ago
commit 4286ffae246c5943dbdc0d830e5e117c900d6baa Author: Gordon Ross <firstname.lastname@example.org> Date: 2017-05-15T19:25:23.000Z 7444 fs/xattr.c should be more transparent (zfs_acl_test) Reviewed by: Dan McDonald <email@example.com> Reviewed by: Yuri Pankov <firstname.lastname@example.org> Approved by: Matthew Ahrens <email@example.com>
Updated by Gordon Ross over 2 years ago
For the record, the follow-up fix was because in the first fix,
the function xattr_dir_access() was missing some logic
that was there in the earlier version. That caused the
ZFS test zfs_acl_chmod_xattr_001_pos to regress.
That test now passes again.
[ Test name corrected per. Yuri -- thx! ]