linux-ext4.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Jan Kara <jack@suse.cz>
To: Harshad Shirwadkar <harshadshirwadkar@gmail.com>
Cc: linux-ext4@vger.kernel.org, tytso@mit.edu, jack@suse.cz
Subject: Re: [PATCH 09/10] ext4: disable fast commit with data journalling
Date: Tue, 3 Nov 2020 18:01:00 +0100	[thread overview]
Message-ID: <20201103170100.GM3440@quack2.suse.cz> (raw)
In-Reply-To: <20201031200518.4178786-10-harshadshirwadkar@gmail.com>

On Sat 31-10-20 13:05:17, Harshad Shirwadkar wrote:
> Fast commits don't work with data journalling. This patch disables the
> fast commit support when data journalling is turned on.
> 
> Suggested-by: Jan Kara <jack@suse.cz>
> Signed-off-by: Harshad Shirwadkar <harshadshirwadkar@gmail.com>

The patch looks good to me. You can add:

Reviewed-by: Jan Kara <jack@suse.cz>

								Honza

> ---
>  fs/ext4/fast_commit.c       | 7 +++++++
>  fs/ext4/fast_commit.h       | 1 +
>  fs/ext4/super.c             | 3 ++-
>  include/trace/events/ext4.h | 6 ++++--
>  4 files changed, 14 insertions(+), 3 deletions(-)
> 
> diff --git a/fs/ext4/fast_commit.c b/fs/ext4/fast_commit.c
> index 4c0a3e858ea3..9ae8ba213961 100644
> --- a/fs/ext4/fast_commit.c
> +++ b/fs/ext4/fast_commit.c
> @@ -472,6 +472,12 @@ void ext4_fc_track_inode(handle_t *handle, struct inode *inode)
>  	if (S_ISDIR(inode->i_mode))
>  		return;
>  
> +	if (ext4_should_journal_data(inode)) {
> +		ext4_fc_mark_ineligible(inode->i_sb,
> +					EXT4_FC_REASON_INODE_JOURNAL_DATA);
> +		return;
> +	}
> +
>  	ret = ext4_fc_track_template(handle, inode, __track_inode, NULL, 1);
>  	trace_ext4_fc_track_inode(inode, ret);
>  }
> @@ -2095,6 +2101,7 @@ const char *fc_ineligible_reasons[] = {
>  	"Resize",
>  	"Dir renamed",
>  	"Falloc range op",
> +	"Data journalling",
>  	"FC Commit Failed"
>  };
>  
> diff --git a/fs/ext4/fast_commit.h b/fs/ext4/fast_commit.h
> index cde86747faf8..cdb36a10dfd0 100644
> --- a/fs/ext4/fast_commit.h
> +++ b/fs/ext4/fast_commit.h
> @@ -105,6 +105,7 @@ enum {
>  	EXT4_FC_REASON_RESIZE,
>  	EXT4_FC_REASON_RENAME_DIR,
>  	EXT4_FC_REASON_FALLOC_RANGE,
> +	EXT4_FC_REASON_INODE_JOURNAL_DATA,
>  	EXT4_FC_COMMIT_FAILED,
>  	EXT4_FC_REASON_MAX
>  };
> diff --git a/fs/ext4/super.c b/fs/ext4/super.c
> index e67d2fa41a78..9333475737ac 100644
> --- a/fs/ext4/super.c
> +++ b/fs/ext4/super.c
> @@ -4340,9 +4340,10 @@ static int ext4_fill_super(struct super_block *sb, void *data, int silent)
>  #endif
>  
>  	if (test_opt(sb, DATA_FLAGS) == EXT4_MOUNT_JOURNAL_DATA) {
> -		printk_once(KERN_WARNING "EXT4-fs: Warning: mounting with data=journal disables delayed allocation, dioread_nolock, and O_DIRECT support!\n");
> +		printk_once(KERN_WARNING "EXT4-fs: Warning: mounting with data=journal disables delayed allocation, dioread_nolock, O_DIRECT and fast_commit support!\n");
>  		/* can't mount with both data=journal and dioread_nolock. */
>  		clear_opt(sb, DIOREAD_NOLOCK);
> +		clear_opt2(sb, JOURNAL_FAST_COMMIT);
>  		if (test_opt2(sb, EXPLICIT_DELALLOC)) {
>  			ext4_msg(sb, KERN_ERR, "can't mount with "
>  				 "both data=journal and delalloc");
> diff --git a/include/trace/events/ext4.h b/include/trace/events/ext4.h
> index 239e98014f9b..ee7362f31eb6 100644
> --- a/include/trace/events/ext4.h
> +++ b/include/trace/events/ext4.h
> @@ -104,7 +104,8 @@ TRACE_DEFINE_ENUM(ES_REFERENCED_B);
>  		{ EXT4_FC_REASON_SWAP_BOOT,	"SWAP_BOOT"},		\
>  		{ EXT4_FC_REASON_RESIZE,	"RESIZE"},		\
>  		{ EXT4_FC_REASON_RENAME_DIR,	"RENAME_DIR"},		\
> -		{ EXT4_FC_REASON_FALLOC_RANGE,	"FALLOC_RANGE"})
> +		{ EXT4_FC_REASON_FALLOC_RANGE,	"FALLOC_RANGE"},	\
> +		{ EXT4_FC_REASON_INODE_JOURNAL_DATA,	"INODE_JOURNAL_DATA"})
>  
>  TRACE_EVENT(ext4_other_inode_update_time,
>  	TP_PROTO(struct inode *inode, ino_t orig_ino),
> @@ -2917,7 +2918,7 @@ TRACE_EVENT(ext4_fc_stats,
>  		    ),
>  
>  	    TP_printk("dev %d:%d fc ineligible reasons:\n"
> -		      "%s:%d, %s:%d, %s:%d, %s:%d, %s:%d, %s:%d, %s:%d, %s:%d; "
> +		      "%s:%d, %s:%d, %s:%d, %s:%d, %s:%d, %s:%d, %s:%d, %s:%d, %s:%d; "
>  		      "num_commits:%ld, ineligible: %ld, numblks: %ld",
>  		      MAJOR(__entry->dev), MINOR(__entry->dev),
>  		      FC_REASON_NAME_STAT(EXT4_FC_REASON_XATTR),
> @@ -2928,6 +2929,7 @@ TRACE_EVENT(ext4_fc_stats,
>  		      FC_REASON_NAME_STAT(EXT4_FC_REASON_RESIZE),
>  		      FC_REASON_NAME_STAT(EXT4_FC_REASON_RENAME_DIR),
>  		      FC_REASON_NAME_STAT(EXT4_FC_REASON_FALLOC_RANGE),
> +		      FC_REASON_NAME_STAT(EXT4_FC_REASON_INODE_JOURNAL_DATA),
>  		      __entry->sbi->s_fc_stats.fc_num_commits,
>  		      __entry->sbi->s_fc_stats.fc_ineligible_commits,
>  		      __entry->sbi->s_fc_stats.fc_numblks)
> -- 
> 2.29.1.341.ge80a0c044ae-goog
> 
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR

  reply	other threads:[~2020-11-03 17:01 UTC|newest]

Thread overview: 32+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-10-31 20:05 [PATCH 00/10] Ext4 fast commit fixes Harshad Shirwadkar
2020-10-31 20:05 ` [PATCH 01/10] ext4: describe fast_commit feature flags Harshad Shirwadkar
2020-11-03 10:33   ` Jan Kara
2020-10-31 20:05 ` [PATCH 02/10] ext4: mark fc ineligible if inode gets evictied due to mem pressure Harshad Shirwadkar
2020-11-03 14:13   ` Jan Kara
2020-11-03 18:33     ` harshad shirwadkar
2020-11-04  9:52       ` Jan Kara
2020-10-31 20:05 ` [PATCH 03/10] ext4: pass handle to ext4_fc_track_* functions Harshad Shirwadkar
2020-11-03 14:46   ` Jan Kara
2020-11-05  1:53     ` harshad shirwadkar
2020-10-31 20:05 ` [PATCH 04/10] ext4: clean up the JBD2 API that initializes fast commits Harshad Shirwadkar
2020-11-03 16:29   ` Jan Kara
2020-11-04 19:52     ` harshad shirwadkar
2020-11-05 10:30       ` Jan Kara
2020-11-05 12:44         ` Jan Kara
     [not found]           ` <CAE1WUT6oxiKtrdF6vzLv5vYFxPjefRhzDE2xfQGF6CqQQdPv1Q@mail.gmail.com>
2020-11-06  3:02             ` harshad shirwadkar
2020-10-31 20:05 ` [PATCH 05/10] jbd2: fix fast commit journalling APIs Harshad Shirwadkar
2020-11-03 16:38   ` Jan Kara
2020-11-04 21:25     ` harshad shirwadkar
2020-10-31 20:05 ` [PATCH 06/10] ext4: dedpulicate the code to wait on inode that's being committed Harshad Shirwadkar
2020-11-03 16:42   ` Jan Kara
2020-11-04 21:35     ` harshad shirwadkar
2020-10-31 20:05 ` [PATCH 07/10] ext4: misc fast commit fixes Harshad Shirwadkar
2020-11-03 16:52   ` Jan Kara
2020-11-06  3:07     ` harshad shirwadkar
2020-10-31 20:05 ` [PATCH 08/10] ext4: fix inode dirty check in case of fast commits Harshad Shirwadkar
2020-11-01  7:09   ` Andrea Righi
2020-11-03 16:58   ` Jan Kara
2020-10-31 20:05 ` [PATCH 09/10] ext4: disable fast commit with data journalling Harshad Shirwadkar
2020-11-03 17:01   ` Jan Kara [this message]
2020-10-31 20:05 ` [PATCH 10/10] ext4: issue fsdev cache flush before starting fast commit Harshad Shirwadkar
2020-11-03 17:02   ` Jan Kara

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20201103170100.GM3440@quack2.suse.cz \
    --to=jack@suse.cz \
    --cc=harshadshirwadkar@gmail.com \
    --cc=linux-ext4@vger.kernel.org \
    --cc=tytso@mit.edu \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).