Project

General

Profile

Bug #7444

fs/xattr.c should be more transparent

Added by Gordon Ross about 3 years ago. Updated over 2 years ago.

Status:
Closed
Priority:
Normal
Assignee:
Category:
filesystems (not ZFS)
Start date:
2016-10-02
Due date:
% Done:

100%

Estimated time:
Difficulty:
Medium
Tags:
needs-triage

Description

Background:

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.

Problems/solutions

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.


Related issues

Related to illumos gate - Bug #8102: zfs-tests suite fails zfs_acl_chmod_xattr_001_posClosed2017-04-24

Actions

History

#2

Updated by Electric Monk about 3 years ago

  • Status changed from New to Closed
  • % Done changed from 0 to 100

git commit 9b24702912cd165acd5682b2c0c853496d320536

commit  9b24702912cd165acd5682b2c0c853496d320536
Author: Gordon Ross <gwr@nexenta.com>
Date:   2016-10-28T22:34:19.000Z

    7444 fs/xattr.c should be more transparent
    Reviewed by: Piotr Jasiukajtis <estibi@me.com>
    Reviewed by: Jerry Jelinek <jerry.jelinek@joyent.com>
    Approved by: Dan McDonald <danmcd@omniti.com>

#3

Updated by Electric Monk over 2 years ago

git commit 4286ffae246c5943dbdc0d830e5e117c900d6baa

commit  4286ffae246c5943dbdc0d830e5e117c900d6baa
Author: Gordon Ross <gwr@nexenta.com>
Date:   2017-05-15T19:25:23.000Z

    7444 fs/xattr.c should be more transparent (zfs_acl_test)
    Reviewed by: Dan McDonald <danmcd@omniti.com>
    Reviewed by: Yuri Pankov <yuri.pankov@gmail.com>
    Approved by: Matthew Ahrens <mahrens@delphix.com>

#4

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! ]

#5

Updated by Yuri Pankov over 2 years ago

Actually, it was zfs_acl_chmod_xattr_001_pos (just for the record).

#6

Updated by Yuri Pankov over 2 years ago

  • Related to Bug #8102: zfs-tests suite fails zfs_acl_chmod_xattr_001_pos added

Also available in: Atom PDF