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?
> + * 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?
> 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() ?
> + 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.
> + 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.
--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
>
