On Fri, Jul 03, 2020 at 10:58:50AM +1000, Dave Chinner wrote:
> 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...
I prefer d_features/DDQFEAT over d_type/DDQTYPE because I don't want
people to mix up "ondisk dquot type (user/group/proj/bigtime)" with
"quota type (user/group/proj)".
--D
> Cheers,
>
> Dave.
> --
> Dave Chinner
> david@xxxxxxxxxxxxx
