xfs
[Top] [All Lists]

Re: [PATCH 6/6] xfs: increase number of ACL entries for V5 superblocks

To: Dave Chinner <david@xxxxxxxxxxxxx>
Subject: Re: [PATCH 6/6] xfs: increase number of ACL entries for V5 superblocks
From: Mark Tinguely <tinguely@xxxxxxx>
Date: Mon, 03 Jun 2013 17:08:38 -0500
Cc: xfs@xxxxxxxxxxx, bpm@xxxxxxx
Delivered-to: xfs@xxxxxxxxxxx
In-reply-to: <1370237332-24757-7-git-send-email-david@xxxxxxxxxxxxx>
References: <1370237332-24757-1-git-send-email-david@xxxxxxxxxxxxx> <1370237332-24757-7-git-send-email-david@xxxxxxxxxxxxx>
User-agent: Mozilla/5.0 (X11; FreeBSD amd64; rv:9.0) Gecko/20120122 Thunderbird/9.0
On 06/03/13 00:28, Dave Chinner wrote:
From: Dave Chinner<dchinner@xxxxxxxxxx>

The limit of 25 ACL entries is arbitrary, but baked into the on-disk
format.  For version 5 superblocks, increase it to the maximum nuber
of ACLs that can fit into a single xattr.

Signed-off-by: Dave Chinner<dchinner@xxxxxxxxxx>
Reviewed-by: Brian Foster<bfoster@xxxxxxxxxx>
---

@@ -189,16 +193,17 @@ xfs_set_acl(struct inode *inode, int type, struct 
posix_acl *acl)

        if (acl) {
                struct xfs_acl *xfs_acl;
-               int len;
+               int len = XFS_ACL_SIZE(ip->i_mount);

-               xfs_acl = kzalloc(sizeof(struct xfs_acl), GFP_KERNEL);
+               xfs_acl = kzalloc(len, GFP_KERNEL);

Isn't that physical contiguous allocator? wouldn't a virtual contiguous be good enough for the acl?

...

        if (type == ACL_TYPE_ACCESS) {
diff --git a/fs/xfs/xfs_acl.h b/fs/xfs/xfs_acl.h
index 39632d9..0da8725 100644
--- a/fs/xfs/xfs_acl.h
+++ b/fs/xfs/xfs_acl.h
@@ -22,19 +22,35 @@ struct inode;
  struct posix_acl;
  struct xfs_inode;

-#define XFS_ACL_MAX_ENTRIES 25
  #define XFS_ACL_NOT_PRESENT (-1)

  /* On-disk XFS access control list structure */
+struct xfs_acl_entry {
+       __be32  ae_tag;
+       __be32  ae_id;
+       __be16  ae_perm;
+       __be16  ae_pad;         /* fill the implicit hole in the structure */
+};
+
  struct xfs_acl {
-       __be32          acl_cnt;
-       struct xfs_acl_entry {
-               __be32  ae_tag;
-               __be32  ae_id;
-               __be16  ae_perm;
-       } acl_entry[XFS_ACL_MAX_ENTRIES];
+       __be32                  acl_cnt;
+       struct xfs_acl_entry    acl_entry[0];
  };

+/*
+ * The number of ACL entries allowed is defined by the on-disk format.
+ * For v4 superblocks, that is limited to 25 entries. For v5 superblocks, it is
+ * limited only by the maximum size of the xattr that stores the information.
+ */
+#define XFS_ACL_MAX_ENTRIES(mp)        \
+       (xfs_sb_version_hascrc(&mp->m_sb) \
+          ?  (XATTR_SIZE_MAX - sizeof(__be32)) / sizeof(struct xfs_acl_entry) \
+          : 25)


XFS_ACL_MAX_ENTRIES(mp)  == (65536 - 4) / 12 == 5461

+
+#define XFS_ACL_SIZE(mp) \
+       (sizeof(struct xfs_acl) + \
+               sizeof(struct xfs_acl_entry) * XFS_ACL_MAX_ENTRIES((mp)))

XFS_ACL_SIZE(mp) == (4 + 12) + 12 * ((64K - 4) / 12) == 65548

Did you want to add in the sizeof(struct xfs_acl) to the first term or the sizeof(__be32)? I would think the acl_entry[0] is the start of the array.

+
  /* On-disk XFS extended attribute names */
  #define SGI_ACL_FILE          (unsigned char *)"SGI_ACL_FILE"
  #define SGI_ACL_DEFAULT               (unsigned char *)"SGI_ACL_DEFAULT"

--Mark.

<Prev in Thread] Current Thread [Next in Thread>