On Sun 18-08-24 04:03:55, Harshad Shirwadkar wrote:
> Fast commit by default makes every inode on which
> ext4_reserve_inode_write() is called. Thus, if that function
> fails for some reason, make the next fast commit ineligible.
>
> Signed-off-by: Harshad Shirwadkar <harshadshirwadkar@xxxxxxxxx>
I think this is a bit pointless. If ext4_reserve_inode_write() fails, we
have a data corruption problems, journal has aborted or similar. Thus I
think data consistency of fsync is kind of the least problem we are having
:). That being said just completely turning of fastcommit as soon as we hit
some filesystem error (in __ext4_std_error() when we don't decide to panic
system / shutdown the filesystem) makes sense to me as a kind of "let's
limit possible damage" measure.
Honza
> ---
> fs/ext4/fast_commit.c | 1 +
> fs/ext4/fast_commit.h | 1 +
> fs/ext4/inode.c | 29 ++++++++++++++++++-----------
> include/trace/events/ext4.h | 7 +++++--
> 4 files changed, 25 insertions(+), 13 deletions(-)
>
> diff --git a/fs/ext4/fast_commit.c b/fs/ext4/fast_commit.c
> index 2fc43b1e2..7525450f1 100644
> --- a/fs/ext4/fast_commit.c
> +++ b/fs/ext4/fast_commit.c
> @@ -2282,6 +2282,7 @@ static const char * const fc_ineligible_reasons[] = {
> [EXT4_FC_REASON_FALLOC_RANGE] = "Falloc range op",
> [EXT4_FC_REASON_INODE_JOURNAL_DATA] = "Data journalling",
> [EXT4_FC_REASON_ENCRYPTED_FILENAME] = "Encrypted filename",
> + [EXT4_FC_REASON_INODE_RSV_WRITE_FAIL] = "Inode reserve write failure"
> };
>
> int ext4_fc_info_show(struct seq_file *seq, void *v)
> diff --git a/fs/ext4/fast_commit.h b/fs/ext4/fast_commit.h
> index 2fadb2c47..f7f85c3dd 100644
> --- a/fs/ext4/fast_commit.h
> +++ b/fs/ext4/fast_commit.h
> @@ -97,6 +97,7 @@ enum {
> EXT4_FC_REASON_FALLOC_RANGE,
> EXT4_FC_REASON_INODE_JOURNAL_DATA,
> EXT4_FC_REASON_ENCRYPTED_FILENAME,
> + EXT4_FC_REASON_INODE_RSV_WRITE_FAIL,
> EXT4_FC_REASON_MAX
> };
>
> diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c
> index c82eba178..5a187902b 100644
> --- a/fs/ext4/inode.c
> +++ b/fs/ext4/inode.c
> @@ -5798,20 +5798,27 @@ ext4_reserve_inode_write(handle_t *handle, struct inode *inode,
> {
> int err;
>
> - if (unlikely(ext4_forced_shutdown(inode->i_sb)))
> - return -EIO;
> + if (unlikely(ext4_forced_shutdown(inode->i_sb))) {
> + err = -EIO;
> + goto out;
> + }
>
> err = ext4_get_inode_loc(inode, iloc);
> - if (!err) {
> - BUFFER_TRACE(iloc->bh, "get_write_access");
> - err = ext4_journal_get_write_access(handle, inode->i_sb,
> - iloc->bh, EXT4_JTR_NONE);
> - if (err) {
> - brelse(iloc->bh);
> - iloc->bh = NULL;
> - }
> - ext4_fc_track_inode(handle, inode);
> + if (err)
> + goto out;
> +
> + BUFFER_TRACE(iloc->bh, "get_write_access");
> + err = ext4_journal_get_write_access(handle, inode->i_sb,
> + iloc->bh, EXT4_JTR_NONE);
> + if (err) {
> + brelse(iloc->bh);
> + iloc->bh = NULL;
> }
> + ext4_fc_track_inode(handle, inode);
> +out:
> + if (err)
> + ext4_fc_mark_ineligible(inode->i_sb,
> + EXT4_FC_REASON_INODE_RSV_WRITE_FAIL, handle);
> ext4_std_error(inode->i_sb, err);
> return err;
> }
> diff --git a/include/trace/events/ext4.h b/include/trace/events/ext4.h
> index cc5e9b7b2..8bab4febd 100644
> --- a/include/trace/events/ext4.h
> +++ b/include/trace/events/ext4.h
> @@ -105,6 +105,7 @@ TRACE_DEFINE_ENUM(EXT4_FC_REASON_RENAME_DIR);
> TRACE_DEFINE_ENUM(EXT4_FC_REASON_FALLOC_RANGE);
> TRACE_DEFINE_ENUM(EXT4_FC_REASON_INODE_JOURNAL_DATA);
> TRACE_DEFINE_ENUM(EXT4_FC_REASON_ENCRYPTED_FILENAME);
> +TRACE_DEFINE_ENUM(EXT4_FC_REASON_INODE_RSV_WRITE_FAIL);
> TRACE_DEFINE_ENUM(EXT4_FC_REASON_MAX);
>
> #define show_fc_reason(reason) \
> @@ -118,7 +119,8 @@ TRACE_DEFINE_ENUM(EXT4_FC_REASON_MAX);
> { EXT4_FC_REASON_RENAME_DIR, "RENAME_DIR"}, \
> { EXT4_FC_REASON_FALLOC_RANGE, "FALLOC_RANGE"}, \
> { EXT4_FC_REASON_INODE_JOURNAL_DATA, "INODE_JOURNAL_DATA"}, \
> - { EXT4_FC_REASON_ENCRYPTED_FILENAME, "ENCRYPTED_FILENAME"})
> + { EXT4_FC_REASON_ENCRYPTED_FILENAME, "ENCRYPTED_FILENAME"}, \
> + { EXT4_FC_REASON_INODE_RSV_WRITE_FAIL, "INODE_RSV_WRITE_FAIL"})
>
> TRACE_DEFINE_ENUM(CR_POWER2_ALIGNED);
> TRACE_DEFINE_ENUM(CR_GOAL_LEN_FAST);
> @@ -2809,7 +2811,7 @@ TRACE_EVENT(ext4_fc_stats,
> ),
>
> TP_printk("dev %d,%d fc ineligible reasons:\n"
> - "%s:%u, %s:%u, %s:%u, %s:%u, %s:%u, %s:%u, %s:%u, %s:%u, %s:%u, %s:%u"
> + "%s:%u, %s:%u, %s:%u, %s:%u, %s:%u, %s:%u, %s:%u, %s:%u, %s:%u, %s:%u, %s:%u"
> "num_commits:%lu, ineligible: %lu, numblks: %lu",
> MAJOR(__entry->dev), MINOR(__entry->dev),
> FC_REASON_NAME_STAT(EXT4_FC_REASON_XATTR),
> @@ -2822,6 +2824,7 @@ TRACE_EVENT(ext4_fc_stats,
> FC_REASON_NAME_STAT(EXT4_FC_REASON_FALLOC_RANGE),
> FC_REASON_NAME_STAT(EXT4_FC_REASON_INODE_JOURNAL_DATA),
> FC_REASON_NAME_STAT(EXT4_FC_REASON_ENCRYPTED_FILENAME),
> + FC_REASON_NAME_STAT(EXT4_FC_REASON_INODE_RSV_WRITE_FAIL),
> __entry->fc_commits, __entry->fc_ineligible_commits,
> __entry->fc_numblks)
> );
> --
> 2.46.0.184.g6999bdac58-goog
>
--
Jan Kara <jack@xxxxxxxx>
SUSE Labs, CR
