On Wed, Jul 01, 2020 at 04:50:31PM -0700, Darrick J. Wong wrote:
> On Thu, Jul 02, 2020 at 09:44:35AM +1000, Dave Chinner wrote:
> > On Wed, Jul 01, 2020 at 04:19:10PM -0700, Darrick J. Wong wrote:
> > > On Thu, Jul 02, 2020 at 08:50:53AM +1000, Dave Chinner wrote:
> > > > On Tue, Jun 30, 2020 at 08:42:16AM -0700, Darrick J. Wong wrote:
> > > > > From: Darrick J. Wong <darrick.wong@xxxxxxxxxx>
> > > > >
> > > > > Use the incore dq_flags to figure out the dquot type. This is the first
> > > > > step towards removing xfs_disk_dquot from the incore dquot.
> > > > >
> > > > > Signed-off-by: Darrick J. Wong <darrick.wong@xxxxxxxxxx>
> > > > > ---
> > > > > fs/xfs/libxfs/xfs_quota_defs.h | 2 ++
> > > > > fs/xfs/scrub/quota.c | 4 ----
> > > > > fs/xfs/xfs_dquot.c | 33 +++++++++++++++++++++++++++++++--
> > > > > fs/xfs/xfs_dquot.h | 2 ++
> > > > > fs/xfs/xfs_dquot_item.c | 6 ++++--
> > > > > fs/xfs/xfs_qm.c | 4 ++--
> > > > > fs/xfs/xfs_qm.h | 2 +-
> > > > > fs/xfs/xfs_qm_syscalls.c | 9 +++------
> > > > > 8 files changed, 45 insertions(+), 17 deletions(-)
> > > > >
> > > > >
> > > > > diff --git a/fs/xfs/libxfs/xfs_quota_defs.h b/fs/xfs/libxfs/xfs_quota_defs.h
> > > > > index 56d9dd787e7b..459023b0a304 100644
> > > > > --- a/fs/xfs/libxfs/xfs_quota_defs.h
> > > > > +++ b/fs/xfs/libxfs/xfs_quota_defs.h
> > > > > @@ -29,6 +29,8 @@ typedef uint16_t xfs_qwarncnt_t;
> > > > >
> > > > > #define XFS_DQ_ALLTYPES (XFS_DQ_USER|XFS_DQ_PROJ|XFS_DQ_GROUP)
> > > > >
> > > > > +#define XFS_DQ_ONDISK (XFS_DQ_ALLTYPES)
> > > >
> > > > That's used as an on-disk flags mask. Perhaps XFS_DQF_ONDISK_MASK?
> > >
> > > Well, based on Christoph's suggestions I broke the incore dquot flags
> > > (XFS_DQ_*) apart from the ondisk dquot flags (XFS_DQFLAG_*). Not sure
> > > if that's really better, but at least the namespaces are separate now.
> >
> > Sure, but the point I was trying to make is that "XFS_DQ_ONDISK"
> > doesn't actually indicate what part of the on-disk dquot it refers
> > to. We use the phrase "on-disk dquot" to refer to the entire on-disk
> > dquot, not a subset of flags in a flags field in the on-disk
> > dquot. Hence the name of this variable needs to be more specific as
> > to what it applies to in the on-disk dquot...
>
> Sorry, I was typing too fast. xfs_format.h now has:
>
> #define XFS_DQFLAG_USER 0x01 /* user dquot record */
> #define XFS_DQFLAG_PROJ 0x02 /* project dquot record */
> #define XFS_DQFLAG_GROUP 0x04 /* group dquot record */
>
> #define XFS_DQFLAG_TYPE_MASK (XFS_DQFLAG_USER | \
> XFS_DQFLAG_PROJ | \
> XFS_DQFLAG_GROUP)
>
> #define XFS_DQFLAG_ALL (XFS_DQFLAG_TYPE_MASK)
>
> /*
> * This is the main portion of the on-disk representation of quota
> * information for a user. This is the q_core of the struct xfs_dquot
> * that is kept in kernel memory. We pad this with some more expansion
> * room to construct the on disk structure.
> */
> struct xfs_disk_dquot {
> __be16 d_magic; /* dquot magic = XFS_DQUOT_MAGIC */
> __u8 d_version; /* dquot version */
> __u8 d_flags; /* XFS_DQFLAG_* */
>
> I'm not particularly thrilled about the DQFLAG/DQ thing though. DDFLAG?
> (Also note that the future bigtime series will add a new ondisk flag
> XFS_DQFLAG_BIGTIME, which ofc will get added to XFS_DQFLAG_ALL.)
/me shrugs
I don't have any good ideas, but I think that DQFLAG is a bad choice
for an on-disk flag namespace because it's way too generic. It's
more a type/feature indicator so perhaps we should rename d_flags to
d_type or d_features and use:
#define XFS_DDQTYPE_USER
#define XFS_DDQTYPE_PROJ
#define XFS_DDQTYPE_GROUP
#define XFS_DDQTYPE_BIGTIME
#define XFS_DDQTYPE_QUOTA_MASK (XFS_DDQTYPE_USER | ...
#define XFS_DDQTYPE_ALL (XFS_DDQTYPE_QUOTA_MASK | XFS_DDQTYPE_BIGTIME)
Or something like that...
Cheers,
Dave.
--
Dave Chinner
david@xxxxxxxxxxxxx
