| Message ID | 517456D7.8090603@inktank.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
----- "Alex Elder" <elder@inktank.com> wrote: > A ceph timespec contains 32-bit unsigned values for its seconds and > nanoseconds components. For a standard timespec, both fields are > signed, and the seconds field is almost surely 64 bits. Is the Ceph timespec going to change at some point? > > Add some explicit casts so the fact that this conversion is taking > place is obvious. Also trip a bug if we ever try to put out of > range (negative or too big) values into a ceph timespec. >
On Mon, 22 Apr 2013, Matt W. Benjamin wrote: > > ----- "Alex Elder" <elder@inktank.com> wrote: > > > A ceph timespec contains 32-bit unsigned values for its seconds and > > nanoseconds components. For a standard timespec, both fields are > > signed, and the seconds field is almost surely 64 bits. > > Is the Ceph timespec going to change at some point? I don't think so. 32-bits is enough for the billion nanoseconds in a second. And I'm not sure if the signedness is used/useful... the ceph utime_t code always normalizes the ns result to be in [0, 1 billion). sage > > > > > Add some explicit casts so the fact that this conversion is taking > > place is obvious. Also trip a bug if we ever try to put out of > > range (negative or too big) values into a ceph timespec. > > > > -- > Matt Benjamin > The Linux Box > 206 South Fifth Ave. Suite 150 > Ann Arbor, MI 48104 > > http://linuxbox.com > > tel. 734-761-4689 > fax. 734-769-8938 > cel. 734-216-5309 > -- > To unsubscribe from this list: send the line "unsubscribe ceph-devel" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > > -- To unsubscribe from this list: send the line "unsubscribe ceph-devel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
I was thinking about the seconds component. ----- "Sage Weil" <sage@inktank.com> wrote: > On Mon, 22 Apr 2013, Matt W. Benjamin wrote: > > > > ----- "Alex Elder" <elder@inktank.com> wrote: > > > > > A ceph timespec contains 32-bit unsigned values for its seconds > and > > > nanoseconds components. For a standard timespec, both fields are > > > signed, and the seconds field is almost surely 64 bits. > > > > Is the Ceph timespec going to change at some point? > > I don't think so. 32-bits is enough for the billion nanoseconds in a > > second. And I'm not sure if the signedness is used/useful... the ceph > > utime_t code always normalizes the ns result to be in [0, 1 billion). > > sage
On Mon, 22 Apr 2013, Matt W. Benjamin wrote: > I was thinking about the seconds component. Oh, right.. that's the unix epoch(alypse) in 2038 or something? That we should probably fix. :) s > > ----- "Sage Weil" <sage@inktank.com> wrote: > > > On Mon, 22 Apr 2013, Matt W. Benjamin wrote: > > > > > > ----- "Alex Elder" <elder@inktank.com> wrote: > > > > > > > A ceph timespec contains 32-bit unsigned values for its seconds > > and > > > > nanoseconds components. For a standard timespec, both fields are > > > > signed, and the seconds field is almost surely 64 bits. > > > > > > Is the Ceph timespec going to change at some point? > > > > I don't think so. 32-bits is enough for the billion nanoseconds in a > > > > second. And I'm not sure if the signedness is used/useful... the ceph > > > > utime_t code always normalizes the ns result to be in [0, 1 billion). > > > > sage > > > -- > Matt Benjamin > The Linux Box > 206 South Fifth Ave. Suite 150 > Ann Arbor, MI 48104 > > http://linuxbox.com > > tel. 734-761-4689 > fax. 734-769-8938 > cel. 734-216-5309 > -- > To unsubscribe from this list: send the line "unsubscribe ceph-devel" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > > -- To unsubscribe from this list: send the line "unsubscribe ceph-devel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 04/22/2013 11:12 AM, Matt W. Benjamin wrote: > I was thinking about the seconds component. I wondered the same thing. It will most likely have to some time in the next 25 years or so. -Alex > ----- "Sage Weil" <sage@inktank.com> wrote: > >> On Mon, 22 Apr 2013, Matt W. Benjamin wrote: >>> >>> ----- "Alex Elder" <elder@inktank.com> wrote: >>> >>>> A ceph timespec contains 32-bit unsigned values for its seconds >> and >>>> nanoseconds components. For a standard timespec, both fields are >>>> signed, and the seconds field is almost surely 64 bits. >>> >>> Is the Ceph timespec going to change at some point? >> >> I don't think so. 32-bits is enough for the billion nanoseconds in a >> >> second. And I'm not sure if the signedness is used/useful... the ceph >> >> utime_t code always normalizes the ns result to be in [0, 1 billion). >> >> sage > > -- To unsubscribe from this list: send the line "unsubscribe ceph-devel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Reviewed-by: Josh Durgin <josh.durgin@inktank.com> On 04/21/2013 02:15 PM, Alex Elder wrote: > A ceph timespec contains 32-bit unsigned values for its seconds and > nanoseconds components. For a standard timespec, both fields are > signed, and the seconds field is almost surely 64 bits. > > Add some explicit casts so the fact that this conversion is taking > place is obvious. Also trip a bug if we ever try to put out of > range (negative or too big) values into a ceph timespec. > > Signed-off-by: Alex Elder <elder@inktank.com> > --- > include/linux/ceph/decode.h | 13 +++++++++---- > 1 file changed, 9 insertions(+), 4 deletions(-) > > diff --git a/include/linux/ceph/decode.h b/include/linux/ceph/decode.h > index 9575a52..379f715 100644 > --- a/include/linux/ceph/decode.h > +++ b/include/linux/ceph/decode.h > @@ -154,14 +154,19 @@ bad: > static inline void ceph_decode_timespec(struct timespec *ts, > const struct ceph_timespec *tv) > { > - ts->tv_sec = le32_to_cpu(tv->tv_sec); > - ts->tv_nsec = le32_to_cpu(tv->tv_nsec); > + ts->tv_sec = (__kernel_time_t)le32_to_cpu(tv->tv_sec); > + ts->tv_nsec = (long)le32_to_cpu(tv->tv_nsec); > } > static inline void ceph_encode_timespec(struct ceph_timespec *tv, > const struct timespec *ts) > { > - tv->tv_sec = cpu_to_le32(ts->tv_sec); > - tv->tv_nsec = cpu_to_le32(ts->tv_nsec); > + BUG_ON(ts->tv_sec < 0); > + BUG_ON(ts->tv_sec > (__kernel_time_t)U32_MAX); > + BUG_ON(ts->tv_nsec < 0); > + BUG_ON(ts->tv_nsec > (long)U32_MAX); > + > + tv->tv_sec = cpu_to_le32((u32)ts->tv_sec); > + tv->tv_nsec = cpu_to_le32((u32)ts->tv_nsec); > } > > /* > -- To unsubscribe from this list: send the line "unsubscribe ceph-devel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/include/linux/ceph/decode.h b/include/linux/ceph/decode.h index 9575a52..379f715 100644 --- a/include/linux/ceph/decode.h +++ b/include/linux/ceph/decode.h @@ -154,14 +154,19 @@ bad: static inline void ceph_decode_timespec(struct timespec *ts, const struct ceph_timespec *tv) { - ts->tv_sec = le32_to_cpu(tv->tv_sec); - ts->tv_nsec = le32_to_cpu(tv->tv_nsec); + ts->tv_sec = (__kernel_time_t)le32_to_cpu(tv->tv_sec); + ts->tv_nsec = (long)le32_to_cpu(tv->tv_nsec); } static inline void ceph_encode_timespec(struct ceph_timespec *tv, const struct timespec *ts) { - tv->tv_sec = cpu_to_le32(ts->tv_sec); - tv->tv_nsec = cpu_to_le32(ts->tv_nsec); + BUG_ON(ts->tv_sec < 0); + BUG_ON(ts->tv_sec > (__kernel_time_t)U32_MAX); + BUG_ON(ts->tv_nsec < 0); + BUG_ON(ts->tv_nsec > (long)U32_MAX); + + tv->tv_sec = cpu_to_le32((u32)ts->tv_sec); + tv->tv_nsec = cpu_to_le32((u32)ts->tv_nsec); } /*
A ceph timespec contains 32-bit unsigned values for its seconds and nanoseconds components. For a standard timespec, both fields are signed, and the seconds field is almost surely 64 bits. Add some explicit casts so the fact that this conversion is taking place is obvious. Also trip a bug if we ever try to put out of range (negative or too big) values into a ceph timespec. Signed-off-by: Alex Elder <elder@inktank.com> --- include/linux/ceph/decode.h | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-)