On Tue, May 10, 2022 at 04:04:51PM -0700, Darrick J. Wong wrote:
> On Mon, May 09, 2022 at 10:41:24AM +1000, Dave Chinner wrote:
> > From: Dave Chinner <dchinner@xxxxxxxxxx>
> >
> > Logged attribute intents only have set and remove types - there is
> > no type of the replace operation. We should ahve a separate type for
>
> "..no separate intent type for a replace operation."?
>
> Also, "ahve" -> "have".
>
> > a replace operation, as it needs to perform operations that neither
> > SET or REMOVE can perform.
> >
> > Add this type to the intent items and rearrange the deferred
> > operation setup to reflect the different operations we are
> > performing.
> >
> > Signed-off-by: Dave Chinner <dchinner@xxxxxxxxxx>
> > Reviewed-by: Allison Henderson<allison.henderson@xxxxxxxxxx>
> > ---
> > fs/xfs/libxfs/xfs_attr.c | 165 +++++++++++++++++++--------------
> > fs/xfs/libxfs/xfs_attr.h | 2 -
> > fs/xfs/libxfs/xfs_log_format.h | 1 +
> > fs/xfs/xfs_attr_item.c | 9 +-
> > fs/xfs/xfs_trace.h | 4 +
> > 5 files changed, 110 insertions(+), 71 deletions(-)
> >
> > diff --git a/fs/xfs/libxfs/xfs_attr.c b/fs/xfs/libxfs/xfs_attr.c
> > index a4b0b20a3bab..817e15740f9c 100644
> > --- a/fs/xfs/libxfs/xfs_attr.c
> > +++ b/fs/xfs/libxfs/xfs_attr.c
> > @@ -671,6 +671,81 @@ xfs_attr_lookup(
> > return xfs_attr_node_hasname(args, NULL);
> > }
> >
> > +static int
> > +xfs_attr_item_init(
> > + struct xfs_da_args *args,
> > + unsigned int op_flags, /* op flag (set or remove) */
> > + struct xfs_attr_item **attr) /* new xfs_attr_item */
> > +{
> > +
> > + struct xfs_attr_item *new;
> > +
> > + new = kmem_zalloc(sizeof(struct xfs_attr_item), KM_NOFS);
>
> Can this fail?
>
> > + new->xattri_op_flags = op_flags;
> > + new->xattri_da_args = args;
> > +
> > + *attr = new;
> > + return 0;
>
> And if it can't, then there's no need for a return value, AFAICT. I
> looked at the end of xfs-5.19-compose and there's no other return
> statements in this function.
>
> And then you don't need any of the error handling in this patch at all,
> right?
>
> *OH*, this is just a hoist of the stuff at the end. Could someone add a
> patch on the end of ... whatever patchset there is to clean that up?
>
> In the meantime, with the commit message typos cleaned up,
> Reviewed-by: Darrick J. Wong <djwong@xxxxxxxxxx>
Thanks.
I'm making notes of all the things that need cleaning up. there's a
few things already, that probably won't happen until the next cycle
at this point...
Cheers,
Dave.
--
Dave Chinner
david@xxxxxxxxxxxxx
