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@xxxxxx> > --- > 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 >