Received-SPF: pass (google.com: best guess record for domain of linux-ext4-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
Date:   Wed, 16 Oct 2019 13:30:39 -0400
From:   "Theodore Y. Ts'o" <tytso@mit.edu>
To:     Harshad Shirwadkar <harshadshirwadkar@gmail.com>
Cc:     linux-ext4@vger.kernel.org
Subject: Re: [PATCH v3 05/13] jbd2: fast-commit recovery path changes
Message-ID: <20191016173039.GE11103@mit.edu>
References: <20191001074101.256523-1-harshadshirwadkar@gmail.com>
 <20191001074101.256523-6-harshadshirwadkar@gmail.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20191001074101.256523-6-harshadshirwadkar@gmail.com>
User-Agent: Mutt/1.10.1 (2018-07-13)
Sender: linux-ext4-owner@vger.kernel.org
Precedence: bulk

On Tue, Oct 01, 2019 at 12:40:54AM -0700, Harshad Shirwadkar wrote:
> diff --git a/fs/jbd2/journal.c b/fs/jbd2/journal.c
> index 14d549445418..e0684212384d 100644
> --- a/fs/jbd2/journal.c
> +++ b/fs/jbd2/journal.c
>  
>  	jbd2_write_superblock(journal, write_op);
>  
> +	if (had_fast_commit)
> +		jbd2_set_feature_fast_commit(journal);
> +

Why the logic with had_fast_commit and (re-)setting the fast commit
feature flag?

This ties back to how we handle the logic around setting the fast
commit flag if requested by the file system....

> @@ -768,6 +816,8 @@ static int do_one_pass(journal_t *journal,
>  			if (err)
>  				goto failed;
>  			continue;
> +		case JBD2_FC_BLOCK:
> +			continue;

Why should a Fast Commit block ever show up in the primary part of the
journal?   It should never happen, right?

In which case, we should probably at least issue a warning, and not
just skip the block.

					- Ted