2012-11-11 13:58:19

by Namjae Jeon

[permalink] [raw]
Subject: [PATCH 2/3] fat: fix time updates for create and delete

Correctly update modification and status change time in case of
file/directory removal and creation.

Signed-off-by: Namjae Jeon <[email protected]>
Signed-off-by: Amit Sahrawat <[email protected]>
---
fs/fat/dir.c | 2 +-
fs/fat/namei_vfat.c | 6 +++---
2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/fs/fat/dir.c b/fs/fat/dir.c
index 2a18234..c9fca7d 100644
--- a/fs/fat/dir.c
+++ b/fs/fat/dir.c
@@ -1042,7 +1042,7 @@ int fat_remove_entries(struct inode *dir, struct fat_slot_info *sinfo)
}
}

- dir->i_mtime = dir->i_atime = CURRENT_TIME_SEC;
+ dir->i_mtime = dir->i_ctime = CURRENT_TIME_SEC;
if (IS_DIRSYNC(dir))
(void)fat_sync_inode(dir);
else
diff --git a/fs/fat/namei_vfat.c b/fs/fat/namei_vfat.c
index ac959d6..23bec0d 100644
--- a/fs/fat/namei_vfat.c
+++ b/fs/fat/namei_vfat.c
@@ -684,7 +684,7 @@ static int vfat_add_entry(struct inode *dir, struct qstr *qname, int is_dir,
goto cleanup;

/* update timestamp */
- dir->i_ctime = dir->i_mtime = dir->i_atime = *ts;
+ dir->i_ctime = dir->i_mtime = *ts;
if (IS_DIRSYNC(dir))
(void)fat_sync_inode(dir);
else
@@ -826,7 +826,7 @@ static int vfat_rmdir(struct inode *dir, struct dentry *dentry)
drop_nlink(dir);

clear_nlink(inode);
- inode->i_mtime = inode->i_atime = CURRENT_TIME_SEC;
+ inode->i_ctime = CURRENT_TIME_SEC;
fat_detach(inode);
out:
mutex_unlock(&MSDOS_SB(sb)->s_lock);
@@ -851,7 +851,7 @@ static int vfat_unlink(struct inode *dir, struct dentry *dentry)
if (err)
goto out;
clear_nlink(inode);
- inode->i_mtime = inode->i_atime = CURRENT_TIME_SEC;
+ inode->i_ctime = CURRENT_TIME_SEC;
fat_detach(inode);
out:
mutex_unlock(&MSDOS_SB(sb)->s_lock);
--
1.7.9.5


2012-11-11 16:09:56

by OGAWA Hirofumi

[permalink] [raw]
Subject: Re: [PATCH 2/3] fat: fix time updates for create and delete

Namjae Jeon <[email protected]> writes:

> Correctly update modification and status change time in case of
> file/directory removal and creation.

This changelog just explain "what", and it doesn't explain "why". Please
explain why we need this change.

IIRC, timestamp handling in FAT driver is strange historically. Anyway,
FAT doesn't have "inode change time". It is "creation time" in FAT.

> Signed-off-by: Namjae Jeon <[email protected]>
> Signed-off-by: Amit Sahrawat <[email protected]>
> ---
> fs/fat/dir.c | 2 +-
> fs/fat/namei_vfat.c | 6 +++---
> 2 files changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/fs/fat/dir.c b/fs/fat/dir.c
> index 2a18234..c9fca7d 100644
> --- a/fs/fat/dir.c
> +++ b/fs/fat/dir.c
> @@ -1042,7 +1042,7 @@ int fat_remove_entries(struct inode *dir, struct fat_slot_info *sinfo)
> }
> }
>
> - dir->i_mtime = dir->i_atime = CURRENT_TIME_SEC;
> + dir->i_mtime = dir->i_ctime = CURRENT_TIME_SEC;
> if (IS_DIRSYNC(dir))
> (void)fat_sync_inode(dir);
> else
> diff --git a/fs/fat/namei_vfat.c b/fs/fat/namei_vfat.c
> index ac959d6..23bec0d 100644
> --- a/fs/fat/namei_vfat.c
> +++ b/fs/fat/namei_vfat.c
> @@ -684,7 +684,7 @@ static int vfat_add_entry(struct inode *dir, struct qstr *qname, int is_dir,
> goto cleanup;
>
> /* update timestamp */
> - dir->i_ctime = dir->i_mtime = dir->i_atime = *ts;
> + dir->i_ctime = dir->i_mtime = *ts;
> if (IS_DIRSYNC(dir))
> (void)fat_sync_inode(dir);
> else
> @@ -826,7 +826,7 @@ static int vfat_rmdir(struct inode *dir, struct dentry *dentry)
> drop_nlink(dir);
>
> clear_nlink(inode);
> - inode->i_mtime = inode->i_atime = CURRENT_TIME_SEC;
> + inode->i_ctime = CURRENT_TIME_SEC;
> fat_detach(inode);
> out:
> mutex_unlock(&MSDOS_SB(sb)->s_lock);
> @@ -851,7 +851,7 @@ static int vfat_unlink(struct inode *dir, struct dentry *dentry)
> if (err)
> goto out;
> clear_nlink(inode);
> - inode->i_mtime = inode->i_atime = CURRENT_TIME_SEC;
> + inode->i_ctime = CURRENT_TIME_SEC;
> fat_detach(inode);
> out:
> mutex_unlock(&MSDOS_SB(sb)->s_lock);

--
OGAWA Hirofumi <[email protected]>

2012-11-12 06:37:23

by Namjae Jeon

[permalink] [raw]
Subject: Re: [PATCH 2/3] fat: fix time updates for create and delete

2012/11/12, OGAWA Hirofumi <[email protected]>:
> Namjae Jeon <[email protected]> writes:
>
>> Correctly update modification and status change time in case of
>> file/directory removal and creation.
>
> This changelog just explain "what", and it doesn't explain "why". Please
> explain why we need this change.
>
> IIRC, timestamp handling in FAT driver is strange historically. Anyway,
> FAT doesn't have "inode change time". It is "creation time" in FAT.
Hi. OGAWA.
I made this patch after comparing timestamp handlings with MSDOS and XFS.
Should timestamp handling of FAT be same with MSDOS ?
Am I missing ?

Thanks.
>
>> Signed-off-by: Namjae Jeon <[email protected]>
>> Signed-off-by: Amit Sahrawat <[email protected]>
>> ---
>> fs/fat/dir.c | 2 +-
>> fs/fat/namei_vfat.c | 6 +++---
>> 2 files changed, 4 insertions(+), 4 deletions(-)
>>
>> diff --git a/fs/fat/dir.c b/fs/fat/dir.c
>> index 2a18234..c9fca7d 100644
>> --- a/fs/fat/dir.c
>> +++ b/fs/fat/dir.c
>> @@ -1042,7 +1042,7 @@ int fat_remove_entries(struct inode *dir, struct
>> fat_slot_info *sinfo)
>> }
>> }
>>
>> - dir->i_mtime = dir->i_atime = CURRENT_TIME_SEC;
>> + dir->i_mtime = dir->i_ctime = CURRENT_TIME_SEC;
>> if (IS_DIRSYNC(dir))
>> (void)fat_sync_inode(dir);
>> else
>> diff --git a/fs/fat/namei_vfat.c b/fs/fat/namei_vfat.c
>> index ac959d6..23bec0d 100644
>> --- a/fs/fat/namei_vfat.c
>> +++ b/fs/fat/namei_vfat.c
>> @@ -684,7 +684,7 @@ static int vfat_add_entry(struct inode *dir, struct
>> qstr *qname, int is_dir,
>> goto cleanup;
>>
>> /* update timestamp */
>> - dir->i_ctime = dir->i_mtime = dir->i_atime = *ts;
>> + dir->i_ctime = dir->i_mtime = *ts;
>> if (IS_DIRSYNC(dir))
>> (void)fat_sync_inode(dir);
>> else
>> @@ -826,7 +826,7 @@ static int vfat_rmdir(struct inode *dir, struct dentry
>> *dentry)
>> drop_nlink(dir);
>>
>> clear_nlink(inode);
>> - inode->i_mtime = inode->i_atime = CURRENT_TIME_SEC;
>> + inode->i_ctime = CURRENT_TIME_SEC;
>> fat_detach(inode);
>> out:
>> mutex_unlock(&MSDOS_SB(sb)->s_lock);
>> @@ -851,7 +851,7 @@ static int vfat_unlink(struct inode *dir, struct
>> dentry *dentry)
>> if (err)
>> goto out;
>> clear_nlink(inode);
>> - inode->i_mtime = inode->i_atime = CURRENT_TIME_SEC;
>> + inode->i_ctime = CURRENT_TIME_SEC;
>> fat_detach(inode);
>> out:
>> mutex_unlock(&MSDOS_SB(sb)->s_lock);
>
> --
> OGAWA Hirofumi <[email protected]>
>

2012-11-12 07:57:18

by OGAWA Hirofumi

[permalink] [raw]
Subject: Re: [PATCH 2/3] fat: fix time updates for create and delete

Namjae Jeon <[email protected]> writes:

> 2012/11/12, OGAWA Hirofumi <[email protected]>:
>> Namjae Jeon <[email protected]> writes:
>>
>>> Correctly update modification and status change time in case of
>>> file/directory removal and creation.
>>
>> This changelog just explain "what", and it doesn't explain "why". Please
>> explain why we need this change.
>>
>> IIRC, timestamp handling in FAT driver is strange historically. Anyway,
>> FAT doesn't have "inode change time". It is "creation time" in FAT.
> Hi. OGAWA.
> I made this patch after comparing timestamp handlings with MSDOS and XFS.
> Should timestamp handling of FAT be same with MSDOS ?
> Am I missing ?

What was difference with XFS, FAT, and MSDOS?
--
OGAWA Hirofumi <[email protected]>

2012-11-12 07:58:06

by OGAWA Hirofumi

[permalink] [raw]
Subject: Re: [PATCH 2/3] fat: fix time updates for create and delete

OGAWA Hirofumi <[email protected]> writes:

> Namjae Jeon <[email protected]> writes:
>
>> 2012/11/12, OGAWA Hirofumi <[email protected]>:
>>> Namjae Jeon <[email protected]> writes:
>>>
>>>> Correctly update modification and status change time in case of
>>>> file/directory removal and creation.
>>>
>>> This changelog just explain "what", and it doesn't explain "why". Please
>>> explain why we need this change.
>>>
>>> IIRC, timestamp handling in FAT driver is strange historically. Anyway,
>>> FAT doesn't have "inode change time". It is "creation time" in FAT.
>> Hi. OGAWA.
>> I made this patch after comparing timestamp handlings with MSDOS and XFS.
>> Should timestamp handling of FAT be same with MSDOS ?
>> Am I missing ?
>
> What was difference with XFS, FAT, and MSDOS?

BTW, I recall I checked this, and yes, it is strange. But it is historical.
--
OGAWA Hirofumi <[email protected]>

2012-11-12 09:12:43

by Namjae Jeon

[permalink] [raw]
Subject: Re: [PATCH 2/3] fat: fix time updates for create and delete

2012/11/12, OGAWA Hirofumi <[email protected]>:
> OGAWA Hirofumi <[email protected]> writes:
>
>> Namjae Jeon <[email protected]> writes:
>>
>>> 2012/11/12, OGAWA Hirofumi <[email protected]>:
>>>> Namjae Jeon <[email protected]> writes:
>>>>
>>>>> Correctly update modification and status change time in case of
>>>>> file/directory removal and creation.
>>>>
>>>> This changelog just explain "what", and it doesn't explain "why".
>>>> Please
>>>> explain why we need this change.
>>>>
>>>> IIRC, timestamp handling in FAT driver is strange historically. Anyway,
>>>> FAT doesn't have "inode change time". It is "creation time" in FAT.
>>> Hi. OGAWA.
>>> I made this patch after comparing timestamp handlings with MSDOS and
>>> XFS.
>>> Should timestamp handling of FAT be same with MSDOS ?
>>> Am I missing ?
>>
>> What was difference with XFS, FAT, and MSDOS?
Okay, the description of patch was lacked, So I will resend patch with
specifical test reseult with other filesystem.
>
> BTW, I recall I checked this, and yes, it is strange. But it is historical.
"historical" means It is difficult to change ?

Thanks.
> --
> OGAWA Hirofumi <[email protected]>
>

2012-11-12 09:21:09

by OGAWA Hirofumi

[permalink] [raw]
Subject: Re: [PATCH 2/3] fat: fix time updates for create and delete

Namjae Jeon <[email protected]> writes:

>>> What was difference with XFS, FAT, and MSDOS?
> Okay, the description of patch was lacked, So I will resend patch with
> specifical test reseult with other filesystem.
>>
>> BTW, I recall I checked this, and yes, it is strange. But it is historical.
> "historical" means It is difficult to change ?

The timestamp handling was not same with unix fs from initial, and FAT
doesn't have inode change timestamp. This historical reason it is not
better to change to same with unix fs, and never be possible to support
inode change timestamp cleanly.
--
OGAWA Hirofumi <[email protected]>

2012-11-12 09:22:15

by OGAWA Hirofumi

[permalink] [raw]
Subject: Re: [PATCH 2/3] fat: fix time updates for create and delete

OGAWA Hirofumi <[email protected]> writes:

> Namjae Jeon <[email protected]> writes:
>
>>>> What was difference with XFS, FAT, and MSDOS?
>> Okay, the description of patch was lacked, So I will resend patch with
>> specifical test reseult with other filesystem.
>>>
>>> BTW, I recall I checked this, and yes, it is strange. But it is historical.
>> "historical" means It is difficult to change ?
>
> The timestamp handling was not same with unix fs from initial, and FAT
> doesn't have inode change timestamp. This historical reason it is not
> better to change to same with unix fs, and never be possible to support
> inode change timestamp cleanly.

I.e. the user visible change without strong reason is the wrong.
--
OGAWA Hirofumi <[email protected]>

2012-11-15 07:02:30

by Namjae Jeon

[permalink] [raw]
Subject: Re: [PATCH 2/3] fat: fix time updates for create and delete

2012/11/12, OGAWA Hirofumi <[email protected]>:
> OGAWA Hirofumi <[email protected]> writes:
>
>> Namjae Jeon <[email protected]> writes:
>>
>>>>> What was difference with XFS, FAT, and MSDOS?
>>> Okay, the description of patch was lacked, So I will resend patch with
>>> specifical test reseult with other filesystem.
>>>>
>>>> BTW, I recall I checked this, and yes, it is strange. But it is
>>>> historical.
>>> "historical" means It is difficult to change ?
>>
>> The timestamp handling was not same with unix fs from initial, and FAT
>> doesn't have inode change timestamp. This historical reason it is not
>> better to change to same with unix fs, and never be possible to support
>> inode change timestamp cleanly.
>
> I.e. the user visible change without strong reason is the wrong.

1)If we consider the code for FAT and MSDOS to be same with respect to
timing updates - there there is difference in code as mentioned below:
a) For:vfat_rmdir()
inode->i_mtime = inode->i_atime = CURRENT_TIME_SEC;
For:msdos_rmdir()
inode->i_ctime = CURRENT_TIME_SEC;

b) For: vfat_unlink()
inode->i_mtime = inode->i_atime = CURRENT_TIME_SEC;
For: msdos_unlink()
inode->i_ctime = CURRENT_TIME_SEC;

So, atleast the uniformity in the code is missing for MSDOS and VFAT.

2) Regarding timings information
Original timings on VFAT:
#> mkdir parent
#> stat parent
File type: directory
I-node number: 30
Mode: 40755 (octal)
Link count: 2
Ownership: UID=0 GID=0
Preferred I/O block size: 4096 bytes
File size: 4096 bytes
Blocks allocated: 8
Last status change: Thu Jan 1 00:01:29 2012
Last file access: Thu Jan 1 00:01:29 2012
Last file modification: Thu Jan 1 00:01:29 2012
#>
#> echo "hello" > parent/childfile
#> stat parent
File type: directory
I-node number: 30
Mode: 40755 (octal)
Link count: 2
Ownership: UID=0 GID=0
Preferred I/O block size: 4096 bytes
File size: 4096 bytes
Blocks allocated: 8
Last status change: Thu Jan 1 00:01:45 2012
Last file access: Thu Jan 1 00:01:45 2012
Last file modification: Thu Jan 1 00:01:45 2012
#>
#> mkdir parent/childdir
#> stat parent
File type: directory
I-node number: 30
Mode: 40755 (octal)
Link count: 3
Ownership: UID=0 GID=0
Preferred I/O block size: 4096 bytes
File size: 4096 bytes
Blocks allocated: 8
Last status change: Thu Jan 1 00:01:56 2012
Last file access: Thu Jan 1 00:01:56 2012
Last file modification: Thu Jan 1 00:01:56 2012
#> rm parent/childfile
#> stat parent
File type: directory
I-node number: 30
Mode: 40755 (octal)
Link count: 3
Ownership: UID=0 GID=0
Preferred I/O block size: 4096 bytes
File size: 4096 bytes
Blocks allocated: 8
Last status change: Thu Jan 1 00:01:56 2012
Last file access: Thu Jan 1 00:02:12 2012
Last file modification: Thu Jan 1 00:02:12 2012
#> rm -rf parent/childdir
#> stat parent
File type: directory
I-node number: 30
Mode: 40755 (octal)
Link count: 2
Ownership: UID=0 GID=0
Preferred I/O block size: 4096 bytes
File size: 4096 bytes
Blocks allocated: 8
Last status change: Thu Jan 1 00:01:56 2012
Last file access: Thu Jan 1 00:02:24 2012
Last file modification: Thu Jan 1 00:02:24 2012


For timing information comparison after changes in VFAT -> for VFAT ,
EXT4 and XFS
########################################################################
VFAT Filesystem with timestamp patch
########################################################################
#> mkdir parent
#> stat parent
File type: directory
I-node number: 29
Mode: 40755 (octal)
Link count: 2
Ownership: UID=0 GID=0
Preferred I/O block size: 4096 bytes
File size: 4096 bytes
Blocks allocated: 8
Last status change: Sun Jan 1 00:11:10 2012
Last file access: Sun Jan 1 00:11:10 2012
Last file modification: Sun Jan 1 00:11:10 2012
#>
#> echo "hello" parent/childfile
hello parent/childfile
#> echo "hello" > parent/childfile
#> stat parent
File type: directory
I-node number: 29
Mode: 40755 (octal)
Link count: 2
Ownership: UID=0 GID=0
Preferred I/O block size: 4096 bytes
File size: 4096 bytes
Blocks allocated: 8
Last status change: Sun Jan 1 00:12:22 2012
Last file access: Sun Jan 1 00:11:10 2012
Last file modification: Sun Jan 1 00:12:22 2012
#>
#>
#> mkdir parent/childdir
#> stat parent
File type: directory
I-node number: 29
Mode: 40755 (octal)
Link count: 3
Ownership: UID=0 GID=0
Preferred I/O block size: 4096 bytes
File size: 4096 bytes
Blocks allocated: 8
Last status change: Sun Jan 1 00:13:10 2012
Last file access: Sun Jan 1 00:11:10 2012
Last file modification: Sun Jan 1 00:13:10 2012
#>
#> rm parent/childfile
#> stat parent
File type: directory
I-node number: 29
Mode: 40755 (octal)
Link count: 3
Ownership: UID=0 GID=0
Preferred I/O block size: 4096 bytes
File size: 4096 bytes
Blocks allocated: 8
Last status change: Sun Jan 1 00:13:38 2012
Last file access: Sun Jan 1 00:11:10 2012
Last file modification: Sun Jan 1 00:13:38 2012
#> rm parent/childdir
rm: parent/childdir: is a directory
#> rm -rf parent/childdir
#> stat parent
File type: directory
I-node number: 29
Mode: 40755 (octal)
Link count: 2
Ownership: UID=0 GID=0
Preferred I/O block size: 4096 bytes
File size: 4096 bytes
Blocks allocated: 8
Last status change: Sun Jan 1 00:13:59 2012
Last file access: Sun Jan 1 00:11:10 2012
Last file modification: Sun Jan 1 00:13:59 2012
#>


########################################################################
EXT4 Filesystem
########################################################################
#> cd /dtv/usb/sdb2
#> pwd
/dtv/usb/sdb2
#>
#> mkdir parent
#> stat parent
File type: directory
I-node number: 12
Mode: 40755 (octal)
Link count: 2
Ownership: UID=0 GID=0
Preferred I/O block size: 4096 bytes
File size: 4096 bytes
Blocks allocated: 8
Last status change: Sun Jan 1 00:14:44 2012
Last file access: Sun Jan 1 00:14:44 2012
Last file modification: Sun Jan 1 00:14:44 2012
#>
#> echo "hello" > parent/childfile
#> stat parent
File type: directory
I-node number: 12
Mode: 40755 (octal)
Link count: 2
Ownership: UID=0 GID=0
Preferred I/O block size: 4096 bytes
File size: 4096 bytes
Blocks allocated: 8
Last status change: Sun Jan 1 00:15:04 2012
Last file access: Sun Jan 1 00:14:44 2012
Last file modification: Sun Jan 1 00:15:04 2012
#> mkdir parent/childdir
#> stat parent
File type: directory
I-node number: 12
Mode: 40755 (octal)
Link count: 3
Ownership: UID=0 GID=0
Preferred I/O block size: 4096 bytes
File size: 4096 bytes
Blocks allocated: 8
Last status change: Sun Jan 1 00:15:34 2012
Last file access: Sun Jan 1 00:14:44 2012
Last file modification: Sun Jan 1 00:15:34 2012
#>
#> rm parent/childfile
#> stat parent
File type: directory
I-node number: 12
Mode: 40755 (octal)
Link count: 3
Ownership: UID=0 GID=0
Preferred I/O block size: 4096 bytes
File size: 4096 bytes
Blocks allocated: 8
Last status change: Sun Jan 1 00:15:53 2012
Last file access: Sun Jan 1 00:14:44 2012
Last file modification: Sun Jan 1 00:15:53 2012
#>
#> rm -rf parent/childdir
#> stat parent
File type: directory
I-node number: 12
Mode: 40755 (octal)
Link count: 2
Ownership: UID=0 GID=0
Preferred I/O block size: 4096 bytes
File size: 4096 bytes
Blocks allocated: 8
Last status change: Sun Jan 1 00:16:09 2012
Last file access: Sun Jan 1 00:14:44 2012
Last file modification: Sun Jan 1 00:16:09 2012
#>





####################################################################
XFS
####################################################################
#> mkdir parent
#> stat parent
File type: directory
I-node number: 131
Mode: 40755 (octal)
Link count: 2
Ownership: UID=0 GID=0
Preferred I/O block size: 4096 bytes
File size: 6 bytes
Blocks allocated: 0
Last status change: Sun Jan 1 00:17:06 2012
Last file access: Sun Jan 1 00:17:06 2012
Last file modification: Sun Jan 1 00:17:06 2012
#>
#> echo "hello" > parent/childfile
#> stat parent
File type: directory
I-node number: 131
Mode: 40755 (octal)
Link count: 2
Ownership: UID=0 GID=0
Preferred I/O block size: 4096 bytes
File size: 22 bytes
Blocks allocated: 0
Last status change: Sun Jan 1 00:17:22 2012
Last file access: Sun Jan 1 00:17:06 2012
Last file modification: Sun Jan 1 00:17:22 2012
#>
#> mkdir parent/childdir
#> stat parent
File type: directory
I-node number: 131
Mode: 40755 (octal)
Link count: 3
Ownership: UID=0 GID=0
Preferred I/O block size: 4096 bytes
File size: 37 bytes
Blocks allocated: 0
Last status change: Sun Jan 1 00:17:36 2012
Last file access: Sun Jan 1 00:17:06 2012
Last file modification: Sun Jan 1 00:17:36 2012
#>
#>
#> rm parent/childfile
#> stat parent
File type: directory
I-node number: 131
Mode: 40755 (octal)
Link count: 3
Ownership: UID=0 GID=0
Preferred I/O block size: 4096 bytes
File size: 21 bytes
Blocks allocated: 0
Last status change: Sun Jan 1 00:17:51 2012
Last file access: Sun Jan 1 00:17:06 2012
Last file modification: Sun Jan 1 00:17:51 2012
#>
#> rm -rf parent/childdir
#> stat parent
File type: directory
I-node number: 131
Mode: 40755 (octal)
Link count: 2
Ownership: UID=0 GID=0
Preferred I/O block size: 4096 bytes
File size: 6 bytes
Blocks allocated: 0
Last status change: Sun Jan 1 00:17:58 2012
Last file access: Sun Jan 1 00:17:06 2012
Last file modification: Sun Jan 1 00:17:58 2012
#>

As can be seen from the 'stat' information - the timing information
appears same for VFAT like EXT4/XFS after changes.
Please let me know your opinion.

Thanks.

> --
> OGAWA Hirofumi <[email protected]>
>

2012-11-15 07:18:50

by OGAWA Hirofumi

[permalink] [raw]
Subject: Re: [PATCH 2/3] fat: fix time updates for create and delete

Namjae Jeon <[email protected]> writes:

> 1)If we consider the code for FAT and MSDOS to be same with respect to
> timing updates - there there is difference in code as mentioned below:
> a) For:vfat_rmdir()
> inode->i_mtime = inode->i_atime = CURRENT_TIME_SEC;
> For:msdos_rmdir()
> inode->i_ctime = CURRENT_TIME_SEC;
>
> b) For: vfat_unlink()
> inode->i_mtime = inode->i_atime = CURRENT_TIME_SEC;
> For: msdos_unlink()
> inode->i_ctime = CURRENT_TIME_SEC;
>
> So, atleast the uniformity in the code is missing for MSDOS and VFAT.

MSDOS doesn't have atime.

> 2) Regarding timings information
> Original timings on VFAT:
> #> mkdir parent
> #> stat parent
> File type: directory
> I-node number: 30
> Mode: 40755 (octal)
> Link count: 2
> Ownership: UID=0 GID=0
> Preferred I/O block size: 4096 bytes
> File size: 4096 bytes
> Blocks allocated: 8
> Last status change: Thu Jan 1 00:01:29 2012
> Last file access: Thu Jan 1 00:01:29 2012
> Last file modification: Thu Jan 1 00:01:29 2012

[...]

> As can be seen from the 'stat' information - the timing information
> appears same for VFAT like EXT4/XFS after changes.
> Please let me know your opinion.

You have to think about compatibility with other FAT, not unix fs.
--
OGAWA Hirofumi <[email protected]>

2012-11-16 10:13:00

by Namjae Jeon

[permalink] [raw]
Subject: Re: [PATCH 2/3] fat: fix time updates for create and delete

2012/11/15, OGAWA Hirofumi <[email protected]>:
> Namjae Jeon <[email protected]> writes:
>
>> 1)If we consider the code for FAT and MSDOS to be same with respect to
>> timing updates - there there is difference in code as mentioned below:
>> a) For:vfat_rmdir()
>> inode->i_mtime = inode->i_atime = CURRENT_TIME_SEC;
>> For:msdos_rmdir()
>> inode->i_ctime = CURRENT_TIME_SEC;
>>
>> b) For: vfat_unlink()
>> inode->i_mtime = inode->i_atime = CURRENT_TIME_SEC;
>> For: msdos_unlink()
>> inode->i_ctime = CURRENT_TIME_SEC;
>>
>> So, atleast the uniformity in the code is missing for MSDOS and VFAT.
>
> MSDOS doesn't have atime.
>
>> 2) Regarding timings information
>> Original timings on VFAT:
>> #> mkdir parent
>> #> stat parent
>> File type: directory
>> I-node number: 30
>> Mode: 40755 (octal)
>> Link count: 2
>> Ownership: UID=0 GID=0
>> Preferred I/O block size: 4096 bytes
>> File size: 4096 bytes
>> Blocks allocated: 8
>> Last status change: Thu Jan 1 00:01:29 2012
>> Last file access: Thu Jan 1 00:01:29 2012
>> Last file modification: Thu Jan 1 00:01:29 2012
>
> [...]
>
>> As can be seen from the 'stat' information - the timing information
>> appears same for VFAT like EXT4/XFS after changes.
>> Please let me know your opinion.
>
> You have to think about compatibility with other FAT, not unix fs.

Agreed, ctime is creation time, and there are comptability issues with
the patch.

But there is confusion about 'ctime' usage in the default code. When
referring the code I found many instances except 'fat_fill_inode'
where 'ctime' is updated as if it is 'change time' instead of
'creation time' like in functions: fat_write_end(), fat_cont_expend(),
fat_free(), vfat_add_entry().

As a case when I check using a simple case:
dd if=/dev/zero of=./samplefile bs=4096 count=10
=> check file timings
wait for 2minutes
Now, append to this file
echo "this is simple string to be appended" >> samplefile
=> check file timings

I can see - it resulted in change in 'ctime' and 'mtime'.
Now, when Connecting this Drive to Windows - it shows the time of
'second write' as the CREATION time as well as "Modification time".
If you agree that this is a strange/problem. I can try to fix the
timestamp of linux FAT checking this compatability pattern to the
nearest.
Let me know your opinion.

Thanks.

> --
> OGAWA Hirofumi <[email protected]>
>

2012-11-16 13:47:35

by OGAWA Hirofumi

[permalink] [raw]
Subject: Re: [PATCH 2/3] fat: fix time updates for create and delete

Namjae Jeon <[email protected]> writes:

>> You have to think about compatibility with other FAT, not unix fs.
>
> Agreed, ctime is creation time, and there are comptability issues with
> the patch.
>
> But there is confusion about 'ctime' usage in the default code. When
> referring the code I found many instances except 'fat_fill_inode'
> where 'ctime' is updated as if it is 'change time' instead of
> 'creation time' like in functions: fat_write_end(), fat_cont_expend(),
> fat_free(), vfat_add_entry().
>
> As a case when I check using a simple case:
> dd if=/dev/zero of=./samplefile bs=4096 count=10
> => check file timings
> wait for 2minutes
> Now, append to this file
> echo "this is simple string to be appended" >> samplefile
> => check file timings
>
> I can see - it resulted in change in 'ctime' and 'mtime'.
> Now, when Connecting this Drive to Windows - it shows the time of
> 'second write' as the CREATION time as well as "Modification time".
> If you agree that this is a strange/problem. I can try to fix the
> timestamp of linux FAT checking this compatability pattern to the
> nearest.
> Let me know your opinion.

Yes. It is strange behavior, and it is what I was calling historical.
If we changed this behavior now, user can be notice to change of
historical behavior. Also, if we didn't change this, it is strange as
FAT-fs compatibility.

If I can design from scratch, probably I will choose FAT-fs
compatibility at first. But we can't, and I don't have strong opinion to
change current behavior.
--
OGAWA Hirofumi <[email protected]>