Linux-ext4 Archive on lore.kernel.org
 help / color / Atom feed
From: Jan Kara <jack@suse.cz>
To: "zhangyi (F)" <yi.zhang@huawei.com>
Cc: linux-ext4@vger.kernel.org, jack@suse.com, tytso@mit.edu,
	adilger.kernel@dilger.ca, liangyun2@huawei.com,
	luoshijie1@huawei.com
Subject: Re: [PATCH v3 4/4] jbd2: clean __jbd2_journal_abort_hard() and __journal_abort_soft()
Date: Wed, 4 Dec 2019 18:05:55 +0100
Message-ID: <20191204170555.GI8206@quack2.suse.cz> (raw)
In-Reply-To: <20191204124614.45424-5-yi.zhang@huawei.com>

On Wed 04-12-19 20:46:14, zhangyi (F) wrote:
> __jbd2_journal_abort_hard() has never been used, now we can merge
> __jbd2_journal_abort_hard() and __journal_abort_soft() these two
> functions into jbd2_journal_abort() and remove them.
> 
> Signed-off-by: zhangyi (F) <yi.zhang@huawei.com>

Looks good to me. You can add:

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

								Honza

> ---
>  fs/jbd2/journal.c    | 103 ++++++++++++++++++-------------------------
>  include/linux/jbd2.h |   1 -
>  2 files changed, 42 insertions(+), 62 deletions(-)
> 
> diff --git a/fs/jbd2/journal.c b/fs/jbd2/journal.c
> index 93be6e0311da..e59d9b6e4596 100644
> --- a/fs/jbd2/journal.c
> +++ b/fs/jbd2/journal.c
> @@ -96,7 +96,6 @@ EXPORT_SYMBOL(jbd2_journal_release_jbd_inode);
>  EXPORT_SYMBOL(jbd2_journal_begin_ordered_truncate);
>  EXPORT_SYMBOL(jbd2_inode_cache);
>  
> -static void __journal_abort_soft (journal_t *journal, int errno);
>  static int jbd2_journal_create_slab(size_t slab_size);
>  
>  #ifdef CONFIG_JBD2_DEBUG
> @@ -805,7 +804,7 @@ int jbd2_journal_bmap(journal_t *journal, unsigned long blocknr,
>  					"at offset %lu on %s\n",
>  			       __func__, blocknr, journal->j_devname);
>  			err = -EIO;
> -			__journal_abort_soft(journal, err);
> +			jbd2_journal_abort(journal, err);
>  		}
>  	} else {
>  		*retp = blocknr; /* +journal->j_blk_offset */
> @@ -2065,64 +2064,6 @@ int jbd2_journal_wipe(journal_t *journal, int write)
>  	return err;
>  }
>  
> -/*
> - * Journal abort has very specific semantics, which we describe
> - * for journal abort.
> - *
> - * Two internal functions, which provide abort to the jbd layer
> - * itself are here.
> - */
> -
> -/*
> - * Quick version for internal journal use (doesn't lock the journal).
> - * Aborts hard --- we mark the abort as occurred, but do _nothing_ else,
> - * and don't attempt to make any other journal updates.
> - */
> -void __jbd2_journal_abort_hard(journal_t *journal)
> -{
> -	transaction_t *transaction;
> -
> -	if (journal->j_flags & JBD2_ABORT)
> -		return;
> -
> -	printk(KERN_ERR "Aborting journal on device %s.\n",
> -	       journal->j_devname);
> -
> -	write_lock(&journal->j_state_lock);
> -	journal->j_flags |= JBD2_ABORT;
> -	transaction = journal->j_running_transaction;
> -	if (transaction)
> -		__jbd2_log_start_commit(journal, transaction->t_tid);
> -	write_unlock(&journal->j_state_lock);
> -}
> -
> -/* Soft abort: record the abort error status in the journal superblock,
> - * but don't do any other IO. */
> -static void __journal_abort_soft (journal_t *journal, int errno)
> -{
> -	int old_errno;
> -
> -	write_lock(&journal->j_state_lock);
> -	old_errno = journal->j_errno;
> -	if (!journal->j_errno || errno == -ESHUTDOWN)
> -		journal->j_errno = errno;
> -
> -	if (journal->j_flags & JBD2_ABORT) {
> -		write_unlock(&journal->j_state_lock);
> -		if (old_errno != -ESHUTDOWN && errno == -ESHUTDOWN)
> -			jbd2_journal_update_sb_errno(journal);
> -		return;
> -	}
> -	write_unlock(&journal->j_state_lock);
> -
> -	__jbd2_journal_abort_hard(journal);
> -
> -	jbd2_journal_update_sb_errno(journal);
> -	write_lock(&journal->j_state_lock);
> -	journal->j_flags |= JBD2_REC_ERR;
> -	write_unlock(&journal->j_state_lock);
> -}
> -
>  /**
>   * void jbd2_journal_abort () - Shutdown the journal immediately.
>   * @journal: the journal to shutdown.
> @@ -2166,7 +2107,47 @@ static void __journal_abort_soft (journal_t *journal, int errno)
>  
>  void jbd2_journal_abort(journal_t *journal, int errno)
>  {
> -	__journal_abort_soft(journal, errno);
> +	transaction_t *transaction;
> +
> +	/*
> +	 * ESHUTDOWN always takes precedence because a file system check
> +	 * caused by any other journal abort error is not required after
> +	 * a shutdown triggered.
> +	 */
> +	write_lock(&journal->j_state_lock);
> +	if (journal->j_flags & JBD2_ABORT) {
> +		int old_errno = journal->j_errno;
> +
> +		write_unlock(&journal->j_state_lock);
> +		if (old_errno != -ESHUTDOWN && errno == -ESHUTDOWN) {
> +			journal->j_errno = errno;
> +			jbd2_journal_update_sb_errno(journal);
> +		}
> +		return;
> +	}
> +
> +	/*
> +	 * Mark the abort as occurred and start current running transaction
> +	 * to release all journaled buffer.
> +	 */
> +	pr_err("Aborting journal on device %s.\n", journal->j_devname);
> +
> +	journal->j_flags |= JBD2_ABORT;
> +	journal->j_errno = errno;
> +	transaction = journal->j_running_transaction;
> +	if (transaction)
> +		__jbd2_log_start_commit(journal, transaction->t_tid);
> +	write_unlock(&journal->j_state_lock);
> +
> +	/*
> +	 * Record errno to the journal super block, so that fsck and jbd2
> +	 * layer could realise that a filesystem check is needed.
> +	 */
> +	jbd2_journal_update_sb_errno(journal);
> +
> +	write_lock(&journal->j_state_lock);
> +	journal->j_flags |= JBD2_REC_ERR;
> +	write_unlock(&journal->j_state_lock);
>  }
>  
>  /**
> diff --git a/include/linux/jbd2.h b/include/linux/jbd2.h
> index 603fbc4e2f70..e3e271bfb0e7 100644
> --- a/include/linux/jbd2.h
> +++ b/include/linux/jbd2.h
> @@ -1402,7 +1402,6 @@ extern int	   jbd2_journal_skip_recovery	(journal_t *);
>  extern void	   jbd2_journal_update_sb_errno(journal_t *);
>  extern int	   jbd2_journal_update_sb_log_tail	(journal_t *, tid_t,
>  				unsigned long, int);
> -extern void	   __jbd2_journal_abort_hard	(journal_t *);
>  extern void	   jbd2_journal_abort      (journal_t *, int);
>  extern int	   jbd2_journal_errno      (journal_t *);
>  extern void	   jbd2_journal_ack_err    (journal_t *);
> -- 
> 2.17.2
> 
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR

      reply index

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-12-04 12:46 [PATCH v3 0/4] ext4, jbd2: improve aborting progress zhangyi (F)
2019-12-04 12:46 ` [PATCH v3 1/4] jbd2: switch to use jbd2_journal_abort() when failed to submit the commit record zhangyi (F)
2019-12-04 12:46 ` [PATCH v3 2/4] ext4, jbd2: ensure panic when aborting with zero errno zhangyi (F)
2019-12-04 12:52   ` Jan Kara
2019-12-04 12:46 ` [PATCH v3 3/4] jbd2: make sure ESHUTDOWN to be recorded in the journal superblock zhangyi (F)
2019-12-04 17:05   ` Jan Kara
2019-12-05  1:23     ` zhangyi (F)
2019-12-04 12:46 ` [PATCH v3 4/4] jbd2: clean __jbd2_journal_abort_hard() and __journal_abort_soft() zhangyi (F)
2019-12-04 17:05   ` Jan Kara [this message]

Reply instructions:

You may reply publically 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=20191204170555.GI8206@quack2.suse.cz \
    --to=jack@suse.cz \
    --cc=adilger.kernel@dilger.ca \
    --cc=jack@suse.com \
    --cc=liangyun2@huawei.com \
    --cc=linux-ext4@vger.kernel.org \
    --cc=luoshijie1@huawei.com \
    --cc=tytso@mit.edu \
    --cc=yi.zhang@huawei.com \
    /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

Linux-ext4 Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-ext4/0 linux-ext4/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-ext4 linux-ext4/ https://lore.kernel.org/linux-ext4 \
		linux-ext4@vger.kernel.org
	public-inbox-index linux-ext4

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-ext4


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git