On Wed, Jul 15, 2020 at 07:39:48PM +0100, Christoph Hellwig wrote: > On Wed, Jul 15, 2020 at 11:38:38AM -0700, Darrick J. Wong wrote: > > On Wed, Jul 15, 2020 at 06:43:40PM +0100, Christoph Hellwig wrote: > > > On Tue, Jul 14, 2020 at 11:05:02AM -0700, Darrick J. Wong wrote: > > > > On Tue, Jul 14, 2020 at 08:57:56AM +0100, Christoph Hellwig wrote: > > > > > On Mon, Jul 13, 2020 at 06:32:00PM -0700, Darrick J. Wong wrote: > > > > > > From: Darrick J. Wong <darrick.wong@xxxxxxxxxx> > > > > > > > > > > > > Create a new type (xfs_dqtype_t) to represent the type of an incore > > > > > > dquot. Break the type field out from the dq_flags field of the incore > > > > > > dquot. > > > > > > > > > > I don't understand why we need separate in-core vs on-disk values for > > > > > the type. Why not something like this on top of the whole series: > > > > > > > > I want to keep the ondisk d_type values separate from the incore q_type > > > > values because they don't describe exactly the same concepts: > > > > > > > > First, the incore qtype has a NONE value that we can pass to the dquot > > > > core verifier when we don't actually know if this is a user, group, or > > > > project dquot. This should never end up on disk. > > > > > > Which we can trivially verify. Or just get rid of NONE, which actually > > > cleans things up a fair bit (patch on top of my previous one below) > > > > Ok, I'll get rid of that usage. > > > > > > Second, xfs_dqtype_t is a (barely concealed) enumeration type for quota > > > > callers to tell us that they want to perform an action on behalf of > > > > user, group, or project quotas. The incore q_flags and the ondisk > > > > d_type contain internal state that should not be exposed to quota > > > > callers. > > > > > > I don't think that is an argument, as we do the same elsewhere. > > > > > > > > > > > I feel a need to reiterate that I'm about to start adding more flags to > > > > d_type (for y2038+ time support), for which it will be very important to > > > > keep d_type and q_{type,flags} separate. > > > > > > Why? We'll just OR the bigtime flag in before writing to disk. > > > > Ugh, fine, I'll rework the whole series yet again, since it doesn't look > > like anyone else is going to have the time to review a 27 patch cleanup > > series. > > Let's just get your series in and I'll send an incremental patch > after the bigtime series.. Seeing as I already refactored yesterday to produce v4 and am now midway through refactoring to produce v5 I just going to keep going with even more ******* cleanups. --D