On Tue, Feb 18, 2020 at 04:47:29PM -0800, Darrick J. Wong wrote: > On Tue, Feb 18, 2020 at 04:48:56PM +0100, Christoph Hellwig wrote: > > On Tue, Feb 18, 2020 at 01:23:34PM +1100, Dave Chinner wrote: > > > > diff --git a/fs/xfs/libxfs/xfs_attr.c b/fs/xfs/libxfs/xfs_attr.c > > > > index d5c112b6dcdd..23e0d8ce39f8 100644 > > > > --- a/fs/xfs/libxfs/xfs_attr.c > > > > +++ b/fs/xfs/libxfs/xfs_attr.c > > > > @@ -898,7 +898,7 @@ xfs_attr_node_addname( > > > > * The INCOMPLETE flag means that we will find the "old" > > > > * attr, not the "new" one. > > > > */ > > > > - args->op_flags |= XFS_DA_OP_INCOMPLETE; > > > > + args->attr_namespace |= XFS_ATTR_INCOMPLETE; > > > > > > So args->attr_namespace is no longer an indication of what attribute > > > namespace to look up? Unless you are now defining incomplete > > > attributes to be in a namespace? > > > > It is the same field on disk. Yes, but the INCOMPLETE flag is not indicating a user specified attribute namespace type like user.*, system.* or secure.*. We're just using the flag to store that prefix in as little space as possible. Indeed, we can have incomplete attributes in all the namespaces, so what we are really using this field and it's flags for is search filtering. > > > If so, I think this needs more explanation than "we can use the > > > on-disk format directly instead". That's just telling me what the > > > patch is doing and doesn't explain why we are considering this > > > specific on disk flag to indicate a new type of "namespace" for > > > attributes lookups. Hence I think this needs more documentation in > > > both the commit and the code as the definition of > > > XFS_ATTR_INCOMPLETE doesn't really make it clear that this is now > > > considered a namespace signifier... > > > > Ok. Also if anyone has a better name for the field, suggestions welcome.. > > The bureaucrat part of my brain suggests "attr_flags", with a > XFS_ATTR_NAMESPACE() helper to extract just the namespace part by using > #define XFS_ATTR_NAMESPACE_MASK (XFS_ATTR_ROOT | XFS_ATTR_SECURE) If it's filtering, then attr_filter might be appropriate. We only return attributes that match the filter mask.... > I kind of dislike that idea because it seems like a lot of overkill to > keep the two namespace bits separate from the actual attr entry flags, > but maybe we need it... *nod* I don't think we need all that jazz (waves jazz hands in the air) if we look at it from the point of view of it being a filter mask rather than a namespace specifier.... Cheers, Dave. -- Dave Chinner david@xxxxxxxxxxxxx
