diff mbox series

[3/9] xfs: separate log cleaning from log quiesce

Message ID 20210106174127.805660-4-bfoster@redhat.com (mailing list archive)
State Superseded
Headers show
Series xfs: rework log quiesce to cover the log | expand

Commit Message

Brian Foster Jan. 6, 2021, 5:41 p.m. UTC
Log quiesce is currently associated with cleaning the log, which is
accomplished by writing an unmount record as the last step of the
quiesce sequence. The quiesce codepath is a bit convoluted in this
regard due to how it is reused from various contexts. In preparation
to create separate log cleaning and log covering interfaces, lift
the write of the unmount record into a new cleaning helper and call
that wherever xfs_log_quiesce() is currently invoked. No functional
changes.

Signed-off-by: Brian Foster <bfoster@redhat.com>
---
 fs/xfs/xfs_log.c   | 8 +++++++-
 fs/xfs/xfs_log.h   | 1 +
 fs/xfs/xfs_super.c | 2 +-
 3 files changed, 9 insertions(+), 2 deletions(-)

Comments

Allison Henderson Jan. 6, 2021, 10:50 p.m. UTC | #1
On 1/6/21 10:41 AM, Brian Foster wrote:
> Log quiesce is currently associated with cleaning the log, which is
> accomplished by writing an unmount record as the last step of the
> quiesce sequence. The quiesce codepath is a bit convoluted in this
> regard due to how it is reused from various contexts. In preparation
> to create separate log cleaning and log covering interfaces, lift
> the write of the unmount record into a new cleaning helper and call
> that wherever xfs_log_quiesce() is currently invoked. No functional
> changes.
> 
> Signed-off-by: Brian Foster <bfoster@redhat.com>
Looks ok to me
Reviewed-by: Allison Henderson <allison.henderson@oracle.com>
> ---
>   fs/xfs/xfs_log.c   | 8 +++++++-
>   fs/xfs/xfs_log.h   | 1 +
>   fs/xfs/xfs_super.c | 2 +-
>   3 files changed, 9 insertions(+), 2 deletions(-)
> 
> diff --git a/fs/xfs/xfs_log.c b/fs/xfs/xfs_log.c
> index 4137ed007111..1b3227a033ad 100644
> --- a/fs/xfs/xfs_log.c
> +++ b/fs/xfs/xfs_log.c
> @@ -957,7 +957,13 @@ xfs_log_quiesce(
>   	xfs_wait_buftarg(mp->m_ddev_targp);
>   	xfs_buf_lock(mp->m_sb_bp);
>   	xfs_buf_unlock(mp->m_sb_bp);
> +}
>   
> +void
> +xfs_log_clean(
> +	struct xfs_mount	*mp)
> +{
> +	xfs_log_quiesce(mp);
>   	xfs_log_unmount_write(mp);
>   }
>   
> @@ -972,7 +978,7 @@ void
>   xfs_log_unmount(
>   	struct xfs_mount	*mp)
>   {
> -	xfs_log_quiesce(mp);
> +	xfs_log_clean(mp);
>   
>   	xfs_trans_ail_destroy(mp);
>   
> diff --git a/fs/xfs/xfs_log.h b/fs/xfs/xfs_log.h
> index 98c913da7587..b0400589f824 100644
> --- a/fs/xfs/xfs_log.h
> +++ b/fs/xfs/xfs_log.h
> @@ -139,6 +139,7 @@ bool	xfs_log_item_in_current_chkpt(struct xfs_log_item *lip);
>   
>   void	xfs_log_work_queue(struct xfs_mount *mp);
>   void	xfs_log_quiesce(struct xfs_mount *mp);
> +void	xfs_log_clean(struct xfs_mount *mp);
>   bool	xfs_log_check_lsn(struct xfs_mount *, xfs_lsn_t);
>   bool	xfs_log_in_recovery(struct xfs_mount *);
>   
> diff --git a/fs/xfs/xfs_super.c b/fs/xfs/xfs_super.c
> index 813be879a5e5..09d956e30fd8 100644
> --- a/fs/xfs/xfs_super.c
> +++ b/fs/xfs/xfs_super.c
> @@ -897,7 +897,7 @@ xfs_quiesce_attr(
>   	if (error)
>   		xfs_warn(mp, "xfs_attr_quiesce: failed to log sb changes. "
>   				"Frozen image may not be consistent.");
> -	xfs_log_quiesce(mp);
> +	xfs_log_clean(mp);
>   }
>   
>   /*
>
Darrick J. Wong Jan. 7, 2021, 7:07 p.m. UTC | #2
On Wed, Jan 06, 2021 at 12:41:21PM -0500, Brian Foster wrote:
> Log quiesce is currently associated with cleaning the log, which is
> accomplished by writing an unmount record as the last step of the
> quiesce sequence. The quiesce codepath is a bit convoluted in this
> regard due to how it is reused from various contexts. In preparation
> to create separate log cleaning and log covering interfaces, lift
> the write of the unmount record into a new cleaning helper and call
> that wherever xfs_log_quiesce() is currently invoked. No functional
> changes.
> 
> Signed-off-by: Brian Foster <bfoster@redhat.com>

Assuming the stuff I rambled about in my reply to patch 2 wasn't a
total braino,

Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com>

--D

> ---
>  fs/xfs/xfs_log.c   | 8 +++++++-
>  fs/xfs/xfs_log.h   | 1 +
>  fs/xfs/xfs_super.c | 2 +-
>  3 files changed, 9 insertions(+), 2 deletions(-)
> 
> diff --git a/fs/xfs/xfs_log.c b/fs/xfs/xfs_log.c
> index 4137ed007111..1b3227a033ad 100644
> --- a/fs/xfs/xfs_log.c
> +++ b/fs/xfs/xfs_log.c
> @@ -957,7 +957,13 @@ xfs_log_quiesce(
>  	xfs_wait_buftarg(mp->m_ddev_targp);
>  	xfs_buf_lock(mp->m_sb_bp);
>  	xfs_buf_unlock(mp->m_sb_bp);
> +}
>  
> +void
> +xfs_log_clean(
> +	struct xfs_mount	*mp)
> +{
> +	xfs_log_quiesce(mp);
>  	xfs_log_unmount_write(mp);
>  }
>  
> @@ -972,7 +978,7 @@ void
>  xfs_log_unmount(
>  	struct xfs_mount	*mp)
>  {
> -	xfs_log_quiesce(mp);
> +	xfs_log_clean(mp);
>  
>  	xfs_trans_ail_destroy(mp);
>  
> diff --git a/fs/xfs/xfs_log.h b/fs/xfs/xfs_log.h
> index 98c913da7587..b0400589f824 100644
> --- a/fs/xfs/xfs_log.h
> +++ b/fs/xfs/xfs_log.h
> @@ -139,6 +139,7 @@ bool	xfs_log_item_in_current_chkpt(struct xfs_log_item *lip);
>  
>  void	xfs_log_work_queue(struct xfs_mount *mp);
>  void	xfs_log_quiesce(struct xfs_mount *mp);
> +void	xfs_log_clean(struct xfs_mount *mp);
>  bool	xfs_log_check_lsn(struct xfs_mount *, xfs_lsn_t);
>  bool	xfs_log_in_recovery(struct xfs_mount *);
>  
> diff --git a/fs/xfs/xfs_super.c b/fs/xfs/xfs_super.c
> index 813be879a5e5..09d956e30fd8 100644
> --- a/fs/xfs/xfs_super.c
> +++ b/fs/xfs/xfs_super.c
> @@ -897,7 +897,7 @@ xfs_quiesce_attr(
>  	if (error)
>  		xfs_warn(mp, "xfs_attr_quiesce: failed to log sb changes. "
>  				"Frozen image may not be consistent.");
> -	xfs_log_quiesce(mp);
> +	xfs_log_clean(mp);
>  }
>  
>  /*
> -- 
> 2.26.2
>
Christoph Hellwig Jan. 13, 2021, 3:30 p.m. UTC | #3
On Wed, Jan 06, 2021 at 12:41:21PM -0500, Brian Foster wrote:
> Log quiesce is currently associated with cleaning the log, which is
> accomplished by writing an unmount record as the last step of the
> quiesce sequence. The quiesce codepath is a bit convoluted in this
> regard due to how it is reused from various contexts. In preparation
> to create separate log cleaning and log covering interfaces, lift
> the write of the unmount record into a new cleaning helper and call
> that wherever xfs_log_quiesce() is currently invoked. No functional
> changes.
> 
> Signed-off-by: Brian Foster <bfoster@redhat.com>

Looks good,

Reviewed-by: Christoph Hellwig <hch@lst.de>
diff mbox series

Patch

diff --git a/fs/xfs/xfs_log.c b/fs/xfs/xfs_log.c
index 4137ed007111..1b3227a033ad 100644
--- a/fs/xfs/xfs_log.c
+++ b/fs/xfs/xfs_log.c
@@ -957,7 +957,13 @@  xfs_log_quiesce(
 	xfs_wait_buftarg(mp->m_ddev_targp);
 	xfs_buf_lock(mp->m_sb_bp);
 	xfs_buf_unlock(mp->m_sb_bp);
+}
 
+void
+xfs_log_clean(
+	struct xfs_mount	*mp)
+{
+	xfs_log_quiesce(mp);
 	xfs_log_unmount_write(mp);
 }
 
@@ -972,7 +978,7 @@  void
 xfs_log_unmount(
 	struct xfs_mount	*mp)
 {
-	xfs_log_quiesce(mp);
+	xfs_log_clean(mp);
 
 	xfs_trans_ail_destroy(mp);
 
diff --git a/fs/xfs/xfs_log.h b/fs/xfs/xfs_log.h
index 98c913da7587..b0400589f824 100644
--- a/fs/xfs/xfs_log.h
+++ b/fs/xfs/xfs_log.h
@@ -139,6 +139,7 @@  bool	xfs_log_item_in_current_chkpt(struct xfs_log_item *lip);
 
 void	xfs_log_work_queue(struct xfs_mount *mp);
 void	xfs_log_quiesce(struct xfs_mount *mp);
+void	xfs_log_clean(struct xfs_mount *mp);
 bool	xfs_log_check_lsn(struct xfs_mount *, xfs_lsn_t);
 bool	xfs_log_in_recovery(struct xfs_mount *);
 
diff --git a/fs/xfs/xfs_super.c b/fs/xfs/xfs_super.c
index 813be879a5e5..09d956e30fd8 100644
--- a/fs/xfs/xfs_super.c
+++ b/fs/xfs/xfs_super.c
@@ -897,7 +897,7 @@  xfs_quiesce_attr(
 	if (error)
 		xfs_warn(mp, "xfs_attr_quiesce: failed to log sb changes. "
 				"Frozen image may not be consistent.");
-	xfs_log_quiesce(mp);
+	xfs_log_clean(mp);
 }
 
 /*