On Thu, Feb 18, 2016 at 5:20 PM, Arnd Bergmann arnd@arndb.de wrote:
On Thursday 18 February 2016 17:02:25 Ilya Dryomov wrote:
If we are going to touch this function at all, let's drop all casts and just add a comment along the lines of "the lack of cast/sign-extension here is intentional, this function has a counterpart on the server side which also lacks cast/sign-extension, etc". To someone who is not up on these subtleties, the __u64 cast is going to be more confusing than explicit...
We certainly a comment, I was just suggesting to add the cast as well.
Again, it's not the lack of the cast on the server that is important here, it's whether it gets interpreted as signed or unsigned at the point where the timestamp is actually used for comparison or printing. Let me suggest a comment here:
I'd argue it's important to have encode/decode code look the same.
I mentioned before that at least in some places it's interpreted as signed and that the interpretation is not totally up to the client. The printing (at least some of it) is right there in the same file (src/include/utime.h) and you can see ... + 1900.
/*
- ceph timestamps are unsigned 32-bit starting at 1970, which is
- different from a signed 32-bit or 64-bit time_t. On 64-bit
- architectures, this gets interpreted as a subset of time_t
- in the range from 1970 to 2106.
- Machines with a 32-bit time_t will incorrectly interpret the
- timestamps with years 2038-2106 as negative numbers in the
- 1902-1969 range, until both kernel and glibc are change to
- using 64-bit time_t.
*/
I think a more accurate description would be "ceph timestamps are signed 32-bit, with the caveat that something somewhere is likely to break on a negative timestamp".
We are not looking at trying to reuse that extra bit for 1970..2106. As I said earlier, utime_t is used in quite a lot of places throughout the code base and is part of both in-core and on-disk data structures. Auditing all those sites for that is probably never going to happen. I think the plan is to live with what we've got until a proper 64-bit sec/nsec conversion is done in a way that was described earlier (message versions, ceph feature bit, etc). Any vfs timespec-handling changes should simply preserve the existing behavior, for now.
Thanks,
Ilya