No-journal option for ext4

No-journal option for ext4

[Peter Kukol]

Due to some interest in having an ext4 option not to use a journal at
all (for various reasons that I'd rather not get into right now), I've
been looking at adding an option to mount and use a volume as ext4
without a journal present, and I'm wondering if folks would be willing
to chime in to help direct this effort (for one thing, I'm fairly new
to the ext4 code base so I can certainly use all the help and guidance
I can get ;-). The way I've gone about this so far is as follows.

To start with, I added an "incompat" option to ext4 allow the journal
not to be present - when the right option string is given at mount
time, the journaling functionality is disabled. The exact naming and
mechanism by which this happens  actually doesn't seem terribly
important to the real "guts" of the making the feature work, but if
anyone feels strongly that it should be done in a certain way please
do let me know.

My first attempt at actually implementing the no-journal option was to
disable all of the journaling logic "early" - i.e. not create handles
and journal instances, and so on. This ran into a bit of trouble in
the handful of journaling functions that only accept handles and have
no obvious way of determining whether journaling is disabled or not
(other than the handle being NULL, which seems like a poor
determinant). In addition to this, I ran into a few places where tests
for a handle being non-NULL guard code that is needed even when
journaling is disabled.

Undeterred, in my next attempt I changed the code to create the
journal instance and allocate handles and transactions and all that,
since this way a handle is always good enough to determine whether
"real" journaling is to happen or not; incidentally, this also allows
the "no-journal" flag to go in the journal superblock which seems like
a pretty good place to keep it. Sadly, this second approach ran into
even more trouble: basically, the number of places where the "are we
really journaling or just pretending to" flag had to be tested seemed
excessive and all the changes all too intrusive (not to mention that
it seems a bit silly to create all those data structures needed only
for the journal and then not do any journaling). So, in the end, I
went back to disabling journal-only functionality "early", i.e.
typically at the points where the ext4_ or jbd2_ functions are called
in ext4 code, and storing the "no-journal" bit in the superblock (not
the journal superblock but the "main" sb of the file system).

One effect of doing things this way is that there are quite a number
of places in the ext4 code that now need to test the "are we really
journaling" bit, though having tried the obvious alternative described
above (i.e. pushing the "real journal" tests farther into the journal
code itself), I think that on balance this seems like a reasonable
approach. By now I've added the necessary tests to enough ext4 code to
be able to mount a volume with no journal and create directories and
files, but there are plenty more places that still need to check the
bit and not call the journal code when it's disabled. To show a
specific example of doing this, the following is my current diff for
mballoc.c:

diff 2.6.26/fs/ext4/mballoc.c noj/2.6.26/fs/ext4/mballoc.c
2906,2908c2906,2910
<     err = ext4_journal_get_write_access(handle, bitmap_bh);
<     if (err)
<         goto out_err;
---
>     if (!EXT4_HAS_INCOMPAT_FEATURE(sb, EXT4_FEATURE_INCOMPAT_NULL_JOURNAL)) {
>         err = ext4_journal_get_write_access(handle, bitmap_bh);
>         if (err)
>             goto out_err;
>     }
2918,2920c2920,2924
<     err = ext4_journal_get_write_access(handle, gdp_bh);
<     if (err)
<         goto out_err;
---
>     if (!EXT4_HAS_INCOMPAT_FEATURE(sb, EXT4_FEATURE_INCOMPAT_NULL_JOURNAL)) {
>         err = ext4_journal_get_write_access(handle, gdp_bh);
>         if (err)
>             goto out_err;
>     }
2943,2945c2947,2951
<         err = ext4_journal_dirty_metadata(handle, bitmap_bh);
<         if (!err)
<             err = -EAGAIN;
---
>         if (!EXT4_HAS_INCOMPAT_FEATURE(sb, EXT4_FEATURE_INCOMPAT_NULL_JOURNAL)) {
>             err = ext4_journal_dirty_metadata(handle, bitmap_bh);
>             if (!err)
>                 err = -EAGAIN;
>         }
2989,2992c2995,3001
<     err = ext4_journal_dirty_metadata(handle, bitmap_bh);
<     if (err)
<         goto out_err;
<     err = ext4_journal_dirty_metadata(handle, gdp_bh);
---
>     if (!EXT4_HAS_INCOMPAT_FEATURE(sb, EXT4_FEATURE_INCOMPAT_NULL_JOURNAL)) {
>         err = ext4_journal_dirty_metadata(handle, bitmap_bh);
>         if (err)
>             goto out_err;
>         err = ext4_journal_dirty_metadata(handle, gdp_bh);
>     } else
>         err = 0;

Of course, the rather long-winded test for the no-journal bit should
probably be wrapped in a simple macro, but the main point is that
explicit checks are required in quite a few places. The few locations
in the code where a NULL handle currently causes code needed even with
no journal to execute are simply changed to check for NULL handle
*and* the new no-journal bit not being set, which doesn't seem too
bad.

The bottom line is, I'm wondering if folks are interested in having an
option to disable journaling in ext4 to begin with, and if the above
sounds like the right approach or if there might be a better way that
I've overlooked. I've done enough work in this direction (I think) to
verify that it's likely doable and not terribly ugly or burdensome,
but before I continue to change all the remaining places that need to
be no-journal-aware I'd like to check with folks to see if perhaps
there might be a simpler / more effective approach. BTW, the complete
diff at this point a few hundred lines long and I'd be happy to post
it to this list or e-mail it to folks who'd like to see more of it.

Thanks!

PeterK

Re: No-journal option for ext4

[Theodore Tso]

On Mon, Oct 06, 2008 at 06:39:50PM -0700, Peter Kukol wrote:
> Due to some interest in having an ext4 option not to use a journal at
> all (for various reasons that I'd rather not get into right now), 

That's OK, I think most of us know why already.  :-)

> To start with, I added an "incompat" option to ext4 allow the journal
> not to be present - when the right option string is given at mount
> time, the journaling functionality is disabled. The exact naming and
> mechanism by which this happens  actually doesn't seem terribly
> important to the real "guts" of the making the feature work, but if
> anyone feels strongly that it should be done in a certain way please
> do let me know.

Um, no.  You shouldn't need an mount option string, and there is
already a compat flag which indicates that a journal is present.
Namely, EXT4_FEATURE_COMPAT_HAS_JOURNAL; this is the feature flag
which indicates that journal is present.  Currently, both the ext3 and
ext4 filesystems require that this flag be set; if the flag is not
set, indicating that a journal is not present, ext4 will fail the
mount.

So what your patches should do is to enhance the ext4 filesystem so
that it can mount filesystems that don't have the COMPAT has_journal
feature.  This has the nice side-effect of allowing the ext4
filesystem code to be able to mount not only ext3 filesystems, but
also ext2 filesystems.

> My first attempt at actually implementing the no-journal option was to
> disable all of the journaling logic "early" - i.e. not create handles
> and journal instances, and so on. This ran into a bit of trouble in
> the handful of journaling functions that only accept handles and have
> no obvious way of determining whether journaling is disabled or not
> (other than the handle being NULL, which seems like a poor
> determinant). In addition to this, I ran into a few places where tests
> for a handle being non-NULL guard code that is needed even when
> journaling is disabled....

Yeah, the problem is that in some cases a NULL handle can mean that
there is an error.  Here's one idea:

#define NO_HANDLE ((handle_t *) ~0)
#define VALID_HANDLE(x) ((x != 0) && (x != NO_HANDLE))

So you can distinguish between NULL handle (which could be from an
error) and NO_HANDLE (which means there's no journal).

Yeah, it's a little gross, but this sort of game has been played
before --- see how SIG_IGN is defined for sigaction().

> So, in the end, I
> went back to disabling journal-only functionality "early", i.e.
> typically at the points where the ext4_ or jbd2_ functions are called
> in ext4 code, and storing the "no-journal" bit in the superblock (not
> the journal superblock but the "main" sb of the file system).

As mentioned above, we already have a "has journal" bit in the
superblock, but in many places, it will be much more efficient to test
for a null s_journal field in the ext4_sb_info structure.  There is no
error handling we need to worry about, since today, s_journal is
assumed to ALWAYS be non-NULL, as it is how we access the core jbd2
structure.  So simply checking for !EXT4_SB(sb)->s_journal should be
much more easier to read, type, and more efficient to execute.

So for example, ext4_force_commit() might look like this:

int ext4_force_commit(struct super_block *sb)
{
	journal_t *journal = EXT4_SB(sb)->s_journal;;
	int ret;

	if ((sb->s_flags & MS_RDONLY) || !journal)
	   return 0;

	sb->s_dirt = 0;
	return ext4_journal_force_commit(journal);
}

and once you set up ext4_journal_start_sb() to start like this:


handle_t *ext4_journal_start_sb(struct super_block *sb, int nblocks)
{
	journal_t *journal = EXT4_SB(sb)->s_journal;

	if (sb->s_flags & MS_RDONLY)
	   return ERR_PTR(-EROFS);

	if (!journal)
	   return NO_HANDLE;

	   ...
}

and you can just arrange to have __ext4_journal_get_write_access()
look like this:

#define ext4_journal_get_undo_access(handle, bh) \
	if ((handle) != NO_HANDLE) __ext4_journal_get_undo_access(__func__, (handle), (bh))

This should keep the patch relatively small, easier to audit, and the
code will be much cleaner and easier to understand in the long-run.

Given all of these benefits, I think the tiny grossness of casting ~0
or 1 to a pointer is well worth it, don't you think?  :-)

> The bottom line is, I'm wondering if folks are interested in having an
> option to disable journaling in ext4 to begin with, 

Yep, we're interested; especially since this allows the ext4
filesystem to be a full functional superset of the ext2 and ext3
filesystem code, which is quite pleasing from an aesthetic point of
view.

I hope the suggestions given above are helpful, and should hopefully
provide for much cleaner approach.

Regards,

						- Ted

Re: No-journal option for ext4

[Peter Kukol]

Thanks for the great suggestions - they all make perfect sense. I did
see the "journal present" flag, BTW, but figured that it might be
safer not to change existing behavior - e.g. when someone accidentally
says ext4 instead of ext2 when mounting an ext2 volume, right now they
get an error. If folks are OK with this changing to a successful
mount, I'm happy - less code/work is good ;-).

I'll rework my patch and send out an update when it's ready.

Thanks again!

PeterK

Re: No-journal option for ext4

[Eric Sandeen]

Peter Kukol wrote:
> Thanks for the great suggestions - they all make perfect sense. I did
> see the "journal present" flag, BTW, but figured that it might be
> safer not to change existing behavior - e.g. when someone accidentally
> says ext4 instead of ext2 when mounting an ext2 volume, right now they
> get an error. If folks are OK with this changing to a successful
> mount, I'm happy - less code/work is good ;-).

As long as we've purged all kernelspace flag updates it's probably ok to
"accidentally" mount ext2 (or ext3) as ext4 - as long as it's really
easy to go back.  It'd be worth one more look though in the long run.

Early on I had some nasty surprises with ext4 "assimilating" my ext3
RHEL5 root disk :)

-Eric

> I'll rework my patch and send out an update when it's ready.
> 
> Thanks again!
> 
> PeterK
> --
> 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