2016-09-22 01:10:43

by Eric Whitney

[permalink] [raw]
Subject: [PATCH 1/2] ext4: create function to read journal inode

Factor out the code used in ext4_get_journal() to read a valid journal
inode from storage, enabling its reuse in other functions.

Signed-off-by: Eric Whitney <[email protected]>
---
fs/ext4/super.c | 32 +++++++++++++++++++++++---------
1 file changed, 23 insertions(+), 9 deletions(-)

diff --git a/fs/ext4/super.c b/fs/ext4/super.c
index 50912cc..4b0ca25 100644
--- a/fs/ext4/super.c
+++ b/fs/ext4/super.c
@@ -78,6 +78,8 @@ static int ext4_feature_set_ok(struct super_block *sb, int readonly);
static void ext4_destroy_lazyinit_thread(void);
static void ext4_unregister_li_request(struct super_block *sb);
static void ext4_clear_request_list(void);
+static struct inode *ext4_get_journal_inode(struct super_block *sb,
+ unsigned int journal_inum);

/*
* Lock ordering
@@ -4237,18 +4239,16 @@ static void ext4_init_journal_params(struct super_block *sb, journal_t *journal)
write_unlock(&journal->j_state_lock);
}

-static journal_t *ext4_get_journal(struct super_block *sb,
- unsigned int journal_inum)
+static struct inode *ext4_get_journal_inode(struct super_block *sb,
+ unsigned int journal_inum)
{
struct inode *journal_inode;
- journal_t *journal;
-
- BUG_ON(!ext4_has_feature_journal(sb));
-
- /* First, test for the existence of a valid inode on disk. Bad
- * things happen if we iget() an unused inode, as the subsequent
- * iput() will try to delete it. */

+ /*
+ * Test for the existence of a valid inode on disk. Bad things
+ * happen if we iget() an unused inode, as the subsequent iput()
+ * will try to delete it.
+ */
journal_inode = ext4_iget(sb, journal_inum);
if (IS_ERR(journal_inode)) {
ext4_msg(sb, KERN_ERR, "no journal found");
@@ -4268,6 +4268,20 @@ static journal_t *ext4_get_journal(struct super_block *sb,
iput(journal_inode);
return NULL;
}
+ return journal_inode;
+}
+
+static journal_t *ext4_get_journal(struct super_block *sb,
+ unsigned int journal_inum)
+{
+ struct inode *journal_inode;
+ journal_t *journal;
+
+ BUG_ON(!ext4_has_feature_journal(sb));
+
+ journal_inode = ext4_get_journal_inode(sb, journal_inum);
+ if (!journal_inode)
+ return NULL;

journal = jbd2_journal_init_inode(journal_inode);
if (!journal) {
--
2.1.4



2016-09-22 15:03:56

by Jan Kara

[permalink] [raw]
Subject: Re: [PATCH 1/2] ext4: create function to read journal inode

On Wed 21-09-16 21:14:50, Eric Whitney wrote:
> Factor out the code used in ext4_get_journal() to read a valid journal
> inode from storage, enabling its reuse in other functions.
>
> Signed-off-by: Eric Whitney <[email protected]>

Looks good. You can add:

Reviewed-by: Jan Kara <[email protected]>

BTW: I didn't see PATCH 2/2?

Honza
> ---
> fs/ext4/super.c | 32 +++++++++++++++++++++++---------
> 1 file changed, 23 insertions(+), 9 deletions(-)
>
> diff --git a/fs/ext4/super.c b/fs/ext4/super.c
> index 50912cc..4b0ca25 100644
> --- a/fs/ext4/super.c
> +++ b/fs/ext4/super.c
> @@ -78,6 +78,8 @@ static int ext4_feature_set_ok(struct super_block *sb, int readonly);
> static void ext4_destroy_lazyinit_thread(void);
> static void ext4_unregister_li_request(struct super_block *sb);
> static void ext4_clear_request_list(void);
> +static struct inode *ext4_get_journal_inode(struct super_block *sb,
> + unsigned int journal_inum);
>
> /*
> * Lock ordering
> @@ -4237,18 +4239,16 @@ static void ext4_init_journal_params(struct super_block *sb, journal_t *journal)
> write_unlock(&journal->j_state_lock);
> }
>
> -static journal_t *ext4_get_journal(struct super_block *sb,
> - unsigned int journal_inum)
> +static struct inode *ext4_get_journal_inode(struct super_block *sb,
> + unsigned int journal_inum)
> {
> struct inode *journal_inode;
> - journal_t *journal;
> -
> - BUG_ON(!ext4_has_feature_journal(sb));
> -
> - /* First, test for the existence of a valid inode on disk. Bad
> - * things happen if we iget() an unused inode, as the subsequent
> - * iput() will try to delete it. */
>
> + /*
> + * Test for the existence of a valid inode on disk. Bad things
> + * happen if we iget() an unused inode, as the subsequent iput()
> + * will try to delete it.
> + */
> journal_inode = ext4_iget(sb, journal_inum);
> if (IS_ERR(journal_inode)) {
> ext4_msg(sb, KERN_ERR, "no journal found");
> @@ -4268,6 +4268,20 @@ static journal_t *ext4_get_journal(struct super_block *sb,
> iput(journal_inode);
> return NULL;
> }
> + return journal_inode;
> +}
> +
> +static journal_t *ext4_get_journal(struct super_block *sb,
> + unsigned int journal_inum)
> +{
> + struct inode *journal_inode;
> + journal_t *journal;
> +
> + BUG_ON(!ext4_has_feature_journal(sb));
> +
> + journal_inode = ext4_get_journal_inode(sb, journal_inum);
> + if (!journal_inode)
> + return NULL;
>
> journal = jbd2_journal_init_inode(journal_inode);
> if (!journal) {
> --
> 2.1.4
>
> --
> 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
--
Jan Kara <[email protected]>
SUSE Labs, CR

2016-09-22 16:34:38

by Eric Whitney

[permalink] [raw]
Subject: Re: [PATCH 1/2] ext4: create function to read journal inode

* Jan Kara <[email protected]>:
> On Wed 21-09-16 21:14:50, Eric Whitney wrote:
> > Factor out the code used in ext4_get_journal() to read a valid journal
> > inode from storage, enabling its reuse in other functions.
> >
> > Signed-off-by: Eric Whitney <[email protected]>
>
> Looks good. You can add:
>
> Reviewed-by: Jan Kara <[email protected]>
>
> BTW: I didn't see PATCH 2/2?
>

Yup, my mistake. I reposted it earlier today - hopefully it's now visible.

Thanks for the review!

Eric

2016-09-30 06:11:51

by Theodore Ts'o

[permalink] [raw]
Subject: Re: [PATCH 1/2] ext4: create function to read journal inode

On Wed, Sep 21, 2016 at 09:14:50PM -0400, Eric Whitney wrote:
> Factor out the code used in ext4_get_journal() to read a valid journal
> inode from storage, enabling its reuse in other functions.
>
> Signed-off-by: Eric Whitney <[email protected]>

Thanks, applied.

- Ted