Re: Correction to nanosecond timestamp patch for 64-bit arch
On Sun, 2007-06-17 at 18:32 +0530, Kalpak Shah wrote:
Index: linux-2.6.21/include/linux/ext4_fs.h
===
--- linux-2.6.21.orig/include/linux/ext4_fs.h
+++ linux-2.6.21/include/linux/ext4_fs.h
@@ -366,9 +366,9 @@ static inline __le32 ext4_encode_extra_t
static inline void ext4_decode_extra_time(struct timespec *time, __le32
extra)
{
if (sizeof(time-tv_sec) 4)
-time-tv_sec |= (__u64)(le32_to_cpu(extra) EXT4_EPOCH_MASK)
- 32;
- time-tv_nsec = (le32_to_cpu(extra) EXT4_NSEC_MASK) 2;
+time-tv_sec |= (__u64)((signed)le32_to_cpu(extra)
+EXT4_EPOCH_MASK) 32;
+ time-tv_nsec = ((signed)le32_to_cpu(extra) EXT4_NSEC_MASK) 2;
}
I am not too sure about the above hunk. tv_sec would not need any
(signed) cast since it is being ORed. And nsec should always lie between
0 and 1e9.
But the following patch is definitely correct and needs to be applied.
Index: linux-2.6.21/include/linux/ext4_fs.h
===
--- linux-2.6.21.orig/include/linux/ext4_fs.h
+++ linux-2.6.21/include/linux/ext4_fs.h
@@ -390,7 +390,7 @@ do {
\
#define EXT4_INODE_GET_XTIME(xtime, inode, raw_inode) \
do { \
- (inode)-xtime.tv_sec = le32_to_cpu((raw_inode)-xtime); \
+ (inode)-xtime.tv_sec = (signed)le32_to_cpu((raw_inode)-xtime); \
if (EXT4_FITS_IN_INODE(raw_inode, EXT4_I(inode), xtime ## _extra)) \
ext4_decode_extra_time((inode)-xtime,\
raw_inode-xtime ## _extra);\
@@ -399,7 +399,8 @@ do {
\
#define EXT4_EINODE_GET_XTIME(xtime, einode, raw_inode)
\
do { \
if (EXT4_FITS_IN_INODE(raw_inode, einode, xtime)) \
- (einode)-xtime.tv_sec = le32_to_cpu((raw_inode)-xtime); \
+ (einode)-xtime.tv_sec = \
+ (signed)le32_to_cpu((raw_inode)-xtime); \
if (EXT4_FITS_IN_INODE(raw_inode, einode, xtime ## _extra))\
ext4_decode_extra_time((einode)-xtime, \
raw_inode-xtime ## _extra);\
Thanks,
Kalpak.
-
To unsubscribe from this list: send the line unsubscribe linux-ext4 in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Correction to nanosecond timestamp patch for 64-bit arch
On Jun 18, 2007 15:39 +0530, Kalpak Shah wrote:
On Sun, 2007-06-17 at 18:32 +0530, Kalpak Shah wrote:
Index: linux-2.6.21/include/linux/ext4_fs.h
===
--- linux-2.6.21.orig/include/linux/ext4_fs.h
+++ linux-2.6.21/include/linux/ext4_fs.h
@@ -366,9 +366,9 @@ static inline __le32 ext4_encode_extra_t
static inline void ext4_decode_extra_time(struct timespec *time, __le32
extra)
{
if (sizeof(time-tv_sec) 4)
- time-tv_sec |= (__u64)(le32_to_cpu(extra) EXT4_EPOCH_MASK)
- 32;
- time-tv_nsec = (le32_to_cpu(extra) EXT4_NSEC_MASK) 2;
+ time-tv_sec |= (__u64)((signed)le32_to_cpu(extra)
+ EXT4_EPOCH_MASK) 32;
+ time-tv_nsec = ((signed)le32_to_cpu(extra) EXT4_NSEC_MASK) 2;
}
I am not too sure about the above hunk. tv_sec would not need any
(signed) cast since it is being ORed. And nsec should always lie between
0 and 1e9.
But the following patch is definitely correct and needs to be applied.
Hmm, this makes me think that the shifting for the epoch needs to be
done with 31 bits instead of 32, otherwise the time between 0x7fff
and 0x will always appear to be negative.
I'm beginning to wonder at the value of trying to save any times with
negative timestamp values. That might have seemed useful in 1970 when
it was only a few weeks or months in the past, but it seems like a waste
of bits today. It would seem prudent to just truncate times on disk to
0 if the timestamp is negative? The original bug report was related to
setting the time to Jan 1 1970 in some timezone that causes this to be
a negative value, so limiting the timestamp to 0 in such cases wouldn't
be considered a limitation.
According to the original bug report, storing negative times isn't even
possible to do with some tools, and might be considered a bug in date
as much as anything. If the problem is the inconsistency of times between
32-bit and 64-bit systems this could be resolved by always treating the
on-disk value as an unsigned integer, and it would be capped at +2^31
(2038) on 32-bit systems for the next couple of years until date is fixed.
#define EXT4_INODE_GET_XTIME(xtime, inode, raw_inode)
\
do {
\
- (inode)-xtime.tv_sec = le32_to_cpu((raw_inode)-xtime);
\
+ (inode)-xtime.tv_sec = (signed)le32_to_cpu((raw_inode)-xtime);
\
if (EXT4_FITS_IN_INODE(raw_inode, EXT4_I(inode), xtime ## _extra))
\
ext4_decode_extra_time((inode)-xtime,
\
raw_inode-xtime ## _extra);
\
@@ -399,7 +399,8 @@ do {
\
#define EXT4_EINODE_GET_XTIME(xtime, einode, raw_inode)
\
do {
\
if (EXT4_FITS_IN_INODE(raw_inode, einode, xtime))
\
- (einode)-xtime.tv_sec = le32_to_cpu((raw_inode)-xtime);
\
+ (einode)-xtime.tv_sec =
\
+ (signed)le32_to_cpu((raw_inode)-xtime);
\
if (EXT4_FITS_IN_INODE(raw_inode, einode, xtime ## _extra))
\
ext4_decode_extra_time((einode)-xtime,
\
raw_inode-xtime ## _extra);
\
Cheers, Andreas
--
Andreas Dilger
Principal Software Engineer
Cluster File Systems, Inc.
-
To unsubscribe from this list: send the line unsubscribe linux-ext4 in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Correction to nanosecond timestamp patch for 64-bit arch
http://bugzilla.kernel.org/show_bug.cgi?id=5079 mentions the problem with
timestamps in ext2/3/4. If the timestamp is set to before epoch, the files have
their dates changed on 64-bit partitions.
The corrections from this bug also need to be incorporated in the nanosecond
patch.
Signed-off-by: Kalpak Shah [EMAIL PROTECTED]
Index: linux-2.6.21/include/linux/ext4_fs.h
===
--- linux-2.6.21.orig/include/linux/ext4_fs.h
+++ linux-2.6.21/include/linux/ext4_fs.h
@@ -366,9 +366,9 @@ static inline __le32 ext4_encode_extra_t
static inline void ext4_decode_extra_time(struct timespec *time, __le32 extra)
{
if (sizeof(time-tv_sec) 4)
- time-tv_sec |= (__u64)(le32_to_cpu(extra) EXT4_EPOCH_MASK)
- 32;
- time-tv_nsec = (le32_to_cpu(extra) EXT4_NSEC_MASK) 2;
+ time-tv_sec |= (__u64)((signed)le32_to_cpu(extra)
+ EXT4_EPOCH_MASK) 32;
+ time-tv_nsec = ((signed)le32_to_cpu(extra) EXT4_NSEC_MASK) 2;
}
#define EXT4_INODE_SET_XTIME(xtime, inode, raw_inode) \
@@ -390,7 +390,7 @@ do {
\
#define EXT4_INODE_GET_XTIME(xtime, inode, raw_inode) \
do { \
- (inode)-xtime.tv_sec = le32_to_cpu((raw_inode)-xtime); \
+ (inode)-xtime.tv_sec = (signed)le32_to_cpu((raw_inode)-xtime); \
if (EXT4_FITS_IN_INODE(raw_inode, EXT4_I(inode), xtime ## _extra)) \
ext4_decode_extra_time((inode)-xtime,\
raw_inode-xtime ## _extra);\
@@ -399,7 +399,8 @@ do {
\
#define EXT4_EINODE_GET_XTIME(xtime, einode, raw_inode)
\
do { \
if (EXT4_FITS_IN_INODE(raw_inode, einode, xtime)) \
- (einode)-xtime.tv_sec = le32_to_cpu((raw_inode)-xtime); \
+ (einode)-xtime.tv_sec = \
+ (signed)le32_to_cpu((raw_inode)-xtime); \
if (EXT4_FITS_IN_INODE(raw_inode, einode, xtime ## _extra))\
ext4_decode_extra_time((einode)-xtime, \
raw_inode-xtime ## _extra);\
-
To unsubscribe from this list: send the line unsubscribe linux-ext4 in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
