| Message ID | 20191213205417.3871055-12-arnd@arndb.de (mailing list archive) |
|---|---|
| State | Superseded, archived |
| Headers | show |
| Series | drivers, fs: y2038 updates | expand |
On Fri, Dec 13, 2019 at 09:53:49PM +0100, Arnd Bergmann wrote: > As a preparation for removing the 32-bit time_t type and > all associated interfaces, change xfs to use time64_t and > ktime_get_real_seconds() for the quota housekeeping. > > Signed-off-by: Arnd Bergmann <arnd@arndb.de> Looks mostly reasonable to me... The bigtime series refactors the triplicated timer handling and whatnot, but I don't think it would be difficult to rebase that series assuming this lands first (which it probably will, I expect a new incompat ondisk feature to take a /long/ time to get through review.) > --- > fs/xfs/xfs_dquot.c | 6 +++--- > fs/xfs/xfs_qm.h | 6 +++--- > fs/xfs/xfs_quotaops.c | 6 +++--- > fs/xfs/xfs_trans_dquot.c | 8 +++++--- > 4 files changed, 14 insertions(+), 12 deletions(-) > > diff --git a/fs/xfs/xfs_dquot.c b/fs/xfs/xfs_dquot.c > index 2bff21ca9d78..9cfd3209f52b 100644 > --- a/fs/xfs/xfs_dquot.c > +++ b/fs/xfs/xfs_dquot.c > @@ -137,7 +137,7 @@ xfs_qm_adjust_dqtimers( > (d->d_blk_hardlimit && > (be64_to_cpu(d->d_bcount) > > be64_to_cpu(d->d_blk_hardlimit)))) { > - d->d_btimer = cpu_to_be32(get_seconds() + > + d->d_btimer = cpu_to_be32(ktime_get_real_seconds() + > mp->m_quotainfo->qi_btimelimit); > } else { > d->d_bwarns = 0; > @@ -160,7 +160,7 @@ xfs_qm_adjust_dqtimers( > (d->d_ino_hardlimit && > (be64_to_cpu(d->d_icount) > > be64_to_cpu(d->d_ino_hardlimit)))) { > - d->d_itimer = cpu_to_be32(get_seconds() + > + d->d_itimer = cpu_to_be32(ktime_get_real_seconds() + > mp->m_quotainfo->qi_itimelimit); > } else { > d->d_iwarns = 0; > @@ -183,7 +183,7 @@ xfs_qm_adjust_dqtimers( > (d->d_rtb_hardlimit && > (be64_to_cpu(d->d_rtbcount) > > be64_to_cpu(d->d_rtb_hardlimit)))) { > - d->d_rtbtimer = cpu_to_be32(get_seconds() + > + d->d_rtbtimer = cpu_to_be32(ktime_get_real_seconds() + > mp->m_quotainfo->qi_rtbtimelimit); Hmm, so one thing that I clean up on the way to bigtime is the total lack of clamping here. If (for example) it's September 2105 and rtbtimelimit is set to 1 year, this will cause an integer overflow. The quota timer will be set to 1970 and expire immediately, rather than what I'd consider the best effort of February 2106. (I'll grant you the current code also behaves like this...) Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com> --D > } else { > d->d_rtbwarns = 0; > diff --git a/fs/xfs/xfs_qm.h b/fs/xfs/xfs_qm.h > index 7823af39008b..4e57edca8bce 100644 > --- a/fs/xfs/xfs_qm.h > +++ b/fs/xfs/xfs_qm.h > @@ -64,9 +64,9 @@ struct xfs_quotainfo { > struct xfs_inode *qi_pquotaip; /* project quota inode */ > struct list_lru qi_lru; > int qi_dquots; > - time_t qi_btimelimit; /* limit for blks timer */ > - time_t qi_itimelimit; /* limit for inodes timer */ > - time_t qi_rtbtimelimit;/* limit for rt blks timer */ > + time64_t qi_btimelimit; /* limit for blks timer */ > + time64_t qi_itimelimit; /* limit for inodes timer */ > + time64_t qi_rtbtimelimit;/* limit for rt blks timer */ > xfs_qwarncnt_t qi_bwarnlimit; /* limit for blks warnings */ > xfs_qwarncnt_t qi_iwarnlimit; /* limit for inodes warnings */ > xfs_qwarncnt_t qi_rtbwarnlimit;/* limit for rt blks warnings */ > diff --git a/fs/xfs/xfs_quotaops.c b/fs/xfs/xfs_quotaops.c > index c7de17deeae6..38669e827206 100644 > --- a/fs/xfs/xfs_quotaops.c > +++ b/fs/xfs/xfs_quotaops.c > @@ -37,9 +37,9 @@ xfs_qm_fill_state( > tstate->flags |= QCI_SYSFILE; > tstate->blocks = ip->i_d.di_nblocks; > tstate->nextents = ip->i_d.di_nextents; > - tstate->spc_timelimit = q->qi_btimelimit; > - tstate->ino_timelimit = q->qi_itimelimit; > - tstate->rt_spc_timelimit = q->qi_rtbtimelimit; > + tstate->spc_timelimit = (u32)q->qi_btimelimit; > + tstate->ino_timelimit = (u32)q->qi_itimelimit; > + tstate->rt_spc_timelimit = (u32)q->qi_rtbtimelimit; > tstate->spc_warnlimit = q->qi_bwarnlimit; > tstate->ino_warnlimit = q->qi_iwarnlimit; > tstate->rt_spc_warnlimit = q->qi_rtbwarnlimit; > diff --git a/fs/xfs/xfs_trans_dquot.c b/fs/xfs/xfs_trans_dquot.c > index a6fe2d8dc40f..d1b9869bc5fa 100644 > --- a/fs/xfs/xfs_trans_dquot.c > +++ b/fs/xfs/xfs_trans_dquot.c > @@ -580,7 +580,7 @@ xfs_trans_dqresv( > { > xfs_qcnt_t hardlimit; > xfs_qcnt_t softlimit; > - time_t timer; > + time64_t timer; > xfs_qwarncnt_t warns; > xfs_qwarncnt_t warnlimit; > xfs_qcnt_t total_count; > @@ -635,7 +635,8 @@ xfs_trans_dqresv( > goto error_return; > } > if (softlimit && total_count > softlimit) { > - if ((timer != 0 && get_seconds() > timer) || > + if ((timer != 0 && > + ktime_get_real_seconds() > timer) || > (warns != 0 && warns >= warnlimit)) { > xfs_quota_warn(mp, dqp, > QUOTA_NL_BSOFTLONGWARN); > @@ -662,7 +663,8 @@ xfs_trans_dqresv( > goto error_return; > } > if (softlimit && total_count > softlimit) { > - if ((timer != 0 && get_seconds() > timer) || > + if ((timer != 0 && > + ktime_get_real_seconds() > timer) || > (warns != 0 && warns >= warnlimit)) { > xfs_quota_warn(mp, dqp, > QUOTA_NL_ISOFTLONGWARN); > -- > 2.20.0 >
On Fri, Dec 13, 2019 at 10:17 PM Darrick J. Wong <darrick.wong@oracle.com> wrote: > > On Fri, Dec 13, 2019 at 09:53:49PM +0100, Arnd Bergmann wrote: > > As a preparation for removing the 32-bit time_t type and > > all associated interfaces, change xfs to use time64_t and > > ktime_get_real_seconds() for the quota housekeeping. > > > > Signed-off-by: Arnd Bergmann <arnd@arndb.de> > > Looks mostly reasonable to me... > > The bigtime series refactors the triplicated timer handling and whatnot, > but I don't think it would be difficult to rebase that series assuming > this lands first (which it probably will, I expect a new incompat ondisk > feature to take a /long/ time to get through review.) Could you just merge my three patches into your tree then once you are happy with all the changes? > > @@ -183,7 +183,7 @@ xfs_qm_adjust_dqtimers( > > (d->d_rtb_hardlimit && > > (be64_to_cpu(d->d_rtbcount) > > > be64_to_cpu(d->d_rtb_hardlimit)))) { > > - d->d_rtbtimer = cpu_to_be32(get_seconds() + > > + d->d_rtbtimer = cpu_to_be32(ktime_get_real_seconds() + > > mp->m_quotainfo->qi_rtbtimelimit); > > Hmm, so one thing that I clean up on the way to bigtime is the total > lack of clamping here. If (for example) it's September 2105 and > rtbtimelimit is set to 1 year, this will cause an integer overflow. The > quota timer will be set to 1970 and expire immediately, rather than what > I'd consider the best effort of February 2106. I don't think clamping would be good here, that just replaces one bug with another at the overflow time. If you would like to have something better before this gets extended, I could try to come up with a version that converts it to the nearest 64-bit timestamp, similar to the way that time_before32() in the kernel or the NTP protocol work. If you think it can get extended properly soon, I'd just leave the patch as it is today in order to remove the get_seconds() interface for v5.6. > (I'll grant you the current code also behaves like this...) > > Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com> Thanks, Arnd
On Mon, Dec 16, 2019 at 5:52 PM Arnd Bergmann <arnd@arndb.de> wrote: > On Fri, Dec 13, 2019 at 10:17 PM Darrick J. Wong <darrick.wong@oracle.com> wrote: >> >> Hmm, so one thing that I clean up on the way to bigtime is the total >> lack of clamping here. If (for example) it's September 2105 and >> rtbtimelimit is set to 1 year, this will cause an integer overflow. The >> quota timer will be set to 1970 and expire immediately, rather than what >> I'd consider the best effort of February 2106. One more hing to note (I will add this to the changelog text) is that on 32-bit architectures, the limit here is y2038, while on 64-bit architectures it's y2106: int xfs_trans_dqresv(...) { time_t timer; /* signed 'long' */ timer = be32_to_cpu(dqp->q_core.d_btimer); /* get_seconds() returns unsigned long */ if ((timer != 0 && get_seconds() > timer)) return -EDQUOT; } > I don't think clamping would be good here, that just replaces > one bug with another at the overflow time. If you would like to > have something better before this gets extended, I could try to > come up with a version that converts it to the nearest 64-bit > timestamp, similar to the way that time_before32() in the kernel > or the NTP protocol work. > > If you think it can get extended properly soon, I'd just leave the > patch as it is today in order to remove the get_seconds() > interface for v5.6. I've tried this now, and but this feels wrong: it adds lots of complexity for corner cases and is still fragile, e.g. when the time is wrong during boot before ntp runs. See that patch below for reference. I also see that quotatool on xfs always uses the old xfs quota interface, so it already overflows on the user space side. Fixing this properly seems to be a bigger effort than I was planning for (on an unpatched 64-bit kernel): $ sudo quotatool -b -u -t 220month /mnt/tmp -r $ rm file ; fallocate -l 11M file $ sudo quotatool -d /mnt/tmp -u arnd 1000 /mnt/tmp 11264 10240 20480 570239975 2 0 00 $ sudo quotatool -b -u -t 222month /mnt/tmp -r $ rm file ; fallocate -l 11M file $ sudo quotatool -d /mnt/tmp -u arnd 1000 /mnt/tmp 11264 10240 20480 18446744069990008316 2 0 00 Arnd diff --git a/fs/xfs/xfs_dquot.c b/fs/xfs/xfs_dquot.c index 9cfd3209f52b..6c9128bb607b 100644 --- a/fs/xfs/xfs_dquot.c +++ b/fs/xfs/xfs_dquot.c @@ -98,6 +98,23 @@ xfs_qm_adjust_dqlimits( xfs_dquot_set_prealloc_limits(dq); } +static __be32 xfs_quota_timeout32(s64 limit) +{ + time64_t now = ktime_get_real_seconds(); + u32 timeout; + + /* avoid overflows in out-of-range limits */ + if ((u64)limit > S32_MAX) + limit = S32_MAX; + timeout = now + limit; + + /* avoid timeout of zero */ + if (lower_32_bits(timeout) == 0) + return cpu_to_be32(1); + + return cpu_to_be32(lower_32_bits(timeout)); +} + /* * Check the limits and timers of a dquot and start or reset timers * if necessary. @@ -137,7 +154,7 @@ xfs_qm_adjust_dqtimers( (d->d_blk_hardlimit && (be64_to_cpu(d->d_bcount) > be64_to_cpu(d->d_blk_hardlimit)))) { - d->d_btimer = cpu_to_be32(ktime_get_real_seconds() + + d->d_btimer = xfs_quota_timeout32( mp->m_quotainfo->qi_btimelimit); } else { d->d_bwarns = 0; @@ -160,7 +177,7 @@ xfs_qm_adjust_dqtimers( (d->d_ino_hardlimit && (be64_to_cpu(d->d_icount) > be64_to_cpu(d->d_ino_hardlimit)))) { - d->d_itimer = cpu_to_be32(ktime_get_real_seconds() + + d->d_itimer = xfs_quota_timeout32( mp->m_quotainfo->qi_itimelimit); } else { d->d_iwarns = 0; @@ -183,7 +200,7 @@ xfs_qm_adjust_dqtimers( (d->d_rtb_hardlimit && (be64_to_cpu(d->d_rtbcount) > be64_to_cpu(d->d_rtb_hardlimit)))) { - d->d_rtbtimer = cpu_to_be32(ktime_get_real_seconds() + + d->d_rtbtimer = xfs_quota_timeout32( mp->m_quotainfo->qi_rtbtimelimit); } else { d->d_rtbwarns = 0; diff --git a/fs/xfs/xfs_qm_syscalls.c b/fs/xfs/xfs_qm_syscalls.c index 1ea82764bf89..2087626b4bee 100644 --- a/fs/xfs/xfs_qm_syscalls.c +++ b/fs/xfs/xfs_qm_syscalls.c @@ -601,6 +601,14 @@ xfs_qm_scall_setqlim( return error; } +/* Assume timers are within +/- 68 years of current wall clock */ +static time64_t xfs_quota_time32_to_time64(time64_t now, __be32 timer) +{ + s32 diff = be32_to_cpu(timer) - lower_32_bits(now); + + return now + diff; +} + /* Fill out the quota context. */ static void xfs_qm_scall_getquota_fill_qc( @@ -609,6 +617,8 @@ xfs_qm_scall_getquota_fill_qc( const struct xfs_dquot *dqp, struct qc_dqblk *dst) { + time64_t now = ktime_get_real_seconds(); + memset(dst, 0, sizeof(*dst)); dst->d_spc_hardlimit = XFS_FSB_TO_B(mp, be64_to_cpu(dqp->q_core.d_blk_hardlimit)); @@ -618,8 +628,8 @@ xfs_qm_scall_getquota_fill_qc( dst->d_ino_softlimit = be64_to_cpu(dqp->q_core.d_ino_softlimit); dst->d_space = XFS_FSB_TO_B(mp, dqp->q_res_bcount); dst->d_ino_count = dqp->q_res_icount; - dst->d_spc_timer = be32_to_cpu(dqp->q_core.d_btimer); - dst->d_ino_timer = be32_to_cpu(dqp->q_core.d_itimer); + dst->d_spc_timer = xfs_quota_time32_to_time64(now, dqp->q_core.d_btimer); + dst->d_ino_timer = xfs_quota_time32_to_time64(now, dqp->q_core.d_itimer); dst->d_ino_warns = be16_to_cpu(dqp->q_core.d_iwarns); dst->d_spc_warns = be16_to_cpu(dqp->q_core.d_bwarns); dst->d_rt_spc_hardlimit = @@ -627,7 +637,7 @@ xfs_qm_scall_getquota_fill_qc( dst->d_rt_spc_softlimit = XFS_FSB_TO_B(mp, be64_to_cpu(dqp->q_core.d_rtb_softlimit)); dst->d_rt_space = XFS_FSB_TO_B(mp, dqp->q_res_rtbcount); - dst->d_rt_spc_timer = be32_to_cpu(dqp->q_core.d_rtbtimer); + dst->d_rt_spc_timer = xfs_quota_time32_to_time64(now, dqp->q_core.d_rtbtimer); dst->d_rt_spc_warns = be16_to_cpu(dqp->q_core.d_rtbwarns); /* diff --git a/fs/xfs/xfs_trans_dquot.c b/fs/xfs/xfs_trans_dquot.c index d1b9869bc5fa..c75887da6546 100644 --- a/fs/xfs/xfs_trans_dquot.c +++ b/fs/xfs/xfs_trans_dquot.c @@ -636,7 +636,8 @@ xfs_trans_dqresv( } if (softlimit && total_count > softlimit) { if ((timer != 0 && - ktime_get_real_seconds() > timer) || + time_after32(ktime_get_real_seconds(), + timer)) || (warns != 0 && warns >= warnlimit)) { xfs_quota_warn(mp, dqp, QUOTA_NL_BSOFTLONGWARN); @@ -664,7 +665,8 @@ xfs_trans_dqresv( } if (softlimit && total_count > softlimit) { if ((timer != 0 && - ktime_get_real_seconds() > timer) || + time_after32(ktime_get_real_seconds(), + timer)) || (warns != 0 && warns >= warnlimit)) { xfs_quota_warn(mp, dqp, QUOTA_NL_ISOFTLONGWARN);
On Tue, Dec 17, 2019 at 04:02:47PM +0100, Arnd Bergmann wrote: > On Mon, Dec 16, 2019 at 5:52 PM Arnd Bergmann <arnd@arndb.de> wrote: > > On Fri, Dec 13, 2019 at 10:17 PM Darrick J. Wong <darrick.wong@oracle.com> wrote: > >> > >> Hmm, so one thing that I clean up on the way to bigtime is the total > >> lack of clamping here. If (for example) it's September 2105 and > >> rtbtimelimit is set to 1 year, this will cause an integer overflow. The > >> quota timer will be set to 1970 and expire immediately, rather than what > >> I'd consider the best effort of February 2106. > > One more hing to note (I will add this to the changelog text) is that on Ok, I'll look for it in the next revision you send out. By the way, would you mind cc'ing the xfs list on all 24 patches? They probably aren't directly relevant to xfs, but it does make it a lot easier for us to look at the other 21 patches and think "Oh, ok, so there isn't some core infrastructure change that we're not seeing". > 32-bit architectures, the limit here is y2038, while on 64-bit > architectures it's y2106: Yikes. I probably just need to send the bigtime series and see what you all think about the mess I created^W^W^Wway I dealt with all that. > int xfs_trans_dqresv(...) > { > time_t timer; /* signed 'long' */ > timer = be32_to_cpu(dqp->q_core.d_btimer); > /* get_seconds() returns unsigned long */ > if ((timer != 0 && get_seconds() > timer)) > return -EDQUOT; > } > > > I don't think clamping would be good here, that just replaces > > one bug with another at the overflow time. If you would like to > > have something better before this gets extended, I could try to > > come up with a version that converts it to the nearest 64-bit > > timestamp, similar to the way that time_before32() in the kernel > > or the NTP protocol work. > > > > If you think it can get extended properly soon, I'd just leave the > > patch as it is today in order to remove the get_seconds() > > interface for v5.6. > > I've tried this now, and but this feels wrong: it adds lots of complexity > for corner cases and is still fragile, e.g. when the time is wrong > during boot before ntp runs. See that patch below for reference. Yeah, that is pretty weird to glue the upper 32 bits of the timestamp onto the expiration timer... --D > I also see that quotatool on xfs always uses the old xfs quota > interface, so it already overflows on the user space side. Fixing > this properly seems to be a bigger effort than I was planning for > (on an unpatched 64-bit kernel): > > $ sudo quotatool -b -u -t 220month /mnt/tmp -r > $ rm file ; fallocate -l 11M file > $ sudo quotatool -d /mnt/tmp -u arnd > 1000 /mnt/tmp 11264 10240 20480 570239975 2 0 00 > $ sudo quotatool -b -u -t 222month /mnt/tmp -r > $ rm file ; fallocate -l 11M file > $ sudo quotatool -d /mnt/tmp -u arnd > 1000 /mnt/tmp 11264 10240 20480 18446744069990008316 2 0 00 > > Arnd > > diff --git a/fs/xfs/xfs_dquot.c b/fs/xfs/xfs_dquot.c > index 9cfd3209f52b..6c9128bb607b 100644 > --- a/fs/xfs/xfs_dquot.c > +++ b/fs/xfs/xfs_dquot.c > @@ -98,6 +98,23 @@ xfs_qm_adjust_dqlimits( > xfs_dquot_set_prealloc_limits(dq); > } > > +static __be32 xfs_quota_timeout32(s64 limit) > +{ > + time64_t now = ktime_get_real_seconds(); > + u32 timeout; > + > + /* avoid overflows in out-of-range limits */ > + if ((u64)limit > S32_MAX) > + limit = S32_MAX; > + timeout = now + limit; > + > + /* avoid timeout of zero */ > + if (lower_32_bits(timeout) == 0) > + return cpu_to_be32(1); > + > + return cpu_to_be32(lower_32_bits(timeout)); > +} > + > /* > * Check the limits and timers of a dquot and start or reset timers > * if necessary. > @@ -137,7 +154,7 @@ xfs_qm_adjust_dqtimers( > (d->d_blk_hardlimit && > (be64_to_cpu(d->d_bcount) > > be64_to_cpu(d->d_blk_hardlimit)))) { > - d->d_btimer = cpu_to_be32(ktime_get_real_seconds() + > + d->d_btimer = xfs_quota_timeout32( > mp->m_quotainfo->qi_btimelimit); > } else { > d->d_bwarns = 0; > @@ -160,7 +177,7 @@ xfs_qm_adjust_dqtimers( > (d->d_ino_hardlimit && > (be64_to_cpu(d->d_icount) > > be64_to_cpu(d->d_ino_hardlimit)))) { > - d->d_itimer = cpu_to_be32(ktime_get_real_seconds() + > + d->d_itimer = xfs_quota_timeout32( > mp->m_quotainfo->qi_itimelimit); > } else { > d->d_iwarns = 0; > @@ -183,7 +200,7 @@ xfs_qm_adjust_dqtimers( > (d->d_rtb_hardlimit && > (be64_to_cpu(d->d_rtbcount) > > be64_to_cpu(d->d_rtb_hardlimit)))) { > - d->d_rtbtimer = cpu_to_be32(ktime_get_real_seconds() + > + d->d_rtbtimer = xfs_quota_timeout32( > mp->m_quotainfo->qi_rtbtimelimit); > } else { > d->d_rtbwarns = 0; > diff --git a/fs/xfs/xfs_qm_syscalls.c b/fs/xfs/xfs_qm_syscalls.c > index 1ea82764bf89..2087626b4bee 100644 > --- a/fs/xfs/xfs_qm_syscalls.c > +++ b/fs/xfs/xfs_qm_syscalls.c > @@ -601,6 +601,14 @@ xfs_qm_scall_setqlim( > return error; > } > > +/* Assume timers are within +/- 68 years of current wall clock */ > +static time64_t xfs_quota_time32_to_time64(time64_t now, __be32 timer) > +{ > + s32 diff = be32_to_cpu(timer) - lower_32_bits(now); > + > + return now + diff; > +} > + > /* Fill out the quota context. */ > static void > xfs_qm_scall_getquota_fill_qc( > @@ -609,6 +617,8 @@ xfs_qm_scall_getquota_fill_qc( > const struct xfs_dquot *dqp, > struct qc_dqblk *dst) > { > + time64_t now = ktime_get_real_seconds(); > + > memset(dst, 0, sizeof(*dst)); > dst->d_spc_hardlimit = > XFS_FSB_TO_B(mp, be64_to_cpu(dqp->q_core.d_blk_hardlimit)); > @@ -618,8 +628,8 @@ xfs_qm_scall_getquota_fill_qc( > dst->d_ino_softlimit = be64_to_cpu(dqp->q_core.d_ino_softlimit); > dst->d_space = XFS_FSB_TO_B(mp, dqp->q_res_bcount); > dst->d_ino_count = dqp->q_res_icount; > - dst->d_spc_timer = be32_to_cpu(dqp->q_core.d_btimer); > - dst->d_ino_timer = be32_to_cpu(dqp->q_core.d_itimer); > + dst->d_spc_timer = xfs_quota_time32_to_time64(now, > dqp->q_core.d_btimer); > + dst->d_ino_timer = xfs_quota_time32_to_time64(now, > dqp->q_core.d_itimer); > dst->d_ino_warns = be16_to_cpu(dqp->q_core.d_iwarns); > dst->d_spc_warns = be16_to_cpu(dqp->q_core.d_bwarns); > dst->d_rt_spc_hardlimit = > @@ -627,7 +637,7 @@ xfs_qm_scall_getquota_fill_qc( > dst->d_rt_spc_softlimit = > XFS_FSB_TO_B(mp, be64_to_cpu(dqp->q_core.d_rtb_softlimit)); > dst->d_rt_space = XFS_FSB_TO_B(mp, dqp->q_res_rtbcount); > - dst->d_rt_spc_timer = be32_to_cpu(dqp->q_core.d_rtbtimer); > + dst->d_rt_spc_timer = xfs_quota_time32_to_time64(now, > dqp->q_core.d_rtbtimer); > dst->d_rt_spc_warns = be16_to_cpu(dqp->q_core.d_rtbwarns); > > /* > diff --git a/fs/xfs/xfs_trans_dquot.c b/fs/xfs/xfs_trans_dquot.c > index d1b9869bc5fa..c75887da6546 100644 > --- a/fs/xfs/xfs_trans_dquot.c > +++ b/fs/xfs/xfs_trans_dquot.c > @@ -636,7 +636,8 @@ xfs_trans_dqresv( > } > if (softlimit && total_count > softlimit) { > if ((timer != 0 && > - ktime_get_real_seconds() > timer) || > + time_after32(ktime_get_real_seconds(), > + timer)) || > (warns != 0 && warns >= warnlimit)) { > xfs_quota_warn(mp, dqp, > QUOTA_NL_BSOFTLONGWARN); > @@ -664,7 +665,8 @@ xfs_trans_dqresv( > } > if (softlimit && total_count > softlimit) { > if ((timer != 0 && > - ktime_get_real_seconds() > timer) || > + time_after32(ktime_get_real_seconds(), > + timer)) || > (warns != 0 && warns >= warnlimit)) { > xfs_quota_warn(mp, dqp, > QUOTA_NL_ISOFTLONGWARN);
On Tue, Dec 17, 2019 at 11:18 PM Darrick J. Wong <darrick.wong@oracle.com> wrote: > On Tue, Dec 17, 2019 at 04:02:47PM +0100, Arnd Bergmann wrote: > > On Mon, Dec 16, 2019 at 5:52 PM Arnd Bergmann <arnd@arndb.de> wrote: > > > On Fri, Dec 13, 2019 at 10:17 PM Darrick J. Wong <darrick.wong@oracle.com> wrote: > > >> > > >> Hmm, so one thing that I clean up on the way to bigtime is the total > > >> lack of clamping here. If (for example) it's September 2105 and > > >> rtbtimelimit is set to 1 year, this will cause an integer overflow. The > > >> quota timer will be set to 1970 and expire immediately, rather than what > > >> I'd consider the best effort of February 2106. > > > > One more hing to note (I will add this to the changelog text) is that on > > Ok, I'll look for it in the next revision you send out. > > By the way, would you mind cc'ing the xfs list on all 24 patches? They > probably aren't directly relevant to xfs, but it does make it a lot > easier for us to look at the other 21 patches and think "Oh, ok, so > there isn't some core infrastructure change that we're not seeing". I wasn't planning on sending the full series once more, as there were very few comments now. I've sent the three XFS patches again by themselves now. If you can pick these up, I'll put the rest into linux-next to give them some more testing, and hopefully have others pick up a couple more before I send a pull request. Arnd
diff --git a/fs/xfs/xfs_dquot.c b/fs/xfs/xfs_dquot.c index 2bff21ca9d78..9cfd3209f52b 100644 --- a/fs/xfs/xfs_dquot.c +++ b/fs/xfs/xfs_dquot.c @@ -137,7 +137,7 @@ xfs_qm_adjust_dqtimers( (d->d_blk_hardlimit && (be64_to_cpu(d->d_bcount) > be64_to_cpu(d->d_blk_hardlimit)))) { - d->d_btimer = cpu_to_be32(get_seconds() + + d->d_btimer = cpu_to_be32(ktime_get_real_seconds() + mp->m_quotainfo->qi_btimelimit); } else { d->d_bwarns = 0; @@ -160,7 +160,7 @@ xfs_qm_adjust_dqtimers( (d->d_ino_hardlimit && (be64_to_cpu(d->d_icount) > be64_to_cpu(d->d_ino_hardlimit)))) { - d->d_itimer = cpu_to_be32(get_seconds() + + d->d_itimer = cpu_to_be32(ktime_get_real_seconds() + mp->m_quotainfo->qi_itimelimit); } else { d->d_iwarns = 0; @@ -183,7 +183,7 @@ xfs_qm_adjust_dqtimers( (d->d_rtb_hardlimit && (be64_to_cpu(d->d_rtbcount) > be64_to_cpu(d->d_rtb_hardlimit)))) { - d->d_rtbtimer = cpu_to_be32(get_seconds() + + d->d_rtbtimer = cpu_to_be32(ktime_get_real_seconds() + mp->m_quotainfo->qi_rtbtimelimit); } else { d->d_rtbwarns = 0; diff --git a/fs/xfs/xfs_qm.h b/fs/xfs/xfs_qm.h index 7823af39008b..4e57edca8bce 100644 --- a/fs/xfs/xfs_qm.h +++ b/fs/xfs/xfs_qm.h @@ -64,9 +64,9 @@ struct xfs_quotainfo { struct xfs_inode *qi_pquotaip; /* project quota inode */ struct list_lru qi_lru; int qi_dquots; - time_t qi_btimelimit; /* limit for blks timer */ - time_t qi_itimelimit; /* limit for inodes timer */ - time_t qi_rtbtimelimit;/* limit for rt blks timer */ + time64_t qi_btimelimit; /* limit for blks timer */ + time64_t qi_itimelimit; /* limit for inodes timer */ + time64_t qi_rtbtimelimit;/* limit for rt blks timer */ xfs_qwarncnt_t qi_bwarnlimit; /* limit for blks warnings */ xfs_qwarncnt_t qi_iwarnlimit; /* limit for inodes warnings */ xfs_qwarncnt_t qi_rtbwarnlimit;/* limit for rt blks warnings */ diff --git a/fs/xfs/xfs_quotaops.c b/fs/xfs/xfs_quotaops.c index c7de17deeae6..38669e827206 100644 --- a/fs/xfs/xfs_quotaops.c +++ b/fs/xfs/xfs_quotaops.c @@ -37,9 +37,9 @@ xfs_qm_fill_state( tstate->flags |= QCI_SYSFILE; tstate->blocks = ip->i_d.di_nblocks; tstate->nextents = ip->i_d.di_nextents; - tstate->spc_timelimit = q->qi_btimelimit; - tstate->ino_timelimit = q->qi_itimelimit; - tstate->rt_spc_timelimit = q->qi_rtbtimelimit; + tstate->spc_timelimit = (u32)q->qi_btimelimit; + tstate->ino_timelimit = (u32)q->qi_itimelimit; + tstate->rt_spc_timelimit = (u32)q->qi_rtbtimelimit; tstate->spc_warnlimit = q->qi_bwarnlimit; tstate->ino_warnlimit = q->qi_iwarnlimit; tstate->rt_spc_warnlimit = q->qi_rtbwarnlimit; diff --git a/fs/xfs/xfs_trans_dquot.c b/fs/xfs/xfs_trans_dquot.c index a6fe2d8dc40f..d1b9869bc5fa 100644 --- a/fs/xfs/xfs_trans_dquot.c +++ b/fs/xfs/xfs_trans_dquot.c @@ -580,7 +580,7 @@ xfs_trans_dqresv( { xfs_qcnt_t hardlimit; xfs_qcnt_t softlimit; - time_t timer; + time64_t timer; xfs_qwarncnt_t warns; xfs_qwarncnt_t warnlimit; xfs_qcnt_t total_count; @@ -635,7 +635,8 @@ xfs_trans_dqresv( goto error_return; } if (softlimit && total_count > softlimit) { - if ((timer != 0 && get_seconds() > timer) || + if ((timer != 0 && + ktime_get_real_seconds() > timer) || (warns != 0 && warns >= warnlimit)) { xfs_quota_warn(mp, dqp, QUOTA_NL_BSOFTLONGWARN); @@ -662,7 +663,8 @@ xfs_trans_dqresv( goto error_return; } if (softlimit && total_count > softlimit) { - if ((timer != 0 && get_seconds() > timer) || + if ((timer != 0 && + ktime_get_real_seconds() > timer) || (warns != 0 && warns >= warnlimit)) { xfs_quota_warn(mp, dqp, QUOTA_NL_ISOFTLONGWARN);
As a preparation for removing the 32-bit time_t type and all associated interfaces, change xfs to use time64_t and ktime_get_real_seconds() for the quota housekeeping. Signed-off-by: Arnd Bergmann <arnd@arndb.de> --- fs/xfs/xfs_dquot.c | 6 +++--- fs/xfs/xfs_qm.h | 6 +++--- fs/xfs/xfs_quotaops.c | 6 +++--- fs/xfs/xfs_trans_dquot.c | 8 +++++--- 4 files changed, 14 insertions(+), 12 deletions(-)