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. > > > 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) 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... --D
