All of lore.kernel.org
 help / color / mirror / Atom feed
From: Dave Chinner <david@fromorbit.com>
To: raghu.prabhu13@gmail.com
Cc: Raghavendra D Prabhu <rprabhu@wnohang.net>, xfs@oss.sgi.com
Subject: Re: [PATCH] [PATCH] Stop periodic syncing if filesystem is already shutdown.
Date: Tue, 8 May 2012 09:53:21 +1000	[thread overview]
Message-ID: <20120507235321.GD5091@dastard> (raw)
In-Reply-To: <a27cbce4f4b35c2a8aee1b58d88c22381fe70ccf.1336378182.git.rprabhu@wnohang.net>

On Mon, May 07, 2012 at 02:44:07PM +0530, raghu.prabhu13@gmail.com wrote:
> From: Raghavendra D Prabhu <rprabhu@wnohang.net>
> 
> This is to prevent syncing from running ad-infinitum till umount if the disk has been forcefully unplugged.
> 
> This is to prevent messages like these from being displayed.
.....
> ---
>  fs/xfs/xfs_sync.c |   12 ++++++++++++
>  1 file changed, 12 insertions(+)
> 
> diff --git a/fs/xfs/xfs_sync.c b/fs/xfs/xfs_sync.c
> index 205ebcb..7ec412c 100644
> --- a/fs/xfs/xfs_sync.c
> +++ b/fs/xfs/xfs_sync.c
> @@ -460,6 +460,12 @@ xfs_sync_worker(
>  					struct xfs_mount, m_sync_work);
>  	int		error;
>  
> +	if (!xfs_fs_writable(mp)) {
> +		xfs_err(mp, "Filesystem not writable / already shutdown.");
> +		xfs_syncd_stop(mp);
> +		return;
> +	}
> +

That is going to kill the xfssyncd on read only and frozen
filesystems as well as shutdowns, so this is certainly not correct.
The xfs_sync_worker should continue to run until the filesystem is
unmounted, even if it does nothing when it runs.

Indeed, all that is needed in xfs_sync_worker() is this:

-	if (!(mp->m_flags & XFS_MOUNT_RDONLY)) {
+	if (!xfs_fs_writable(mp)) {

and the error message won't appear. It fixes the problem for the
shutdown case, as well as handles frozen and read-only filesystems
correctly.

>  		/* dgc: errors ignored here */
>  		if (mp->m_super->s_frozen == SB_UNFROZEN &&
> @@ -551,6 +557,12 @@ xfs_flush_worker(
>  	struct xfs_mount *mp = container_of(work,
>  					struct xfs_mount, m_flush_work);
>  
> +	if (!xfs_fs_writable(mp)) {
> +		xfs_err(mp, "Filesystem not writable / already shutdown.");
> +		xfs_syncd_stop(mp);
> +		return;
> +	}
> +
>  	xfs_sync_data(mp, SYNC_TRYLOCK);
>  	xfs_sync_data(mp, SYNC_TRYLOCK | SYNC_WAIT);

This is not necessary, either, because xfs_sync_data() has shutdown
checks and xfs_flush_worker() should never be called on a shutdown
filesystem....

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

  parent reply	other threads:[~2012-05-07 23:53 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-05-07  9:14 [PATCH] [PATCH] Stop periodic syncing if filesystem is already shutdown raghu.prabhu13
2012-05-07  9:41 ` Raghavendra D Prabhu
2012-05-07 12:07 ` Raghavendra D Prabhu
2012-05-07 23:53 ` Dave Chinner [this message]
2012-05-09 20:07   ` Raghavendra D Prabhu
2012-05-09 20:34     ` Raghavendra D Prabhu

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=20120507235321.GD5091@dastard \
    --to=david@fromorbit.com \
    --cc=raghu.prabhu13@gmail.com \
    --cc=rprabhu@wnohang.net \
    --cc=xfs@oss.sgi.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
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.