Re: [PATCH v7 02/19] xfs: Embed struct xfs_name in xfs_da_args

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 





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.

Allison


Dave.






[Index of Archives]     [XFS Filesystem Development (older mail)]     [Linux Filesystem Development]     [Linux Audio Users]     [Yosemite Trails]     [Linux Kernel]     [Linux RAID]     [Linux SCSI]


  Powered by Linux