public inbox for linux-xfs@vger.kernel.org
 help / color / mirror / Atom feed
From: "Darrick J. Wong" <djwong@kernel.org>
To: Dave Chinner <david@fromorbit.com>
Cc: linux-xfs@vger.kernel.org
Subject: Re: [PATCH 1/9] xfs: convert XLOG_FORCED_SHUTDOWN() to xlog_is_shutdown()
Date: Wed, 30 Jun 2021 09:10:00 -0700	[thread overview]
Message-ID: <20210630161000.GO13784@locust> (raw)
In-Reply-To: <20210630063813.1751007-2-david@fromorbit.com>

On Wed, Jun 30, 2021 at 04:38:05PM +1000, Dave Chinner wrote:
> From: Dave Chinner <dchinner@redhat.com>
> 
> Make it less shouty and a static inline before adding more calls
> through the log code.
> 
> Signed-off-by: Dave Chinner <dchinner@redhat.com>

Easy peasy,
Reviewed-by: Darrick J. Wong <djwong@kernel.org>

--D

> ---
>  fs/xfs/xfs_log.c         | 32 ++++++++++++++++----------------
>  fs/xfs/xfs_log_cil.c     | 10 +++++-----
>  fs/xfs/xfs_log_priv.h    |  7 +++++--
>  fs/xfs/xfs_log_recover.c |  9 +++------
>  fs/xfs/xfs_trans.c       |  2 +-
>  5 files changed, 30 insertions(+), 30 deletions(-)
> 
> diff --git a/fs/xfs/xfs_log.c b/fs/xfs/xfs_log.c
> index 71fd8c0cad92..5ae11e7bf2fd 100644
> --- a/fs/xfs/xfs_log.c
> +++ b/fs/xfs/xfs_log.c
> @@ -247,7 +247,7 @@ xlog_grant_head_wait(
>  	list_add_tail(&tic->t_queue, &head->waiters);
>  
>  	do {
> -		if (XLOG_FORCED_SHUTDOWN(log))
> +		if (xlog_is_shutdown(log))
>  			goto shutdown;
>  		xlog_grant_push_ail(log, need_bytes);
>  
> @@ -261,7 +261,7 @@ xlog_grant_head_wait(
>  		trace_xfs_log_grant_wake(log, tic);
>  
>  		spin_lock(&head->lock);
> -		if (XLOG_FORCED_SHUTDOWN(log))
> +		if (xlog_is_shutdown(log))
>  			goto shutdown;
>  	} while (xlog_space_left(log, &head->grant) < need_bytes);
>  
> @@ -366,7 +366,7 @@ xfs_log_writable(
>  		return false;
>  	if (xfs_readonly_buftarg(mp->m_log->l_targ))
>  		return false;
> -	if (XFS_FORCED_SHUTDOWN(mp))
> +	if (xlog_is_shutdown(mp->m_log))
>  		return false;
>  	return true;
>  }
> @@ -383,7 +383,7 @@ xfs_log_regrant(
>  	int			need_bytes;
>  	int			error = 0;
>  
> -	if (XLOG_FORCED_SHUTDOWN(log))
> +	if (xlog_is_shutdown(log))
>  		return -EIO;
>  
>  	XFS_STATS_INC(mp, xs_try_logspace);
> @@ -451,7 +451,7 @@ xfs_log_reserve(
>  
>  	ASSERT(client == XFS_TRANSACTION || client == XFS_LOG);
>  
> -	if (XLOG_FORCED_SHUTDOWN(log))
> +	if (xlog_is_shutdown(log))
>  		return -EIO;
>  
>  	XFS_STATS_INC(mp, xs_try_logspace);
> @@ -787,7 +787,7 @@ xlog_wait_on_iclog(
>  	struct xlog		*log = iclog->ic_log;
>  
>  	trace_xlog_iclog_wait_on(iclog, _RET_IP_);
> -	if (!XLOG_FORCED_SHUTDOWN(log) &&
> +	if (!xlog_is_shutdown(log) &&
>  	    iclog->ic_state != XLOG_STATE_ACTIVE &&
>  	    iclog->ic_state != XLOG_STATE_DIRTY) {
>  		XFS_STATS_INC(log->l_mp, xs_log_force_sleep);
> @@ -796,7 +796,7 @@ xlog_wait_on_iclog(
>  		spin_unlock(&log->l_icloglock);
>  	}
>  
> -	if (XLOG_FORCED_SHUTDOWN(log))
> +	if (xlog_is_shutdown(log))
>  		return -EIO;
>  	return 0;
>  }
> @@ -915,7 +915,7 @@ xfs_log_unmount_write(
>  
>  	xfs_log_force(mp, XFS_LOG_SYNC);
>  
> -	if (XLOG_FORCED_SHUTDOWN(log))
> +	if (xlog_is_shutdown(log))
>  		return;
>  
>  	/*
> @@ -1024,7 +1024,7 @@ xfs_log_space_wake(
>  	struct xlog		*log = mp->m_log;
>  	int			free_bytes;
>  
> -	if (XLOG_FORCED_SHUTDOWN(log))
> +	if (xlog_is_shutdown(log))
>  		return;
>  
>  	if (!list_empty_careful(&log->l_write_head.waiters)) {
> @@ -1115,7 +1115,7 @@ xfs_log_cover(
>  
>  	ASSERT((xlog_cil_empty(mp->m_log) && xlog_iclogs_empty(mp->m_log) &&
>  	        !xfs_ail_min_lsn(mp->m_log->l_ailp)) ||
> -	       XFS_FORCED_SHUTDOWN(mp));
> +		xlog_is_shutdown(mp->m_log));
>  
>  	if (!xfs_log_writable(mp))
>  		return 0;
> @@ -1546,7 +1546,7 @@ xlog_commit_record(
>  	};
>  	int	error;
>  
> -	if (XLOG_FORCED_SHUTDOWN(log))
> +	if (xlog_is_shutdown(log))
>  		return -EIO;
>  
>  	error = xlog_write(log, &vec, ticket, lsn, iclog, XLOG_COMMIT_TRANS);
> @@ -1627,7 +1627,7 @@ xlog_grant_push_ail(
>  	xfs_lsn_t	threshold_lsn;
>  
>  	threshold_lsn = xlog_grant_push_threshold(log, need_bytes);
> -	if (threshold_lsn == NULLCOMMITLSN || XLOG_FORCED_SHUTDOWN(log))
> +	if (threshold_lsn == NULLCOMMITLSN || xlog_is_shutdown(log))
>  		return;
>  
>  	/*
> @@ -2806,7 +2806,7 @@ xlog_state_do_callback(
>  			cycled_icloglock = true;
>  
>  			spin_lock(&log->l_icloglock);
> -			if (XLOG_FORCED_SHUTDOWN(log))
> +			if (xlog_is_shutdown(log))
>  				wake_up_all(&iclog->ic_force_wait);
>  			else
>  				xlog_state_clean_iclog(log, iclog);
> @@ -2858,7 +2858,7 @@ xlog_state_done_syncing(
>  	 * split log writes, on the second, we shut down the file system and
>  	 * no iclogs should ever be attempted to be written to disk again.
>  	 */
> -	if (!XLOG_FORCED_SHUTDOWN(log)) {
> +	if (!xlog_is_shutdown(log)) {
>  		ASSERT(iclog->ic_state == XLOG_STATE_SYNCING);
>  		iclog->ic_state = XLOG_STATE_DONE_SYNC;
>  	}
> @@ -2906,7 +2906,7 @@ xlog_state_get_iclog_space(
>  
>  restart:
>  	spin_lock(&log->l_icloglock);
> -	if (XLOG_FORCED_SHUTDOWN(log)) {
> +	if (xlog_is_shutdown(log)) {
>  		spin_unlock(&log->l_icloglock);
>  		return -EIO;
>  	}
> @@ -3754,7 +3754,7 @@ xfs_log_force_umount(
>  	 * No need to get locks for this.
>  	 */
>  	if (logerror && log->l_iclog->ic_state == XLOG_STATE_IOERROR) {
> -		ASSERT(XLOG_FORCED_SHUTDOWN(log));
> +		ASSERT(xlog_is_shutdown(log));
>  		return 1;
>  	}
>  
> diff --git a/fs/xfs/xfs_log_cil.c b/fs/xfs/xfs_log_cil.c
> index d162e8b83e90..23eec4f76f19 100644
> --- a/fs/xfs/xfs_log_cil.c
> +++ b/fs/xfs/xfs_log_cil.c
> @@ -584,7 +584,7 @@ xlog_cil_committed(
>  	struct xfs_cil_ctx	*ctx)
>  {
>  	struct xfs_mount	*mp = ctx->cil->xc_log->l_mp;
> -	bool			abort = XLOG_FORCED_SHUTDOWN(ctx->cil->xc_log);
> +	bool			abort = xlog_is_shutdown(ctx->cil->xc_log);
>  
>  	/*
>  	 * If the I/O failed, we're aborting the commit and already shutdown.
> @@ -853,7 +853,7 @@ xlog_cil_push_work(
>  		 * shutdown, but then went back to sleep once already in the
>  		 * shutdown state.
>  		 */
> -		if (XLOG_FORCED_SHUTDOWN(log)) {
> +		if (xlog_is_shutdown(log)) {
>  			spin_unlock(&cil->xc_push_lock);
>  			goto out_abort_free_ticket;
>  		}
> @@ -962,7 +962,7 @@ xlog_cil_push_work(
>  out_abort_free_ticket:
>  	xfs_log_ticket_ungrant(log, tic);
>  out_abort:
> -	ASSERT(XLOG_FORCED_SHUTDOWN(log));
> +	ASSERT(xlog_is_shutdown(log));
>  	xlog_cil_committed(ctx);
>  }
>  
> @@ -1115,7 +1115,7 @@ xlog_cil_commit(
>  
>  	xlog_cil_insert_items(log, tp);
>  
> -	if (regrant && !XLOG_FORCED_SHUTDOWN(log))
> +	if (regrant && !xlog_is_shutdown(log))
>  		xfs_log_ticket_regrant(log, tp->t_ticket);
>  	else
>  		xfs_log_ticket_ungrant(log, tp->t_ticket);
> @@ -1188,7 +1188,7 @@ xlog_cil_force_seq(
>  		 * shutdown, but then went back to sleep once already in the
>  		 * shutdown state.
>  		 */
> -		if (XLOG_FORCED_SHUTDOWN(log))
> +		if (xlog_is_shutdown(log))
>  			goto out_shutdown;
>  		if (ctx->sequence > sequence)
>  			continue;
> diff --git a/fs/xfs/xfs_log_priv.h b/fs/xfs/xfs_log_priv.h
> index 4c41bbfa33b0..80d4e1325e1d 100644
> --- a/fs/xfs/xfs_log_priv.h
> +++ b/fs/xfs/xfs_log_priv.h
> @@ -454,8 +454,11 @@ struct xlog {
>  #define XLOG_BUF_CANCEL_BUCKET(log, blkno) \
>  	((log)->l_buf_cancel_table + ((uint64_t)blkno % XLOG_BC_TABLE_SIZE))
>  
> -#define XLOG_FORCED_SHUTDOWN(log) \
> -	(unlikely((log)->l_flags & XLOG_IO_ERROR))
> +static inline bool
> +xlog_is_shutdown(struct xlog *log)
> +{
> +	return (log->l_flags & XLOG_IO_ERROR);
> +}
>  
>  /* common routines */
>  extern int
> diff --git a/fs/xfs/xfs_log_recover.c b/fs/xfs/xfs_log_recover.c
> index 1a291bf50776..f1b828dedb25 100644
> --- a/fs/xfs/xfs_log_recover.c
> +++ b/fs/xfs/xfs_log_recover.c
> @@ -144,7 +144,7 @@ xlog_do_io(
>  
>  	error = xfs_rw_bdev(log->l_targ->bt_bdev, log->l_logBBstart + blk_no,
>  			BBTOB(nbblks), data, op);
> -	if (error && !XFS_FORCED_SHUTDOWN(log->l_mp)) {
> +	if (error && !xlog_is_shutdown(log)) {
>  		xfs_alert(log->l_mp,
>  			  "log recovery %s I/O error at daddr 0x%llx len %d error %d",
>  			  op == REQ_OP_WRITE ? "write" : "read",
> @@ -3278,10 +3278,7 @@ xlog_do_recover(
>  	if (error)
>  		return error;
>  
> -	/*
> -	 * If IO errors happened during recovery, bail out.
> -	 */
> -	if (XFS_FORCED_SHUTDOWN(mp))
> +	if (xlog_is_shutdown(log))
>  		return -EIO;
>  
>  	/*
> @@ -3303,7 +3300,7 @@ xlog_do_recover(
>  	xfs_buf_hold(bp);
>  	error = _xfs_buf_read(bp, XBF_READ);
>  	if (error) {
> -		if (!XFS_FORCED_SHUTDOWN(mp)) {
> +		if (!xlog_is_shutdown(log)) {
>  			xfs_buf_ioerror_alert(bp, __this_address);
>  			ASSERT(0);
>  		}
> diff --git a/fs/xfs/xfs_trans.c b/fs/xfs/xfs_trans.c
> index 87bffd12c20c..e26ade9fc630 100644
> --- a/fs/xfs/xfs_trans.c
> +++ b/fs/xfs/xfs_trans.c
> @@ -908,7 +908,7 @@ __xfs_trans_commit(
>  	 */
>  	xfs_trans_unreserve_and_mod_dquots(tp);
>  	if (tp->t_ticket) {
> -		if (regrant && !XLOG_FORCED_SHUTDOWN(mp->m_log))
> +		if (regrant && !xlog_is_shutdown(mp->m_log))
>  			xfs_log_ticket_regrant(mp->m_log, tp->t_ticket);
>  		else
>  			xfs_log_ticket_ungrant(mp->m_log, tp->t_ticket);
> -- 
> 2.31.1
> 

  reply	other threads:[~2021-06-30 16:10 UTC|newest]

Thread overview: 37+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-06-30  6:38 [PATCH 0/9] xfs: shutdown is a racy mess Dave Chinner
2021-06-30  6:38 ` [PATCH 1/9] xfs: convert XLOG_FORCED_SHUTDOWN() to xlog_is_shutdown() Dave Chinner
2021-06-30 16:10   ` Darrick J. Wong [this message]
2021-07-02  7:48   ` Christoph Hellwig
2021-07-02  8:45     ` Dave Chinner
2021-07-02  9:27       ` Christoph Hellwig
2021-06-30  6:38 ` [PATCH 2/9] xfs: XLOG_STATE_IOERROR must die Dave Chinner
2021-06-30 15:22   ` kernel test robot
2021-06-30 15:22   ` [PATCH] xfs: fix semicolon.cocci warnings kernel test robot
2021-07-02  8:00   ` [PATCH 2/9] xfs: XLOG_STATE_IOERROR must die Christoph Hellwig
2021-07-02  8:55     ` Dave Chinner
2021-06-30  6:38 ` [PATCH 3/9] xfs: move recovery needed state updates to xfs_log_mount_finish Dave Chinner
2021-07-02  8:10   ` Christoph Hellwig
2021-06-30  6:38 ` [PATCH 4/9] xfs: convert log flags to an operational state field Dave Chinner
2021-07-02  8:15   ` Christoph Hellwig
2021-07-09  4:33     ` Darrick J. Wong
2021-06-30  6:38 ` [PATCH 5/9] xfs: make forced shutdown processing atomic Dave Chinner
2021-07-02  8:24   ` Christoph Hellwig
2021-07-05  1:28     ` Dave Chinner
2021-07-09  4:40   ` Darrick J. Wong
2021-07-14  3:15     ` Dave Chinner
2021-07-14  5:02       ` Darrick J. Wong
2021-06-30  6:38 ` [PATCH 6/9] xfs: reowrk up xlog_state_do_callback Dave Chinner
2021-07-02  8:28   ` Christoph Hellwig
2021-07-09  4:42   ` Darrick J. Wong
2021-06-30  6:38 ` [PATCH 7/9] xfs: separate out log shutdown callback processing Dave Chinner
2021-07-02  8:36   ` Christoph Hellwig
2021-07-05  1:46     ` Dave Chinner
2021-06-30  6:38 ` [PATCH 8/9] xfs: don't run shutdown callbacks on active iclogs Dave Chinner
2021-07-02  8:48   ` Christoph Hellwig
2021-07-05  2:04     ` Dave Chinner
2021-06-30  6:38 ` [PATCH 9/9] xfs: log head and tail aren't reliable during shutdown Dave Chinner
2021-07-02  8:53   ` Christoph Hellwig
2021-07-05  2:22     ` Dave Chinner
  -- strict thread matches above, loose matches on Subject: below --
2021-07-14  3:19 [PATCH 0/9 v2] xfs: shutdown is a racy mess Dave Chinner
2021-07-14  3:19 ` [PATCH 1/9] xfs: convert XLOG_FORCED_SHUTDOWN() to xlog_is_shutdown() Dave Chinner
2021-07-14  6:09   ` Christoph Hellwig
2021-08-10  5:18 [PATCH 0/9 v3] xfs: shutdown is a racy mess Dave Chinner
2021-08-10  5:18 ` [PATCH 1/9] xfs: convert XLOG_FORCED_SHUTDOWN() to xlog_is_shutdown() Dave Chinner

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=20210630161000.GO13784@locust \
    --to=djwong@kernel.org \
    --cc=david@fromorbit.com \
    --cc=linux-xfs@vger.kernel.org \
    /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