* [PATCH 1/3] xfs: remove support for disabling quota accounting on a mounted file system
2021-07-12 11:14 don't allow disabling quota accounting on a mounted file system Christoph Hellwig
@ 2021-07-12 11:14 ` Christoph Hellwig
2021-07-14 11:10 ` Carlos Maiolino
` (2 more replies)
2021-07-12 11:14 ` [PATCH 2/3] xfs: remove the flags argument to xfs_qm_dquot_walk Christoph Hellwig
2021-07-12 11:14 ` [PATCH 3/3] xfs: remove the active vs running quota differentiation Christoph Hellwig
2 siblings, 3 replies; 14+ messages in thread
From: Christoph Hellwig @ 2021-07-12 11:14 UTC (permalink / raw)
To: linux-xfs
Disabling quota accounting is hairy, racy code with all kinds of pitfalls.
And it has a very strange mind set, as quota accounting (unlike
enforcement) really is a propery of the on-disk format. There is no good
use case for supporting this.
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
fs/xfs/libxfs/xfs_trans_resv.c | 30 ----
fs/xfs/libxfs/xfs_trans_resv.h | 2 -
fs/xfs/xfs_dquot_item.c | 134 ------------------
fs/xfs/xfs_dquot_item.h | 17 ---
fs/xfs/xfs_qm.c | 2 +-
fs/xfs/xfs_qm.h | 3 -
fs/xfs/xfs_qm_syscalls.c | 241 ++-------------------------------
fs/xfs/xfs_trans_dquot.c | 38 ------
8 files changed, 13 insertions(+), 454 deletions(-)
diff --git a/fs/xfs/libxfs/xfs_trans_resv.c b/fs/xfs/libxfs/xfs_trans_resv.c
index d1a0848cb52eca..ce12c8142bd18e 100644
--- a/fs/xfs/libxfs/xfs_trans_resv.c
+++ b/fs/xfs/libxfs/xfs_trans_resv.c
@@ -798,29 +798,6 @@ xfs_calc_qm_dqalloc_reservation(
XFS_FSB_TO_B(mp, XFS_DQUOT_CLUSTER_SIZE_FSB) - 1);
}
-/*
- * Turning off quotas.
- * the quota off logitems: sizeof(struct xfs_qoff_logitem) * 2
- * the superblock for the quota flags: sector size
- */
-STATIC uint
-xfs_calc_qm_quotaoff_reservation(
- struct xfs_mount *mp)
-{
- return sizeof(struct xfs_qoff_logitem) * 2 +
- xfs_calc_buf_res(1, mp->m_sb.sb_sectsize);
-}
-
-/*
- * End of turning off quotas.
- * the quota off logitems: sizeof(struct xfs_qoff_logitem) * 2
- */
-STATIC uint
-xfs_calc_qm_quotaoff_end_reservation(void)
-{
- return sizeof(struct xfs_qoff_logitem) * 2;
-}
-
/*
* Syncing the incore super block changes to disk.
* the super block to reflect the changes: sector size
@@ -923,13 +900,6 @@ xfs_trans_resv_calc(
resp->tr_qm_setqlim.tr_logres = xfs_calc_qm_setqlim_reservation();
resp->tr_qm_setqlim.tr_logcount = XFS_DEFAULT_LOG_COUNT;
- resp->tr_qm_quotaoff.tr_logres = xfs_calc_qm_quotaoff_reservation(mp);
- resp->tr_qm_quotaoff.tr_logcount = XFS_DEFAULT_LOG_COUNT;
-
- resp->tr_qm_equotaoff.tr_logres =
- xfs_calc_qm_quotaoff_end_reservation();
- resp->tr_qm_equotaoff.tr_logcount = XFS_DEFAULT_LOG_COUNT;
-
resp->tr_sb.tr_logres = xfs_calc_sb_reservation(mp);
resp->tr_sb.tr_logcount = XFS_DEFAULT_LOG_COUNT;
diff --git a/fs/xfs/libxfs/xfs_trans_resv.h b/fs/xfs/libxfs/xfs_trans_resv.h
index 7241ab28cf84fe..fc4e9b369a3ae6 100644
--- a/fs/xfs/libxfs/xfs_trans_resv.h
+++ b/fs/xfs/libxfs/xfs_trans_resv.h
@@ -46,8 +46,6 @@ struct xfs_trans_resv {
struct xfs_trans_res tr_growrtfree; /* grow realtime freeing */
struct xfs_trans_res tr_qm_setqlim; /* adjust quota limits */
struct xfs_trans_res tr_qm_dqalloc; /* allocate quota on disk */
- struct xfs_trans_res tr_qm_quotaoff; /* turn quota off */
- struct xfs_trans_res tr_qm_equotaoff;/* end of turn quota off */
struct xfs_trans_res tr_sb; /* modify superblock */
struct xfs_trans_res tr_fsyncts; /* update timestamps on fsync */
};
diff --git a/fs/xfs/xfs_dquot_item.c b/fs/xfs/xfs_dquot_item.c
index 8ed47b739b6ccc..6a1aae799cf16d 100644
--- a/fs/xfs/xfs_dquot_item.c
+++ b/fs/xfs/xfs_dquot_item.c
@@ -218,137 +218,3 @@ xfs_qm_dquot_logitem_init(
&xfs_dquot_item_ops);
lp->qli_dquot = dqp;
}
-
-/*------------------ QUOTAOFF LOG ITEMS -------------------*/
-
-static inline struct xfs_qoff_logitem *QOFF_ITEM(struct xfs_log_item *lip)
-{
- return container_of(lip, struct xfs_qoff_logitem, qql_item);
-}
-
-
-/*
- * This returns the number of iovecs needed to log the given quotaoff item.
- * We only need 1 iovec for an quotaoff item. It just logs the
- * quotaoff_log_format structure.
- */
-STATIC void
-xfs_qm_qoff_logitem_size(
- struct xfs_log_item *lip,
- int *nvecs,
- int *nbytes)
-{
- *nvecs += 1;
- *nbytes += sizeof(struct xfs_qoff_logitem);
-}
-
-STATIC void
-xfs_qm_qoff_logitem_format(
- struct xfs_log_item *lip,
- struct xfs_log_vec *lv)
-{
- struct xfs_qoff_logitem *qflip = QOFF_ITEM(lip);
- struct xfs_log_iovec *vecp = NULL;
- struct xfs_qoff_logformat *qlf;
-
- qlf = xlog_prepare_iovec(lv, &vecp, XLOG_REG_TYPE_QUOTAOFF);
- qlf->qf_type = XFS_LI_QUOTAOFF;
- qlf->qf_size = 1;
- qlf->qf_flags = qflip->qql_flags;
- xlog_finish_iovec(lv, vecp, sizeof(struct xfs_qoff_logitem));
-}
-
-/*
- * There isn't much you can do to push a quotaoff item. It is simply
- * stuck waiting for the log to be flushed to disk.
- */
-STATIC uint
-xfs_qm_qoff_logitem_push(
- struct xfs_log_item *lip,
- struct list_head *buffer_list)
-{
- return XFS_ITEM_LOCKED;
-}
-
-STATIC xfs_lsn_t
-xfs_qm_qoffend_logitem_committed(
- struct xfs_log_item *lip,
- xfs_lsn_t lsn)
-{
- struct xfs_qoff_logitem *qfe = QOFF_ITEM(lip);
- struct xfs_qoff_logitem *qfs = qfe->qql_start_lip;
-
- xfs_qm_qoff_logitem_relse(qfs);
-
- kmem_free(lip->li_lv_shadow);
- kmem_free(qfe);
- return (xfs_lsn_t)-1;
-}
-
-STATIC void
-xfs_qm_qoff_logitem_release(
- struct xfs_log_item *lip)
-{
- struct xfs_qoff_logitem *qoff = QOFF_ITEM(lip);
-
- if (test_bit(XFS_LI_ABORTED, &lip->li_flags)) {
- if (qoff->qql_start_lip)
- xfs_qm_qoff_logitem_relse(qoff->qql_start_lip);
- xfs_qm_qoff_logitem_relse(qoff);
- }
-}
-
-static const struct xfs_item_ops xfs_qm_qoffend_logitem_ops = {
- .iop_size = xfs_qm_qoff_logitem_size,
- .iop_format = xfs_qm_qoff_logitem_format,
- .iop_committed = xfs_qm_qoffend_logitem_committed,
- .iop_push = xfs_qm_qoff_logitem_push,
- .iop_release = xfs_qm_qoff_logitem_release,
-};
-
-static const struct xfs_item_ops xfs_qm_qoff_logitem_ops = {
- .iop_size = xfs_qm_qoff_logitem_size,
- .iop_format = xfs_qm_qoff_logitem_format,
- .iop_push = xfs_qm_qoff_logitem_push,
- .iop_release = xfs_qm_qoff_logitem_release,
-};
-
-/*
- * Delete the quotaoff intent from the AIL and free it. On success,
- * this should only be called for the start item. It can be used for
- * either on shutdown or abort.
- */
-void
-xfs_qm_qoff_logitem_relse(
- struct xfs_qoff_logitem *qoff)
-{
- struct xfs_log_item *lip = &qoff->qql_item;
-
- ASSERT(test_bit(XFS_LI_IN_AIL, &lip->li_flags) ||
- test_bit(XFS_LI_ABORTED, &lip->li_flags) ||
- XFS_FORCED_SHUTDOWN(lip->li_mountp));
- xfs_trans_ail_delete(lip, 0);
- kmem_free(lip->li_lv_shadow);
- kmem_free(qoff);
-}
-
-/*
- * Allocate and initialize an quotaoff item of the correct quota type(s).
- */
-struct xfs_qoff_logitem *
-xfs_qm_qoff_logitem_init(
- struct xfs_mount *mp,
- struct xfs_qoff_logitem *start,
- uint flags)
-{
- struct xfs_qoff_logitem *qf;
-
- qf = kmem_zalloc(sizeof(struct xfs_qoff_logitem), 0);
-
- xfs_log_item_init(mp, &qf->qql_item, XFS_LI_QUOTAOFF, start ?
- &xfs_qm_qoffend_logitem_ops : &xfs_qm_qoff_logitem_ops);
- qf->qql_item.li_mountp = mp;
- qf->qql_start_lip = start;
- qf->qql_flags = flags;
- return qf;
-}
diff --git a/fs/xfs/xfs_dquot_item.h b/fs/xfs/xfs_dquot_item.h
index 2b86a43d7ce2ec..794710c2447493 100644
--- a/fs/xfs/xfs_dquot_item.h
+++ b/fs/xfs/xfs_dquot_item.h
@@ -9,7 +9,6 @@
struct xfs_dquot;
struct xfs_trans;
struct xfs_mount;
-struct xfs_qoff_logitem;
struct xfs_dq_logitem {
struct xfs_log_item qli_item; /* common portion */
@@ -17,22 +16,6 @@ struct xfs_dq_logitem {
xfs_lsn_t qli_flush_lsn; /* lsn at last flush */
};
-struct xfs_qoff_logitem {
- struct xfs_log_item qql_item; /* common portion */
- struct xfs_qoff_logitem *qql_start_lip; /* qoff-start logitem, if any */
- unsigned int qql_flags;
-};
-
-
void xfs_qm_dquot_logitem_init(struct xfs_dquot *dqp);
-struct xfs_qoff_logitem *xfs_qm_qoff_logitem_init(struct xfs_mount *mp,
- struct xfs_qoff_logitem *start,
- uint flags);
-void xfs_qm_qoff_logitem_relse(struct xfs_qoff_logitem *);
-struct xfs_qoff_logitem *xfs_trans_get_qoff_item(struct xfs_trans *tp,
- struct xfs_qoff_logitem *startqoff,
- uint flags);
-void xfs_trans_log_quotaoff_item(struct xfs_trans *tp,
- struct xfs_qoff_logitem *qlp);
#endif /* __XFS_DQUOT_ITEM_H__ */
diff --git a/fs/xfs/xfs_qm.c b/fs/xfs/xfs_qm.c
index fe341f3fd41901..580b9dba21122b 100644
--- a/fs/xfs/xfs_qm.c
+++ b/fs/xfs/xfs_qm.c
@@ -185,7 +185,7 @@ xfs_qm_dqpurge(
/*
* Purge the dquot cache.
*/
-void
+static void
xfs_qm_dqpurge_all(
struct xfs_mount *mp,
uint flags)
diff --git a/fs/xfs/xfs_qm.h b/fs/xfs/xfs_qm.h
index ebbb484c49dc7e..442a0f97a9d439 100644
--- a/fs/xfs/xfs_qm.h
+++ b/fs/xfs/xfs_qm.h
@@ -140,9 +140,6 @@ struct xfs_dquot_acct {
extern void xfs_qm_destroy_quotainfo(struct xfs_mount *);
-/* dquot stuff */
-extern void xfs_qm_dqpurge_all(struct xfs_mount *, uint);
-
/* quota ops */
extern int xfs_qm_scall_trunc_qfiles(struct xfs_mount *, uint);
extern int xfs_qm_scall_getquota(struct xfs_mount *mp,
diff --git a/fs/xfs/xfs_qm_syscalls.c b/fs/xfs/xfs_qm_syscalls.c
index 13a56e1ea15ce1..e00067a142a094 100644
--- a/fs/xfs/xfs_qm_syscalls.c
+++ b/fs/xfs/xfs_qm_syscalls.c
@@ -19,91 +19,11 @@
#include "xfs_qm.h"
#include "xfs_icache.h"
-STATIC int
-xfs_qm_log_quotaoff(
- struct xfs_mount *mp,
- struct xfs_qoff_logitem **qoffstartp,
- uint flags)
-{
- struct xfs_trans *tp;
- int error;
- struct xfs_qoff_logitem *qoffi;
-
- error = xfs_trans_alloc(mp, &M_RES(mp)->tr_qm_quotaoff, 0, 0, 0, &tp);
- if (error)
- goto out;
-
- qoffi = xfs_trans_get_qoff_item(tp, NULL, flags & XFS_ALL_QUOTA_ACCT);
- xfs_trans_log_quotaoff_item(tp, qoffi);
-
- spin_lock(&mp->m_sb_lock);
- mp->m_sb.sb_qflags = (mp->m_qflags & ~(flags)) & XFS_MOUNT_QUOTA_ALL;
- spin_unlock(&mp->m_sb_lock);
-
- xfs_log_sb(tp);
-
- /*
- * We have to make sure that the transaction is secure on disk before we
- * return and actually stop quota accounting. So, make it synchronous.
- * We don't care about quotoff's performance.
- */
- xfs_trans_set_sync(tp);
- error = xfs_trans_commit(tp);
- if (error)
- goto out;
-
- *qoffstartp = qoffi;
-out:
- return error;
-}
-
-STATIC int
-xfs_qm_log_quotaoff_end(
- struct xfs_mount *mp,
- struct xfs_qoff_logitem **startqoff,
- uint flags)
-{
- struct xfs_trans *tp;
- int error;
- struct xfs_qoff_logitem *qoffi;
-
- error = xfs_trans_alloc(mp, &M_RES(mp)->tr_qm_equotaoff, 0, 0, 0, &tp);
- if (error)
- return error;
-
- qoffi = xfs_trans_get_qoff_item(tp, *startqoff,
- flags & XFS_ALL_QUOTA_ACCT);
- xfs_trans_log_quotaoff_item(tp, qoffi);
- *startqoff = NULL;
-
- /*
- * We have to make sure that the transaction is secure on disk before we
- * return and actually stop quota accounting. So, make it synchronous.
- * We don't care about quotoff's performance.
- */
- xfs_trans_set_sync(tp);
- return xfs_trans_commit(tp);
-}
-
-/*
- * Turn off quota accounting and/or enforcement for all udquots and/or
- * gdquots. Called only at unmount time.
- *
- * This assumes that there are no dquots of this file system cached
- * incore, and modifies the ondisk dquot directly. Therefore, for example,
- * it is an error to call this twice, without purging the cache.
- */
int
xfs_qm_scall_quotaoff(
xfs_mount_t *mp,
uint flags)
{
- struct xfs_quotainfo *q = mp->m_quotainfo;
- uint dqtype;
- int error;
- uint inactivate_flags;
- struct xfs_qoff_logitem *qoffstart = NULL;
-
/*
* No file system can have quotas enabled on disk but not in core.
* Note that quota utilities (like quotaoff) _expect_
@@ -111,160 +31,23 @@ xfs_qm_scall_quotaoff(
*/
if ((mp->m_qflags & flags) == 0)
return -EEXIST;
- error = 0;
-
- flags &= (XFS_ALL_QUOTA_ACCT | XFS_ALL_QUOTA_ENFD);
/*
- * We don't want to deal with two quotaoffs messing up each other,
- * so we're going to serialize it. quotaoff isn't exactly a performance
- * critical thing.
- * If quotaoff, then we must be dealing with the root filesystem.
+ * We do not support actually turning off quota accounting any more.
+ * Just log a warning and ignored the accounting related flags.
*/
- ASSERT(q);
- mutex_lock(&q->qi_quotaofflock);
+ if (flags & XFS_ALL_QUOTA_ACCT)
+ xfs_info(mp, "disabling of quota accounting not supported.");
- /*
- * If we're just turning off quota enforcement, change mp and go.
- */
- if ((flags & XFS_ALL_QUOTA_ACCT) == 0) {
- mp->m_qflags &= ~(flags);
-
- spin_lock(&mp->m_sb_lock);
- mp->m_sb.sb_qflags = mp->m_qflags;
- spin_unlock(&mp->m_sb_lock);
- mutex_unlock(&q->qi_quotaofflock);
-
- /* XXX what to do if error ? Revert back to old vals incore ? */
- return xfs_sync_sb(mp, false);
- }
-
- dqtype = 0;
- inactivate_flags = 0;
- /*
- * If accounting is off, we must turn enforcement off, clear the
- * quota 'CHKD' certificate to make it known that we have to
- * do a quotacheck the next time this quota is turned on.
- */
- if (flags & XFS_UQUOTA_ACCT) {
- dqtype |= XFS_QMOPT_UQUOTA;
- flags |= (XFS_UQUOTA_CHKD | XFS_UQUOTA_ENFD);
- inactivate_flags |= XFS_UQUOTA_ACTIVE;
- }
- if (flags & XFS_GQUOTA_ACCT) {
- dqtype |= XFS_QMOPT_GQUOTA;
- flags |= (XFS_GQUOTA_CHKD | XFS_GQUOTA_ENFD);
- inactivate_flags |= XFS_GQUOTA_ACTIVE;
- }
- if (flags & XFS_PQUOTA_ACCT) {
- dqtype |= XFS_QMOPT_PQUOTA;
- flags |= (XFS_PQUOTA_CHKD | XFS_PQUOTA_ENFD);
- inactivate_flags |= XFS_PQUOTA_ACTIVE;
- }
-
- /*
- * Nothing to do? Don't complain. This happens when we're just
- * turning off quota enforcement.
- */
- if ((mp->m_qflags & flags) == 0)
- goto out_unlock;
-
- /*
- * Write the LI_QUOTAOFF log record, and do SB changes atomically,
- * and synchronously. If we fail to write, we should abort the
- * operation as it cannot be recovered safely if we crash.
- */
- error = xfs_qm_log_quotaoff(mp, &qoffstart, flags);
- if (error)
- goto out_unlock;
-
- /*
- * Next we clear the XFS_MOUNT_*DQ_ACTIVE bit(s) in the mount struct
- * to take care of the race between dqget and quotaoff. We don't take
- * any special locks to reset these bits. All processes need to check
- * these bits *after* taking inode lock(s) to see if the particular
- * quota type is in the process of being turned off. If *ACTIVE, it is
- * guaranteed that all dquot structures and all quotainode ptrs will all
- * stay valid as long as that inode is kept locked.
- *
- * There is no turning back after this.
- */
- mp->m_qflags &= ~inactivate_flags;
-
- /*
- * Give back all the dquot reference(s) held by inodes.
- * Here we go thru every single incore inode in this file system, and
- * do a dqrele on the i_udquot/i_gdquot that it may have.
- * Essentially, as long as somebody has an inode locked, this guarantees
- * that quotas will not be turned off. This is handy because in a
- * transaction once we lock the inode(s) and check for quotaon, we can
- * depend on the quota inodes (and other things) being valid as long as
- * we keep the lock(s).
- */
- error = xfs_dqrele_all_inodes(mp, flags);
- ASSERT(!error);
-
- /*
- * Next we make the changes in the quota flag in the mount struct.
- * This isn't protected by a particular lock directly, because we
- * don't want to take a mrlock every time we depend on quotas being on.
- */
- mp->m_qflags &= ~flags;
-
- /*
- * Go through all the dquots of this file system and purge them,
- * according to what was turned off.
- */
- xfs_qm_dqpurge_all(mp, dqtype);
-
- /*
- * Transactions that had started before ACTIVE state bit was cleared
- * could have logged many dquots, so they'd have higher LSNs than
- * the first QUOTAOFF log record does. If we happen to crash when
- * the tail of the log has gone past the QUOTAOFF record, but
- * before the last dquot modification, those dquots __will__
- * recover, and that's not good.
- *
- * So, we have QUOTAOFF start and end logitems; the start
- * logitem won't get overwritten until the end logitem appears...
- */
- error = xfs_qm_log_quotaoff_end(mp, &qoffstart, flags);
- if (error) {
- /* We're screwed now. Shutdown is the only option. */
- xfs_force_shutdown(mp, SHUTDOWN_CORRUPT_INCORE);
- goto out_unlock;
- }
-
- /*
- * If all quotas are completely turned off, close shop.
- */
- if (mp->m_qflags == 0) {
- mutex_unlock(&q->qi_quotaofflock);
- xfs_qm_destroy_quotainfo(mp);
- return 0;
- }
-
- /*
- * Release our quotainode references if we don't need them anymore.
- */
- if ((dqtype & XFS_QMOPT_UQUOTA) && q->qi_uquotaip) {
- xfs_irele(q->qi_uquotaip);
- q->qi_uquotaip = NULL;
- }
- if ((dqtype & XFS_QMOPT_GQUOTA) && q->qi_gquotaip) {
- xfs_irele(q->qi_gquotaip);
- q->qi_gquotaip = NULL;
- }
- if ((dqtype & XFS_QMOPT_PQUOTA) && q->qi_pquotaip) {
- xfs_irele(q->qi_pquotaip);
- q->qi_pquotaip = NULL;
- }
+ mutex_lock(&mp->m_quotainfo->qi_quotaofflock);
+ mp->m_qflags &= ~(flags & XFS_ALL_QUOTA_ENFD);
+ spin_lock(&mp->m_sb_lock);
+ mp->m_sb.sb_qflags = mp->m_qflags;
+ spin_unlock(&mp->m_sb_lock);
+ mutex_unlock(&mp->m_quotainfo->qi_quotaofflock);
-out_unlock:
- if (error && qoffstart)
- xfs_qm_qoff_logitem_relse(qoffstart);
- mutex_unlock(&q->qi_quotaofflock);
- return error;
+ /* XXX what to do if error ? Revert back to old vals incore ? */
+ return xfs_sync_sb(mp, false);
}
STATIC int
diff --git a/fs/xfs/xfs_trans_dquot.c b/fs/xfs/xfs_trans_dquot.c
index 48e09ea30ee539..b7e4b05a559bdb 100644
--- a/fs/xfs/xfs_trans_dquot.c
+++ b/fs/xfs/xfs_trans_dquot.c
@@ -843,44 +843,6 @@ xfs_trans_reserve_quota_icreate(
dblocks, 1, XFS_QMOPT_RES_REGBLKS);
}
-/*
- * This routine is called to allocate a quotaoff log item.
- */
-struct xfs_qoff_logitem *
-xfs_trans_get_qoff_item(
- struct xfs_trans *tp,
- struct xfs_qoff_logitem *startqoff,
- uint flags)
-{
- struct xfs_qoff_logitem *q;
-
- ASSERT(tp != NULL);
-
- q = xfs_qm_qoff_logitem_init(tp->t_mountp, startqoff, flags);
- ASSERT(q != NULL);
-
- /*
- * Get a log_item_desc to point at the new item.
- */
- xfs_trans_add_item(tp, &q->qql_item);
- return q;
-}
-
-
-/*
- * This is called to mark the quotaoff logitem as needing
- * to be logged when the transaction is committed. The logitem must
- * already be associated with the given transaction.
- */
-void
-xfs_trans_log_quotaoff_item(
- struct xfs_trans *tp,
- struct xfs_qoff_logitem *qlp)
-{
- tp->t_flags |= XFS_TRANS_DIRTY;
- set_bit(XFS_LI_DIRTY, &qlp->qql_item.li_flags);
-}
-
STATIC void
xfs_trans_alloc_dqinfo(
xfs_trans_t *tp)
--
2.30.2
^ permalink raw reply related [flat|nested] 14+ messages in thread* Re: [PATCH 1/3] xfs: remove support for disabling quota accounting on a mounted file system
2021-07-12 11:14 ` [PATCH 1/3] xfs: remove support for " Christoph Hellwig
@ 2021-07-14 11:10 ` Carlos Maiolino
2021-07-20 6:33 ` Christoph Hellwig
2021-07-14 21:44 ` Darrick J. Wong
2021-07-15 17:15 ` Darrick J. Wong
2 siblings, 1 reply; 14+ messages in thread
From: Carlos Maiolino @ 2021-07-14 11:10 UTC (permalink / raw)
To: Christoph Hellwig; +Cc: linux-xfs
On Mon, Jul 12, 2021 at 01:14:24PM +0200, Christoph Hellwig wrote:
> Disabling quota accounting is hairy, racy code with all kinds of pitfalls.
> And it has a very strange mind set, as quota accounting (unlike
> enforcement) really is a propery of the on-disk format. There is no good
> use case for supporting this.
>
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> - if ((dqtype & XFS_QMOPT_UQUOTA) && q->qi_uquotaip) {
> - xfs_irele(q->qi_uquotaip);
> - q->qi_uquotaip = NULL;
> - }
> - if ((dqtype & XFS_QMOPT_GQUOTA) && q->qi_gquotaip) {
> - xfs_irele(q->qi_gquotaip);
> - q->qi_gquotaip = NULL;
> - }
> - if ((dqtype & XFS_QMOPT_PQUOTA) && q->qi_pquotaip) {
> - xfs_irele(q->qi_pquotaip);
> - q->qi_pquotaip = NULL;
> - }
> + mutex_lock(&mp->m_quotainfo->qi_quotaofflock);
> + mp->m_qflags &= ~(flags & XFS_ALL_QUOTA_ENFD);
> + spin_lock(&mp->m_sb_lock);
> + mp->m_sb.sb_qflags = mp->m_qflags;
> + spin_unlock(&mp->m_sb_lock);
> + mutex_unlock(&mp->m_quotainfo->qi_quotaofflock);
>
> -out_unlock:
> - if (error && qoffstart)
> - xfs_qm_qoff_logitem_relse(qoffstart);
> - mutex_unlock(&q->qi_quotaofflock);
> - return error;
> + /* XXX what to do if error ? Revert back to old vals incore ? */
> + return xfs_sync_sb(mp, false);
May be too strict, but I wonder if we shouldn't shut the FS down in case we fail
here?
Patch looks good though.
Reviewed-by: Carlos Maiolino <cmaiolino@redhat.com>
--
Carlos
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [PATCH 1/3] xfs: remove support for disabling quota accounting on a mounted file system
2021-07-14 11:10 ` Carlos Maiolino
@ 2021-07-20 6:33 ` Christoph Hellwig
0 siblings, 0 replies; 14+ messages in thread
From: Christoph Hellwig @ 2021-07-20 6:33 UTC (permalink / raw)
To: Christoph Hellwig, linux-xfs
On Wed, Jul 14, 2021 at 01:10:49PM +0200, Carlos Maiolino wrote:
> > - mutex_unlock(&q->qi_quotaofflock);
> > - return error;
> > + /* XXX what to do if error ? Revert back to old vals incore ? */
>
> May be too strict, but I wonder if we shouldn't shut the FS down in case we fail
> here?
This is just the existing code being unindented by one level. The
usual transaction commit failure cases will cause a shutdown anyway.
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 1/3] xfs: remove support for disabling quota accounting on a mounted file system
2021-07-12 11:14 ` [PATCH 1/3] xfs: remove support for " Christoph Hellwig
2021-07-14 11:10 ` Carlos Maiolino
@ 2021-07-14 21:44 ` Darrick J. Wong
2021-07-15 17:15 ` Darrick J. Wong
2 siblings, 0 replies; 14+ messages in thread
From: Darrick J. Wong @ 2021-07-14 21:44 UTC (permalink / raw)
To: Christoph Hellwig; +Cc: linux-xfs
On Mon, Jul 12, 2021 at 01:14:24PM +0200, Christoph Hellwig wrote:
> Disabling quota accounting is hairy, racy code with all kinds of pitfalls.
> And it has a very strange mind set, as quota accounting (unlike
> enforcement) really is a propery of the on-disk format. There is no good
> use case for supporting this.
>
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
> fs/xfs/libxfs/xfs_trans_resv.c | 30 ----
> fs/xfs/libxfs/xfs_trans_resv.h | 2 -
> fs/xfs/xfs_dquot_item.c | 134 ------------------
> fs/xfs/xfs_dquot_item.h | 17 ---
> fs/xfs/xfs_qm.c | 2 +-
> fs/xfs/xfs_qm.h | 3 -
> fs/xfs/xfs_qm_syscalls.c | 241 ++-------------------------------
> fs/xfs/xfs_trans_dquot.c | 38 ------
> 8 files changed, 13 insertions(+), 454 deletions(-)
>
> diff --git a/fs/xfs/libxfs/xfs_trans_resv.c b/fs/xfs/libxfs/xfs_trans_resv.c
> index d1a0848cb52eca..ce12c8142bd18e 100644
> --- a/fs/xfs/libxfs/xfs_trans_resv.c
> +++ b/fs/xfs/libxfs/xfs_trans_resv.c
> @@ -798,29 +798,6 @@ xfs_calc_qm_dqalloc_reservation(
> XFS_FSB_TO_B(mp, XFS_DQUOT_CLUSTER_SIZE_FSB) - 1);
> }
>
> -/*
> - * Turning off quotas.
> - * the quota off logitems: sizeof(struct xfs_qoff_logitem) * 2
> - * the superblock for the quota flags: sector size
> - */
> -STATIC uint
> -xfs_calc_qm_quotaoff_reservation(
> - struct xfs_mount *mp)
> -{
> - return sizeof(struct xfs_qoff_logitem) * 2 +
> - xfs_calc_buf_res(1, mp->m_sb.sb_sectsize);
> -}
> -
> -/*
> - * End of turning off quotas.
> - * the quota off logitems: sizeof(struct xfs_qoff_logitem) * 2
> - */
> -STATIC uint
> -xfs_calc_qm_quotaoff_end_reservation(void)
> -{
> - return sizeof(struct xfs_qoff_logitem) * 2;
> -}
> -
> /*
> * Syncing the incore super block changes to disk.
> * the super block to reflect the changes: sector size
> @@ -923,13 +900,6 @@ xfs_trans_resv_calc(
> resp->tr_qm_setqlim.tr_logres = xfs_calc_qm_setqlim_reservation();
> resp->tr_qm_setqlim.tr_logcount = XFS_DEFAULT_LOG_COUNT;
>
> - resp->tr_qm_quotaoff.tr_logres = xfs_calc_qm_quotaoff_reservation(mp);
> - resp->tr_qm_quotaoff.tr_logcount = XFS_DEFAULT_LOG_COUNT;
> -
> - resp->tr_qm_equotaoff.tr_logres =
> - xfs_calc_qm_quotaoff_end_reservation();
> - resp->tr_qm_equotaoff.tr_logcount = XFS_DEFAULT_LOG_COUNT;
> -
> resp->tr_sb.tr_logres = xfs_calc_sb_reservation(mp);
> resp->tr_sb.tr_logcount = XFS_DEFAULT_LOG_COUNT;
>
> diff --git a/fs/xfs/libxfs/xfs_trans_resv.h b/fs/xfs/libxfs/xfs_trans_resv.h
> index 7241ab28cf84fe..fc4e9b369a3ae6 100644
> --- a/fs/xfs/libxfs/xfs_trans_resv.h
> +++ b/fs/xfs/libxfs/xfs_trans_resv.h
> @@ -46,8 +46,6 @@ struct xfs_trans_resv {
> struct xfs_trans_res tr_growrtfree; /* grow realtime freeing */
> struct xfs_trans_res tr_qm_setqlim; /* adjust quota limits */
> struct xfs_trans_res tr_qm_dqalloc; /* allocate quota on disk */
> - struct xfs_trans_res tr_qm_quotaoff; /* turn quota off */
> - struct xfs_trans_res tr_qm_equotaoff;/* end of turn quota off */
> struct xfs_trans_res tr_sb; /* modify superblock */
> struct xfs_trans_res tr_fsyncts; /* update timestamps on fsync */
> };
> diff --git a/fs/xfs/xfs_dquot_item.c b/fs/xfs/xfs_dquot_item.c
> index 8ed47b739b6ccc..6a1aae799cf16d 100644
> --- a/fs/xfs/xfs_dquot_item.c
> +++ b/fs/xfs/xfs_dquot_item.c
> @@ -218,137 +218,3 @@ xfs_qm_dquot_logitem_init(
> &xfs_dquot_item_ops);
> lp->qli_dquot = dqp;
> }
> -
> -/*------------------ QUOTAOFF LOG ITEMS -------------------*/
> -
> -static inline struct xfs_qoff_logitem *QOFF_ITEM(struct xfs_log_item *lip)
> -{
> - return container_of(lip, struct xfs_qoff_logitem, qql_item);
> -}
> -
> -
> -/*
> - * This returns the number of iovecs needed to log the given quotaoff item.
> - * We only need 1 iovec for an quotaoff item. It just logs the
> - * quotaoff_log_format structure.
> - */
> -STATIC void
> -xfs_qm_qoff_logitem_size(
> - struct xfs_log_item *lip,
> - int *nvecs,
> - int *nbytes)
> -{
> - *nvecs += 1;
> - *nbytes += sizeof(struct xfs_qoff_logitem);
> -}
> -
> -STATIC void
> -xfs_qm_qoff_logitem_format(
> - struct xfs_log_item *lip,
> - struct xfs_log_vec *lv)
> -{
> - struct xfs_qoff_logitem *qflip = QOFF_ITEM(lip);
> - struct xfs_log_iovec *vecp = NULL;
> - struct xfs_qoff_logformat *qlf;
> -
> - qlf = xlog_prepare_iovec(lv, &vecp, XLOG_REG_TYPE_QUOTAOFF);
> - qlf->qf_type = XFS_LI_QUOTAOFF;
> - qlf->qf_size = 1;
> - qlf->qf_flags = qflip->qql_flags;
> - xlog_finish_iovec(lv, vecp, sizeof(struct xfs_qoff_logitem));
> -}
> -
> -/*
> - * There isn't much you can do to push a quotaoff item. It is simply
> - * stuck waiting for the log to be flushed to disk.
> - */
> -STATIC uint
> -xfs_qm_qoff_logitem_push(
> - struct xfs_log_item *lip,
> - struct list_head *buffer_list)
> -{
> - return XFS_ITEM_LOCKED;
> -}
> -
> -STATIC xfs_lsn_t
> -xfs_qm_qoffend_logitem_committed(
> - struct xfs_log_item *lip,
> - xfs_lsn_t lsn)
> -{
> - struct xfs_qoff_logitem *qfe = QOFF_ITEM(lip);
> - struct xfs_qoff_logitem *qfs = qfe->qql_start_lip;
> -
> - xfs_qm_qoff_logitem_relse(qfs);
> -
> - kmem_free(lip->li_lv_shadow);
> - kmem_free(qfe);
> - return (xfs_lsn_t)-1;
> -}
> -
> -STATIC void
> -xfs_qm_qoff_logitem_release(
> - struct xfs_log_item *lip)
> -{
> - struct xfs_qoff_logitem *qoff = QOFF_ITEM(lip);
> -
> - if (test_bit(XFS_LI_ABORTED, &lip->li_flags)) {
> - if (qoff->qql_start_lip)
> - xfs_qm_qoff_logitem_relse(qoff->qql_start_lip);
> - xfs_qm_qoff_logitem_relse(qoff);
> - }
> -}
> -
> -static const struct xfs_item_ops xfs_qm_qoffend_logitem_ops = {
> - .iop_size = xfs_qm_qoff_logitem_size,
> - .iop_format = xfs_qm_qoff_logitem_format,
> - .iop_committed = xfs_qm_qoffend_logitem_committed,
> - .iop_push = xfs_qm_qoff_logitem_push,
> - .iop_release = xfs_qm_qoff_logitem_release,
> -};
> -
> -static const struct xfs_item_ops xfs_qm_qoff_logitem_ops = {
> - .iop_size = xfs_qm_qoff_logitem_size,
> - .iop_format = xfs_qm_qoff_logitem_format,
> - .iop_push = xfs_qm_qoff_logitem_push,
> - .iop_release = xfs_qm_qoff_logitem_release,
> -};
> -
> -/*
> - * Delete the quotaoff intent from the AIL and free it. On success,
> - * this should only be called for the start item. It can be used for
> - * either on shutdown or abort.
> - */
> -void
> -xfs_qm_qoff_logitem_relse(
> - struct xfs_qoff_logitem *qoff)
> -{
> - struct xfs_log_item *lip = &qoff->qql_item;
> -
> - ASSERT(test_bit(XFS_LI_IN_AIL, &lip->li_flags) ||
> - test_bit(XFS_LI_ABORTED, &lip->li_flags) ||
> - XFS_FORCED_SHUTDOWN(lip->li_mountp));
> - xfs_trans_ail_delete(lip, 0);
> - kmem_free(lip->li_lv_shadow);
> - kmem_free(qoff);
> -}
> -
> -/*
> - * Allocate and initialize an quotaoff item of the correct quota type(s).
> - */
> -struct xfs_qoff_logitem *
> -xfs_qm_qoff_logitem_init(
> - struct xfs_mount *mp,
> - struct xfs_qoff_logitem *start,
> - uint flags)
> -{
> - struct xfs_qoff_logitem *qf;
> -
> - qf = kmem_zalloc(sizeof(struct xfs_qoff_logitem), 0);
> -
> - xfs_log_item_init(mp, &qf->qql_item, XFS_LI_QUOTAOFF, start ?
> - &xfs_qm_qoffend_logitem_ops : &xfs_qm_qoff_logitem_ops);
> - qf->qql_item.li_mountp = mp;
> - qf->qql_start_lip = start;
> - qf->qql_flags = flags;
> - return qf;
> -}
> diff --git a/fs/xfs/xfs_dquot_item.h b/fs/xfs/xfs_dquot_item.h
> index 2b86a43d7ce2ec..794710c2447493 100644
> --- a/fs/xfs/xfs_dquot_item.h
> +++ b/fs/xfs/xfs_dquot_item.h
> @@ -9,7 +9,6 @@
> struct xfs_dquot;
> struct xfs_trans;
> struct xfs_mount;
> -struct xfs_qoff_logitem;
>
> struct xfs_dq_logitem {
> struct xfs_log_item qli_item; /* common portion */
> @@ -17,22 +16,6 @@ struct xfs_dq_logitem {
> xfs_lsn_t qli_flush_lsn; /* lsn at last flush */
> };
>
> -struct xfs_qoff_logitem {
> - struct xfs_log_item qql_item; /* common portion */
> - struct xfs_qoff_logitem *qql_start_lip; /* qoff-start logitem, if any */
> - unsigned int qql_flags;
> -};
> -
> -
> void xfs_qm_dquot_logitem_init(struct xfs_dquot *dqp);
> -struct xfs_qoff_logitem *xfs_qm_qoff_logitem_init(struct xfs_mount *mp,
> - struct xfs_qoff_logitem *start,
> - uint flags);
> -void xfs_qm_qoff_logitem_relse(struct xfs_qoff_logitem *);
> -struct xfs_qoff_logitem *xfs_trans_get_qoff_item(struct xfs_trans *tp,
> - struct xfs_qoff_logitem *startqoff,
> - uint flags);
> -void xfs_trans_log_quotaoff_item(struct xfs_trans *tp,
> - struct xfs_qoff_logitem *qlp);
>
> #endif /* __XFS_DQUOT_ITEM_H__ */
> diff --git a/fs/xfs/xfs_qm.c b/fs/xfs/xfs_qm.c
> index fe341f3fd41901..580b9dba21122b 100644
> --- a/fs/xfs/xfs_qm.c
> +++ b/fs/xfs/xfs_qm.c
> @@ -185,7 +185,7 @@ xfs_qm_dqpurge(
> /*
> * Purge the dquot cache.
> */
> -void
> +static void
> xfs_qm_dqpurge_all(
> struct xfs_mount *mp,
> uint flags)
> diff --git a/fs/xfs/xfs_qm.h b/fs/xfs/xfs_qm.h
> index ebbb484c49dc7e..442a0f97a9d439 100644
> --- a/fs/xfs/xfs_qm.h
> +++ b/fs/xfs/xfs_qm.h
> @@ -140,9 +140,6 @@ struct xfs_dquot_acct {
>
> extern void xfs_qm_destroy_quotainfo(struct xfs_mount *);
>
> -/* dquot stuff */
> -extern void xfs_qm_dqpurge_all(struct xfs_mount *, uint);
> -
> /* quota ops */
> extern int xfs_qm_scall_trunc_qfiles(struct xfs_mount *, uint);
> extern int xfs_qm_scall_getquota(struct xfs_mount *mp,
> diff --git a/fs/xfs/xfs_qm_syscalls.c b/fs/xfs/xfs_qm_syscalls.c
> index 13a56e1ea15ce1..e00067a142a094 100644
> --- a/fs/xfs/xfs_qm_syscalls.c
> +++ b/fs/xfs/xfs_qm_syscalls.c
> @@ -19,91 +19,11 @@
> #include "xfs_qm.h"
> #include "xfs_icache.h"
>
> -STATIC int
> -xfs_qm_log_quotaoff(
> - struct xfs_mount *mp,
> - struct xfs_qoff_logitem **qoffstartp,
> - uint flags)
> -{
> - struct xfs_trans *tp;
> - int error;
> - struct xfs_qoff_logitem *qoffi;
> -
> - error = xfs_trans_alloc(mp, &M_RES(mp)->tr_qm_quotaoff, 0, 0, 0, &tp);
> - if (error)
> - goto out;
> -
> - qoffi = xfs_trans_get_qoff_item(tp, NULL, flags & XFS_ALL_QUOTA_ACCT);
> - xfs_trans_log_quotaoff_item(tp, qoffi);
> -
> - spin_lock(&mp->m_sb_lock);
> - mp->m_sb.sb_qflags = (mp->m_qflags & ~(flags)) & XFS_MOUNT_QUOTA_ALL;
> - spin_unlock(&mp->m_sb_lock);
> -
> - xfs_log_sb(tp);
> -
> - /*
> - * We have to make sure that the transaction is secure on disk before we
> - * return and actually stop quota accounting. So, make it synchronous.
> - * We don't care about quotoff's performance.
> - */
> - xfs_trans_set_sync(tp);
> - error = xfs_trans_commit(tp);
> - if (error)
> - goto out;
> -
> - *qoffstartp = qoffi;
> -out:
> - return error;
> -}
> -
> -STATIC int
> -xfs_qm_log_quotaoff_end(
> - struct xfs_mount *mp,
> - struct xfs_qoff_logitem **startqoff,
> - uint flags)
> -{
> - struct xfs_trans *tp;
> - int error;
> - struct xfs_qoff_logitem *qoffi;
> -
> - error = xfs_trans_alloc(mp, &M_RES(mp)->tr_qm_equotaoff, 0, 0, 0, &tp);
> - if (error)
> - return error;
> -
> - qoffi = xfs_trans_get_qoff_item(tp, *startqoff,
> - flags & XFS_ALL_QUOTA_ACCT);
> - xfs_trans_log_quotaoff_item(tp, qoffi);
> - *startqoff = NULL;
> -
> - /*
> - * We have to make sure that the transaction is secure on disk before we
> - * return and actually stop quota accounting. So, make it synchronous.
> - * We don't care about quotoff's performance.
> - */
> - xfs_trans_set_sync(tp);
> - return xfs_trans_commit(tp);
> -}
> -
> -/*
> - * Turn off quota accounting and/or enforcement for all udquots and/or
> - * gdquots. Called only at unmount time.
> - *
> - * This assumes that there are no dquots of this file system cached
> - * incore, and modifies the ondisk dquot directly. Therefore, for example,
> - * it is an error to call this twice, without purging the cache.
> - */
> int
> xfs_qm_scall_quotaoff(
> xfs_mount_t *mp,
> uint flags)
> {
> - struct xfs_quotainfo *q = mp->m_quotainfo;
> - uint dqtype;
> - int error;
> - uint inactivate_flags;
> - struct xfs_qoff_logitem *qoffstart = NULL;
> -
> /*
> * No file system can have quotas enabled on disk but not in core.
> * Note that quota utilities (like quotaoff) _expect_
> @@ -111,160 +31,23 @@ xfs_qm_scall_quotaoff(
> */
> if ((mp->m_qflags & flags) == 0)
> return -EEXIST;
> - error = 0;
> -
> - flags &= (XFS_ALL_QUOTA_ACCT | XFS_ALL_QUOTA_ENFD);
>
> /*
> - * We don't want to deal with two quotaoffs messing up each other,
> - * so we're going to serialize it. quotaoff isn't exactly a performance
> - * critical thing.
> - * If quotaoff, then we must be dealing with the root filesystem.
> + * We do not support actually turning off quota accounting any more.
> + * Just log a warning and ignored the accounting related flags.
s/ignored/ignore/
> */
> - ASSERT(q);
> - mutex_lock(&q->qi_quotaofflock);
> + if (flags & XFS_ALL_QUOTA_ACCT)
> + xfs_info(mp, "disabling of quota accounting not supported.");
Why not return EOPNOTSUPP here?
--D
>
> - /*
> - * If we're just turning off quota enforcement, change mp and go.
> - */
> - if ((flags & XFS_ALL_QUOTA_ACCT) == 0) {
> - mp->m_qflags &= ~(flags);
> -
> - spin_lock(&mp->m_sb_lock);
> - mp->m_sb.sb_qflags = mp->m_qflags;
> - spin_unlock(&mp->m_sb_lock);
> - mutex_unlock(&q->qi_quotaofflock);
> -
> - /* XXX what to do if error ? Revert back to old vals incore ? */
> - return xfs_sync_sb(mp, false);
> - }
> -
> - dqtype = 0;
> - inactivate_flags = 0;
> - /*
> - * If accounting is off, we must turn enforcement off, clear the
> - * quota 'CHKD' certificate to make it known that we have to
> - * do a quotacheck the next time this quota is turned on.
> - */
> - if (flags & XFS_UQUOTA_ACCT) {
> - dqtype |= XFS_QMOPT_UQUOTA;
> - flags |= (XFS_UQUOTA_CHKD | XFS_UQUOTA_ENFD);
> - inactivate_flags |= XFS_UQUOTA_ACTIVE;
> - }
> - if (flags & XFS_GQUOTA_ACCT) {
> - dqtype |= XFS_QMOPT_GQUOTA;
> - flags |= (XFS_GQUOTA_CHKD | XFS_GQUOTA_ENFD);
> - inactivate_flags |= XFS_GQUOTA_ACTIVE;
> - }
> - if (flags & XFS_PQUOTA_ACCT) {
> - dqtype |= XFS_QMOPT_PQUOTA;
> - flags |= (XFS_PQUOTA_CHKD | XFS_PQUOTA_ENFD);
> - inactivate_flags |= XFS_PQUOTA_ACTIVE;
> - }
> -
> - /*
> - * Nothing to do? Don't complain. This happens when we're just
> - * turning off quota enforcement.
> - */
> - if ((mp->m_qflags & flags) == 0)
> - goto out_unlock;
> -
> - /*
> - * Write the LI_QUOTAOFF log record, and do SB changes atomically,
> - * and synchronously. If we fail to write, we should abort the
> - * operation as it cannot be recovered safely if we crash.
> - */
> - error = xfs_qm_log_quotaoff(mp, &qoffstart, flags);
> - if (error)
> - goto out_unlock;
> -
> - /*
> - * Next we clear the XFS_MOUNT_*DQ_ACTIVE bit(s) in the mount struct
> - * to take care of the race between dqget and quotaoff. We don't take
> - * any special locks to reset these bits. All processes need to check
> - * these bits *after* taking inode lock(s) to see if the particular
> - * quota type is in the process of being turned off. If *ACTIVE, it is
> - * guaranteed that all dquot structures and all quotainode ptrs will all
> - * stay valid as long as that inode is kept locked.
> - *
> - * There is no turning back after this.
> - */
> - mp->m_qflags &= ~inactivate_flags;
> -
> - /*
> - * Give back all the dquot reference(s) held by inodes.
> - * Here we go thru every single incore inode in this file system, and
> - * do a dqrele on the i_udquot/i_gdquot that it may have.
> - * Essentially, as long as somebody has an inode locked, this guarantees
> - * that quotas will not be turned off. This is handy because in a
> - * transaction once we lock the inode(s) and check for quotaon, we can
> - * depend on the quota inodes (and other things) being valid as long as
> - * we keep the lock(s).
> - */
> - error = xfs_dqrele_all_inodes(mp, flags);
> - ASSERT(!error);
> -
> - /*
> - * Next we make the changes in the quota flag in the mount struct.
> - * This isn't protected by a particular lock directly, because we
> - * don't want to take a mrlock every time we depend on quotas being on.
> - */
> - mp->m_qflags &= ~flags;
> -
> - /*
> - * Go through all the dquots of this file system and purge them,
> - * according to what was turned off.
> - */
> - xfs_qm_dqpurge_all(mp, dqtype);
> -
> - /*
> - * Transactions that had started before ACTIVE state bit was cleared
> - * could have logged many dquots, so they'd have higher LSNs than
> - * the first QUOTAOFF log record does. If we happen to crash when
> - * the tail of the log has gone past the QUOTAOFF record, but
> - * before the last dquot modification, those dquots __will__
> - * recover, and that's not good.
> - *
> - * So, we have QUOTAOFF start and end logitems; the start
> - * logitem won't get overwritten until the end logitem appears...
> - */
> - error = xfs_qm_log_quotaoff_end(mp, &qoffstart, flags);
> - if (error) {
> - /* We're screwed now. Shutdown is the only option. */
> - xfs_force_shutdown(mp, SHUTDOWN_CORRUPT_INCORE);
> - goto out_unlock;
> - }
> -
> - /*
> - * If all quotas are completely turned off, close shop.
> - */
> - if (mp->m_qflags == 0) {
> - mutex_unlock(&q->qi_quotaofflock);
> - xfs_qm_destroy_quotainfo(mp);
> - return 0;
> - }
> -
> - /*
> - * Release our quotainode references if we don't need them anymore.
> - */
> - if ((dqtype & XFS_QMOPT_UQUOTA) && q->qi_uquotaip) {
> - xfs_irele(q->qi_uquotaip);
> - q->qi_uquotaip = NULL;
> - }
> - if ((dqtype & XFS_QMOPT_GQUOTA) && q->qi_gquotaip) {
> - xfs_irele(q->qi_gquotaip);
> - q->qi_gquotaip = NULL;
> - }
> - if ((dqtype & XFS_QMOPT_PQUOTA) && q->qi_pquotaip) {
> - xfs_irele(q->qi_pquotaip);
> - q->qi_pquotaip = NULL;
> - }
> + mutex_lock(&mp->m_quotainfo->qi_quotaofflock);
> + mp->m_qflags &= ~(flags & XFS_ALL_QUOTA_ENFD);
> + spin_lock(&mp->m_sb_lock);
> + mp->m_sb.sb_qflags = mp->m_qflags;
> + spin_unlock(&mp->m_sb_lock);
> + mutex_unlock(&mp->m_quotainfo->qi_quotaofflock);
>
> -out_unlock:
> - if (error && qoffstart)
> - xfs_qm_qoff_logitem_relse(qoffstart);
> - mutex_unlock(&q->qi_quotaofflock);
> - return error;
> + /* XXX what to do if error ? Revert back to old vals incore ? */
> + return xfs_sync_sb(mp, false);
> }
>
> STATIC int
> diff --git a/fs/xfs/xfs_trans_dquot.c b/fs/xfs/xfs_trans_dquot.c
> index 48e09ea30ee539..b7e4b05a559bdb 100644
> --- a/fs/xfs/xfs_trans_dquot.c
> +++ b/fs/xfs/xfs_trans_dquot.c
> @@ -843,44 +843,6 @@ xfs_trans_reserve_quota_icreate(
> dblocks, 1, XFS_QMOPT_RES_REGBLKS);
> }
>
> -/*
> - * This routine is called to allocate a quotaoff log item.
> - */
> -struct xfs_qoff_logitem *
> -xfs_trans_get_qoff_item(
> - struct xfs_trans *tp,
> - struct xfs_qoff_logitem *startqoff,
> - uint flags)
> -{
> - struct xfs_qoff_logitem *q;
> -
> - ASSERT(tp != NULL);
> -
> - q = xfs_qm_qoff_logitem_init(tp->t_mountp, startqoff, flags);
> - ASSERT(q != NULL);
> -
> - /*
> - * Get a log_item_desc to point at the new item.
> - */
> - xfs_trans_add_item(tp, &q->qql_item);
> - return q;
> -}
> -
> -
> -/*
> - * This is called to mark the quotaoff logitem as needing
> - * to be logged when the transaction is committed. The logitem must
> - * already be associated with the given transaction.
> - */
> -void
> -xfs_trans_log_quotaoff_item(
> - struct xfs_trans *tp,
> - struct xfs_qoff_logitem *qlp)
> -{
> - tp->t_flags |= XFS_TRANS_DIRTY;
> - set_bit(XFS_LI_DIRTY, &qlp->qql_item.li_flags);
> -}
> -
> STATIC void
> xfs_trans_alloc_dqinfo(
> xfs_trans_t *tp)
> --
> 2.30.2
>
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [PATCH 1/3] xfs: remove support for disabling quota accounting on a mounted file system
2021-07-12 11:14 ` [PATCH 1/3] xfs: remove support for " Christoph Hellwig
2021-07-14 11:10 ` Carlos Maiolino
2021-07-14 21:44 ` Darrick J. Wong
@ 2021-07-15 17:15 ` Darrick J. Wong
2021-07-20 6:35 ` Christoph Hellwig
2 siblings, 1 reply; 14+ messages in thread
From: Darrick J. Wong @ 2021-07-15 17:15 UTC (permalink / raw)
To: Christoph Hellwig; +Cc: linux-xfs
On Mon, Jul 12, 2021 at 01:14:24PM +0200, Christoph Hellwig wrote:
> Disabling quota accounting is hairy, racy code with all kinds of pitfalls.
> And it has a very strange mind set, as quota accounting (unlike
> enforcement) really is a propery of the on-disk format. There is no good
> use case for supporting this.
>
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
> fs/xfs/libxfs/xfs_trans_resv.c | 30 ----
> fs/xfs/libxfs/xfs_trans_resv.h | 2 -
> fs/xfs/xfs_dquot_item.c | 134 ------------------
> fs/xfs/xfs_dquot_item.h | 17 ---
> fs/xfs/xfs_qm.c | 2 +-
> fs/xfs/xfs_qm.h | 3 -
> fs/xfs/xfs_qm_syscalls.c | 241 ++-------------------------------
> fs/xfs/xfs_trans_dquot.c | 38 ------
> 8 files changed, 13 insertions(+), 454 deletions(-)
>
<snip> since Christoph didn't find the reply the first time...
> diff --git a/fs/xfs/xfs_qm_syscalls.c b/fs/xfs/xfs_qm_syscalls.c
> index 13a56e1ea15ce1..e00067a142a094 100644
> --- a/fs/xfs/xfs_qm_syscalls.c
> +++ b/fs/xfs/xfs_qm_syscalls.c
<snip>
> @@ -111,160 +31,23 @@ xfs_qm_scall_quotaoff(
> */
> if ((mp->m_qflags & flags) == 0)
> return -EEXIST;
> - error = 0;
> -
> - flags &= (XFS_ALL_QUOTA_ACCT | XFS_ALL_QUOTA_ENFD);
>
> /*
> - * We don't want to deal with two quotaoffs messing up each other,
> - * so we're going to serialize it. quotaoff isn't exactly a performance
> - * critical thing.
> - * If quotaoff, then we must be dealing with the root filesystem.
> + * We do not support actually turning off quota accounting any more.
> + * Just log a warning and ignored the accounting related flags.
s/ignored/ignore/
> */
> - ASSERT(q);
> - mutex_lock(&q->qi_quotaofflock);
> + if (flags & XFS_ALL_QUOTA_ACCT)
> + xfs_info(mp, "disabling of quota accounting not supported.");
Why not return EOPNOTSUPP here? We're not going to turn off accounting,
so we're not doing what the admin asked.
--D
>
> - /*
> - * If we're just turning off quota enforcement, change mp and go.
> - */
> - if ((flags & XFS_ALL_QUOTA_ACCT) == 0) {
> - mp->m_qflags &= ~(flags);
> -
> - spin_lock(&mp->m_sb_lock);
> - mp->m_sb.sb_qflags = mp->m_qflags;
> - spin_unlock(&mp->m_sb_lock);
> - mutex_unlock(&q->qi_quotaofflock);
> -
> - /* XXX what to do if error ? Revert back to old vals incore ? */
> - return xfs_sync_sb(mp, false);
> - }
> -
> - dqtype = 0;
> - inactivate_flags = 0;
> - /*
> - * If accounting is off, we must turn enforcement off, clear the
> - * quota 'CHKD' certificate to make it known that we have to
> - * do a quotacheck the next time this quota is turned on.
> - */
> - if (flags & XFS_UQUOTA_ACCT) {
> - dqtype |= XFS_QMOPT_UQUOTA;
> - flags |= (XFS_UQUOTA_CHKD | XFS_UQUOTA_ENFD);
> - inactivate_flags |= XFS_UQUOTA_ACTIVE;
> - }
> - if (flags & XFS_GQUOTA_ACCT) {
> - dqtype |= XFS_QMOPT_GQUOTA;
> - flags |= (XFS_GQUOTA_CHKD | XFS_GQUOTA_ENFD);
> - inactivate_flags |= XFS_GQUOTA_ACTIVE;
> - }
> - if (flags & XFS_PQUOTA_ACCT) {
> - dqtype |= XFS_QMOPT_PQUOTA;
> - flags |= (XFS_PQUOTA_CHKD | XFS_PQUOTA_ENFD);
> - inactivate_flags |= XFS_PQUOTA_ACTIVE;
> - }
> -
> - /*
> - * Nothing to do? Don't complain. This happens when we're just
> - * turning off quota enforcement.
> - */
> - if ((mp->m_qflags & flags) == 0)
> - goto out_unlock;
> -
> - /*
> - * Write the LI_QUOTAOFF log record, and do SB changes atomically,
> - * and synchronously. If we fail to write, we should abort the
> - * operation as it cannot be recovered safely if we crash.
> - */
> - error = xfs_qm_log_quotaoff(mp, &qoffstart, flags);
> - if (error)
> - goto out_unlock;
> -
> - /*
> - * Next we clear the XFS_MOUNT_*DQ_ACTIVE bit(s) in the mount struct
> - * to take care of the race between dqget and quotaoff. We don't take
> - * any special locks to reset these bits. All processes need to check
> - * these bits *after* taking inode lock(s) to see if the particular
> - * quota type is in the process of being turned off. If *ACTIVE, it is
> - * guaranteed that all dquot structures and all quotainode ptrs will all
> - * stay valid as long as that inode is kept locked.
> - *
> - * There is no turning back after this.
> - */
> - mp->m_qflags &= ~inactivate_flags;
> -
> - /*
> - * Give back all the dquot reference(s) held by inodes.
> - * Here we go thru every single incore inode in this file system, and
> - * do a dqrele on the i_udquot/i_gdquot that it may have.
> - * Essentially, as long as somebody has an inode locked, this guarantees
> - * that quotas will not be turned off. This is handy because in a
> - * transaction once we lock the inode(s) and check for quotaon, we can
> - * depend on the quota inodes (and other things) being valid as long as
> - * we keep the lock(s).
> - */
> - error = xfs_dqrele_all_inodes(mp, flags);
> - ASSERT(!error);
> -
> - /*
> - * Next we make the changes in the quota flag in the mount struct.
> - * This isn't protected by a particular lock directly, because we
> - * don't want to take a mrlock every time we depend on quotas being on.
> - */
> - mp->m_qflags &= ~flags;
> -
> - /*
> - * Go through all the dquots of this file system and purge them,
> - * according to what was turned off.
> - */
> - xfs_qm_dqpurge_all(mp, dqtype);
> -
> - /*
> - * Transactions that had started before ACTIVE state bit was cleared
> - * could have logged many dquots, so they'd have higher LSNs than
> - * the first QUOTAOFF log record does. If we happen to crash when
> - * the tail of the log has gone past the QUOTAOFF record, but
> - * before the last dquot modification, those dquots __will__
> - * recover, and that's not good.
> - *
> - * So, we have QUOTAOFF start and end logitems; the start
> - * logitem won't get overwritten until the end logitem appears...
> - */
> - error = xfs_qm_log_quotaoff_end(mp, &qoffstart, flags);
> - if (error) {
> - /* We're screwed now. Shutdown is the only option. */
> - xfs_force_shutdown(mp, SHUTDOWN_CORRUPT_INCORE);
> - goto out_unlock;
> - }
> -
> - /*
> - * If all quotas are completely turned off, close shop.
> - */
> - if (mp->m_qflags == 0) {
> - mutex_unlock(&q->qi_quotaofflock);
> - xfs_qm_destroy_quotainfo(mp);
> - return 0;
> - }
> -
> - /*
> - * Release our quotainode references if we don't need them anymore.
> - */
> - if ((dqtype & XFS_QMOPT_UQUOTA) && q->qi_uquotaip) {
> - xfs_irele(q->qi_uquotaip);
> - q->qi_uquotaip = NULL;
> - }
> - if ((dqtype & XFS_QMOPT_GQUOTA) && q->qi_gquotaip) {
> - xfs_irele(q->qi_gquotaip);
> - q->qi_gquotaip = NULL;
> - }
> - if ((dqtype & XFS_QMOPT_PQUOTA) && q->qi_pquotaip) {
> - xfs_irele(q->qi_pquotaip);
> - q->qi_pquotaip = NULL;
> - }
> + mutex_lock(&mp->m_quotainfo->qi_quotaofflock);
> + mp->m_qflags &= ~(flags & XFS_ALL_QUOTA_ENFD);
> + spin_lock(&mp->m_sb_lock);
> + mp->m_sb.sb_qflags = mp->m_qflags;
> + spin_unlock(&mp->m_sb_lock);
> + mutex_unlock(&mp->m_quotainfo->qi_quotaofflock);
>
> -out_unlock:
> - if (error && qoffstart)
> - xfs_qm_qoff_logitem_relse(qoffstart);
> - mutex_unlock(&q->qi_quotaofflock);
> - return error;
> + /* XXX what to do if error ? Revert back to old vals incore ? */
> + return xfs_sync_sb(mp, false);
> }
>
> STATIC int
> diff --git a/fs/xfs/xfs_trans_dquot.c b/fs/xfs/xfs_trans_dquot.c
> index 48e09ea30ee539..b7e4b05a559bdb 100644
> --- a/fs/xfs/xfs_trans_dquot.c
> +++ b/fs/xfs/xfs_trans_dquot.c
> @@ -843,44 +843,6 @@ xfs_trans_reserve_quota_icreate(
> dblocks, 1, XFS_QMOPT_RES_REGBLKS);
> }
>
> -/*
> - * This routine is called to allocate a quotaoff log item.
> - */
> -struct xfs_qoff_logitem *
> -xfs_trans_get_qoff_item(
> - struct xfs_trans *tp,
> - struct xfs_qoff_logitem *startqoff,
> - uint flags)
> -{
> - struct xfs_qoff_logitem *q;
> -
> - ASSERT(tp != NULL);
> -
> - q = xfs_qm_qoff_logitem_init(tp->t_mountp, startqoff, flags);
> - ASSERT(q != NULL);
> -
> - /*
> - * Get a log_item_desc to point at the new item.
> - */
> - xfs_trans_add_item(tp, &q->qql_item);
> - return q;
> -}
> -
> -
> -/*
> - * This is called to mark the quotaoff logitem as needing
> - * to be logged when the transaction is committed. The logitem must
> - * already be associated with the given transaction.
> - */
> -void
> -xfs_trans_log_quotaoff_item(
> - struct xfs_trans *tp,
> - struct xfs_qoff_logitem *qlp)
> -{
> - tp->t_flags |= XFS_TRANS_DIRTY;
> - set_bit(XFS_LI_DIRTY, &qlp->qql_item.li_flags);
> -}
> -
> STATIC void
> xfs_trans_alloc_dqinfo(
> xfs_trans_t *tp)
> --
> 2.30.2
>
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [PATCH 1/3] xfs: remove support for disabling quota accounting on a mounted file system
2021-07-15 17:15 ` Darrick J. Wong
@ 2021-07-20 6:35 ` Christoph Hellwig
0 siblings, 0 replies; 14+ messages in thread
From: Christoph Hellwig @ 2021-07-20 6:35 UTC (permalink / raw)
To: Darrick J. Wong; +Cc: Christoph Hellwig, linux-xfs
On Thu, Jul 15, 2021 at 10:15:49AM -0700, Darrick J. Wong wrote:
> > + if (flags & XFS_ALL_QUOTA_ACCT)
> > + xfs_info(mp, "disabling of quota accounting not supported.");
>
> Why not return EOPNOTSUPP here? We're not going to turn off accounting,
> so we're not doing what the admin asked.
Because what the user usually wants is to disable visible effects of
quota, and we still give that. The fact that we keep accounting underneath
isn't quite as relevant as there is no different user behavior, just a little
overhead.
[snipping the rest of the full quote..]
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH 2/3] xfs: remove the flags argument to xfs_qm_dquot_walk
2021-07-12 11:14 don't allow disabling quota accounting on a mounted file system Christoph Hellwig
2021-07-12 11:14 ` [PATCH 1/3] xfs: remove support for " Christoph Hellwig
@ 2021-07-12 11:14 ` Christoph Hellwig
2021-07-14 11:11 ` Carlos Maiolino
2021-07-14 21:45 ` Darrick J. Wong
2021-07-12 11:14 ` [PATCH 3/3] xfs: remove the active vs running quota differentiation Christoph Hellwig
2 siblings, 2 replies; 14+ messages in thread
From: Christoph Hellwig @ 2021-07-12 11:14 UTC (permalink / raw)
To: linux-xfs
We always purge all dquots now, so drop the argument.
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
fs/xfs/xfs_qm.c | 16 ++++++----------
1 file changed, 6 insertions(+), 10 deletions(-)
diff --git a/fs/xfs/xfs_qm.c b/fs/xfs/xfs_qm.c
index 580b9dba21122b..2b34273d0405e7 100644
--- a/fs/xfs/xfs_qm.c
+++ b/fs/xfs/xfs_qm.c
@@ -187,15 +187,11 @@ xfs_qm_dqpurge(
*/
static void
xfs_qm_dqpurge_all(
- struct xfs_mount *mp,
- uint flags)
+ struct xfs_mount *mp)
{
- if (flags & XFS_QMOPT_UQUOTA)
- xfs_qm_dquot_walk(mp, XFS_DQTYPE_USER, xfs_qm_dqpurge, NULL);
- if (flags & XFS_QMOPT_GQUOTA)
- xfs_qm_dquot_walk(mp, XFS_DQTYPE_GROUP, xfs_qm_dqpurge, NULL);
- if (flags & XFS_QMOPT_PQUOTA)
- xfs_qm_dquot_walk(mp, XFS_DQTYPE_PROJ, xfs_qm_dqpurge, NULL);
+ xfs_qm_dquot_walk(mp, XFS_DQTYPE_USER, xfs_qm_dqpurge, NULL);
+ xfs_qm_dquot_walk(mp, XFS_DQTYPE_GROUP, xfs_qm_dqpurge, NULL);
+ xfs_qm_dquot_walk(mp, XFS_DQTYPE_PROJ, xfs_qm_dqpurge, NULL);
}
/*
@@ -206,7 +202,7 @@ xfs_qm_unmount(
struct xfs_mount *mp)
{
if (mp->m_quotainfo) {
- xfs_qm_dqpurge_all(mp, XFS_QMOPT_QUOTALL);
+ xfs_qm_dqpurge_all(mp);
xfs_qm_destroy_quotainfo(mp);
}
}
@@ -1359,7 +1355,7 @@ xfs_qm_quotacheck(
* at this point (because we intentionally didn't in dqget_noattach).
*/
if (error) {
- xfs_qm_dqpurge_all(mp, XFS_QMOPT_QUOTALL);
+ xfs_qm_dqpurge_all(mp);
goto error_return;
}
--
2.30.2
^ permalink raw reply related [flat|nested] 14+ messages in thread* Re: [PATCH 2/3] xfs: remove the flags argument to xfs_qm_dquot_walk
2021-07-12 11:14 ` [PATCH 2/3] xfs: remove the flags argument to xfs_qm_dquot_walk Christoph Hellwig
@ 2021-07-14 11:11 ` Carlos Maiolino
2021-07-14 21:45 ` Darrick J. Wong
1 sibling, 0 replies; 14+ messages in thread
From: Carlos Maiolino @ 2021-07-14 11:11 UTC (permalink / raw)
To: Christoph Hellwig; +Cc: linux-xfs
On Mon, Jul 12, 2021 at 01:14:25PM +0200, Christoph Hellwig wrote:
> We always purge all dquots now, so drop the argument.
>
Reviewed-by: Carlos Maiolino <cmaiolino@redhat.com>
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
> fs/xfs/xfs_qm.c | 16 ++++++----------
> 1 file changed, 6 insertions(+), 10 deletions(-)
>
> diff --git a/fs/xfs/xfs_qm.c b/fs/xfs/xfs_qm.c
> index 580b9dba21122b..2b34273d0405e7 100644
> --- a/fs/xfs/xfs_qm.c
> +++ b/fs/xfs/xfs_qm.c
> @@ -187,15 +187,11 @@ xfs_qm_dqpurge(
> */
> static void
> xfs_qm_dqpurge_all(
> - struct xfs_mount *mp,
> - uint flags)
> + struct xfs_mount *mp)
> {
> - if (flags & XFS_QMOPT_UQUOTA)
> - xfs_qm_dquot_walk(mp, XFS_DQTYPE_USER, xfs_qm_dqpurge, NULL);
> - if (flags & XFS_QMOPT_GQUOTA)
> - xfs_qm_dquot_walk(mp, XFS_DQTYPE_GROUP, xfs_qm_dqpurge, NULL);
> - if (flags & XFS_QMOPT_PQUOTA)
> - xfs_qm_dquot_walk(mp, XFS_DQTYPE_PROJ, xfs_qm_dqpurge, NULL);
> + xfs_qm_dquot_walk(mp, XFS_DQTYPE_USER, xfs_qm_dqpurge, NULL);
> + xfs_qm_dquot_walk(mp, XFS_DQTYPE_GROUP, xfs_qm_dqpurge, NULL);
> + xfs_qm_dquot_walk(mp, XFS_DQTYPE_PROJ, xfs_qm_dqpurge, NULL);
> }
>
> /*
> @@ -206,7 +202,7 @@ xfs_qm_unmount(
> struct xfs_mount *mp)
> {
> if (mp->m_quotainfo) {
> - xfs_qm_dqpurge_all(mp, XFS_QMOPT_QUOTALL);
> + xfs_qm_dqpurge_all(mp);
> xfs_qm_destroy_quotainfo(mp);
> }
> }
> @@ -1359,7 +1355,7 @@ xfs_qm_quotacheck(
> * at this point (because we intentionally didn't in dqget_noattach).
> */
> if (error) {
> - xfs_qm_dqpurge_all(mp, XFS_QMOPT_QUOTALL);
> + xfs_qm_dqpurge_all(mp);
> goto error_return;
> }
>
> --
> 2.30.2
>
--
Carlos
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [PATCH 2/3] xfs: remove the flags argument to xfs_qm_dquot_walk
2021-07-12 11:14 ` [PATCH 2/3] xfs: remove the flags argument to xfs_qm_dquot_walk Christoph Hellwig
2021-07-14 11:11 ` Carlos Maiolino
@ 2021-07-14 21:45 ` Darrick J. Wong
1 sibling, 0 replies; 14+ messages in thread
From: Darrick J. Wong @ 2021-07-14 21:45 UTC (permalink / raw)
To: Christoph Hellwig; +Cc: linux-xfs
On Mon, Jul 12, 2021 at 01:14:25PM +0200, Christoph Hellwig wrote:
> We always purge all dquots now, so drop the argument.
>
> Signed-off-by: Christoph Hellwig <hch@lst.de>
Pretty straightforward, so
Reviewed-by: Darrick J. Wong <djwong@kernel.org>
--D
> ---
> fs/xfs/xfs_qm.c | 16 ++++++----------
> 1 file changed, 6 insertions(+), 10 deletions(-)
>
> diff --git a/fs/xfs/xfs_qm.c b/fs/xfs/xfs_qm.c
> index 580b9dba21122b..2b34273d0405e7 100644
> --- a/fs/xfs/xfs_qm.c
> +++ b/fs/xfs/xfs_qm.c
> @@ -187,15 +187,11 @@ xfs_qm_dqpurge(
> */
> static void
> xfs_qm_dqpurge_all(
> - struct xfs_mount *mp,
> - uint flags)
> + struct xfs_mount *mp)
> {
> - if (flags & XFS_QMOPT_UQUOTA)
> - xfs_qm_dquot_walk(mp, XFS_DQTYPE_USER, xfs_qm_dqpurge, NULL);
> - if (flags & XFS_QMOPT_GQUOTA)
> - xfs_qm_dquot_walk(mp, XFS_DQTYPE_GROUP, xfs_qm_dqpurge, NULL);
> - if (flags & XFS_QMOPT_PQUOTA)
> - xfs_qm_dquot_walk(mp, XFS_DQTYPE_PROJ, xfs_qm_dqpurge, NULL);
> + xfs_qm_dquot_walk(mp, XFS_DQTYPE_USER, xfs_qm_dqpurge, NULL);
> + xfs_qm_dquot_walk(mp, XFS_DQTYPE_GROUP, xfs_qm_dqpurge, NULL);
> + xfs_qm_dquot_walk(mp, XFS_DQTYPE_PROJ, xfs_qm_dqpurge, NULL);
> }
>
> /*
> @@ -206,7 +202,7 @@ xfs_qm_unmount(
> struct xfs_mount *mp)
> {
> if (mp->m_quotainfo) {
> - xfs_qm_dqpurge_all(mp, XFS_QMOPT_QUOTALL);
> + xfs_qm_dqpurge_all(mp);
> xfs_qm_destroy_quotainfo(mp);
> }
> }
> @@ -1359,7 +1355,7 @@ xfs_qm_quotacheck(
> * at this point (because we intentionally didn't in dqget_noattach).
> */
> if (error) {
> - xfs_qm_dqpurge_all(mp, XFS_QMOPT_QUOTALL);
> + xfs_qm_dqpurge_all(mp);
> goto error_return;
> }
>
> --
> 2.30.2
>
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH 3/3] xfs: remove the active vs running quota differentiation
2021-07-12 11:14 don't allow disabling quota accounting on a mounted file system Christoph Hellwig
2021-07-12 11:14 ` [PATCH 1/3] xfs: remove support for " Christoph Hellwig
2021-07-12 11:14 ` [PATCH 2/3] xfs: remove the flags argument to xfs_qm_dquot_walk Christoph Hellwig
@ 2021-07-12 11:14 ` Christoph Hellwig
2021-07-14 11:47 ` Carlos Maiolino
2021-07-14 21:47 ` Darrick J. Wong
2 siblings, 2 replies; 14+ messages in thread
From: Christoph Hellwig @ 2021-07-12 11:14 UTC (permalink / raw)
To: linux-xfs
These only made a difference when quotaoff supported disabling quota
accounting on a mounted file system, so we can switch everyone to use
a single set of flags and helpers now. Note that the *QUOTA_ON naming
for the helpers is kept as it was the much more commonly used one.
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
fs/xfs/libxfs/xfs_quota_defs.h | 30 +++-----------------
fs/xfs/scrub/quota.c | 2 +-
fs/xfs/xfs_dquot.c | 3 --
fs/xfs/xfs_ioctl.c | 2 +-
fs/xfs/xfs_iops.c | 4 +--
fs/xfs/xfs_mount.c | 4 +--
fs/xfs/xfs_qm.c | 26 ++++++++---------
fs/xfs/xfs_qm_syscalls.c | 2 +-
fs/xfs/xfs_quotaops.c | 30 +++++++-------------
fs/xfs/xfs_super.c | 51 ++++++++++++++--------------------
fs/xfs/xfs_trans_dquot.c | 11 ++++----
11 files changed, 58 insertions(+), 107 deletions(-)
diff --git a/fs/xfs/libxfs/xfs_quota_defs.h b/fs/xfs/libxfs/xfs_quota_defs.h
index 0f0af4e3503293..a02c5062f9b292 100644
--- a/fs/xfs/libxfs/xfs_quota_defs.h
+++ b/fs/xfs/libxfs/xfs_quota_defs.h
@@ -60,36 +60,14 @@ typedef uint8_t xfs_dqtype_t;
#define XFS_DQUOT_LOGRES(mp) \
((sizeof(struct xfs_dq_logformat) + sizeof(struct xfs_disk_dquot)) * 6)
-#define XFS_IS_QUOTA_RUNNING(mp) ((mp)->m_qflags & XFS_ALL_QUOTA_ACCT)
-#define XFS_IS_UQUOTA_RUNNING(mp) ((mp)->m_qflags & XFS_UQUOTA_ACCT)
-#define XFS_IS_PQUOTA_RUNNING(mp) ((mp)->m_qflags & XFS_PQUOTA_ACCT)
-#define XFS_IS_GQUOTA_RUNNING(mp) ((mp)->m_qflags & XFS_GQUOTA_ACCT)
+#define XFS_IS_QUOTA_ON(mp) ((mp)->m_qflags & XFS_ALL_QUOTA_ACCT)
+#define XFS_IS_UQUOTA_ON(mp) ((mp)->m_qflags & XFS_UQUOTA_ACCT)
+#define XFS_IS_PQUOTA_ON(mp) ((mp)->m_qflags & XFS_PQUOTA_ACCT)
+#define XFS_IS_GQUOTA_ON(mp) ((mp)->m_qflags & XFS_GQUOTA_ACCT)
#define XFS_IS_UQUOTA_ENFORCED(mp) ((mp)->m_qflags & XFS_UQUOTA_ENFD)
#define XFS_IS_GQUOTA_ENFORCED(mp) ((mp)->m_qflags & XFS_GQUOTA_ENFD)
#define XFS_IS_PQUOTA_ENFORCED(mp) ((mp)->m_qflags & XFS_PQUOTA_ENFD)
-/*
- * Incore only flags for quotaoff - these bits get cleared when quota(s)
- * are in the process of getting turned off. These flags are in m_qflags but
- * never in sb_qflags.
- */
-#define XFS_UQUOTA_ACTIVE 0x1000 /* uquotas are being turned off */
-#define XFS_GQUOTA_ACTIVE 0x2000 /* gquotas are being turned off */
-#define XFS_PQUOTA_ACTIVE 0x4000 /* pquotas are being turned off */
-#define XFS_ALL_QUOTA_ACTIVE \
- (XFS_UQUOTA_ACTIVE | XFS_GQUOTA_ACTIVE | XFS_PQUOTA_ACTIVE)
-
-/*
- * Checking XFS_IS_*QUOTA_ON() while holding any inode lock guarantees
- * quota will be not be switched off as long as that inode lock is held.
- */
-#define XFS_IS_QUOTA_ON(mp) ((mp)->m_qflags & (XFS_UQUOTA_ACTIVE | \
- XFS_GQUOTA_ACTIVE | \
- XFS_PQUOTA_ACTIVE))
-#define XFS_IS_UQUOTA_ON(mp) ((mp)->m_qflags & XFS_UQUOTA_ACTIVE)
-#define XFS_IS_GQUOTA_ON(mp) ((mp)->m_qflags & XFS_GQUOTA_ACTIVE)
-#define XFS_IS_PQUOTA_ON(mp) ((mp)->m_qflags & XFS_PQUOTA_ACTIVE)
-
/*
* Flags to tell various functions what to do. Not all of these are meaningful
* to a single function. None of these XFS_QMOPT_* flags are meant to have
diff --git a/fs/xfs/scrub/quota.c b/fs/xfs/scrub/quota.c
index acbb9839d42fbd..3cccd6d5b57755 100644
--- a/fs/xfs/scrub/quota.c
+++ b/fs/xfs/scrub/quota.c
@@ -42,7 +42,7 @@ xchk_setup_quota(
xfs_dqtype_t dqtype;
int error;
- if (!XFS_IS_QUOTA_RUNNING(sc->mp) || !XFS_IS_QUOTA_ON(sc->mp))
+ if (!XFS_IS_QUOTA_ON(sc->mp))
return -ENOENT;
dqtype = xchk_quota_to_dqtype(sc);
diff --git a/fs/xfs/xfs_dquot.c b/fs/xfs/xfs_dquot.c
index ecd5059d6928f7..c301b18b7685b1 100644
--- a/fs/xfs/xfs_dquot.c
+++ b/fs/xfs/xfs_dquot.c
@@ -847,9 +847,6 @@ xfs_qm_dqget_checks(
struct xfs_mount *mp,
xfs_dqtype_t type)
{
- if (WARN_ON_ONCE(!XFS_IS_QUOTA_RUNNING(mp)))
- return -ESRCH;
-
switch (type) {
case XFS_DQTYPE_USER:
if (!XFS_IS_UQUOTA_ON(mp))
diff --git a/fs/xfs/xfs_ioctl.c b/fs/xfs/xfs_ioctl.c
index 65270e63c032a0..9f4aa1477e2fca 100644
--- a/fs/xfs/xfs_ioctl.c
+++ b/fs/xfs/xfs_ioctl.c
@@ -1433,7 +1433,7 @@ xfs_fileattr_set(
/* Change the ownerships and register project quota modifications */
if (ip->i_projid != fa->fsx_projid) {
- if (XFS_IS_QUOTA_RUNNING(mp) && XFS_IS_PQUOTA_ON(mp)) {
+ if (XFS_IS_PQUOTA_ON(mp)) {
olddquot = xfs_qm_vop_chown(tp, ip,
&ip->i_pdquot, pdqp);
}
diff --git a/fs/xfs/xfs_iops.c b/fs/xfs/xfs_iops.c
index 93c082db04b78c..327d65ef1e268c 100644
--- a/fs/xfs/xfs_iops.c
+++ b/fs/xfs/xfs_iops.c
@@ -778,7 +778,7 @@ xfs_setattr_nonsize(
* in the transaction.
*/
if (!uid_eq(iuid, uid)) {
- if (XFS_IS_QUOTA_RUNNING(mp) && XFS_IS_UQUOTA_ON(mp)) {
+ if (XFS_IS_UQUOTA_ON(mp)) {
ASSERT(mask & ATTR_UID);
ASSERT(udqp);
olddquot1 = xfs_qm_vop_chown(tp, ip,
@@ -787,7 +787,7 @@ xfs_setattr_nonsize(
inode->i_uid = uid;
}
if (!gid_eq(igid, gid)) {
- if (XFS_IS_QUOTA_RUNNING(mp) && XFS_IS_GQUOTA_ON(mp)) {
+ if (XFS_IS_GQUOTA_ON(mp)) {
ASSERT(xfs_sb_version_has_pquotino(&mp->m_sb) ||
!XFS_IS_PQUOTA_ON(mp));
ASSERT(mask & ATTR_GID);
diff --git a/fs/xfs/xfs_mount.c b/fs/xfs/xfs_mount.c
index d0755494597f99..baf7b323cb15eb 100644
--- a/fs/xfs/xfs_mount.c
+++ b/fs/xfs/xfs_mount.c
@@ -836,13 +836,11 @@ xfs_mountfs(
/*
* Initialise the XFS quota management subsystem for this mount
*/
- if (XFS_IS_QUOTA_RUNNING(mp)) {
+ if (XFS_IS_QUOTA_ON(mp)) {
error = xfs_qm_newmount(mp, "amount, "aflags);
if (error)
goto out_rtunmount;
} else {
- ASSERT(!XFS_IS_QUOTA_ON(mp));
-
/*
* If a file system had quotas running earlier, but decided to
* mount without -o uquota/pquota/gquota options, revoke the
diff --git a/fs/xfs/xfs_qm.c b/fs/xfs/xfs_qm.c
index 2b34273d0405e7..351d99bc52e5f8 100644
--- a/fs/xfs/xfs_qm.c
+++ b/fs/xfs/xfs_qm.c
@@ -295,8 +295,6 @@ xfs_qm_need_dqattach(
{
struct xfs_mount *mp = ip->i_mount;
- if (!XFS_IS_QUOTA_RUNNING(mp))
- return false;
if (!XFS_IS_QUOTA_ON(mp))
return false;
if (!XFS_NOT_DQATTACHED(mp, ip))
@@ -631,7 +629,7 @@ xfs_qm_init_quotainfo(
struct xfs_quotainfo *qinf;
int error;
- ASSERT(XFS_IS_QUOTA_RUNNING(mp));
+ ASSERT(XFS_IS_QUOTA_ON(mp));
qinf = mp->m_quotainfo = kmem_zalloc(sizeof(struct xfs_quotainfo), 0);
@@ -676,11 +674,11 @@ xfs_qm_init_quotainfo(
xfs_qm_init_timelimits(mp, XFS_DQTYPE_GROUP);
xfs_qm_init_timelimits(mp, XFS_DQTYPE_PROJ);
- if (XFS_IS_UQUOTA_RUNNING(mp))
+ if (XFS_IS_UQUOTA_ON(mp))
xfs_qm_set_defquota(mp, XFS_DQTYPE_USER, qinf);
- if (XFS_IS_GQUOTA_RUNNING(mp))
+ if (XFS_IS_GQUOTA_ON(mp))
xfs_qm_set_defquota(mp, XFS_DQTYPE_GROUP, qinf);
- if (XFS_IS_PQUOTA_RUNNING(mp))
+ if (XFS_IS_PQUOTA_ON(mp))
xfs_qm_set_defquota(mp, XFS_DQTYPE_PROJ, qinf);
qinf->qi_shrinker.count_objects = xfs_qm_shrink_count;
@@ -1143,7 +1141,7 @@ xfs_qm_dqusage_adjust(
xfs_filblks_t rtblks = 0; /* total rt blks */
int error;
- ASSERT(XFS_IS_QUOTA_RUNNING(mp));
+ ASSERT(XFS_IS_QUOTA_ON(mp));
/*
* rootino must have its resources accounted for, not so with the quota
@@ -1284,7 +1282,7 @@ xfs_qm_quotacheck(
flags = 0;
ASSERT(uip || gip || pip);
- ASSERT(XFS_IS_QUOTA_RUNNING(mp));
+ ASSERT(XFS_IS_QUOTA_ON(mp));
xfs_notice(mp, "Quotacheck needed: Please wait.");
@@ -1414,7 +1412,7 @@ xfs_qm_mount_quotas(
goto write_changes;
}
- ASSERT(XFS_IS_QUOTA_RUNNING(mp));
+ ASSERT(XFS_IS_QUOTA_ON(mp));
/*
* Allocate the quotainfo structure inside the mount struct, and
@@ -1469,7 +1467,7 @@ xfs_qm_mount_quotas(
* the incore structures are convinced that quotas are
* off, but the on disk superblock doesn't know that !
*/
- ASSERT(!(XFS_IS_QUOTA_RUNNING(mp)));
+ ASSERT(!(XFS_IS_QUOTA_ON(mp)));
xfs_alert(mp, "%s: Superblock update failed!",
__func__);
}
@@ -1641,7 +1639,7 @@ xfs_qm_vop_dqalloc(
int error;
uint lockflags;
- if (!XFS_IS_QUOTA_RUNNING(mp) || !XFS_IS_QUOTA_ON(mp))
+ if (!XFS_IS_QUOTA_ON(mp))
return 0;
lockflags = XFS_ILOCK_EXCL;
@@ -1772,7 +1770,7 @@ xfs_qm_vop_chown(
ASSERT(xfs_isilocked(ip, XFS_ILOCK_EXCL));
- ASSERT(XFS_IS_QUOTA_RUNNING(ip->i_mount));
+ ASSERT(XFS_IS_QUOTA_ON(ip->i_mount));
/* old dquot */
prevdq = *IO_olddq;
@@ -1825,7 +1823,7 @@ xfs_qm_vop_rename_dqattach(
struct xfs_mount *mp = i_tab[0]->i_mount;
int i;
- if (!XFS_IS_QUOTA_RUNNING(mp) || !XFS_IS_QUOTA_ON(mp))
+ if (!XFS_IS_QUOTA_ON(mp))
return 0;
for (i = 0; (i < 4 && i_tab[i]); i++) {
@@ -1856,7 +1854,7 @@ xfs_qm_vop_create_dqattach(
{
struct xfs_mount *mp = tp->t_mountp;
- if (!XFS_IS_QUOTA_RUNNING(mp) || !XFS_IS_QUOTA_ON(mp))
+ if (!XFS_IS_QUOTA_ON(mp))
return;
ASSERT(xfs_isilocked(ip, XFS_ILOCK_EXCL));
diff --git a/fs/xfs/xfs_qm_syscalls.c b/fs/xfs/xfs_qm_syscalls.c
index e00067a142a094..b5685ce666f70d 100644
--- a/fs/xfs/xfs_qm_syscalls.c
+++ b/fs/xfs/xfs_qm_syscalls.c
@@ -204,7 +204,7 @@ xfs_qm_scall_quotaon(
(mp->m_qflags & XFS_GQUOTA_ACCT)))
return 0;
- if (! XFS_IS_QUOTA_RUNNING(mp))
+ if (!XFS_IS_QUOTA_ON(mp))
return -ESRCH;
/*
diff --git a/fs/xfs/xfs_quotaops.c b/fs/xfs/xfs_quotaops.c
index 88d70c236a5445..07989bd677289a 100644
--- a/fs/xfs/xfs_quotaops.c
+++ b/fs/xfs/xfs_quotaops.c
@@ -60,18 +60,18 @@ xfs_fs_get_quota_state(
struct xfs_quotainfo *q = mp->m_quotainfo;
memset(state, 0, sizeof(*state));
- if (!XFS_IS_QUOTA_RUNNING(mp))
+ if (!XFS_IS_QUOTA_ON(mp))
return 0;
state->s_incoredqs = q->qi_dquots;
- if (XFS_IS_UQUOTA_RUNNING(mp))
+ if (XFS_IS_UQUOTA_ON(mp))
state->s_state[USRQUOTA].flags |= QCI_ACCT_ENABLED;
if (XFS_IS_UQUOTA_ENFORCED(mp))
state->s_state[USRQUOTA].flags |= QCI_LIMITS_ENFORCED;
- if (XFS_IS_GQUOTA_RUNNING(mp))
+ if (XFS_IS_GQUOTA_ON(mp))
state->s_state[GRPQUOTA].flags |= QCI_ACCT_ENABLED;
if (XFS_IS_GQUOTA_ENFORCED(mp))
state->s_state[GRPQUOTA].flags |= QCI_LIMITS_ENFORCED;
- if (XFS_IS_PQUOTA_RUNNING(mp))
+ if (XFS_IS_PQUOTA_ON(mp))
state->s_state[PRJQUOTA].flags |= QCI_ACCT_ENABLED;
if (XFS_IS_PQUOTA_ENFORCED(mp))
state->s_state[PRJQUOTA].flags |= QCI_LIMITS_ENFORCED;
@@ -114,10 +114,8 @@ xfs_fs_set_info(
if (sb_rdonly(sb))
return -EROFS;
- if (!XFS_IS_QUOTA_RUNNING(mp))
- return -ENOSYS;
if (!XFS_IS_QUOTA_ON(mp))
- return -ESRCH;
+ return -ENOSYS;
if (info->i_fieldmask & ~XFS_QC_SETINFO_MASK)
return -EINVAL;
if ((info->i_fieldmask & XFS_QC_SETINFO_MASK) == 0)
@@ -164,7 +162,7 @@ xfs_quota_enable(
if (sb_rdonly(sb))
return -EROFS;
- if (!XFS_IS_QUOTA_RUNNING(mp))
+ if (!XFS_IS_QUOTA_ON(mp))
return -ENOSYS;
return xfs_qm_scall_quotaon(mp, xfs_quota_flags(uflags));
@@ -179,10 +177,8 @@ xfs_quota_disable(
if (sb_rdonly(sb))
return -EROFS;
- if (!XFS_IS_QUOTA_RUNNING(mp))
- return -ENOSYS;
if (!XFS_IS_QUOTA_ON(mp))
- return -EINVAL;
+ return -ENOSYS;
return xfs_qm_scall_quotaoff(mp, xfs_quota_flags(uflags));
}
@@ -223,10 +219,8 @@ xfs_fs_get_dqblk(
struct xfs_mount *mp = XFS_M(sb);
xfs_dqid_t id;
- if (!XFS_IS_QUOTA_RUNNING(mp))
- return -ENOSYS;
if (!XFS_IS_QUOTA_ON(mp))
- return -ESRCH;
+ return -ENOSYS;
id = from_kqid(&init_user_ns, qid);
return xfs_qm_scall_getquota(mp, id, xfs_quota_type(qid.type), qdq);
@@ -243,10 +237,8 @@ xfs_fs_get_nextdqblk(
struct xfs_mount *mp = XFS_M(sb);
xfs_dqid_t id;
- if (!XFS_IS_QUOTA_RUNNING(mp))
- return -ENOSYS;
if (!XFS_IS_QUOTA_ON(mp))
- return -ESRCH;
+ return -ENOSYS;
id = from_kqid(&init_user_ns, *qid);
ret = xfs_qm_scall_getquota_next(mp, &id, xfs_quota_type(qid->type),
@@ -269,10 +261,8 @@ xfs_fs_set_dqblk(
if (sb_rdonly(sb))
return -EROFS;
- if (!XFS_IS_QUOTA_RUNNING(mp))
- return -ENOSYS;
if (!XFS_IS_QUOTA_ON(mp))
- return -ESRCH;
+ return -ENOSYS;
return xfs_qm_scall_setqlim(mp, from_kqid(&init_user_ns, qid),
xfs_quota_type(qid.type), qdq);
diff --git a/fs/xfs/xfs_super.c b/fs/xfs/xfs_super.c
index 2c9e26a44546b8..36fc81e52dc22a 100644
--- a/fs/xfs/xfs_super.c
+++ b/fs/xfs/xfs_super.c
@@ -201,25 +201,20 @@ xfs_fs_show_options(
seq_printf(m, ",swidth=%d",
(int)XFS_FSB_TO_BB(mp, mp->m_swidth));
- if (mp->m_qflags & XFS_UQUOTA_ACCT) {
- if (mp->m_qflags & XFS_UQUOTA_ENFD)
- seq_puts(m, ",usrquota");
- else
- seq_puts(m, ",uqnoenforce");
- }
+ if (mp->m_qflags & XFS_UQUOTA_ENFD)
+ seq_puts(m, ",usrquota");
+ else if (mp->m_qflags & XFS_UQUOTA_ACCT)
+ seq_puts(m, ",uqnoenforce");
- if (mp->m_qflags & XFS_PQUOTA_ACCT) {
- if (mp->m_qflags & XFS_PQUOTA_ENFD)
- seq_puts(m, ",prjquota");
- else
- seq_puts(m, ",pqnoenforce");
- }
- if (mp->m_qflags & XFS_GQUOTA_ACCT) {
- if (mp->m_qflags & XFS_GQUOTA_ENFD)
- seq_puts(m, ",grpquota");
- else
- seq_puts(m, ",gqnoenforce");
- }
+ if (mp->m_qflags & XFS_PQUOTA_ENFD)
+ seq_puts(m, ",prjquota");
+ else if (mp->m_qflags & XFS_PQUOTA_ACCT)
+ seq_puts(m, ",pqnoenforce");
+
+ if (mp->m_qflags & XFS_GQUOTA_ENFD)
+ seq_puts(m, ",grpquota");
+ else if (mp->m_qflags & XFS_GQUOTA_ACCT)
+ seq_puts(m, ",gqnoenforce");
if (!(mp->m_qflags & XFS_ALL_QUOTA_ACCT))
seq_puts(m, ",noquota");
@@ -962,8 +957,8 @@ xfs_finish_flags(
return -EROFS;
}
- if ((mp->m_qflags & (XFS_GQUOTA_ACCT | XFS_GQUOTA_ACTIVE)) &&
- (mp->m_qflags & (XFS_PQUOTA_ACCT | XFS_PQUOTA_ACTIVE)) &&
+ if ((mp->m_qflags & XFS_GQUOTA_ACCT) &&
+ (mp->m_qflags & XFS_PQUOTA_ACCT) &&
!xfs_sb_version_has_pquotino(&mp->m_sb)) {
xfs_warn(mp,
"Super block does not support project and group quota together");
@@ -1228,35 +1223,31 @@ xfs_fs_parse_param(
case Opt_noquota:
parsing_mp->m_qflags &= ~XFS_ALL_QUOTA_ACCT;
parsing_mp->m_qflags &= ~XFS_ALL_QUOTA_ENFD;
- parsing_mp->m_qflags &= ~XFS_ALL_QUOTA_ACTIVE;
return 0;
case Opt_quota:
case Opt_uquota:
case Opt_usrquota:
- parsing_mp->m_qflags |= (XFS_UQUOTA_ACCT | XFS_UQUOTA_ACTIVE |
- XFS_UQUOTA_ENFD);
+ parsing_mp->m_qflags |= (XFS_UQUOTA_ACCT | XFS_UQUOTA_ENFD);
return 0;
case Opt_qnoenforce:
case Opt_uqnoenforce:
- parsing_mp->m_qflags |= (XFS_UQUOTA_ACCT | XFS_UQUOTA_ACTIVE);
+ parsing_mp->m_qflags |= XFS_UQUOTA_ACCT;
parsing_mp->m_qflags &= ~XFS_UQUOTA_ENFD;
return 0;
case Opt_pquota:
case Opt_prjquota:
- parsing_mp->m_qflags |= (XFS_PQUOTA_ACCT | XFS_PQUOTA_ACTIVE |
- XFS_PQUOTA_ENFD);
+ parsing_mp->m_qflags |= (XFS_PQUOTA_ACCT | XFS_PQUOTA_ENFD);
return 0;
case Opt_pqnoenforce:
- parsing_mp->m_qflags |= (XFS_PQUOTA_ACCT | XFS_PQUOTA_ACTIVE);
+ parsing_mp->m_qflags |= XFS_PQUOTA_ACCT;
parsing_mp->m_qflags &= ~XFS_PQUOTA_ENFD;
return 0;
case Opt_gquota:
case Opt_grpquota:
- parsing_mp->m_qflags |= (XFS_GQUOTA_ACCT | XFS_GQUOTA_ACTIVE |
- XFS_GQUOTA_ENFD);
+ parsing_mp->m_qflags |= (XFS_GQUOTA_ACCT | XFS_GQUOTA_ENFD);
return 0;
case Opt_gqnoenforce:
- parsing_mp->m_qflags |= (XFS_GQUOTA_ACCT | XFS_GQUOTA_ACTIVE);
+ parsing_mp->m_qflags |= XFS_GQUOTA_ACCT;
parsing_mp->m_qflags &= ~XFS_GQUOTA_ENFD;
return 0;
case Opt_discard:
diff --git a/fs/xfs/xfs_trans_dquot.c b/fs/xfs/xfs_trans_dquot.c
index b7e4b05a559bdb..eb76bc5bed9d0a 100644
--- a/fs/xfs/xfs_trans_dquot.c
+++ b/fs/xfs/xfs_trans_dquot.c
@@ -132,8 +132,7 @@ xfs_trans_mod_dquot_byino(
{
xfs_mount_t *mp = tp->t_mountp;
- if (!XFS_IS_QUOTA_RUNNING(mp) ||
- !XFS_IS_QUOTA_ON(mp) ||
+ if (!XFS_IS_QUOTA_ON(mp) ||
xfs_is_quota_inode(&mp->m_sb, ip->i_ino))
return;
@@ -192,7 +191,7 @@ xfs_trans_mod_dquot(
struct xfs_dqtrx *qtrx;
ASSERT(tp);
- ASSERT(XFS_IS_QUOTA_RUNNING(tp->t_mountp));
+ ASSERT(XFS_IS_QUOTA_ON(tp->t_mountp));
qtrx = NULL;
if (!delta)
@@ -738,7 +737,7 @@ xfs_trans_reserve_quota_bydquots(
{
int error;
- if (!XFS_IS_QUOTA_RUNNING(mp) || !XFS_IS_QUOTA_ON(mp))
+ if (!XFS_IS_QUOTA_ON(mp))
return 0;
ASSERT(flags & XFS_QMOPT_RESBLK_MASK);
@@ -795,7 +794,7 @@ xfs_trans_reserve_quota_nblks(
unsigned int qflags = 0;
int error;
- if (!XFS_IS_QUOTA_RUNNING(mp) || !XFS_IS_QUOTA_ON(mp))
+ if (!XFS_IS_QUOTA_ON(mp))
return 0;
ASSERT(!xfs_is_quota_inode(&mp->m_sb, ip->i_ino));
@@ -836,7 +835,7 @@ xfs_trans_reserve_quota_icreate(
{
struct xfs_mount *mp = tp->t_mountp;
- if (!XFS_IS_QUOTA_RUNNING(mp) || !XFS_IS_QUOTA_ON(mp))
+ if (!XFS_IS_QUOTA_ON(mp))
return 0;
return xfs_trans_reserve_quota_bydquots(tp, mp, udqp, gdqp, pdqp,
--
2.30.2
^ permalink raw reply related [flat|nested] 14+ messages in thread* Re: [PATCH 3/3] xfs: remove the active vs running quota differentiation
2021-07-12 11:14 ` [PATCH 3/3] xfs: remove the active vs running quota differentiation Christoph Hellwig
@ 2021-07-14 11:47 ` Carlos Maiolino
2021-07-14 21:47 ` Darrick J. Wong
1 sibling, 0 replies; 14+ messages in thread
From: Carlos Maiolino @ 2021-07-14 11:47 UTC (permalink / raw)
To: Christoph Hellwig; +Cc: linux-xfs
On Mon, Jul 12, 2021 at 01:14:26PM +0200, Christoph Hellwig wrote:
> These only made a difference when quotaoff supported disabling quota
> accounting on a mounted file system, so we can switch everyone to use
> a single set of flags and helpers now. Note that the *QUOTA_ON naming
> for the helpers is kept as it was the much more commonly used one.
>
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
> fs/xfs/libxfs/xfs_quota_defs.h | 30 +++-----------------
> fs/xfs/scrub/quota.c | 2 +-
> fs/xfs/xfs_dquot.c | 3 --
> fs/xfs/xfs_ioctl.c | 2 +-
> fs/xfs/xfs_iops.c | 4 +--
> fs/xfs/xfs_mount.c | 4 +--
> fs/xfs/xfs_qm.c | 26 ++++++++---------
> fs/xfs/xfs_qm_syscalls.c | 2 +-
> fs/xfs/xfs_quotaops.c | 30 +++++++-------------
> fs/xfs/xfs_super.c | 51 ++++++++++++++--------------------
> fs/xfs/xfs_trans_dquot.c | 11 ++++----
> 11 files changed, 58 insertions(+), 107 deletions(-)
Looks good
Reviewed-by: Carlos Maiolino <cmaiolino@redhat.com>
>
> diff --git a/fs/xfs/libxfs/xfs_quota_defs.h b/fs/xfs/libxfs/xfs_quota_defs.h
> index 0f0af4e3503293..a02c5062f9b292 100644
> --- a/fs/xfs/libxfs/xfs_quota_defs.h
> +++ b/fs/xfs/libxfs/xfs_quota_defs.h
> @@ -60,36 +60,14 @@ typedef uint8_t xfs_dqtype_t;
> #define XFS_DQUOT_LOGRES(mp) \
> ((sizeof(struct xfs_dq_logformat) + sizeof(struct xfs_disk_dquot)) * 6)
>
> -#define XFS_IS_QUOTA_RUNNING(mp) ((mp)->m_qflags & XFS_ALL_QUOTA_ACCT)
> -#define XFS_IS_UQUOTA_RUNNING(mp) ((mp)->m_qflags & XFS_UQUOTA_ACCT)
> -#define XFS_IS_PQUOTA_RUNNING(mp) ((mp)->m_qflags & XFS_PQUOTA_ACCT)
> -#define XFS_IS_GQUOTA_RUNNING(mp) ((mp)->m_qflags & XFS_GQUOTA_ACCT)
> +#define XFS_IS_QUOTA_ON(mp) ((mp)->m_qflags & XFS_ALL_QUOTA_ACCT)
> +#define XFS_IS_UQUOTA_ON(mp) ((mp)->m_qflags & XFS_UQUOTA_ACCT)
> +#define XFS_IS_PQUOTA_ON(mp) ((mp)->m_qflags & XFS_PQUOTA_ACCT)
> +#define XFS_IS_GQUOTA_ON(mp) ((mp)->m_qflags & XFS_GQUOTA_ACCT)
> #define XFS_IS_UQUOTA_ENFORCED(mp) ((mp)->m_qflags & XFS_UQUOTA_ENFD)
> #define XFS_IS_GQUOTA_ENFORCED(mp) ((mp)->m_qflags & XFS_GQUOTA_ENFD)
> #define XFS_IS_PQUOTA_ENFORCED(mp) ((mp)->m_qflags & XFS_PQUOTA_ENFD)
>
> -/*
> - * Incore only flags for quotaoff - these bits get cleared when quota(s)
> - * are in the process of getting turned off. These flags are in m_qflags but
> - * never in sb_qflags.
> - */
> -#define XFS_UQUOTA_ACTIVE 0x1000 /* uquotas are being turned off */
> -#define XFS_GQUOTA_ACTIVE 0x2000 /* gquotas are being turned off */
> -#define XFS_PQUOTA_ACTIVE 0x4000 /* pquotas are being turned off */
> -#define XFS_ALL_QUOTA_ACTIVE \
> - (XFS_UQUOTA_ACTIVE | XFS_GQUOTA_ACTIVE | XFS_PQUOTA_ACTIVE)
> -
> -/*
> - * Checking XFS_IS_*QUOTA_ON() while holding any inode lock guarantees
> - * quota will be not be switched off as long as that inode lock is held.
> - */
> -#define XFS_IS_QUOTA_ON(mp) ((mp)->m_qflags & (XFS_UQUOTA_ACTIVE | \
> - XFS_GQUOTA_ACTIVE | \
> - XFS_PQUOTA_ACTIVE))
> -#define XFS_IS_UQUOTA_ON(mp) ((mp)->m_qflags & XFS_UQUOTA_ACTIVE)
> -#define XFS_IS_GQUOTA_ON(mp) ((mp)->m_qflags & XFS_GQUOTA_ACTIVE)
> -#define XFS_IS_PQUOTA_ON(mp) ((mp)->m_qflags & XFS_PQUOTA_ACTIVE)
> -
> /*
> * Flags to tell various functions what to do. Not all of these are meaningful
> * to a single function. None of these XFS_QMOPT_* flags are meant to have
> diff --git a/fs/xfs/scrub/quota.c b/fs/xfs/scrub/quota.c
> index acbb9839d42fbd..3cccd6d5b57755 100644
> --- a/fs/xfs/scrub/quota.c
> +++ b/fs/xfs/scrub/quota.c
> @@ -42,7 +42,7 @@ xchk_setup_quota(
> xfs_dqtype_t dqtype;
> int error;
>
> - if (!XFS_IS_QUOTA_RUNNING(sc->mp) || !XFS_IS_QUOTA_ON(sc->mp))
> + if (!XFS_IS_QUOTA_ON(sc->mp))
> return -ENOENT;
>
> dqtype = xchk_quota_to_dqtype(sc);
> diff --git a/fs/xfs/xfs_dquot.c b/fs/xfs/xfs_dquot.c
> index ecd5059d6928f7..c301b18b7685b1 100644
> --- a/fs/xfs/xfs_dquot.c
> +++ b/fs/xfs/xfs_dquot.c
> @@ -847,9 +847,6 @@ xfs_qm_dqget_checks(
> struct xfs_mount *mp,
> xfs_dqtype_t type)
> {
> - if (WARN_ON_ONCE(!XFS_IS_QUOTA_RUNNING(mp)))
> - return -ESRCH;
> -
> switch (type) {
> case XFS_DQTYPE_USER:
> if (!XFS_IS_UQUOTA_ON(mp))
> diff --git a/fs/xfs/xfs_ioctl.c b/fs/xfs/xfs_ioctl.c
> index 65270e63c032a0..9f4aa1477e2fca 100644
> --- a/fs/xfs/xfs_ioctl.c
> +++ b/fs/xfs/xfs_ioctl.c
> @@ -1433,7 +1433,7 @@ xfs_fileattr_set(
>
> /* Change the ownerships and register project quota modifications */
> if (ip->i_projid != fa->fsx_projid) {
> - if (XFS_IS_QUOTA_RUNNING(mp) && XFS_IS_PQUOTA_ON(mp)) {
> + if (XFS_IS_PQUOTA_ON(mp)) {
> olddquot = xfs_qm_vop_chown(tp, ip,
> &ip->i_pdquot, pdqp);
> }
> diff --git a/fs/xfs/xfs_iops.c b/fs/xfs/xfs_iops.c
> index 93c082db04b78c..327d65ef1e268c 100644
> --- a/fs/xfs/xfs_iops.c
> +++ b/fs/xfs/xfs_iops.c
> @@ -778,7 +778,7 @@ xfs_setattr_nonsize(
> * in the transaction.
> */
> if (!uid_eq(iuid, uid)) {
> - if (XFS_IS_QUOTA_RUNNING(mp) && XFS_IS_UQUOTA_ON(mp)) {
> + if (XFS_IS_UQUOTA_ON(mp)) {
> ASSERT(mask & ATTR_UID);
> ASSERT(udqp);
> olddquot1 = xfs_qm_vop_chown(tp, ip,
> @@ -787,7 +787,7 @@ xfs_setattr_nonsize(
> inode->i_uid = uid;
> }
> if (!gid_eq(igid, gid)) {
> - if (XFS_IS_QUOTA_RUNNING(mp) && XFS_IS_GQUOTA_ON(mp)) {
> + if (XFS_IS_GQUOTA_ON(mp)) {
> ASSERT(xfs_sb_version_has_pquotino(&mp->m_sb) ||
> !XFS_IS_PQUOTA_ON(mp));
> ASSERT(mask & ATTR_GID);
> diff --git a/fs/xfs/xfs_mount.c b/fs/xfs/xfs_mount.c
> index d0755494597f99..baf7b323cb15eb 100644
> --- a/fs/xfs/xfs_mount.c
> +++ b/fs/xfs/xfs_mount.c
> @@ -836,13 +836,11 @@ xfs_mountfs(
> /*
> * Initialise the XFS quota management subsystem for this mount
> */
> - if (XFS_IS_QUOTA_RUNNING(mp)) {
> + if (XFS_IS_QUOTA_ON(mp)) {
> error = xfs_qm_newmount(mp, "amount, "aflags);
> if (error)
> goto out_rtunmount;
> } else {
> - ASSERT(!XFS_IS_QUOTA_ON(mp));
> -
> /*
> * If a file system had quotas running earlier, but decided to
> * mount without -o uquota/pquota/gquota options, revoke the
> diff --git a/fs/xfs/xfs_qm.c b/fs/xfs/xfs_qm.c
> index 2b34273d0405e7..351d99bc52e5f8 100644
> --- a/fs/xfs/xfs_qm.c
> +++ b/fs/xfs/xfs_qm.c
> @@ -295,8 +295,6 @@ xfs_qm_need_dqattach(
> {
> struct xfs_mount *mp = ip->i_mount;
>
> - if (!XFS_IS_QUOTA_RUNNING(mp))
> - return false;
> if (!XFS_IS_QUOTA_ON(mp))
> return false;
> if (!XFS_NOT_DQATTACHED(mp, ip))
> @@ -631,7 +629,7 @@ xfs_qm_init_quotainfo(
> struct xfs_quotainfo *qinf;
> int error;
>
> - ASSERT(XFS_IS_QUOTA_RUNNING(mp));
> + ASSERT(XFS_IS_QUOTA_ON(mp));
>
> qinf = mp->m_quotainfo = kmem_zalloc(sizeof(struct xfs_quotainfo), 0);
>
> @@ -676,11 +674,11 @@ xfs_qm_init_quotainfo(
> xfs_qm_init_timelimits(mp, XFS_DQTYPE_GROUP);
> xfs_qm_init_timelimits(mp, XFS_DQTYPE_PROJ);
>
> - if (XFS_IS_UQUOTA_RUNNING(mp))
> + if (XFS_IS_UQUOTA_ON(mp))
> xfs_qm_set_defquota(mp, XFS_DQTYPE_USER, qinf);
> - if (XFS_IS_GQUOTA_RUNNING(mp))
> + if (XFS_IS_GQUOTA_ON(mp))
> xfs_qm_set_defquota(mp, XFS_DQTYPE_GROUP, qinf);
> - if (XFS_IS_PQUOTA_RUNNING(mp))
> + if (XFS_IS_PQUOTA_ON(mp))
> xfs_qm_set_defquota(mp, XFS_DQTYPE_PROJ, qinf);
>
> qinf->qi_shrinker.count_objects = xfs_qm_shrink_count;
> @@ -1143,7 +1141,7 @@ xfs_qm_dqusage_adjust(
> xfs_filblks_t rtblks = 0; /* total rt blks */
> int error;
>
> - ASSERT(XFS_IS_QUOTA_RUNNING(mp));
> + ASSERT(XFS_IS_QUOTA_ON(mp));
>
> /*
> * rootino must have its resources accounted for, not so with the quota
> @@ -1284,7 +1282,7 @@ xfs_qm_quotacheck(
> flags = 0;
>
> ASSERT(uip || gip || pip);
> - ASSERT(XFS_IS_QUOTA_RUNNING(mp));
> + ASSERT(XFS_IS_QUOTA_ON(mp));
>
> xfs_notice(mp, "Quotacheck needed: Please wait.");
>
> @@ -1414,7 +1412,7 @@ xfs_qm_mount_quotas(
> goto write_changes;
> }
>
> - ASSERT(XFS_IS_QUOTA_RUNNING(mp));
> + ASSERT(XFS_IS_QUOTA_ON(mp));
>
> /*
> * Allocate the quotainfo structure inside the mount struct, and
> @@ -1469,7 +1467,7 @@ xfs_qm_mount_quotas(
> * the incore structures are convinced that quotas are
> * off, but the on disk superblock doesn't know that !
> */
> - ASSERT(!(XFS_IS_QUOTA_RUNNING(mp)));
> + ASSERT(!(XFS_IS_QUOTA_ON(mp)));
> xfs_alert(mp, "%s: Superblock update failed!",
> __func__);
> }
> @@ -1641,7 +1639,7 @@ xfs_qm_vop_dqalloc(
> int error;
> uint lockflags;
>
> - if (!XFS_IS_QUOTA_RUNNING(mp) || !XFS_IS_QUOTA_ON(mp))
> + if (!XFS_IS_QUOTA_ON(mp))
> return 0;
>
> lockflags = XFS_ILOCK_EXCL;
> @@ -1772,7 +1770,7 @@ xfs_qm_vop_chown(
>
>
> ASSERT(xfs_isilocked(ip, XFS_ILOCK_EXCL));
> - ASSERT(XFS_IS_QUOTA_RUNNING(ip->i_mount));
> + ASSERT(XFS_IS_QUOTA_ON(ip->i_mount));
>
> /* old dquot */
> prevdq = *IO_olddq;
> @@ -1825,7 +1823,7 @@ xfs_qm_vop_rename_dqattach(
> struct xfs_mount *mp = i_tab[0]->i_mount;
> int i;
>
> - if (!XFS_IS_QUOTA_RUNNING(mp) || !XFS_IS_QUOTA_ON(mp))
> + if (!XFS_IS_QUOTA_ON(mp))
> return 0;
>
> for (i = 0; (i < 4 && i_tab[i]); i++) {
> @@ -1856,7 +1854,7 @@ xfs_qm_vop_create_dqattach(
> {
> struct xfs_mount *mp = tp->t_mountp;
>
> - if (!XFS_IS_QUOTA_RUNNING(mp) || !XFS_IS_QUOTA_ON(mp))
> + if (!XFS_IS_QUOTA_ON(mp))
> return;
>
> ASSERT(xfs_isilocked(ip, XFS_ILOCK_EXCL));
> diff --git a/fs/xfs/xfs_qm_syscalls.c b/fs/xfs/xfs_qm_syscalls.c
> index e00067a142a094..b5685ce666f70d 100644
> --- a/fs/xfs/xfs_qm_syscalls.c
> +++ b/fs/xfs/xfs_qm_syscalls.c
> @@ -204,7 +204,7 @@ xfs_qm_scall_quotaon(
> (mp->m_qflags & XFS_GQUOTA_ACCT)))
> return 0;
>
> - if (! XFS_IS_QUOTA_RUNNING(mp))
> + if (!XFS_IS_QUOTA_ON(mp))
> return -ESRCH;
>
> /*
> diff --git a/fs/xfs/xfs_quotaops.c b/fs/xfs/xfs_quotaops.c
> index 88d70c236a5445..07989bd677289a 100644
> --- a/fs/xfs/xfs_quotaops.c
> +++ b/fs/xfs/xfs_quotaops.c
> @@ -60,18 +60,18 @@ xfs_fs_get_quota_state(
> struct xfs_quotainfo *q = mp->m_quotainfo;
>
> memset(state, 0, sizeof(*state));
> - if (!XFS_IS_QUOTA_RUNNING(mp))
> + if (!XFS_IS_QUOTA_ON(mp))
> return 0;
> state->s_incoredqs = q->qi_dquots;
> - if (XFS_IS_UQUOTA_RUNNING(mp))
> + if (XFS_IS_UQUOTA_ON(mp))
> state->s_state[USRQUOTA].flags |= QCI_ACCT_ENABLED;
> if (XFS_IS_UQUOTA_ENFORCED(mp))
> state->s_state[USRQUOTA].flags |= QCI_LIMITS_ENFORCED;
> - if (XFS_IS_GQUOTA_RUNNING(mp))
> + if (XFS_IS_GQUOTA_ON(mp))
> state->s_state[GRPQUOTA].flags |= QCI_ACCT_ENABLED;
> if (XFS_IS_GQUOTA_ENFORCED(mp))
> state->s_state[GRPQUOTA].flags |= QCI_LIMITS_ENFORCED;
> - if (XFS_IS_PQUOTA_RUNNING(mp))
> + if (XFS_IS_PQUOTA_ON(mp))
> state->s_state[PRJQUOTA].flags |= QCI_ACCT_ENABLED;
> if (XFS_IS_PQUOTA_ENFORCED(mp))
> state->s_state[PRJQUOTA].flags |= QCI_LIMITS_ENFORCED;
> @@ -114,10 +114,8 @@ xfs_fs_set_info(
>
> if (sb_rdonly(sb))
> return -EROFS;
> - if (!XFS_IS_QUOTA_RUNNING(mp))
> - return -ENOSYS;
> if (!XFS_IS_QUOTA_ON(mp))
> - return -ESRCH;
> + return -ENOSYS;
> if (info->i_fieldmask & ~XFS_QC_SETINFO_MASK)
> return -EINVAL;
> if ((info->i_fieldmask & XFS_QC_SETINFO_MASK) == 0)
> @@ -164,7 +162,7 @@ xfs_quota_enable(
>
> if (sb_rdonly(sb))
> return -EROFS;
> - if (!XFS_IS_QUOTA_RUNNING(mp))
> + if (!XFS_IS_QUOTA_ON(mp))
> return -ENOSYS;
>
> return xfs_qm_scall_quotaon(mp, xfs_quota_flags(uflags));
> @@ -179,10 +177,8 @@ xfs_quota_disable(
>
> if (sb_rdonly(sb))
> return -EROFS;
> - if (!XFS_IS_QUOTA_RUNNING(mp))
> - return -ENOSYS;
> if (!XFS_IS_QUOTA_ON(mp))
> - return -EINVAL;
> + return -ENOSYS;
>
> return xfs_qm_scall_quotaoff(mp, xfs_quota_flags(uflags));
> }
> @@ -223,10 +219,8 @@ xfs_fs_get_dqblk(
> struct xfs_mount *mp = XFS_M(sb);
> xfs_dqid_t id;
>
> - if (!XFS_IS_QUOTA_RUNNING(mp))
> - return -ENOSYS;
> if (!XFS_IS_QUOTA_ON(mp))
> - return -ESRCH;
> + return -ENOSYS;
>
> id = from_kqid(&init_user_ns, qid);
> return xfs_qm_scall_getquota(mp, id, xfs_quota_type(qid.type), qdq);
> @@ -243,10 +237,8 @@ xfs_fs_get_nextdqblk(
> struct xfs_mount *mp = XFS_M(sb);
> xfs_dqid_t id;
>
> - if (!XFS_IS_QUOTA_RUNNING(mp))
> - return -ENOSYS;
> if (!XFS_IS_QUOTA_ON(mp))
> - return -ESRCH;
> + return -ENOSYS;
>
> id = from_kqid(&init_user_ns, *qid);
> ret = xfs_qm_scall_getquota_next(mp, &id, xfs_quota_type(qid->type),
> @@ -269,10 +261,8 @@ xfs_fs_set_dqblk(
>
> if (sb_rdonly(sb))
> return -EROFS;
> - if (!XFS_IS_QUOTA_RUNNING(mp))
> - return -ENOSYS;
> if (!XFS_IS_QUOTA_ON(mp))
> - return -ESRCH;
> + return -ENOSYS;
>
> return xfs_qm_scall_setqlim(mp, from_kqid(&init_user_ns, qid),
> xfs_quota_type(qid.type), qdq);
> diff --git a/fs/xfs/xfs_super.c b/fs/xfs/xfs_super.c
> index 2c9e26a44546b8..36fc81e52dc22a 100644
> --- a/fs/xfs/xfs_super.c
> +++ b/fs/xfs/xfs_super.c
> @@ -201,25 +201,20 @@ xfs_fs_show_options(
> seq_printf(m, ",swidth=%d",
> (int)XFS_FSB_TO_BB(mp, mp->m_swidth));
>
> - if (mp->m_qflags & XFS_UQUOTA_ACCT) {
> - if (mp->m_qflags & XFS_UQUOTA_ENFD)
> - seq_puts(m, ",usrquota");
> - else
> - seq_puts(m, ",uqnoenforce");
> - }
> + if (mp->m_qflags & XFS_UQUOTA_ENFD)
> + seq_puts(m, ",usrquota");
> + else if (mp->m_qflags & XFS_UQUOTA_ACCT)
> + seq_puts(m, ",uqnoenforce");
>
> - if (mp->m_qflags & XFS_PQUOTA_ACCT) {
> - if (mp->m_qflags & XFS_PQUOTA_ENFD)
> - seq_puts(m, ",prjquota");
> - else
> - seq_puts(m, ",pqnoenforce");
> - }
> - if (mp->m_qflags & XFS_GQUOTA_ACCT) {
> - if (mp->m_qflags & XFS_GQUOTA_ENFD)
> - seq_puts(m, ",grpquota");
> - else
> - seq_puts(m, ",gqnoenforce");
> - }
> + if (mp->m_qflags & XFS_PQUOTA_ENFD)
> + seq_puts(m, ",prjquota");
> + else if (mp->m_qflags & XFS_PQUOTA_ACCT)
> + seq_puts(m, ",pqnoenforce");
> +
> + if (mp->m_qflags & XFS_GQUOTA_ENFD)
> + seq_puts(m, ",grpquota");
> + else if (mp->m_qflags & XFS_GQUOTA_ACCT)
> + seq_puts(m, ",gqnoenforce");
>
> if (!(mp->m_qflags & XFS_ALL_QUOTA_ACCT))
> seq_puts(m, ",noquota");
> @@ -962,8 +957,8 @@ xfs_finish_flags(
> return -EROFS;
> }
>
> - if ((mp->m_qflags & (XFS_GQUOTA_ACCT | XFS_GQUOTA_ACTIVE)) &&
> - (mp->m_qflags & (XFS_PQUOTA_ACCT | XFS_PQUOTA_ACTIVE)) &&
> + if ((mp->m_qflags & XFS_GQUOTA_ACCT) &&
> + (mp->m_qflags & XFS_PQUOTA_ACCT) &&
> !xfs_sb_version_has_pquotino(&mp->m_sb)) {
> xfs_warn(mp,
> "Super block does not support project and group quota together");
> @@ -1228,35 +1223,31 @@ xfs_fs_parse_param(
> case Opt_noquota:
> parsing_mp->m_qflags &= ~XFS_ALL_QUOTA_ACCT;
> parsing_mp->m_qflags &= ~XFS_ALL_QUOTA_ENFD;
> - parsing_mp->m_qflags &= ~XFS_ALL_QUOTA_ACTIVE;
> return 0;
> case Opt_quota:
> case Opt_uquota:
> case Opt_usrquota:
> - parsing_mp->m_qflags |= (XFS_UQUOTA_ACCT | XFS_UQUOTA_ACTIVE |
> - XFS_UQUOTA_ENFD);
> + parsing_mp->m_qflags |= (XFS_UQUOTA_ACCT | XFS_UQUOTA_ENFD);
> return 0;
> case Opt_qnoenforce:
> case Opt_uqnoenforce:
> - parsing_mp->m_qflags |= (XFS_UQUOTA_ACCT | XFS_UQUOTA_ACTIVE);
> + parsing_mp->m_qflags |= XFS_UQUOTA_ACCT;
> parsing_mp->m_qflags &= ~XFS_UQUOTA_ENFD;
> return 0;
> case Opt_pquota:
> case Opt_prjquota:
> - parsing_mp->m_qflags |= (XFS_PQUOTA_ACCT | XFS_PQUOTA_ACTIVE |
> - XFS_PQUOTA_ENFD);
> + parsing_mp->m_qflags |= (XFS_PQUOTA_ACCT | XFS_PQUOTA_ENFD);
> return 0;
> case Opt_pqnoenforce:
> - parsing_mp->m_qflags |= (XFS_PQUOTA_ACCT | XFS_PQUOTA_ACTIVE);
> + parsing_mp->m_qflags |= XFS_PQUOTA_ACCT;
> parsing_mp->m_qflags &= ~XFS_PQUOTA_ENFD;
> return 0;
> case Opt_gquota:
> case Opt_grpquota:
> - parsing_mp->m_qflags |= (XFS_GQUOTA_ACCT | XFS_GQUOTA_ACTIVE |
> - XFS_GQUOTA_ENFD);
> + parsing_mp->m_qflags |= (XFS_GQUOTA_ACCT | XFS_GQUOTA_ENFD);
> return 0;
> case Opt_gqnoenforce:
> - parsing_mp->m_qflags |= (XFS_GQUOTA_ACCT | XFS_GQUOTA_ACTIVE);
> + parsing_mp->m_qflags |= XFS_GQUOTA_ACCT;
> parsing_mp->m_qflags &= ~XFS_GQUOTA_ENFD;
> return 0;
> case Opt_discard:
> diff --git a/fs/xfs/xfs_trans_dquot.c b/fs/xfs/xfs_trans_dquot.c
> index b7e4b05a559bdb..eb76bc5bed9d0a 100644
> --- a/fs/xfs/xfs_trans_dquot.c
> +++ b/fs/xfs/xfs_trans_dquot.c
> @@ -132,8 +132,7 @@ xfs_trans_mod_dquot_byino(
> {
> xfs_mount_t *mp = tp->t_mountp;
>
> - if (!XFS_IS_QUOTA_RUNNING(mp) ||
> - !XFS_IS_QUOTA_ON(mp) ||
> + if (!XFS_IS_QUOTA_ON(mp) ||
> xfs_is_quota_inode(&mp->m_sb, ip->i_ino))
> return;
>
> @@ -192,7 +191,7 @@ xfs_trans_mod_dquot(
> struct xfs_dqtrx *qtrx;
>
> ASSERT(tp);
> - ASSERT(XFS_IS_QUOTA_RUNNING(tp->t_mountp));
> + ASSERT(XFS_IS_QUOTA_ON(tp->t_mountp));
> qtrx = NULL;
>
> if (!delta)
> @@ -738,7 +737,7 @@ xfs_trans_reserve_quota_bydquots(
> {
> int error;
>
> - if (!XFS_IS_QUOTA_RUNNING(mp) || !XFS_IS_QUOTA_ON(mp))
> + if (!XFS_IS_QUOTA_ON(mp))
> return 0;
>
> ASSERT(flags & XFS_QMOPT_RESBLK_MASK);
> @@ -795,7 +794,7 @@ xfs_trans_reserve_quota_nblks(
> unsigned int qflags = 0;
> int error;
>
> - if (!XFS_IS_QUOTA_RUNNING(mp) || !XFS_IS_QUOTA_ON(mp))
> + if (!XFS_IS_QUOTA_ON(mp))
> return 0;
>
> ASSERT(!xfs_is_quota_inode(&mp->m_sb, ip->i_ino));
> @@ -836,7 +835,7 @@ xfs_trans_reserve_quota_icreate(
> {
> struct xfs_mount *mp = tp->t_mountp;
>
> - if (!XFS_IS_QUOTA_RUNNING(mp) || !XFS_IS_QUOTA_ON(mp))
> + if (!XFS_IS_QUOTA_ON(mp))
> return 0;
>
> return xfs_trans_reserve_quota_bydquots(tp, mp, udqp, gdqp, pdqp,
> --
> 2.30.2
>
--
Carlos
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [PATCH 3/3] xfs: remove the active vs running quota differentiation
2021-07-12 11:14 ` [PATCH 3/3] xfs: remove the active vs running quota differentiation Christoph Hellwig
2021-07-14 11:47 ` Carlos Maiolino
@ 2021-07-14 21:47 ` Darrick J. Wong
1 sibling, 0 replies; 14+ messages in thread
From: Darrick J. Wong @ 2021-07-14 21:47 UTC (permalink / raw)
To: Christoph Hellwig; +Cc: linux-xfs
On Mon, Jul 12, 2021 at 01:14:26PM +0200, Christoph Hellwig wrote:
> These only made a difference when quotaoff supported disabling quota
> accounting on a mounted file system, so we can switch everyone to use
Ugh, I hated that distinction. Thanks for making it go away.
> a single set of flags and helpers now. Note that the *QUOTA_ON naming
> for the helpers is kept as it was the much more commonly used one.
>
> Signed-off-by: Christoph Hellwig <hch@lst.de>
Nice cleanup!
Reviewed-by: Darrick J. Wong <djwong@kernel.org>
--D
> ---
> fs/xfs/libxfs/xfs_quota_defs.h | 30 +++-----------------
> fs/xfs/scrub/quota.c | 2 +-
> fs/xfs/xfs_dquot.c | 3 --
> fs/xfs/xfs_ioctl.c | 2 +-
> fs/xfs/xfs_iops.c | 4 +--
> fs/xfs/xfs_mount.c | 4 +--
> fs/xfs/xfs_qm.c | 26 ++++++++---------
> fs/xfs/xfs_qm_syscalls.c | 2 +-
> fs/xfs/xfs_quotaops.c | 30 +++++++-------------
> fs/xfs/xfs_super.c | 51 ++++++++++++++--------------------
> fs/xfs/xfs_trans_dquot.c | 11 ++++----
> 11 files changed, 58 insertions(+), 107 deletions(-)
>
> diff --git a/fs/xfs/libxfs/xfs_quota_defs.h b/fs/xfs/libxfs/xfs_quota_defs.h
> index 0f0af4e3503293..a02c5062f9b292 100644
> --- a/fs/xfs/libxfs/xfs_quota_defs.h
> +++ b/fs/xfs/libxfs/xfs_quota_defs.h
> @@ -60,36 +60,14 @@ typedef uint8_t xfs_dqtype_t;
> #define XFS_DQUOT_LOGRES(mp) \
> ((sizeof(struct xfs_dq_logformat) + sizeof(struct xfs_disk_dquot)) * 6)
>
> -#define XFS_IS_QUOTA_RUNNING(mp) ((mp)->m_qflags & XFS_ALL_QUOTA_ACCT)
> -#define XFS_IS_UQUOTA_RUNNING(mp) ((mp)->m_qflags & XFS_UQUOTA_ACCT)
> -#define XFS_IS_PQUOTA_RUNNING(mp) ((mp)->m_qflags & XFS_PQUOTA_ACCT)
> -#define XFS_IS_GQUOTA_RUNNING(mp) ((mp)->m_qflags & XFS_GQUOTA_ACCT)
> +#define XFS_IS_QUOTA_ON(mp) ((mp)->m_qflags & XFS_ALL_QUOTA_ACCT)
> +#define XFS_IS_UQUOTA_ON(mp) ((mp)->m_qflags & XFS_UQUOTA_ACCT)
> +#define XFS_IS_PQUOTA_ON(mp) ((mp)->m_qflags & XFS_PQUOTA_ACCT)
> +#define XFS_IS_GQUOTA_ON(mp) ((mp)->m_qflags & XFS_GQUOTA_ACCT)
> #define XFS_IS_UQUOTA_ENFORCED(mp) ((mp)->m_qflags & XFS_UQUOTA_ENFD)
> #define XFS_IS_GQUOTA_ENFORCED(mp) ((mp)->m_qflags & XFS_GQUOTA_ENFD)
> #define XFS_IS_PQUOTA_ENFORCED(mp) ((mp)->m_qflags & XFS_PQUOTA_ENFD)
>
> -/*
> - * Incore only flags for quotaoff - these bits get cleared when quota(s)
> - * are in the process of getting turned off. These flags are in m_qflags but
> - * never in sb_qflags.
> - */
> -#define XFS_UQUOTA_ACTIVE 0x1000 /* uquotas are being turned off */
> -#define XFS_GQUOTA_ACTIVE 0x2000 /* gquotas are being turned off */
> -#define XFS_PQUOTA_ACTIVE 0x4000 /* pquotas are being turned off */
> -#define XFS_ALL_QUOTA_ACTIVE \
> - (XFS_UQUOTA_ACTIVE | XFS_GQUOTA_ACTIVE | XFS_PQUOTA_ACTIVE)
> -
> -/*
> - * Checking XFS_IS_*QUOTA_ON() while holding any inode lock guarantees
> - * quota will be not be switched off as long as that inode lock is held.
> - */
> -#define XFS_IS_QUOTA_ON(mp) ((mp)->m_qflags & (XFS_UQUOTA_ACTIVE | \
> - XFS_GQUOTA_ACTIVE | \
> - XFS_PQUOTA_ACTIVE))
> -#define XFS_IS_UQUOTA_ON(mp) ((mp)->m_qflags & XFS_UQUOTA_ACTIVE)
> -#define XFS_IS_GQUOTA_ON(mp) ((mp)->m_qflags & XFS_GQUOTA_ACTIVE)
> -#define XFS_IS_PQUOTA_ON(mp) ((mp)->m_qflags & XFS_PQUOTA_ACTIVE)
> -
> /*
> * Flags to tell various functions what to do. Not all of these are meaningful
> * to a single function. None of these XFS_QMOPT_* flags are meant to have
> diff --git a/fs/xfs/scrub/quota.c b/fs/xfs/scrub/quota.c
> index acbb9839d42fbd..3cccd6d5b57755 100644
> --- a/fs/xfs/scrub/quota.c
> +++ b/fs/xfs/scrub/quota.c
> @@ -42,7 +42,7 @@ xchk_setup_quota(
> xfs_dqtype_t dqtype;
> int error;
>
> - if (!XFS_IS_QUOTA_RUNNING(sc->mp) || !XFS_IS_QUOTA_ON(sc->mp))
> + if (!XFS_IS_QUOTA_ON(sc->mp))
> return -ENOENT;
>
> dqtype = xchk_quota_to_dqtype(sc);
> diff --git a/fs/xfs/xfs_dquot.c b/fs/xfs/xfs_dquot.c
> index ecd5059d6928f7..c301b18b7685b1 100644
> --- a/fs/xfs/xfs_dquot.c
> +++ b/fs/xfs/xfs_dquot.c
> @@ -847,9 +847,6 @@ xfs_qm_dqget_checks(
> struct xfs_mount *mp,
> xfs_dqtype_t type)
> {
> - if (WARN_ON_ONCE(!XFS_IS_QUOTA_RUNNING(mp)))
> - return -ESRCH;
> -
> switch (type) {
> case XFS_DQTYPE_USER:
> if (!XFS_IS_UQUOTA_ON(mp))
> diff --git a/fs/xfs/xfs_ioctl.c b/fs/xfs/xfs_ioctl.c
> index 65270e63c032a0..9f4aa1477e2fca 100644
> --- a/fs/xfs/xfs_ioctl.c
> +++ b/fs/xfs/xfs_ioctl.c
> @@ -1433,7 +1433,7 @@ xfs_fileattr_set(
>
> /* Change the ownerships and register project quota modifications */
> if (ip->i_projid != fa->fsx_projid) {
> - if (XFS_IS_QUOTA_RUNNING(mp) && XFS_IS_PQUOTA_ON(mp)) {
> + if (XFS_IS_PQUOTA_ON(mp)) {
> olddquot = xfs_qm_vop_chown(tp, ip,
> &ip->i_pdquot, pdqp);
> }
> diff --git a/fs/xfs/xfs_iops.c b/fs/xfs/xfs_iops.c
> index 93c082db04b78c..327d65ef1e268c 100644
> --- a/fs/xfs/xfs_iops.c
> +++ b/fs/xfs/xfs_iops.c
> @@ -778,7 +778,7 @@ xfs_setattr_nonsize(
> * in the transaction.
> */
> if (!uid_eq(iuid, uid)) {
> - if (XFS_IS_QUOTA_RUNNING(mp) && XFS_IS_UQUOTA_ON(mp)) {
> + if (XFS_IS_UQUOTA_ON(mp)) {
> ASSERT(mask & ATTR_UID);
> ASSERT(udqp);
> olddquot1 = xfs_qm_vop_chown(tp, ip,
> @@ -787,7 +787,7 @@ xfs_setattr_nonsize(
> inode->i_uid = uid;
> }
> if (!gid_eq(igid, gid)) {
> - if (XFS_IS_QUOTA_RUNNING(mp) && XFS_IS_GQUOTA_ON(mp)) {
> + if (XFS_IS_GQUOTA_ON(mp)) {
> ASSERT(xfs_sb_version_has_pquotino(&mp->m_sb) ||
> !XFS_IS_PQUOTA_ON(mp));
> ASSERT(mask & ATTR_GID);
> diff --git a/fs/xfs/xfs_mount.c b/fs/xfs/xfs_mount.c
> index d0755494597f99..baf7b323cb15eb 100644
> --- a/fs/xfs/xfs_mount.c
> +++ b/fs/xfs/xfs_mount.c
> @@ -836,13 +836,11 @@ xfs_mountfs(
> /*
> * Initialise the XFS quota management subsystem for this mount
> */
> - if (XFS_IS_QUOTA_RUNNING(mp)) {
> + if (XFS_IS_QUOTA_ON(mp)) {
> error = xfs_qm_newmount(mp, "amount, "aflags);
> if (error)
> goto out_rtunmount;
> } else {
> - ASSERT(!XFS_IS_QUOTA_ON(mp));
> -
> /*
> * If a file system had quotas running earlier, but decided to
> * mount without -o uquota/pquota/gquota options, revoke the
> diff --git a/fs/xfs/xfs_qm.c b/fs/xfs/xfs_qm.c
> index 2b34273d0405e7..351d99bc52e5f8 100644
> --- a/fs/xfs/xfs_qm.c
> +++ b/fs/xfs/xfs_qm.c
> @@ -295,8 +295,6 @@ xfs_qm_need_dqattach(
> {
> struct xfs_mount *mp = ip->i_mount;
>
> - if (!XFS_IS_QUOTA_RUNNING(mp))
> - return false;
> if (!XFS_IS_QUOTA_ON(mp))
> return false;
> if (!XFS_NOT_DQATTACHED(mp, ip))
> @@ -631,7 +629,7 @@ xfs_qm_init_quotainfo(
> struct xfs_quotainfo *qinf;
> int error;
>
> - ASSERT(XFS_IS_QUOTA_RUNNING(mp));
> + ASSERT(XFS_IS_QUOTA_ON(mp));
>
> qinf = mp->m_quotainfo = kmem_zalloc(sizeof(struct xfs_quotainfo), 0);
>
> @@ -676,11 +674,11 @@ xfs_qm_init_quotainfo(
> xfs_qm_init_timelimits(mp, XFS_DQTYPE_GROUP);
> xfs_qm_init_timelimits(mp, XFS_DQTYPE_PROJ);
>
> - if (XFS_IS_UQUOTA_RUNNING(mp))
> + if (XFS_IS_UQUOTA_ON(mp))
> xfs_qm_set_defquota(mp, XFS_DQTYPE_USER, qinf);
> - if (XFS_IS_GQUOTA_RUNNING(mp))
> + if (XFS_IS_GQUOTA_ON(mp))
> xfs_qm_set_defquota(mp, XFS_DQTYPE_GROUP, qinf);
> - if (XFS_IS_PQUOTA_RUNNING(mp))
> + if (XFS_IS_PQUOTA_ON(mp))
> xfs_qm_set_defquota(mp, XFS_DQTYPE_PROJ, qinf);
>
> qinf->qi_shrinker.count_objects = xfs_qm_shrink_count;
> @@ -1143,7 +1141,7 @@ xfs_qm_dqusage_adjust(
> xfs_filblks_t rtblks = 0; /* total rt blks */
> int error;
>
> - ASSERT(XFS_IS_QUOTA_RUNNING(mp));
> + ASSERT(XFS_IS_QUOTA_ON(mp));
>
> /*
> * rootino must have its resources accounted for, not so with the quota
> @@ -1284,7 +1282,7 @@ xfs_qm_quotacheck(
> flags = 0;
>
> ASSERT(uip || gip || pip);
> - ASSERT(XFS_IS_QUOTA_RUNNING(mp));
> + ASSERT(XFS_IS_QUOTA_ON(mp));
>
> xfs_notice(mp, "Quotacheck needed: Please wait.");
>
> @@ -1414,7 +1412,7 @@ xfs_qm_mount_quotas(
> goto write_changes;
> }
>
> - ASSERT(XFS_IS_QUOTA_RUNNING(mp));
> + ASSERT(XFS_IS_QUOTA_ON(mp));
>
> /*
> * Allocate the quotainfo structure inside the mount struct, and
> @@ -1469,7 +1467,7 @@ xfs_qm_mount_quotas(
> * the incore structures are convinced that quotas are
> * off, but the on disk superblock doesn't know that !
> */
> - ASSERT(!(XFS_IS_QUOTA_RUNNING(mp)));
> + ASSERT(!(XFS_IS_QUOTA_ON(mp)));
> xfs_alert(mp, "%s: Superblock update failed!",
> __func__);
> }
> @@ -1641,7 +1639,7 @@ xfs_qm_vop_dqalloc(
> int error;
> uint lockflags;
>
> - if (!XFS_IS_QUOTA_RUNNING(mp) || !XFS_IS_QUOTA_ON(mp))
> + if (!XFS_IS_QUOTA_ON(mp))
> return 0;
>
> lockflags = XFS_ILOCK_EXCL;
> @@ -1772,7 +1770,7 @@ xfs_qm_vop_chown(
>
>
> ASSERT(xfs_isilocked(ip, XFS_ILOCK_EXCL));
> - ASSERT(XFS_IS_QUOTA_RUNNING(ip->i_mount));
> + ASSERT(XFS_IS_QUOTA_ON(ip->i_mount));
>
> /* old dquot */
> prevdq = *IO_olddq;
> @@ -1825,7 +1823,7 @@ xfs_qm_vop_rename_dqattach(
> struct xfs_mount *mp = i_tab[0]->i_mount;
> int i;
>
> - if (!XFS_IS_QUOTA_RUNNING(mp) || !XFS_IS_QUOTA_ON(mp))
> + if (!XFS_IS_QUOTA_ON(mp))
> return 0;
>
> for (i = 0; (i < 4 && i_tab[i]); i++) {
> @@ -1856,7 +1854,7 @@ xfs_qm_vop_create_dqattach(
> {
> struct xfs_mount *mp = tp->t_mountp;
>
> - if (!XFS_IS_QUOTA_RUNNING(mp) || !XFS_IS_QUOTA_ON(mp))
> + if (!XFS_IS_QUOTA_ON(mp))
> return;
>
> ASSERT(xfs_isilocked(ip, XFS_ILOCK_EXCL));
> diff --git a/fs/xfs/xfs_qm_syscalls.c b/fs/xfs/xfs_qm_syscalls.c
> index e00067a142a094..b5685ce666f70d 100644
> --- a/fs/xfs/xfs_qm_syscalls.c
> +++ b/fs/xfs/xfs_qm_syscalls.c
> @@ -204,7 +204,7 @@ xfs_qm_scall_quotaon(
> (mp->m_qflags & XFS_GQUOTA_ACCT)))
> return 0;
>
> - if (! XFS_IS_QUOTA_RUNNING(mp))
> + if (!XFS_IS_QUOTA_ON(mp))
> return -ESRCH;
>
> /*
> diff --git a/fs/xfs/xfs_quotaops.c b/fs/xfs/xfs_quotaops.c
> index 88d70c236a5445..07989bd677289a 100644
> --- a/fs/xfs/xfs_quotaops.c
> +++ b/fs/xfs/xfs_quotaops.c
> @@ -60,18 +60,18 @@ xfs_fs_get_quota_state(
> struct xfs_quotainfo *q = mp->m_quotainfo;
>
> memset(state, 0, sizeof(*state));
> - if (!XFS_IS_QUOTA_RUNNING(mp))
> + if (!XFS_IS_QUOTA_ON(mp))
> return 0;
> state->s_incoredqs = q->qi_dquots;
> - if (XFS_IS_UQUOTA_RUNNING(mp))
> + if (XFS_IS_UQUOTA_ON(mp))
> state->s_state[USRQUOTA].flags |= QCI_ACCT_ENABLED;
> if (XFS_IS_UQUOTA_ENFORCED(mp))
> state->s_state[USRQUOTA].flags |= QCI_LIMITS_ENFORCED;
> - if (XFS_IS_GQUOTA_RUNNING(mp))
> + if (XFS_IS_GQUOTA_ON(mp))
> state->s_state[GRPQUOTA].flags |= QCI_ACCT_ENABLED;
> if (XFS_IS_GQUOTA_ENFORCED(mp))
> state->s_state[GRPQUOTA].flags |= QCI_LIMITS_ENFORCED;
> - if (XFS_IS_PQUOTA_RUNNING(mp))
> + if (XFS_IS_PQUOTA_ON(mp))
> state->s_state[PRJQUOTA].flags |= QCI_ACCT_ENABLED;
> if (XFS_IS_PQUOTA_ENFORCED(mp))
> state->s_state[PRJQUOTA].flags |= QCI_LIMITS_ENFORCED;
> @@ -114,10 +114,8 @@ xfs_fs_set_info(
>
> if (sb_rdonly(sb))
> return -EROFS;
> - if (!XFS_IS_QUOTA_RUNNING(mp))
> - return -ENOSYS;
> if (!XFS_IS_QUOTA_ON(mp))
> - return -ESRCH;
> + return -ENOSYS;
> if (info->i_fieldmask & ~XFS_QC_SETINFO_MASK)
> return -EINVAL;
> if ((info->i_fieldmask & XFS_QC_SETINFO_MASK) == 0)
> @@ -164,7 +162,7 @@ xfs_quota_enable(
>
> if (sb_rdonly(sb))
> return -EROFS;
> - if (!XFS_IS_QUOTA_RUNNING(mp))
> + if (!XFS_IS_QUOTA_ON(mp))
> return -ENOSYS;
>
> return xfs_qm_scall_quotaon(mp, xfs_quota_flags(uflags));
> @@ -179,10 +177,8 @@ xfs_quota_disable(
>
> if (sb_rdonly(sb))
> return -EROFS;
> - if (!XFS_IS_QUOTA_RUNNING(mp))
> - return -ENOSYS;
> if (!XFS_IS_QUOTA_ON(mp))
> - return -EINVAL;
> + return -ENOSYS;
>
> return xfs_qm_scall_quotaoff(mp, xfs_quota_flags(uflags));
> }
> @@ -223,10 +219,8 @@ xfs_fs_get_dqblk(
> struct xfs_mount *mp = XFS_M(sb);
> xfs_dqid_t id;
>
> - if (!XFS_IS_QUOTA_RUNNING(mp))
> - return -ENOSYS;
> if (!XFS_IS_QUOTA_ON(mp))
> - return -ESRCH;
> + return -ENOSYS;
>
> id = from_kqid(&init_user_ns, qid);
> return xfs_qm_scall_getquota(mp, id, xfs_quota_type(qid.type), qdq);
> @@ -243,10 +237,8 @@ xfs_fs_get_nextdqblk(
> struct xfs_mount *mp = XFS_M(sb);
> xfs_dqid_t id;
>
> - if (!XFS_IS_QUOTA_RUNNING(mp))
> - return -ENOSYS;
> if (!XFS_IS_QUOTA_ON(mp))
> - return -ESRCH;
> + return -ENOSYS;
>
> id = from_kqid(&init_user_ns, *qid);
> ret = xfs_qm_scall_getquota_next(mp, &id, xfs_quota_type(qid->type),
> @@ -269,10 +261,8 @@ xfs_fs_set_dqblk(
>
> if (sb_rdonly(sb))
> return -EROFS;
> - if (!XFS_IS_QUOTA_RUNNING(mp))
> - return -ENOSYS;
> if (!XFS_IS_QUOTA_ON(mp))
> - return -ESRCH;
> + return -ENOSYS;
>
> return xfs_qm_scall_setqlim(mp, from_kqid(&init_user_ns, qid),
> xfs_quota_type(qid.type), qdq);
> diff --git a/fs/xfs/xfs_super.c b/fs/xfs/xfs_super.c
> index 2c9e26a44546b8..36fc81e52dc22a 100644
> --- a/fs/xfs/xfs_super.c
> +++ b/fs/xfs/xfs_super.c
> @@ -201,25 +201,20 @@ xfs_fs_show_options(
> seq_printf(m, ",swidth=%d",
> (int)XFS_FSB_TO_BB(mp, mp->m_swidth));
>
> - if (mp->m_qflags & XFS_UQUOTA_ACCT) {
> - if (mp->m_qflags & XFS_UQUOTA_ENFD)
> - seq_puts(m, ",usrquota");
> - else
> - seq_puts(m, ",uqnoenforce");
> - }
> + if (mp->m_qflags & XFS_UQUOTA_ENFD)
> + seq_puts(m, ",usrquota");
> + else if (mp->m_qflags & XFS_UQUOTA_ACCT)
> + seq_puts(m, ",uqnoenforce");
>
> - if (mp->m_qflags & XFS_PQUOTA_ACCT) {
> - if (mp->m_qflags & XFS_PQUOTA_ENFD)
> - seq_puts(m, ",prjquota");
> - else
> - seq_puts(m, ",pqnoenforce");
> - }
> - if (mp->m_qflags & XFS_GQUOTA_ACCT) {
> - if (mp->m_qflags & XFS_GQUOTA_ENFD)
> - seq_puts(m, ",grpquota");
> - else
> - seq_puts(m, ",gqnoenforce");
> - }
> + if (mp->m_qflags & XFS_PQUOTA_ENFD)
> + seq_puts(m, ",prjquota");
> + else if (mp->m_qflags & XFS_PQUOTA_ACCT)
> + seq_puts(m, ",pqnoenforce");
> +
> + if (mp->m_qflags & XFS_GQUOTA_ENFD)
> + seq_puts(m, ",grpquota");
> + else if (mp->m_qflags & XFS_GQUOTA_ACCT)
> + seq_puts(m, ",gqnoenforce");
>
> if (!(mp->m_qflags & XFS_ALL_QUOTA_ACCT))
> seq_puts(m, ",noquota");
> @@ -962,8 +957,8 @@ xfs_finish_flags(
> return -EROFS;
> }
>
> - if ((mp->m_qflags & (XFS_GQUOTA_ACCT | XFS_GQUOTA_ACTIVE)) &&
> - (mp->m_qflags & (XFS_PQUOTA_ACCT | XFS_PQUOTA_ACTIVE)) &&
> + if ((mp->m_qflags & XFS_GQUOTA_ACCT) &&
> + (mp->m_qflags & XFS_PQUOTA_ACCT) &&
> !xfs_sb_version_has_pquotino(&mp->m_sb)) {
> xfs_warn(mp,
> "Super block does not support project and group quota together");
> @@ -1228,35 +1223,31 @@ xfs_fs_parse_param(
> case Opt_noquota:
> parsing_mp->m_qflags &= ~XFS_ALL_QUOTA_ACCT;
> parsing_mp->m_qflags &= ~XFS_ALL_QUOTA_ENFD;
> - parsing_mp->m_qflags &= ~XFS_ALL_QUOTA_ACTIVE;
> return 0;
> case Opt_quota:
> case Opt_uquota:
> case Opt_usrquota:
> - parsing_mp->m_qflags |= (XFS_UQUOTA_ACCT | XFS_UQUOTA_ACTIVE |
> - XFS_UQUOTA_ENFD);
> + parsing_mp->m_qflags |= (XFS_UQUOTA_ACCT | XFS_UQUOTA_ENFD);
> return 0;
> case Opt_qnoenforce:
> case Opt_uqnoenforce:
> - parsing_mp->m_qflags |= (XFS_UQUOTA_ACCT | XFS_UQUOTA_ACTIVE);
> + parsing_mp->m_qflags |= XFS_UQUOTA_ACCT;
> parsing_mp->m_qflags &= ~XFS_UQUOTA_ENFD;
> return 0;
> case Opt_pquota:
> case Opt_prjquota:
> - parsing_mp->m_qflags |= (XFS_PQUOTA_ACCT | XFS_PQUOTA_ACTIVE |
> - XFS_PQUOTA_ENFD);
> + parsing_mp->m_qflags |= (XFS_PQUOTA_ACCT | XFS_PQUOTA_ENFD);
> return 0;
> case Opt_pqnoenforce:
> - parsing_mp->m_qflags |= (XFS_PQUOTA_ACCT | XFS_PQUOTA_ACTIVE);
> + parsing_mp->m_qflags |= XFS_PQUOTA_ACCT;
> parsing_mp->m_qflags &= ~XFS_PQUOTA_ENFD;
> return 0;
> case Opt_gquota:
> case Opt_grpquota:
> - parsing_mp->m_qflags |= (XFS_GQUOTA_ACCT | XFS_GQUOTA_ACTIVE |
> - XFS_GQUOTA_ENFD);
> + parsing_mp->m_qflags |= (XFS_GQUOTA_ACCT | XFS_GQUOTA_ENFD);
> return 0;
> case Opt_gqnoenforce:
> - parsing_mp->m_qflags |= (XFS_GQUOTA_ACCT | XFS_GQUOTA_ACTIVE);
> + parsing_mp->m_qflags |= XFS_GQUOTA_ACCT;
> parsing_mp->m_qflags &= ~XFS_GQUOTA_ENFD;
> return 0;
> case Opt_discard:
> diff --git a/fs/xfs/xfs_trans_dquot.c b/fs/xfs/xfs_trans_dquot.c
> index b7e4b05a559bdb..eb76bc5bed9d0a 100644
> --- a/fs/xfs/xfs_trans_dquot.c
> +++ b/fs/xfs/xfs_trans_dquot.c
> @@ -132,8 +132,7 @@ xfs_trans_mod_dquot_byino(
> {
> xfs_mount_t *mp = tp->t_mountp;
>
> - if (!XFS_IS_QUOTA_RUNNING(mp) ||
> - !XFS_IS_QUOTA_ON(mp) ||
> + if (!XFS_IS_QUOTA_ON(mp) ||
> xfs_is_quota_inode(&mp->m_sb, ip->i_ino))
> return;
>
> @@ -192,7 +191,7 @@ xfs_trans_mod_dquot(
> struct xfs_dqtrx *qtrx;
>
> ASSERT(tp);
> - ASSERT(XFS_IS_QUOTA_RUNNING(tp->t_mountp));
> + ASSERT(XFS_IS_QUOTA_ON(tp->t_mountp));
> qtrx = NULL;
>
> if (!delta)
> @@ -738,7 +737,7 @@ xfs_trans_reserve_quota_bydquots(
> {
> int error;
>
> - if (!XFS_IS_QUOTA_RUNNING(mp) || !XFS_IS_QUOTA_ON(mp))
> + if (!XFS_IS_QUOTA_ON(mp))
> return 0;
>
> ASSERT(flags & XFS_QMOPT_RESBLK_MASK);
> @@ -795,7 +794,7 @@ xfs_trans_reserve_quota_nblks(
> unsigned int qflags = 0;
> int error;
>
> - if (!XFS_IS_QUOTA_RUNNING(mp) || !XFS_IS_QUOTA_ON(mp))
> + if (!XFS_IS_QUOTA_ON(mp))
> return 0;
>
> ASSERT(!xfs_is_quota_inode(&mp->m_sb, ip->i_ino));
> @@ -836,7 +835,7 @@ xfs_trans_reserve_quota_icreate(
> {
> struct xfs_mount *mp = tp->t_mountp;
>
> - if (!XFS_IS_QUOTA_RUNNING(mp) || !XFS_IS_QUOTA_ON(mp))
> + if (!XFS_IS_QUOTA_ON(mp))
> return 0;
>
> return xfs_trans_reserve_quota_bydquots(tp, mp, udqp, gdqp, pdqp,
> --
> 2.30.2
>
^ permalink raw reply [flat|nested] 14+ messages in thread