On Mon, Feb 24, 2020 at 09:19:58PM -0700, Allison Collins wrote: > > > On 2/24/20 9:06 PM, Dave Chinner wrote: > > On Mon, Feb 24, 2020 at 07:00:35PM -0700, Allison Collins wrote: > > > > > > > > > On 2/24/20 5:57 PM, Dave Chinner wrote: > > > > On Sat, Feb 22, 2020 at 07:05:54PM -0700, Allison Collins wrote: > > > > > This patch embeds an xfs_name in xfs_da_args, replacing the name, namelen, and flags > > > > > members. This helps to clean up the xfs_da_args structure and make it more uniform > > > > > with the new xfs_name parameter being passed around. > > > > > > > > Commit message should wrap at 68-72 columns. > > > > > > > > > > > > > > Signed-off-by: Allison Collins <allison.henderson@xxxxxxxxxx> > > > > > Reviewed-by: Brian Foster <bfoster@xxxxxxxxxx> > > > > > --- > > > > > fs/xfs/libxfs/xfs_attr.c | 37 +++++++------- > > > > > fs/xfs/libxfs/xfs_attr_leaf.c | 104 +++++++++++++++++++++------------------- > > > > > fs/xfs/libxfs/xfs_attr_remote.c | 2 +- > > > > > fs/xfs/libxfs/xfs_da_btree.c | 6 ++- > > > > > fs/xfs/libxfs/xfs_da_btree.h | 4 +- > > > > > fs/xfs/libxfs/xfs_dir2.c | 18 +++---- > > > > > fs/xfs/libxfs/xfs_dir2_block.c | 6 +-- > > > > > fs/xfs/libxfs/xfs_dir2_leaf.c | 6 +-- > > > > > fs/xfs/libxfs/xfs_dir2_node.c | 8 ++-- > > > > > fs/xfs/libxfs/xfs_dir2_sf.c | 30 ++++++------ > > > > > fs/xfs/scrub/attr.c | 12 ++--- > > > > > fs/xfs/xfs_trace.h | 20 ++++---- > > > > > 12 files changed, 130 insertions(+), 123 deletions(-) > > > > > > > > > > diff --git a/fs/xfs/libxfs/xfs_attr.c b/fs/xfs/libxfs/xfs_attr.c > > > > > index 6717f47..9acdb23 100644 > > > > > --- a/fs/xfs/libxfs/xfs_attr.c > > > > > +++ b/fs/xfs/libxfs/xfs_attr.c > > > > > @@ -72,13 +72,12 @@ xfs_attr_args_init( > > > > > args->geo = dp->i_mount->m_attr_geo; > > > > > args->whichfork = XFS_ATTR_FORK; > > > > > args->dp = dp; > > > > > - args->flags = flags; > > > > > - args->name = name->name; > > > > > - args->namelen = name->len; > > > > > - if (args->namelen >= MAXNAMELEN) > > > > > + memcpy(&args->name, name, sizeof(struct xfs_name)); > > > > > + args->name.type = flags; > > > > > > > > This doesn't play well with Christoph's cleanup series which fixes > > > > up all the confusion with internal versus API flags. I guess the > > > > namespace is part of the attribute name, but I think this would be a > > > > much clearer conversion when placed on top of the way Christoph > > > > cleaned all this up... > > > > > > > > Have you looked at rebasing this on top of that cleanup series? > > > > > > > > Cheers, > > > > > > > Yes, there is some conflict between the sets here and there, but I think > > > folks wanted to keep them separate for now. > > > > That makes it really hard to form a clear view of what the code > > looks like after both patchsets have been applied. :( > > > > > Are you referring to > > > "[780d29057781] xfs: fix misuse of the XFS_ATTR_INCOMPLETE flag"? I'm > > > pretty sure this set is already seated on top of that one. This one is > > > based on the latest for-next. > > > > No, I'm talking about the series that ends up undoing that commit > > (i.e. the DA_OP_INCOMPLETE flag goes away again) and turns > > args->flags into args->attr_filter as the namespace filter for > > lookups. THis also turn adds XFS_ATTR_INCOMPLETE into a lookup > > filter. > > > > With this separation of ops vs lookup filters, moving the lookup > > filter into the xfs_name makes a bit more sense (i.e. the namespace > > filter is passed with the attribute name), but as a standalone > > movement it creates a bit of an impedence mismatch between the xname > > and the use of these flags. > > > > I think the end result will be fine, but it's making it hard for me > > to reconcile the changes in the two patchsets... > > > > Cheers, > > I'd be happy to go through the sets and find the intersections. Or make a > big super set if you like. I got the impression though that Christoph didnt > particularly like the delayed attr series or the idea of blending them. But > I do think it would be a good idea to take into consideration what the > combination of them is going to look like. At this point you might as well wait for me to actually put hch's attr interface refactoring series into for-next (unless this series is already based off of that??) though Christoph might be a bit time constrained this week... --D > Allison > > > > > Dave. > > > >