Uploaded image for project: 'Lustre'
  1. Lustre
  2. LU-3660

Can't disable ACL support with ZFS MDT

Details

    • 3
    • 9444

    Description

      With ldiskfs, the MDT can be mounted with -o noacl to disable POSIX ACL support. With ZFS, that option seems to have no effect, and clients are still able to set and honor ACLs. It would be desirable to be able to turn off ACLs, as they can be detrimental to performance. For example, the cp -a command tries to write the xattr system.posix_acl_access on every destination file, regardless of whether the source file has that attribute. The ACL update is handled synchronously on the MDT, so the request handler has to wait for a ZFS transaction group to sync. This can introduce significant latency on a busy MDS, effectively limiting the per-file copy rate to the txg sync rate. On ldiskfs, cp -a performance is significantly improved by disabling ACLs.

      (This performance problem is a separate issue that may be helped by integrating ZIL support in Lustre. Turning off ACLs is a temporary workaround.)

      LLNL-bug-ID: TOSS-2207

      Attachments

        Issue Links

          Activity

            [LU-3660] Can't disable ACL support with ZFS MDT
            pjones Peter Jones added a comment -

            Landed for 2.7

            pjones Peter Jones added a comment - Landed for 2.7
            emoly.liu Emoly Liu added a comment -

            Lai's patch for this problem is at http://review.whamcloud.com/#/c/10850/ .

            emoly.liu Emoly Liu added a comment - Lai's patch for this problem is at http://review.whamcloud.com/#/c/10850/ .
            laisiyao Lai Siyao added a comment -

            osd_conf_get() for zfs osd doesn't really get mount options from super_block like ldiskfs, but enables ACL by default. I'll see how to get it work.

            laisiyao Lai Siyao added a comment - osd_conf_get() for zfs osd doesn't really get mount options from super_block like ldiskfs, but enables ACL by default. I'll see how to get it work.

            Lowered priority to minor because disabling ACL support is not really an effective workaround to the performance issue described here. But, we should still decide if this should be fixed and document ACL functionality as it relates to ZFS backends.

            nedbass Ned Bass (Inactive) added a comment - Lowered priority to minor because disabling ACL support is not really an effective workaround to the performance issue described here. But, we should still decide if this should be fixed and document ACL functionality as it relates to ZFS backends.

            Created LU-3671 to discuss the synchronous behavior. This issue is to discuss whether we need a mechanism to disable ACL support with ZFS.

            nedbass Ned Bass (Inactive) added a comment - Created LU-3671 to discuss the synchronous behavior. This issue is to discuss whether we need a mechanism to disable ACL support with ZFS.
            pjones Peter Jones added a comment -

            Lai

            Could you please advise on this one?

            Thanks

            Peter

            pjones Peter Jones added a comment - Lai Could you please advise on this one? Thanks Peter

            I don't know why they're done synchronously. In fact my claim that it is synchronous is based on the observation that the request handler seems to block until the txg syncs, not on a close reading of the code. It think it's related to this comment, but it doesn't explain why.

            mdt_xattr.c:mdt_reint_setxattr()
            334         /* Revoke all clients' lookup lock, since the access
            335          * permissions for this inode is changed when ACL_ACCESS is
            336          * set. This isn't needed for ACL_DEFAULT, since that does
            337          * not change the access permissions of this inode, nor any
            338          * other existing inodes. It is setting the ACLs inherited
            339          * by new directories/files at create time. */
            340         /* We need revoke both LOOKUP|PERM lock here, see mdt_attr_set. */
            341         if (!strcmp(xattr_name, XATTR_NAME_ACL_ACCESS))
            342                 lockpart |= MDS_INODELOCK_PERM | MDS_INODELOCK_LOOKUP;
            
            nedbass Ned Bass (Inactive) added a comment - I don't know why they're done synchronously. In fact my claim that it is synchronous is based on the observation that the request handler seems to block until the txg syncs, not on a close reading of the code. It think it's related to this comment, but it doesn't explain why . mdt_xattr.c:mdt_reint_setxattr() 334 /* Revoke all clients' lookup lock, since the access 335 * permissions for this inode is changed when ACL_ACCESS is 336 * set. This isn't needed for ACL_DEFAULT, since that does 337 * not change the access permissions of this inode, nor any 338 * other existing inodes. It is setting the ACLs inherited 339 * by new directories/files at create time. */ 340 /* We need revoke both LOOKUP|PERM lock here, see mdt_attr_set. */ 341 if (!strcmp(xattr_name, XATTR_NAME_ACL_ACCESS)) 342 lockpart |= MDS_INODELOCK_PERM | MDS_INODELOCK_LOOKUP;

            Is there a reason that ACL updates are done synchronously on the MDS? I don't think there is a valid security rationale for this, since (AFAIK) chmod(), chown(), or chgrp() operations are not synchronous either, and they can have the same security impact as any ACL change. It would be far more beneficial to disable synchronous updates on the MDS entirely for this operation than to make them somewhat more efficient with the ZIL.

            adilger Andreas Dilger added a comment - Is there a reason that ACL updates are done synchronously on the MDS? I don't think there is a valid security rationale for this, since (AFAIK) chmod(), chown(), or chgrp() operations are not synchronous either, and they can have the same security impact as any ACL change. It would be far more beneficial to disable synchronous updates on the MDS entirely for this operation than to make them somewhat more efficient with the ZIL.

            People

              laisiyao Lai Siyao
              nedbass Ned Bass (Inactive)
              Votes:
              0 Vote for this issue
              Watchers:
              8 Start watching this issue

              Dates

                Created:
                Updated:
                Resolved: