Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Mount the external journal device and write accessible extent metadata #59

Closed
wants to merge 5 commits into from

Conversation

jason50123
Copy link
Contributor

This pull request currently adds the parse_option in fill super to mount the external journal device to simplefs. It also writes the currently accessible extent metadata to the journal during write operations. The image below demonstrates that it is already possible to mount the external journal device in the v6.8 environment.

  • Implement external journal device initialization
  • Add journal to write function

螢幕快照 2024-06-20 10-17-03

During fill super, call simplefs_parse_options to find the external
device path and set it as the journal device. Additionally, add the
`make journal` parameter in the Makefile to create an external journal
device.
In the current design of simplefs, we can only access the extent
metadata and perform the 'journal' action on it. To write inode and
other metadata, we might need to emulate `ext4_dirty_inode`.
This method primarily uses the content of struct ext4_iloc to find the
correct location of the inode on disk and records it in the journal.
However, in our current situation, simply using `mark_inode_dirty` to
handle dirty inodes does not allow us to obtain the inode's location,
thus preventing us from recording it in the journal.

But currently, the idea is that we might create a new buffer head, use
`jbd2_journal_get_create_access` to put the dirty inode in it, and then
use `jbd2_journal_dirty_metadata` to write it to the journal.
@jserv
Copy link
Collaborator

jserv commented Jun 20, 2024

Avoid uploading screenshots that contain only text. Instead, use Markdown formatting to present command lists, source code, and any text-based content. This approach is more accessible for individuals with visual impairments.

@jserv jserv requested a review from HotMercury June 20, 2024 03:20
file.c Outdated Show resolved Hide resolved
@@ -196,6 +201,251 @@ static int simplefs_statfs(struct dentry *dentry, struct kstatfs *stat)
return 0;
}

/* journal relation code */

Copy link
Collaborator

Choose a reason for hiding this comment

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

Remove one blank line.

super.c Outdated Show resolved Hide resolved
super.c Outdated Show resolved Hide resolved
super.c Outdated Show resolved Hide resolved
Copy link
Collaborator

@jserv jserv left a comment

Choose a reason for hiding this comment

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

Append the test suite to reflect journaling support.

@jserv
Copy link
Collaborator

jserv commented Jun 20, 2024

Drop unnecessary pr_info statements.

@jserv
Copy link
Collaborator

jserv commented Jun 20, 2024

Fix compatibility with Linux v6.5:

simplefs/super.c: In function ‘simplefs_get_journal_blkdev’:
simplefs/super.c:221:19: error: implicit declaration of function ‘bdev_open_by_dev’; did you mean ‘blkdev_get_by_dev’? [-Werror=implicit-function-declaration]
  221 |     bdev_handle = bdev_open_by_dev(
      |                   ^~~~~~~~~~~~~~~~
      |                   blkdev_get_by_dev
simplefs/super.c:222:48: error: ‘BLK_OPEN_RESTRICT_WRITES’ undeclared (first use in this function)
  222 |         jdev, BLK_OPEN_READ | BLK_OPEN_WRITE | BLK_OPEN_RESTRICT_WRITES, sb,
      |                                                ^~~~~~~~~~~~~~~~~~~~~~~~

@jserv jserv changed the title External journal device initial and have some thoughts with writing metadata with journal Mount the external journal device and write accessible extent metadata Jun 20, 2024
Based on feedback from previous pull requests, addressed the mentioned
issues and ensured that the related external device journal loading
function operates correctly in kernel version 6.5.
@jason50123
Copy link
Contributor Author

The latest commit has addressed the mentioned issues.
Evidence of the journal functioning correctly will be provided later.

Copy link
Collaborator

@jserv jserv left a comment

Choose a reason for hiding this comment

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

Use git rebase -i to rework the commits.

@@ -332,8 +401,7 @@ static int simplefs_load_journal(struct super_block *sb,
struct simplefs_sb_info *sbi = SIMPLEFS_SB(sb);
dev_t journal_dev;
int err = 0;
pr_info("simplefs_load_journal: loading journal from device number %lu\n",
journal_devnum);

Copy link
Collaborator

Choose a reason for hiding this comment

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

Remove this blank line.

Copy link
Collaborator

@jserv jserv left a comment

Choose a reason for hiding this comment

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

Ensure that journal device can be mounted for Linux kernels prior to v6.5. At least, tweak for version 6.1, 5.15, and 5.10.
See https://kernel.org/ for LTS.

@jason50123
Copy link
Contributor Author

@jserv Could you please clarify if you would like me to rebase all the commits into a single commit, or if there is a specific way you prefer me to rework them? Thank you!

@jserv
Copy link
Collaborator

jserv commented Jun 22, 2024

@jserv Could you please clarify if you would like me to rebase all the commits into a single commit, or if there is a specific way you prefer me to rework them? Thank you!

There should be only one commit rebasing the latest master branch for reviewing. See https://git-scm.com/docs/git-rebase

@jserv jserv requested a review from RoyWFHuang June 22, 2024 06:36
@jason50123 jason50123 closed this by deleting the head repository Jun 22, 2024
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.

None yet

2 participants