Patchwork [1/2] JBD2: Allow feature checks before journal recovery

login
register
mail settings
Submitter Patrick J. LoPresti
Date July 11, 2010, 5:03 p.m.
Message ID <871vbax86w.fsf@patl.com>
Download mbox | patch
Permalink /patch/58528/
State Superseded
Headers show

Comments

Patrick J. LoPresti - July 11, 2010, 5:03 p.m.
Before we start accessing a huge (> 16 TiB) OCFS2 volume, we need to
confirm that its journal supports 64-bit offsets.  So we need to check
the journal's feature bits before recovering the journal.

This is not possible with JBD2 at present, because the journal
superblock (where the feature bits reside) is not loaded from disk until
the journal is recovered.

This patch loads the journal superblock in
jbd2_journal_check_used_features() if it has not already been loaded,
allowing us to check the feature bits before journal recovery.

Signed-off-by: Patrick LoPresti <lopresti@gmail.com>

--
To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Jan Kara - July 21, 2010, 5:27 p.m.
> Before we start accessing a huge (> 16 TiB) OCFS2 volume, we need to
> confirm that its journal supports 64-bit offsets.  So we need to check
> the journal's feature bits before recovering the journal.
> 
> This is not possible with JBD2 at present, because the journal
> superblock (where the feature bits reside) is not loaded from disk until
> the journal is recovered.
> 
> This patch loads the journal superblock in
> jbd2_journal_check_used_features() if it has not already been loaded,
> allowing us to check the feature bits before journal recovery.
> 
> Signed-off-by: Patrick LoPresti <lopresti@gmail.com>
> 
> diff --git a/fs/jbd2/journal.c b/fs/jbd2/journal.c
> index bc2ff59..c5a864f 100644
> --- a/fs/jbd2/journal.c
> +++ b/fs/jbd2/journal.c
> @@ -1365,6 +1365,8 @@ int jbd2_journal_check_used_features (journal_t *journal, unsigned long compat,
>  
>  	if (!compat && !ro && !incompat)
>  		return 1;
> +	if (journal_get_superblock(journal))
> +		return 0;
>  	if (journal->j_format_version == 1)
>  		return 0;
  This looks OK in principle. It would be even nicer to avoid all the checks
journal_get_superblock() when the superblock is actually loaded so that we
don't do them each time jbd2_journal_check_used_features is called...

								Honza
Patrick J. LoPresti - July 21, 2010, 5:42 p.m.
On Wed, Jul 21, 2010 at 10:27 AM, Jan Kara <jack@suse.cz> wrote:
>> Signed-off-by: Patrick LoPresti <lopresti@gmail.com>
>>
>> diff --git a/fs/jbd2/journal.c b/fs/jbd2/journal.c
>> index bc2ff59..c5a864f 100644
>> --- a/fs/jbd2/journal.c
>> +++ b/fs/jbd2/journal.c
>> @@ -1365,6 +1365,8 @@ int jbd2_journal_check_used_features (journal_t *journal, unsigned long compat,
>>
>>       if (!compat && !ro && !incompat)
>>               return 1;
>> +     if (journal_get_superblock(journal))
>> +             return 0;
>>       if (journal->j_format_version == 1)
>>               return 0;
>
> This looks OK in principle. It would be even nicer to avoid all the checks
> journal_get_superblock() when the superblock is actually loaded so that we
> don't do them each time jbd2_journal_check_used_features is called...

How about this?

         if (!compat && !ro && !incompat)
                return 1;
+        if (journal->j_format_version == 0 &&
journal_get_superblock(journal) != 0)
+               return 0;
        if (journal->j_format_version == 1)
                return 0;

journal_init_common() uses kzalloc() to allocate the journal_t, and
journal_get_superblock() fills it in, so I believe this is a valid
test.

 - Pat
--
To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Jan Kara - July 21, 2010, 5:50 p.m.
> On Wed, Jul 21, 2010 at 10:27 AM, Jan Kara <jack@suse.cz> wrote:
> >> Signed-off-by: Patrick LoPresti <lopresti@gmail.com>
> >>
> >> diff --git a/fs/jbd2/journal.c b/fs/jbd2/journal.c
> >> index bc2ff59..c5a864f 100644
> >> --- a/fs/jbd2/journal.c
> >> +++ b/fs/jbd2/journal.c
> >> @@ -1365,6 +1365,8 @@ int jbd2_journal_check_used_features (journal_t *journal, unsigned long compat,
> >>
> >>       if (!compat && !ro && !incompat)
> >>               return 1;
> >> +     if (journal_get_superblock(journal))
> >> +             return 0;
> >>       if (journal->j_format_version == 1)
> >>               return 0;
> >
> > This looks OK in principle. It would be even nicer to avoid all the checks
> > journal_get_superblock() when the superblock is actually loaded so that we
> > don't do them each time jbd2_journal_check_used_features is called...
> 
> How about this?
> 
>          if (!compat && !ro && !incompat)
>                 return 1;
> +        if (journal->j_format_version == 0 &&
> journal_get_superblock(journal) != 0)
> +               return 0;
>         if (journal->j_format_version == 1)
>                 return 0;
> 
> journal_init_common() uses kzalloc() to allocate the journal_t, and
> journal_get_superblock() fills it in, so I believe this is a valid
> test.
  Yes, this looks OK. Just add a comment before the check like
/* Load journal super block if it isn't loaded yet */

								Honza

Patch

diff --git a/fs/jbd2/journal.c b/fs/jbd2/journal.c
index bc2ff59..c5a864f 100644
--- a/fs/jbd2/journal.c
+++ b/fs/jbd2/journal.c
@@ -1365,6 +1365,8 @@  int jbd2_journal_check_used_features (journal_t *journal, unsigned long compat,
 
 	if (!compat && !ro && !incompat)
 		return 1;
+	if (journal_get_superblock(journal))
+		return 0;
 	if (journal->j_format_version == 1)
 		return 0;