Re: [PATCH v3] quota: widen timestamps for the fs_disk_quota structure

[Jan Kara <jack@suse.cz>]

On Tue 08-09-20 19:29:09, Darrick J. Wong wrote:
> On Wed, Sep 09, 2020 at 02:49:33AM +0100, Matthew Wilcox wrote:
> > On Tue, Sep 08, 2020 at 06:32:51PM -0700, Darrick J. Wong wrote:
> > > +static inline void copy_to_xfs_dqblk_ts(const struct fs_disk_quota *d,
> > > +		__s32 *timer_lo, __s8 *timer_hi, s64 timer)
> > > +{
> > > +	*timer_lo = timer;
> > > +	if (d->d_fieldmask & FS_DQ_BIGTIME)
> > > +		*timer_hi = timer >> 32;
> > > +	else
> > > +		*timer_hi = 0;
> > > +}
> > 
> > I'm still confused by this.  What breaks if you just do:
> > 
> > 	*timer_lo = timer;
> > 	*timer_hi = timer >> 32;
> 
> "I don't know."
> 
> The manpage for quotactl doesn't actually specify the behavior of the
> padding fields.  The /implementation/ is careful enough to zero
> everything, but the interface specification doesn't explicitly require
> software to do so.
> 
> Because the contents of the padding fields aren't defined by the
> documentation, the kernel cannot simply start using the d_padding2 field
> because there could be an old kernel that doesn't zero the padding,
> which would lead to confusion if the new userspace were mated to such a
> kernel.
> 
> Therefore, we have to add a flag that states explicitly that we are
> using the timer_hi fields.  This is also the only way that an old
> program can detect that it's being fed a structure that it might not
> recognise.

Well, this is in the direction from kernel to userspace and what Matthew
suggests would just make kernel posssibly store non-zero value in *timer_hi
without setting FS_DQ_BIGTIME flag (for negative values of timer). I don't
think it would break anything but I agree the complication isn't big so
let's be careful and only set *timer_hi to non-zero if FS_DQ_BIGTIME is
set.

								Honza
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR