[PATCH v6] Stop periodic syncing if filesystem is already shutdown.

[PATCH v6] Stop periodic syncing if filesystem is already shutdown.

[raghu.prabhu13]

From: Raghavendra D Prabhu <rprabhu@wnohang.net>

This is to prevent xfs_log_force from printing error message continuously (due
to xfs_sync and others) till umount if the disk has been forcefully unplugged.

This is to prevent messages like these from being displayed repeatedly.

[ 3873.009329] XFS (sdb3): xfs_log_force: error 5 returned.

Note, that even after xfs_do_force_shutdown has been called, xfs_log_force
doesn't stop till the filesystem has been unmounted (and it keeps printing
"error 5 returned" to kernel log).

To fix this, added condition over XFS_FORCED_SHUTDOWN to xfs_log_force and
xfs_fs_sync_fs.

To simulate it, mount an xfs filesystem located on external disk, and then pull
the power to the disk.

Now, the dmesg looks,

[  268.307303] XFS (sdb2): xfs_do_force_shutdown(0x1) called from line 1031 of file fs/xfs/xfs_buf.c.  Return address = 0xffffffff8127c13a
[  268.307318] XFS (sdb2): I/O Error Detected. Shutting down filesystem
[  268.307323] XFS (sdb2): Please umount the filesystem and rectify the problem(s)

Version 1: Removed calling xfs_syncd_stop from xfs_sync_worker.
Version 2: Removed calling xfs_fs_writable in xfs_sync_worker and xfs_flush_worker.
Version 3: Removed calling xfs_syncd_stop in xfs_bwrite.
Version 4: Added return statements to xfs_log_force and xfs_fs_sync_fs.
Version 5: As per suggestion, removed EIO return in xfs_log_force.

Signed-off-by: Raghavendra D Prabhu <rprabhu@wnohang.net>
---
 fs/xfs/xfs_log.c   | 5 ++++-
 fs/xfs/xfs_super.c | 4 ++++
 2 files changed, 8 insertions(+), 1 deletion(-)

diff --git a/fs/xfs/xfs_log.c b/fs/xfs/xfs_log.c
index 7f4f937..161c925 100644
--- a/fs/xfs/xfs_log.c
+++ b/fs/xfs/xfs_log.c
@@ -3002,7 +3002,10 @@ xfs_log_force(
 
 	trace_xfs_log_force(mp, 0);
 	error = _xfs_log_force(mp, flags, NULL);
-	if (error)
+	/*
+	 * Avoid warning when the filesystem has already shutdown.
+	 */
+	if (error && !XFS_FORCED_SHUTDOWN(mp))
 		xfs_warn(mp, "%s: error %d returned.", __func__, error);
 }
 
diff --git a/fs/xfs/xfs_super.c b/fs/xfs/xfs_super.c
index 19e2380..8df3387 100644
--- a/fs/xfs/xfs_super.c
+++ b/fs/xfs/xfs_super.c
@@ -944,6 +944,10 @@ xfs_fs_sync_fs(
 	if (!wait)
 		return 0;
 
+	if (XFS_FORCED_SHUTDOWN(mp)) {
+		return XFS_ERROR(EIO);
+	}
+
 	error = xfs_quiesce_data(mp);
 	if (error)
 		return -error;
-- 
1.7.12.1

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

Re: [PATCH v6] Stop periodic syncing if filesystem is already shutdown.

[Carlos Maiolino]

On Fri, Sep 21, 2012 at 10:24:04AM +0530, raghu.prabhu13@gmail.com wrote:
> From: Raghavendra D Prabhu <rprabhu@wnohang.net>
> 
> This is to prevent xfs_log_force from printing error message continuously (due
> to xfs_sync and others) till umount if the disk has been forcefully unplugged.
> 
> This is to prevent messages like these from being displayed repeatedly.
> 
> [ 3873.009329] XFS (sdb3): xfs_log_force: error 5 returned.
> 
> Note, that even after xfs_do_force_shutdown has been called, xfs_log_force
> doesn't stop till the filesystem has been unmounted (and it keeps printing
> "error 5 returned" to kernel log).
> 
> To fix this, added condition over XFS_FORCED_SHUTDOWN to xfs_log_force and
> xfs_fs_sync_fs.
> 
> To simulate it, mount an xfs filesystem located on external disk, and then pull
> the power to the disk.
> 
> Now, the dmesg looks,
> 
> [  268.307303] XFS (sdb2): xfs_do_force_shutdown(0x1) called from line 1031 of file fs/xfs/xfs_buf.c.  Return address = 0xffffffff8127c13a
> [  268.307318] XFS (sdb2): I/O Error Detected. Shutting down filesystem
> [  268.307323] XFS (sdb2): Please umount the filesystem and rectify the problem(s)
> 
> Version 1: Removed calling xfs_syncd_stop from xfs_sync_worker.
> Version 2: Removed calling xfs_fs_writable in xfs_sync_worker and xfs_flush_worker.
> Version 3: Removed calling xfs_syncd_stop in xfs_bwrite.
> Version 4: Added return statements to xfs_log_force and xfs_fs_sync_fs.
> Version 5: As per suggestion, removed EIO return in xfs_log_force.
> 
> Signed-off-by: Raghavendra D Prabhu <rprabhu@wnohang.net>
> ---
>  fs/xfs/xfs_log.c   | 5 ++++-
>  fs/xfs/xfs_super.c | 4 ++++
>  2 files changed, 8 insertions(+), 1 deletion(-)
> 
> diff --git a/fs/xfs/xfs_log.c b/fs/xfs/xfs_log.c
> index 7f4f937..161c925 100644
> --- a/fs/xfs/xfs_log.c
> +++ b/fs/xfs/xfs_log.c
> @@ -3002,7 +3002,10 @@ xfs_log_force(
>  
>  	trace_xfs_log_force(mp, 0);
>  	error = _xfs_log_force(mp, flags, NULL);
> -	if (error)
> +	/*
> +	 * Avoid warning when the filesystem has already shutdown.
> +	 */
> +	if (error && !XFS_FORCED_SHUTDOWN(mp))
>  		xfs_warn(mp, "%s: error %d returned.", __func__, error);
>  }
>  
> diff --git a/fs/xfs/xfs_super.c b/fs/xfs/xfs_super.c
> index 19e2380..8df3387 100644
> --- a/fs/xfs/xfs_super.c
> +++ b/fs/xfs/xfs_super.c
> @@ -944,6 +944,10 @@ xfs_fs_sync_fs(
>  	if (!wait)
>  		return 0;
>  
> +	if (XFS_FORCED_SHUTDOWN(mp)) {
> +		return XFS_ERROR(EIO);
> +	}
> +
>  	error = xfs_quiesce_data(mp);
>  	if (error)
>  		return -error;
> -- 
> 1.7.12.1
> 
Looks good to me.

Reviewd-by: Carlos Maiolino <cmaiolino@redhat.com>

-- 
--Carlos

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

Re: [PATCH v6] Stop periodic syncing if filesystem is already shutdown.

[Christoph Hellwig]

Looks good, but one thing I'd love here is to actually print a useful
error for the typical EIO case, as the "error 5 occured" message just
confuses the heck out of people.  What it means is that we got an
EIO error from the block device, and that's what we should print.

Probably worth doing in a separate patch, so:

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

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

Re: [PATCH v6] Stop periodic syncing if filesystem is already shutdown.

[Raghavendra Prabhu]

Thank you. Are there any comments on this patch or has is it been acked?

On Tue, Sep 25, 2012 at 3:10 PM, Christoph Hellwig <hch@infradead.org> wrote:
> Looks good, but one thing I'd love here is to actually print a useful
> error for the typical EIO case, as the "error 5 occured" message just
> confuses the heck out of people.  What it means is that we got an
> EIO error from the block device, and that's what we should print.
>
> Probably worth doing in a separate patch, so:
>
> Reviewed-by: Christoph Hellwig <hch@lst.de>

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

Re: [PATCH v6] Stop periodic syncing if filesystem is already shutdown.

[Ben Myers]

Hi Raghavendra,

On Thu, Oct 25, 2012 at 02:27:31AM +0530, Raghavendra Prabhu wrote:
> Thank you. Are there any comments on this patch or has is it been acked?

This patch no longer applies cleanly on the master brach.  Could you resolve
the conflicts and repost?

Regards,
	Ben

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