On Tue, Jun 04, 2013 at 01:26:40PM +1000, Dave Chinner wrote:
> On Mon, Jun 03, 2013 at 05:08:38PM -0500, Mark Tinguely wrote:
> > 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>
> > >---
....
> > > /* 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.
>
> Ugh, I lost that in translation somewhere. Good catch.
Actually, I didn't lose anything - your calculation of sizeof(struct
xfs_acl) is wrong. I went back to the output from pahole to check
this:
struct xfs_acl {
__be32 acl_cnt; /* 0 4 */
struct xfs_acl_entry acl_entry[0]; /* 4 0 */
/* size: 4, cachelines: 1, members: 2 */
/* last cacheline: 4 bytes */
};
sizeof(struct xfs_acl) = 4, not 16 as you calculated above. Therefore
code as posted is correct, if not immediately obvious.
The needed fix is to use sizeof(struct xfs_acl) in the
XFS_ACL_MAX_ENTRIES() calcluation rather than sizeof(__be32) so that
the two macros are clearly using the same structures for the
calculations...
-Dave.
--
Dave Chinner
david@xxxxxxxxxxxxx
_______________________________________________
xfs mailing list
xfs@xxxxxxxxxxx
http://oss.sgi.com/mailman/listinfo/xfs
