diff mbox

[v2,09/24] fs: udf: Replace CURRENT_TIME with current_time()

Message ID 1466382443-11063-10-git-send-email-deepa.kernel@gmail.com (mailing list archive)
State New, archived
Headers show

Commit Message

Deepa Dinamani June 20, 2016, 12:27 a.m. UTC 
CURRENT_TIME is not y2038 safe.

CURRENT_TIME macro is also not appropriate for filesystems
as it doesn't use the right granularity for filesystem
timestamps.

Logical Volume Integrity format is described to have the
same timestamp format for "Recording Date and time" as
the other [a,c,m]timestamps.
Hence using current_time() instead here promises to
maintain the same granularity as other timestamps.

This is also in preparation for the patch that transitions
vfs timestamps to use 64 bit time and hence make them
y2038 safe. As part of the effort current_fs_time() will be
extended to do range checks.

Signed-off-by: Deepa Dinamani <deepa.kernel@gmail.com>
Cc: Jan Kara <jack@suse.com>
Reviewed-by: Jan Kara <jack@suse.cz>
---
 fs/udf/super.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

Comments

Arnd Bergmann June 22, 2016, 1:54 p.m. UTC | #1 
On Sunday, June 19, 2016 5:27:08 PM CEST Deepa Dinamani wrote:
>         mutex_lock(&sbi->s_alloc_mutex);
>         lvidiu->impIdent.identSuffix[0] = UDF_OS_CLASS_UNIX;
>         lvidiu->impIdent.identSuffix[1] = UDF_OS_ID_LINUX;
> +       ktime_get_real_ts(&ts);
>         udf_time_to_disk_stamp(&lvid->recordingDateAndTime,
> -                               CURRENT_TIME);
> +                               timespec_trunc(ts, sb->s_time_gran));
>         lvid->integrityType = cpu_to_le32(LVID_INTEGRITY_TYPE_OPEN);
>  
>         lvid->descTag.descCRC = cpu_to_le16(
> 

I think we don't need the timespec_trunc here, and introducing the
call might complicate matters in the future.

IMHO timespec_trunc() really only makes sense when assigning into
an inode timestamp, whereas udf_time_to_disk_stamp() already truncates
the resulting nanoseconds to microseconds.

	Arnd
--
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Deepa Dinamani June 23, 2016, 1:59 a.m. UTC | #2 
On Wed, Jun 22, 2016 at 6:54 AM, Arnd Bergmann <arnd@arndb.de> wrote:
> On Sunday, June 19, 2016 5:27:08 PM CEST Deepa Dinamani wrote:
>>         mutex_lock(&sbi->s_alloc_mutex);
>>         lvidiu->impIdent.identSuffix[0] = UDF_OS_CLASS_UNIX;
>>         lvidiu->impIdent.identSuffix[1] = UDF_OS_ID_LINUX;
>> +       ktime_get_real_ts(&ts);
>>         udf_time_to_disk_stamp(&lvid->recordingDateAndTime,
>> -                               CURRENT_TIME);
>> +                               timespec_trunc(ts, sb->s_time_gran));
>>         lvid->integrityType = cpu_to_le32(LVID_INTEGRITY_TYPE_OPEN);
>>
>>         lvid->descTag.descCRC = cpu_to_le16(
>>
>
> I think we don't need the timespec_trunc here, and introducing the
> call might complicate matters in the future.

This was done so that it could be similar to inode timestamps
as both have same format and use same formatting functions.

> IMHO timespec_trunc() really only makes sense when assigning into
> an inode timestamp, whereas udf_time_to_disk_stamp() already truncates
> the resulting nanoseconds to microseconds.

I reconsidered our discussion.
I think you are correct.
I missed that the prints using microseconds format would actually
convert seconds
also to microseconds and not just nanoseconds.

I will remove the timespec_trunc here and post a v3.

Thanks,
-Deepa
--
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/fs/udf/super.c b/fs/udf/super.c
index 4942549..134c63a 100644
--- a/fs/udf/super.c
+++ b/fs/udf/super.c
@@ -1986,6 +1986,7 @@  static void udf_open_lvid(struct super_block *sb)
 	struct buffer_head *bh = sbi->s_lvid_bh;
 	struct logicalVolIntegrityDesc *lvid;
 	struct logicalVolIntegrityDescImpUse *lvidiu;
+	struct timespec ts;
 
 	if (!bh)
 		return;
@@ -1997,8 +1998,9 @@  static void udf_open_lvid(struct super_block *sb)
 	mutex_lock(&sbi->s_alloc_mutex);
 	lvidiu->impIdent.identSuffix[0] = UDF_OS_CLASS_UNIX;
 	lvidiu->impIdent.identSuffix[1] = UDF_OS_ID_LINUX;
+	ktime_get_real_ts(&ts);
 	udf_time_to_disk_stamp(&lvid->recordingDateAndTime,
-				CURRENT_TIME);
+				timespec_trunc(ts, sb->s_time_gran));
 	lvid->integrityType = cpu_to_le32(LVID_INTEGRITY_TYPE_OPEN);
 
 	lvid->descTag.descCRC = cpu_to_le16(
@@ -2019,6 +2021,7 @@  static void udf_close_lvid(struct super_block *sb)
 	struct buffer_head *bh = sbi->s_lvid_bh;
 	struct logicalVolIntegrityDesc *lvid;
 	struct logicalVolIntegrityDescImpUse *lvidiu;
+	struct timespec ts;
 
 	if (!bh)
 		return;
@@ -2030,7 +2033,9 @@  static void udf_close_lvid(struct super_block *sb)
 	mutex_lock(&sbi->s_alloc_mutex);
 	lvidiu->impIdent.identSuffix[0] = UDF_OS_CLASS_UNIX;
 	lvidiu->impIdent.identSuffix[1] = UDF_OS_ID_LINUX;
-	udf_time_to_disk_stamp(&lvid->recordingDateAndTime, CURRENT_TIME);
+	ktime_get_real_ts(&ts);
+	udf_time_to_disk_stamp(&lvid->recordingDateAndTime,
+		timespec_trunc(ts, sb->s_time_gran));
 	if (UDF_MAX_WRITE_VERSION > le16_to_cpu(lvidiu->maxUDFWriteRev))
 		lvidiu->maxUDFWriteRev = cpu_to_le16(UDF_MAX_WRITE_VERSION);
 	if (sbi->s_udfrev > le16_to_cpu(lvidiu->minUDFReadRev))