On Mon, 3 Nov 2008 12:14:28 -0800
Arthur Jones <ajones@xxxxxxxxxxxx> wrote:
> Hi Andrew, ...
>
> On Mon, Nov 03, 2008 at 11:33:18AM -0800, Andrew Morton wrote:
> > [...]
> > > --- a/fs/ext3/super.c
> > > +++ b/fs/ext3/super.c
> > > @@ -2392,7 +2392,13 @@ static int ext3_sync_fs(struct super_block *sb, int wait)
> > > if (journal_start_commit(EXT3_SB(sb)->s_journal, &target)) {
> > > if (wait)
> > > log_wait_commit(EXT3_SB(sb)->s_journal, target);
> > > - }
> > > + } else if (wait)
> > > + /*
> > > + * We may have a commit in progress, clear it out
> > > + * before we go on...
> > > + */
> > > + ext3_force_commit(sb);
> > > +
> > > return 0;
> > > }
> >
> > Can we do
> >
> > sb->s_dirt = 0;
> > if (wait)
> > ext3_force_commit(...);
> > else
> > journal_start_commit(...);
>
> I think this is what you had in mind:
yup.
>
> diff --git a/fs/ext3/super.c b/fs/ext3/super.c
> index 18eaa78..2b48b85 100644
> --- a/fs/ext3/super.c
> +++ b/fs/ext3/super.c
> @@ -2386,13 +2386,12 @@ static void ext3_write_super (struct super_block * sb)
>
> static int ext3_sync_fs(struct super_block *sb, int wait)
> {
> - tid_t target;
> -
> sb->s_dirt = 0;
> - if (journal_start_commit(EXT3_SB(sb)->s_journal, &target)) {
> - if (wait)
> - log_wait_commit(EXT3_SB(sb)->s_journal, target);
> - }
> + if (wait)
> + ext3_force_commit(sb);
> + else
> + journal_start_commit(EXT3_SB(sb)->s_journal, NULL);
> +
> return 0;
> }
>
> I tried this and it too fixes the problem. FWIW I agree it
> looks better...
>
OK. But still, questions remain.
- should we clear s_dirt if log_wait_commit() didn't work?
- ext3_force_commit() clears s_dirt also
- should ext3_sync_fs() be dropping the ext3_force_commit() return
value on the floor?
This is all rather worrisome.
--
To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
