public inbox for linux-xfs@vger.kernel.org
 help / color / mirror / Atom feed
From: Brian Foster <bfoster@redhat.com>
To: "Darrick J. Wong" <djwong@kernel.org>
Cc: linux-xfs@vger.kernel.org, hch@infradead.org, david@fromorbit.com
Subject: Re: [PATCH 13/13] xfs: clean up xfs_trans_reserve_quota_chown a bit
Date: Thu, 28 Jan 2021 13:23:37 -0500	[thread overview]
Message-ID: <20210128182337.GL2619139@bfoster> (raw)
In-Reply-To: <161181373829.1523592.77926677559106032.stgit@magnolia>

On Wed, Jan 27, 2021 at 10:02:18PM -0800, Darrick J. Wong wrote:
> From: Darrick J. Wong <djwong@kernel.org>
> 
> Clean up the calling conventions of xfs_trans_reserve_quota_chown a
> littel bit -- we can pass in a boolean force parameter since that's the
> only qmopt that caller care about, and make the obvious deficiencies
> more obvious for anyone who someday wants to clean up rt quota.
> 
> Signed-off-by: Darrick J. Wong <djwong@kernel.org>
> ---

Reviewed-by: Brian Foster <bfoster@redhat.com>

>  fs/xfs/xfs_ioctl.c       |    2 +-
>  fs/xfs/xfs_iops.c        |    3 +--
>  fs/xfs/xfs_quota.h       |    4 ++--
>  fs/xfs/xfs_trans_dquot.c |   38 +++++++++++++++++++++-----------------
>  4 files changed, 25 insertions(+), 22 deletions(-)
> 
> 
> diff --git a/fs/xfs/xfs_ioctl.c b/fs/xfs/xfs_ioctl.c
> index e299fbd9ef13..73cfee8007a8 100644
> --- a/fs/xfs/xfs_ioctl.c
> +++ b/fs/xfs/xfs_ioctl.c
> @@ -1471,7 +1471,7 @@ xfs_ioctl_setattr(
>  	if (XFS_IS_QUOTA_RUNNING(mp) && XFS_IS_PQUOTA_ON(mp) &&
>  	    ip->i_d.di_projid != fa->fsx_projid) {
>  		code = xfs_trans_reserve_quota_chown(tp, ip, NULL, NULL, pdqp,
> -				capable(CAP_FOWNER) ?  XFS_QMOPT_FORCE_RES : 0);
> +				capable(CAP_FOWNER));
>  		if (code)	/* out of quota */
>  			goto error_trans_cancel;
>  	}
> diff --git a/fs/xfs/xfs_iops.c b/fs/xfs/xfs_iops.c
> index cb68be87e0a4..51c877ce90bc 100644
> --- a/fs/xfs/xfs_iops.c
> +++ b/fs/xfs/xfs_iops.c
> @@ -731,8 +731,7 @@ xfs_setattr_nonsize(
>  		     (XFS_IS_GQUOTA_ON(mp) && !gid_eq(igid, gid)))) {
>  			ASSERT(tp);
>  			error = xfs_trans_reserve_quota_chown(tp, ip, udqp,
> -					gdqp, NULL, capable(CAP_FOWNER) ?
> -					XFS_QMOPT_FORCE_RES : 0);
> +					gdqp, NULL, capable(CAP_FOWNER));
>  			if (error)	/* out of quota */
>  				goto out_cancel;
>  		}
> diff --git a/fs/xfs/xfs_quota.h b/fs/xfs/xfs_quota.h
> index d3876c71be8f..c3a5b48f5860 100644
> --- a/fs/xfs/xfs_quota.h
> +++ b/fs/xfs/xfs_quota.h
> @@ -100,7 +100,7 @@ extern struct xfs_dquot *xfs_qm_vop_chown(struct xfs_trans *,
>  		struct xfs_inode *, struct xfs_dquot **, struct xfs_dquot *);
>  int xfs_trans_reserve_quota_chown(struct xfs_trans *tp, struct xfs_inode *ip,
>  		struct xfs_dquot *udqp, struct xfs_dquot *gdqp,
> -		struct xfs_dquot *pdqp, unsigned int flags);
> +		struct xfs_dquot *pdqp, bool force);
>  extern int xfs_qm_dqattach(struct xfs_inode *);
>  extern int xfs_qm_dqattach_locked(struct xfs_inode *ip, bool doalloc);
>  extern void xfs_qm_dqdetach(struct xfs_inode *);
> @@ -165,7 +165,7 @@ xfs_trans_reserve_quota_icreate(struct xfs_trans *tp, struct xfs_dquot *udqp,
>  static inline int
>  xfs_trans_reserve_quota_chown(struct xfs_trans *tp, struct xfs_inode *ip,
>  		struct xfs_dquot *udqp, struct xfs_dquot *gdqp,
> -		struct xfs_dquot *pdqp, unsigned int flags)
> +		struct xfs_dquot *pdqp, bool force)
>  {
>  	return 0;
>  }
> diff --git a/fs/xfs/xfs_trans_dquot.c b/fs/xfs/xfs_trans_dquot.c
> index 88146280a177..73ef5994d09d 100644
> --- a/fs/xfs/xfs_trans_dquot.c
> +++ b/fs/xfs/xfs_trans_dquot.c
> @@ -846,26 +846,30 @@ xfs_trans_reserve_quota_chown(
>  	struct xfs_dquot	*udqp,
>  	struct xfs_dquot	*gdqp,
>  	struct xfs_dquot	*pdqp,
> -	unsigned int		flags)
> +	bool			force)
>  {
>  	struct xfs_mount	*mp = ip->i_mount;
> -	uint64_t		delblks;
> -	unsigned int		blkflags;
> -	struct xfs_dquot	*udq_unres = NULL;
> +	struct xfs_dquot	*udq_unres = NULL;	/* old dquots */
>  	struct xfs_dquot	*gdq_unres = NULL;
>  	struct xfs_dquot	*pdq_unres = NULL;
> -	struct xfs_dquot	*udq_delblks = NULL;
> +	struct xfs_dquot	*udq_delblks = NULL;	/* new dquots */
>  	struct xfs_dquot	*gdq_delblks = NULL;
>  	struct xfs_dquot	*pdq_delblks = NULL;
> +	uint64_t		delblks;
> +	unsigned int		qflags = XFS_QMOPT_RES_REGBLKS;
>  	int			error;
>  
> -
> -	ASSERT(xfs_isilocked(ip, XFS_ILOCK_EXCL|XFS_ILOCK_SHARED));
> +	/*
> +	 * XXX: This function doesn't handle rt quota counts correctly.  We
> +	 * don't support mounting with rt+quota so leave this breadcrumb.
> +	 */
> +	ASSERT(!XFS_IS_REALTIME_INODE(ip));
> +	ASSERT(xfs_isilocked(ip, XFS_ILOCK_EXCL));
>  	ASSERT(XFS_IS_QUOTA_RUNNING(mp));
>  
>  	delblks = ip->i_delayed_blks;
> -	blkflags = XFS_IS_REALTIME_INODE(ip) ?
> -			XFS_QMOPT_RES_RTBLKS : XFS_QMOPT_RES_REGBLKS;
> +	if (force)
> +		qflags |= XFS_QMOPT_FORCE_RES;
>  
>  	if (XFS_IS_UQUOTA_ON(mp) && udqp &&
>  	    i_uid_read(VFS_I(ip)) != udqp->q_id) {
> @@ -898,9 +902,9 @@ xfs_trans_reserve_quota_chown(
>  		}
>  	}
>  
> -	error = xfs_trans_reserve_quota_bydquots(tp, ip->i_mount,
> -				udq_delblks, gdq_delblks, pdq_delblks,
> -				ip->i_d.di_nblocks, 1, flags | blkflags);
> +	error = xfs_trans_reserve_quota_bydquots(tp, ip->i_mount, udq_delblks,
> +			gdq_delblks, pdq_delblks, ip->i_d.di_nblocks, 1,
> +			qflags);
>  	if (error)
>  		return error;
>  
> @@ -917,13 +921,13 @@ xfs_trans_reserve_quota_chown(
>  		ASSERT(udq_delblks || gdq_delblks || pdq_delblks);
>  		ASSERT(udq_unres || gdq_unres || pdq_unres);
>  		error = xfs_trans_reserve_quota_bydquots(NULL, ip->i_mount,
> -			    udq_delblks, gdq_delblks, pdq_delblks,
> -			    (xfs_qcnt_t)delblks, 0, flags | blkflags);
> +				udq_delblks, gdq_delblks, pdq_delblks,
> +				(xfs_qcnt_t)delblks, 0, qflags);
>  		if (error)
>  			return error;
> -		xfs_trans_reserve_quota_bydquots(NULL, ip->i_mount,
> -				udq_unres, gdq_unres, pdq_unres,
> -				-((xfs_qcnt_t)delblks), 0, blkflags);
> +		xfs_trans_reserve_quota_bydquots(NULL, ip->i_mount, udq_unres,
> +				gdq_unres, pdq_unres, -((xfs_qcnt_t)delblks),
> +				0, qflags);
>  	}
>  
>  	return 0;
> 


  parent reply	other threads:[~2021-01-28 18:27 UTC|newest]

Thread overview: 42+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-01-28  6:01 [PATCHSET v3 00/13] xfs: minor cleanups of the quota functions Darrick J. Wong
2021-01-28  6:01 ` [PATCH 01/13] xfs: clean up quota reservation callsites Darrick J. Wong
2021-01-28  6:01 ` [PATCH 02/13] xfs: create convenience wrappers for incore quota block reservations Darrick J. Wong
2021-01-28 18:08   ` Brian Foster
2021-01-28  6:01 ` [PATCH 03/13] xfs: remove xfs_trans_unreserve_quota_nblks completely Darrick J. Wong
2021-01-28  9:46   ` Christoph Hellwig
2021-01-28 18:08   ` Brian Foster
2021-01-28  6:01 ` [PATCH 04/13] xfs: clean up icreate quota reservation calls Darrick J. Wong
2021-01-28 18:09   ` Brian Foster
2021-01-28  6:01 ` [PATCH 05/13] xfs: fix up build warnings when quotas are disabled Darrick J. Wong
2021-01-28 18:09   ` Brian Foster
2021-01-28 18:22     ` Darrick J. Wong
2021-01-28 18:23       ` Christoph Hellwig
2021-01-28  6:01 ` [PATCH 06/13] xfs: reserve data and rt quota at the same time Darrick J. Wong
2021-01-28  9:49   ` Christoph Hellwig
2021-01-28 18:01     ` Darrick J. Wong
2021-01-28 18:10   ` Brian Foster
2021-01-28 18:52     ` Darrick J. Wong
2021-01-28  6:01 ` [PATCH 07/13] xfs: refactor common transaction/inode/quota allocation idiom Darrick J. Wong
2021-01-28  9:50   ` Christoph Hellwig
2021-01-28 18:22   ` Brian Foster
2021-01-28  6:01 ` [PATCH 08/13] xfs: allow reservation of rtblocks with xfs_trans_alloc_inode Darrick J. Wong
2021-01-28  9:51   ` Christoph Hellwig
2021-01-28 18:22   ` Brian Foster
2021-01-28  6:01 ` [PATCH 09/13] xfs: refactor reflink functions to use xfs_trans_alloc_inode Darrick J. Wong
2021-01-28  9:53   ` Christoph Hellwig
2021-01-28 18:06     ` Darrick J. Wong
2021-01-28 18:23   ` Brian Foster
2021-01-28  6:02 ` [PATCH 10/13] xfs: try worst case space reservation upfront in xfs_reflink_remap_extent Darrick J. Wong
2021-01-28  9:55   ` Christoph Hellwig
2021-01-28 18:23   ` Brian Foster
2021-01-28  6:02 ` [PATCH 11/13] xfs: refactor inode creation transaction/inode/quota allocation idiom Darrick J. Wong
2021-01-28  9:57   ` Christoph Hellwig
2021-01-28 18:18     ` Darrick J. Wong
2021-01-28 18:23   ` Brian Foster
2021-01-28  6:02 ` [PATCH 12/13] xfs: move xfs_qm_vop_chown_reserve to xfs_trans_dquot.c Darrick J. Wong
2021-01-28  9:58   ` Christoph Hellwig
2021-01-28 18:23   ` Brian Foster
2021-01-28  6:02 ` [PATCH 13/13] xfs: clean up xfs_trans_reserve_quota_chown a bit Darrick J. Wong
2021-01-28 10:00   ` Christoph Hellwig
2021-01-28 18:23   ` Brian Foster [this message]
  -- strict thread matches above, loose matches on Subject: below --
2021-01-29  2:16 [PATCHSET v4 00/13] xfs: minor cleanups of the quota functions Darrick J. Wong
2021-01-29  2:17 ` [PATCH 13/13] xfs: clean up xfs_trans_reserve_quota_chown a bit 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=20210128182337.GL2619139@bfoster \
    --to=bfoster@redhat.com \
    --cc=david@fromorbit.com \
    --cc=djwong@kernel.org \
    --cc=hch@infradead.org \
    --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