Linux-XFS Archive mirror
 help / color / mirror / Atom feed
From: "Darrick J. Wong" <djwong@kernel.org>
To: Christoph Hellwig <hch@infradead.org>
Cc: linux-xfs@vger.kernel.org, hch@lst.de
Subject: Re: [PATCH 4/5] xfs: make attr removal an explicit operation
Date: Tue, 16 Apr 2024 10:31:28 -0700	[thread overview]
Message-ID: <20240416173128.GS11948@frogsfrogsfrogs> (raw)
In-Reply-To: <Zh4IrLX10rprLUiV@infradead.org>

On Mon, Apr 15, 2024 at 10:12:12PM -0700, Christoph Hellwig wrote:
> > +	if (op != XFS_ATTRUPDATE_REMOVE || xfs_inode_hasattr(dp)) {
> >  		error = xfs_iext_count_may_overflow(dp, XFS_ATTR_FORK,
> >  				XFS_IEXT_ATTR_MANIP_CNT(rmt_blks));
> >  		if (error == -EFBIG)
> 
> No need to change this in the current patch, but checking for a remove
> on an inodes without attrs just to skip the extent count upgrade
> and not fail the whole operation is a little silly :)

Yeah, I don't think there's a point to checking @op here.  The only
code we need here is skipping the overflow checks if !xfs_inode_hasattr.
Separate patch.

> > @@ -203,7 +203,8 @@ __xfs_set_acl(struct inode *inode, struct posix_acl *acl, int type)
> >  		xfs_acl_to_disk(args.value, acl);
> >  	}
> >  
> > -	error = xfs_attr_change(&args, XFS_ATTRUPDATE_UPSERT);
> > +	error = xfs_attr_change(&args, args.value ? XFS_ATTRUPDATE_UPSERT :
> > +						    XFS_ATTRUPDATE_REMOVE);
> 
> Given that we have a conditional for removing vs setting right above
> i'd clean this up a bit:
> 
> 		xfs_acl_to_disk(args.value, acl);
> 		error = xfs_attr_change(&args, XFS_ATTRUPDATE_UPSERT)
> 		kvfree(args.value);
> 	} else {
> 		error = xfs_attr_change(&args, XFS_ATTRUPDATE_REMOVE)
> 		/*
> 		 * If the attribute didn't exist to start with that's fine.
> 		 */
> 		if (error == -ENOATTR)
> 			error = 0;
>   	}

Done.

> Otherwise looks good:
> 
> Reviewed-by: Christoph Hellwig <hch@lst.de>

Thanks!

--D

  reply	other threads:[~2024-04-16 17:31 UTC|newest]

Thread overview: 135+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-04-16  1:16 [PATCHBOMB v13.2] xfs: directory parent pointers Darrick J. Wong
2024-04-16  1:19 ` [PATCHSET v13.2 1/7] xfs: shrink struct xfs_da_args Darrick J. Wong
2024-04-16  1:21   ` [PATCH 1/5] xfs: remove XFS_DA_OP_REMOVE Darrick J. Wong
2024-04-16  1:21   ` [PATCH 2/5] xfs: remove XFS_DA_OP_NOTIME Darrick J. Wong
2024-04-16  1:21   ` [PATCH 3/5] xfs: remove xfs_da_args.attr_flags Darrick J. Wong
2024-04-16  5:07     ` Christoph Hellwig
2024-04-16  1:22   ` [PATCH 4/5] xfs: make attr removal an explicit operation Darrick J. Wong
2024-04-16  5:12     ` Christoph Hellwig
2024-04-16 17:31       ` Darrick J. Wong [this message]
2024-04-16  1:22   ` [PATCH 5/5] xfs: rearrange xfs_da_args a bit to use less space Darrick J. Wong
2024-04-16  5:13     ` Christoph Hellwig
2024-04-16  1:19 ` [PATCHSET v13.2 2/7] xfs: improve extended attribute validation Darrick J. Wong
2024-04-16  1:22   ` [PATCH 01/14] xfs: attr fork iext must be loaded before calling xfs_attr_is_leaf Darrick J. Wong
2024-04-16  1:22   ` [PATCH 02/14] xfs: require XFS_SB_FEAT_INCOMPAT_LOG_XATTRS for attr log intent item recovery Darrick J. Wong
2024-04-16  1:23   ` [PATCH 03/14] xfs: use an XFS_OPSTATE_ flag for detecting if logged xattrs are available Darrick J. Wong
2024-04-16  1:23   ` [PATCH 04/14] xfs: check opcode and iovec count match in xlog_recover_attri_commit_pass2 Darrick J. Wong
2024-04-16  1:23   ` [PATCH 05/14] xfs: fix missing check for invalid attr flags Darrick J. Wong
2024-04-16  1:23   ` [PATCH 06/14] xfs: check shortform attr entry flags specifically Darrick J. Wong
2024-04-16  5:13     ` Christoph Hellwig
2024-04-16  1:24   ` [PATCH 07/14] xfs: restructure xfs_attr_complete_op a bit Darrick J. Wong
2024-04-16  1:24   ` [PATCH 08/14] xfs: use helpers to extract xattr op from opflags Darrick J. Wong
2024-04-16  1:24   ` [PATCH 09/14] xfs: validate recovered name buffers when recovering xattr items Darrick J. Wong
2024-04-16  1:24   ` [PATCH 10/14] xfs: always set args->value in xfs_attri_item_recover Darrick J. Wong
2024-04-16  1:25   ` [PATCH 11/14] xfs: use local variables for name and value length in _attri_commit_pass2 Darrick J. Wong
2024-04-16  1:25   ` [PATCH 12/14] xfs: refactor name/length checks in xfs_attri_validate Darrick J. Wong
2024-04-16  1:25   ` [PATCH 13/14] xfs: refactor name/value iovec validation in xlog_recover_attri_commit_pass2 Darrick J. Wong
2024-04-16  5:15     ` Christoph Hellwig
2024-04-16  1:26   ` [PATCH 14/14] xfs: enforce one namespace per attribute Darrick J. Wong
2024-04-16  1:20 ` [PATCHSET v13.2 3/7] xfs: Parent Pointers Darrick J. Wong
2024-04-16  1:26   ` [PATCH 01/31] xfs: rearrange xfs_attr_match parameters Darrick J. Wong
2024-04-16  1:26   ` [PATCH 02/31] xfs: check the flags earlier in xfs_attr_match Darrick J. Wong
2024-04-16  1:26   ` [PATCH 03/31] xfs: move xfs_attr_defer_add to xfs_attr_item.c Darrick J. Wong
2024-04-16  1:27   ` [PATCH 04/31] xfs: create a separate hashname function for extended attributes Darrick J. Wong
2024-04-16  1:27   ` [PATCH 05/31] xfs: add parent pointer support to attribute code Darrick J. Wong
2024-04-16  1:27   ` [PATCH 06/31] xfs: define parent pointer ondisk extended attribute format Darrick J. Wong
2024-04-16  1:27   ` [PATCH 07/31] xfs: allow xattr matching on name and value for parent pointers Darrick J. Wong
2024-04-16  1:28   ` [PATCH 08/31] xfs: refactor xfs_is_using_logged_xattrs checks in attr item recovery Darrick J. Wong
2024-04-16  5:15     ` Christoph Hellwig
2024-04-16  1:28   ` [PATCH 09/31] xfs: create attr log item opcodes and formats for parent pointers Darrick J. Wong
2024-04-16  5:16     ` Christoph Hellwig
2024-04-17  2:52     ` [PATCH v13.2.1 " Darrick J. Wong
2024-04-16  1:28   ` [PATCH 10/31] xfs: record inode generation in xattr update log intent items Darrick J. Wong
2024-04-16  5:17     ` Christoph Hellwig
2024-04-16  1:28   ` [PATCH 11/31] xfs: Expose init_xattrs in xfs_create_tmpfile Darrick J. Wong
2024-04-16  1:29   ` [PATCH 12/31] xfs: add parent pointer validator functions Darrick J. Wong
2024-04-16  1:29   ` [PATCH 13/31] xfs: extend transaction reservations for parent attributes Darrick J. Wong
2024-04-16  1:29   ` [PATCH 14/31] xfs: create a hashname function for parent pointers Darrick J. Wong
2024-04-16  1:29   ` [PATCH 15/31] xfs: parent pointer attribute creation Darrick J. Wong
2024-04-16  1:30   ` [PATCH 16/31] xfs: add parent attributes to link Darrick J. Wong
2024-04-16  1:30   ` [PATCH 17/31] xfs: add parent attributes to symlink Darrick J. Wong
2024-04-16  1:30   ` [PATCH 18/31] xfs: remove parent pointers in unlink Darrick J. Wong
2024-04-16  1:30   ` [PATCH 19/31] xfs: Add parent pointers to rename Darrick J. Wong
2024-04-16  1:31   ` [PATCH 20/31] xfs: Add parent pointers to xfs_cross_rename Darrick J. Wong
2024-04-16  1:31   ` [PATCH 21/31] xfs: don't return XFS_ATTR_PARENT attributes via listxattr Darrick J. Wong
2024-04-16  5:17     ` Christoph Hellwig
2024-04-16  1:31   ` [PATCH 22/31] xfs: pass the attr value to put_listent when possible Darrick J. Wong
2024-04-16  1:32   ` [PATCH 23/31] xfs: move handle ioctl code to xfs_handle.c Darrick J. Wong
2024-04-16  1:32   ` [PATCH 24/31] xfs: split out handle management helpers a bit Darrick J. Wong
2024-04-16  1:32   ` [PATCH 25/31] xfs: actually check the fsid of a handle Darrick J. Wong
2024-04-16  5:19     ` Christoph Hellwig
2024-04-16 17:44       ` Darrick J. Wong
2024-04-16  1:32   ` [PATCH 26/31] xfs: add parent pointer ioctls Darrick J. Wong
2024-04-16  5:21     ` Christoph Hellwig
2024-04-16 17:59       ` Darrick J. Wong
2024-04-16 18:08         ` Christoph Hellwig
2024-04-16 18:12           ` Darrick J. Wong
2024-04-16 18:50             ` Christoph Hellwig
2024-04-17  2:49     ` [PATCH v13.2.1 " Darrick J. Wong
2024-04-17 22:25       ` Darrick J. Wong
2024-04-18  4:21         ` Christoph Hellwig
2024-04-18 16:49           ` Darrick J. Wong
2024-04-16  1:33   ` [PATCH 27/31] xfs: don't remove the attr fork when parent pointers are enabled Darrick J. Wong
2024-04-16  1:33   ` [PATCH 28/31] xfs: add a incompat feature bit for parent pointers Darrick J. Wong
2024-04-16  1:33   ` [PATCH 29/31] xfs: fix unit conversion error in xfs_log_calc_max_attrsetm_res Darrick J. Wong
2024-04-16  1:33   ` [PATCH 30/31] xfs: drop compatibility minimum log size computations for reflink Darrick J. Wong
2024-04-16  1:34   ` [PATCH 31/31] xfs: enable parent pointers Darrick J. Wong
2024-04-16  1:20 ` [PATCHSET v13.2 4/7] xfs: scrubbing for " Darrick J. Wong
2024-04-16  1:34   ` [PATCH 1/7] xfs: revert commit 44af6c7e59b12 Darrick J. Wong
2024-04-16  5:23     ` Christoph Hellwig
2024-04-16  1:34   ` [PATCH 2/7] xfs: check dirents have parent pointers Darrick J. Wong
2024-04-16  1:34   ` [PATCH 3/7] xfs: deferred scrub of dirents Darrick J. Wong
2024-04-16  1:35   ` [PATCH 4/7] xfs: scrub parent pointers Darrick J. Wong
2024-04-16  1:35   ` [PATCH 5/7] xfs: deferred scrub of " Darrick J. Wong
2024-04-16  1:35   ` [PATCH 6/7] xfs: walk directory parent pointers to determine backref count Darrick J. Wong
2024-04-16  1:35   ` [PATCH 7/7] xfs: check parent pointer xattrs when scrubbing Darrick J. Wong
2024-04-16  1:20 ` [PATCHSET v13.2 5/7] xfs: online repair for parent pointers Darrick J. Wong
2024-04-16  1:36   ` [PATCH 01/17] xfs: remove some boilerplate from xfs_attr_set Darrick J. Wong
2024-04-16  5:26     ` Christoph Hellwig
2024-04-16 18:15       ` Darrick J. Wong
2024-04-16  1:36   ` [PATCH 02/17] xfs: make the reserved block permission flag explicit in xfs_attr_set Darrick J. Wong
2024-04-16  5:26     ` Christoph Hellwig
2024-04-16  1:36   ` [PATCH 03/17] xfs: use xfs_attr_defer_parent for calling xfs_attr_set on pptrs Darrick J. Wong
2024-04-16  5:29     ` Christoph Hellwig
2024-04-16 16:05       ` Darrick J. Wong
2024-04-16 16:28         ` Christoph Hellwig
2024-04-16 18:41           ` Darrick J. Wong
2024-04-16 18:51             ` Christoph Hellwig
2024-04-17  2:54               ` Darrick J. Wong
2024-04-17  5:00                 ` Christoph Hellwig
2024-04-16  1:36   ` [PATCH 04/17] xfs: salvage parent pointers when rebuilding xattr structures Darrick J. Wong
2024-04-16  1:37   ` [PATCH 05/17] xfs: add raw parent pointer apis to support repair Darrick J. Wong
2024-04-16  1:37   ` [PATCH 06/17] xfs: repair directories by scanning directory parent pointers Darrick J. Wong
2024-04-16  1:37   ` [PATCH 07/17] xfs: implement live updates for directory repairs Darrick J. Wong
2024-04-16  1:38   ` [PATCH 08/17] xfs: replay unlocked parent pointer updates that accrue during xattr repair Darrick J. Wong
2024-04-16  1:38   ` [PATCH 09/17] xfs: repair directory parent pointers by scanning for dirents Darrick J. Wong
2024-04-16  1:38   ` [PATCH 10/17] xfs: implement live updates for parent pointer repairs Darrick J. Wong
2024-04-16  1:38   ` [PATCH 11/17] xfs: remove pointless unlocked assertion Darrick J. Wong
2024-04-16  1:39   ` [PATCH 12/17] xfs: split xfs_bmap_add_attrfork into two pieces Darrick J. Wong
2024-04-16  1:39   ` [PATCH 13/17] xfs: add a per-leaf block callback to xchk_xattr_walk Darrick J. Wong
2024-04-16  1:39   ` [PATCH 14/17] xfs: actually rebuild the parent pointer xattrs Darrick J. Wong
2024-04-16  1:39   ` [PATCH 15/17] xfs: adapt the orphanage code to handle parent pointers Darrick J. Wong
2024-04-16  1:40   ` [PATCH 16/17] xfs: repair link count of nondirectories after rebuilding " Darrick J. Wong
2024-04-16  1:40   ` [PATCH 17/17] xfs: inode repair should ensure there's an attr fork to store " Darrick J. Wong
2024-04-16  1:20 ` [PATCHSET v13.2 6/7] xfs: detect and correct directory tree problems Darrick J. Wong
2024-04-16  1:40   ` [PATCH 1/4] xfs: teach online scrub to find directory tree structure problems Darrick J. Wong
2024-04-16  1:40   ` [PATCH 2/4] xfs: invalidate dirloop scrub path data when concurrent updates happen Darrick J. Wong
2024-04-16  1:41   ` [PATCH 3/4] xfs: report directory tree corruption in the health information Darrick J. Wong
2024-04-16  1:41   ` [PATCH 4/4] xfs: fix corruptions in the directory tree Darrick J. Wong
2024-04-16  1:21 ` [PATCHSET v13.2 7/7] xfs: vectorize scrub kernel calls Darrick J. Wong
2024-04-16  1:41   ` [PATCH 1/4] xfs: reduce the rate of cond_resched calls inside scrub Darrick J. Wong
2024-04-16  1:41   ` [PATCH 2/4] xfs: move xfs_ioc_scrub_metadata to scrub.c Darrick J. Wong
2024-04-16  5:31     ` Christoph Hellwig
2024-04-16  1:42   ` [PATCH 3/4] xfs: introduce vectored scrub mode Darrick J. Wong
2024-04-16  5:33     ` Christoph Hellwig
2024-04-16 18:46       ` Darrick J. Wong
2024-04-16 18:56         ` Christoph Hellwig
2024-04-16 19:06           ` Darrick J. Wong
2024-04-17  2:55     ` [PATCH v13.2.1 " Darrick J. Wong
2024-04-16  1:42   ` [PATCH 4/4] xfs: only iget the file once when doing vectored scrub-by-handle Darrick J. Wong
2024-04-16  5:35     ` Christoph Hellwig
2024-04-16 22:31       ` Darrick J. Wong
2024-04-16 22:51         ` Darrick J. Wong
2024-04-17  5:02           ` Christoph Hellwig
2024-04-17  5:01         ` Christoph Hellwig
  -- strict thread matches above, loose matches on Subject: below --
2024-04-24  3:05 [PATCHSET v13.4 1/9] xfs: shrink struct xfs_da_args Darrick J. Wong
2024-04-24  3:08 ` [PATCH 4/5] xfs: make attr removal an explicit operation Darrick J. Wong

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20240416173128.GS11948@frogsfrogsfrogs \
    --to=djwong@kernel.org \
    --cc=hch@infradead.org \
    --cc=hch@lst.de \
    --cc=linux-xfs@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).