Skip to content

Fix incorrect timestamp assignment when writing inode#92

Merged
jserv merged 1 commit into
sysprog21:masterfrom
EricKim27:clean_base
Jul 23, 2026
Merged

Fix incorrect timestamp assignment when writing inode#92
jserv merged 1 commit into
sysprog21:masterfrom
EricKim27:clean_base

Conversation

@EricKim27

@EricKim27 EricKim27 commented Jul 20, 2026

Copy link
Copy Markdown
Contributor

fix simplefs_write_inode to write the access time into disk_inode->i_atime and the modification time into disk_inode->i_mtime correctly.

After an inode has been modified or created, simplefs_write_inode(assigned to write_inode in simplefs_super_ops) is called to write the contents in the vfs inode to the relevant raw inode on disk. Because of a typo in simplefs_write_inode, i_atime gets assigned twice; once for writing access time, and again for writing modification time to it.

As a result, the raw inode metadata contains modification time on its access time field, and the modification time is never updated. Upon calling system calls like stat(), it returns modification time as access time, and an incorrect modification time(since it is never updated, and all bytes are zeroed out at mkfs, it defaults to 0, January 1st, 1970 in unix time).

Fix the assignments so access time is written to i_atime field and modification time is written to i_mtime field on on-disk inode structure.


Summary by cubic

Fixes incorrect timestamp assignment in simplefs_write_inode for kernel >= 6.7: write access time to disk_inode->i_atime and modification time to disk_inode->i_mtime instead of writing both to i_atime. Prevents i_mtime from being left zero so files report the correct modification time instead of 1970-01-01.

Written for commit d62bf18. Summary will update on new commits.

Review in cubic

cubic-dev-ai[bot]

This comment was marked as resolved.

@jserv jserv left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Improve git commit messages per https://cbea.ms/git-commit/

@EricKim27 EricKim27 changed the title fix: fix mtime from not updating correctly Fix simplefs_write_inode to write modification and access time correctly to disk inode Jul 22, 2026
@EricKim27

EricKim27 commented Jul 22, 2026

Copy link
Copy Markdown
Contributor Author

@jserv Modified the commit message as requested. Thanks for your feedback.

@jserv

jserv commented Jul 22, 2026

Copy link
Copy Markdown
Collaborator

Modified the commit message as requested.

Check https://cbea.ms/git-commit/ carefully and improve git commit messages to address motivations and considerations.

@EricKim27 EricKim27 changed the title Fix simplefs_write_inode to write modification and access time correctly to disk inode Fix incorrect timestamp assignment when writing inode Jul 22, 2026
@EricKim27

EricKim27 commented Jul 22, 2026

Copy link
Copy Markdown
Contributor Author

Modified the commit message as requested.

Check https://cbea.ms/git-commit/ carefully and improve git commit messages to address motivations and considerations.

I read the article you provided to me and improved the messages that would address my motivations and considerations. Thanks for the feedback.

@jserv

jserv commented Jul 23, 2026

Copy link
Copy Markdown
Collaborator

I read the article you provided to me and improved the messages that would address my motivations and considerations. Thanks for the feedback.

Instead of changing the title of this pull request on GitHub, you should reword the commit message using git commit --amend, then force-push the updated commit.

After running gh pr checkout 92 followed by git show HEAD, I get the following one-line commit message:

fix: fix mtime from not updating correctly

This commit message violates the project's commit message guidelines.

@EricKim27

Copy link
Copy Markdown
Contributor Author

I read the article you provided to me and improved the messages that would address my motivations and considerations. Thanks for the feedback.

Instead of changing the title of this pull request on GitHub, you should reword the commit message using git commit --amend, then force-push the updated commit.

After running gh pr checkout 92 followed by git show HEAD, I get the following one-line commit message:

fix: fix mtime from not updating correctly

This commit message violates the project's commit message guidelines

Thank you for the clarification. I misunderstood and thought you were referring to the pull request title and description.

I've amended my commit and pushed the updated commit. Sorry for the misunderstanding, and thank you for pointing it out.

@EricKim27
EricKim27 requested a review from jserv July 23, 2026 04:06
@jserv

jserv commented Jul 23, 2026

Copy link
Copy Markdown
Collaborator

I've amended my commit and pushed the updated commit. Sorry for the misunderstanding, and thank you for pointing it out.

Check https://cbea.ms/git-commit/ carefully and apply the rules. In particular,

@EricKim27

EricKim27 commented Jul 23, 2026

Copy link
Copy Markdown
Contributor Author

I've amended my commit and pushed the updated commit. Sorry for the misunderstanding, and thank you for pointing it out.

Check https://cbea.ms/git-commit/ carefully and apply the rules. In particular,

@jserv I wrapped the body and shortened the subject line and made some adjustments as you suggested. Thanks for the feedback.

simplefs_write_inode() incorrectly assigns the
modification time to the i_atime due to a typo.
As a result, disk_inode->i_atime field on disk contains
the modification time, while disk_inode->i_mtime is
never updated and remains zero.

Fix the timestamp assignments so that access time
is written to disk_inode->i_atime and modification
time is written to disk_inode->i_mtime.
@jserv
jserv merged commit e59b6a5 into sysprog21:master Jul 23, 2026
4 checks passed
@jserv

jserv commented Jul 23, 2026

Copy link
Copy Markdown
Collaborator

Thank @EricKim27 for contributing!

I amended the commit messages.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants