On 11/8/19 2:17 PM, Darrick J. Wong wrote:
On Wed, Nov 06, 2019 at 06:27:55PM -0700, Allison Collins wrote:
And new helper functions xfs_attr3_leaf_flag_is_set and
xfs_attr3_leaf_flagsflipped. These routines check to see
if xfs_attr3_leaf_setflag or xfs_attr3_leaf_flipflags have
already been run. We will need this later for delayed
attributes since routines may be recalled several times
when -EAGAIN is returned.
Signed-off-by: Allison Collins <allison.henderson@xxxxxxxxxx>
---
fs/xfs/libxfs/xfs_attr_leaf.c | 94 +++++++++++++++++++++++++++++++++++++++++++
fs/xfs/libxfs/xfs_attr_leaf.h | 2 +
2 files changed, 96 insertions(+)
diff --git a/fs/xfs/libxfs/xfs_attr_leaf.c b/fs/xfs/libxfs/xfs_attr_leaf.c
index 42c037e..023c616 100644
--- a/fs/xfs/libxfs/xfs_attr_leaf.c
+++ b/fs/xfs/libxfs/xfs_attr_leaf.c
@@ -2809,6 +2809,40 @@ xfs_attr3_leaf_clearflag(
}
/*
+ * Check if the INCOMPLETE flag on an entry in a leaf block is set. This
+ * function can be used to check if xfs_attr3_leaf_setflag has already been
+ * called. The INCOMPLETE flag is used during attr rename operations to mark
+ * entries that are being renamed. Since renames should be atomic, only one of
It's also used when creating an xattr with a value stored in a remote
block so that we can commit the name entry to the log (with INCOMPLETE
set), allocate/write the remote value with ordered buffers, and then
commit a second transaction clearing the INCOMPLETE flag.
Now that I think about it ... this predicate is for non-rename setting
of attrs with remote values, and the "flagsflipped" predicate is for
rename operations, aren't they?\
Yes, but I'm realizing now, that we may not need these helpers if we
continue to move forward with the state machine. With the state
machine, we can simply flip the flag, set the next state, and move on
since that region of code will not execute again. The state switch will
jump us forward to where we were without having to check if we already
did it.
+ * them should appear as a completed attribute.
+ *
+ * isset is set to true if the flag is set or false otherwise
+ */
+int
+xfs_attr3_leaf_flag_is_set(
+ struct xfs_da_args *args,
+ bool *isset)
+{
+ struct xfs_attr_leafblock *leaf;
+ struct xfs_attr_leaf_entry *entry;
+ struct xfs_buf *bp;
+ struct xfs_inode *dp = args->dp;
+ int error = 0;
+
+ error = xfs_attr3_leaf_read(args->trans, dp, args->blkno,
+ XFS_DABUF_MAP_NOMAPPING, &bp);
+ if (error)
+ return error;
+
+ leaf = bp->b_addr;
+ entry = &xfs_attr3_leaf_entryp(leaf)[args->index];
+
+ *isset = ((entry->flags & XFS_ATTR_INCOMPLETE) != 0);
+ xfs_trans_brelse(args->trans, bp);
+
+ return 0;
+}
+
+/*
* Set the INCOMPLETE flag on an entry in a leaf block.
*/
int
@@ -2972,3 +3006,63 @@ xfs_attr3_leaf_flipflags(
return error;
}
+
+/*
+ * On a leaf entry, check to see if the INCOMPLETE flag is cleared
+ * in args->blkno/index and set in args->blkno2/index2.
Might be worth mentioning here that args->blkno is the old entry and
args->blkno2 is the new entry. This predicate will be used (by the
deferred attr item recovery code) to decide if we have to finish that
part of a rename operation, right?
No, it was used in the previous series xfs_attr_set_later where I was
aiming to rely more on the state of the fork rather than the state
machine. But with the new state machine in this series, I may be able
to just drop this patch.
Note that they could be
+ * in different blocks, or in the same block. This function can be used to
+ * check if xfs_attr3_leaf_flipflags has already been called. The INCOMPLETE
+ * flag is used during attr rename operations to mark entries that are being
+ * renamed. Since renames should be atomic, only one of them should appear as a
+ * completed attribute.
+ *
+ * isflipped is set to true if flags are flipped or false otherwise
+ */
+int
+xfs_attr3_leaf_flagsflipped(
I don't like "flagsflipped" because it's not clear to me what "flipped"
means.
xfs_attr3_leaf_rename_is_incomplete() ?
Sure, if I keep this patch in the set I will update it. Things are
still pretty wiggly, so i may still find a use for it, but hopefully I
can let it go.
+ struct xfs_da_args *args,
+ bool *isflipped)
+{
+ struct xfs_attr_leafblock *leaf1;
+ struct xfs_attr_leafblock *leaf2;
+ struct xfs_attr_leaf_entry *entry1;
+ struct xfs_attr_leaf_entry *entry2;
+ struct xfs_buf *bp1;
+ struct xfs_buf *bp2;
+ struct xfs_inode *dp = args->dp;
+ int error = 0;
+
+ /*
+ * Read the block containing the "old" attr
+ */
+ error = xfs_attr3_leaf_read(args->trans, dp, args->blkno,
+ XFS_DABUF_MAP_NOMAPPING, &bp1);
+ if (error)
+ return error;
+
+ /*
+ * Read the block containing the "new" attr, if it is different
+ */
+ if (args->blkno2 != args->blkno) {
+ error = xfs_attr3_leaf_read(args->trans, args->dp, args->blkno2,
+ -1, &bp2);
+ if (error)
bp1 leaks here, I think.
ok, will make a note.
+ return error;
+ } else {
+ bp2 = bp1;
+ }
+
+ leaf1 = bp1->b_addr;
+ entry1 = &xfs_attr3_leaf_entryp(leaf1)[args->index];
+
+ leaf2 = bp2->b_addr;
+ entry2 = &xfs_attr3_leaf_entryp(leaf2)[args->index2];
+
+ *isflipped = (((entry1->flags & XFS_ATTR_INCOMPLETE) == 0) &&
Nit: ((entry1->flags & XFS_ATTR_INCOMPLETE) == 0) could be written as
!(entry1->flags & XFS_ATTR_INCOMPLETE)
+ (entry2->flags & XFS_ATTR_INCOMPLETE));
+
+ xfs_trans_brelse(args->trans, bp1);
+ xfs_trans_brelse(args->trans, bp2);
This double-frees bp2 if bp1 == bp2.
Ok, will fix if needed. Thanks for the review! Sorry for the stray patch!
Allison
--D
+
+ return 0;
+}
diff --git a/fs/xfs/libxfs/xfs_attr_leaf.h b/fs/xfs/libxfs/xfs_attr_leaf.h
index e108b37..12283cf 100644
--- a/fs/xfs/libxfs/xfs_attr_leaf.h
+++ b/fs/xfs/libxfs/xfs_attr_leaf.h
@@ -57,7 +57,9 @@ int xfs_attr3_leaf_to_shortform(struct xfs_buf *bp,
struct xfs_da_args *args, int forkoff);
int xfs_attr3_leaf_clearflag(struct xfs_da_args *args);
int xfs_attr3_leaf_setflag(struct xfs_da_args *args);
+int xfs_attr3_leaf_flag_is_set(struct xfs_da_args *args, bool *isset);
int xfs_attr3_leaf_flipflags(struct xfs_da_args *args);
+int xfs_attr3_leaf_flagsflipped(struct xfs_da_args *args, bool *isflipped);
/*
* Routines used for growing the Btree.
--
2.7.4
