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

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

[raghu.prabhu13]

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.

==========================================
[ 3816.416570] XFS (sdb3): xfs_do_force_shutdown(0x1) called from line 1031 of file fs/xfs/xfs_buf.c.  Return address = 0xffffffff8127c13a
[ 3816.416586] XFS (sdb3): I/O Error Detected. Shutting down filesystem
[ 3816.416592] XFS (sdb3): Please umount the filesystem and rectify the problem(s)
[ 3842.941953] XFS (sdb3): xfs_log_force: error 5 returned.
[ 3873.009329] XFS (sdb3): xfs_log_force: error 5 returned.
[ 3878.913310] XFS (sdb3): xfs_log_force: error 5 returned.
[ 3878.913322] XFS (sdb3): xfs_do_force_shutdown(0x1) called from line 1031 of file fs/xfs/xfs_buf.c.  Return address = 0xffffffff8127c13a
[ 3878.913350] XFS (sdb3): xfs_log_force: error 5 returned.
[ 3878.913362] 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 simulate it, mount an xfs filesystem located on external disk, and then pull
the power to the disk (non-usb powered disk).

Tested it on latest linus tree. 
Now, the kernel log looks,

[ 2477.305280] XFS (sdc3): xfs_do_force_shutdown(0x1) called from line 1031 of file fs/xfs/xfs_buf.c.  Return address = 0xffffffff8127c13a
[ 2477.305295] XFS (sdc3): I/O Error Detected. Shutting down filesystem
[ 2477.305300] XFS (sdc3): Please umount the filesystem and rectify the problem(s)
[ 2477.305588] XFS (sdc2): xfs_do_force_shutdown(0x1) called from line 1031 of file fs/xfs/xfs_buf.c.  Return address = 0xffffffff8127c13a
[ 2477.305600] XFS (sdc2): I/O Error Detected. Shutting down filesystem
[ 2477.305604] XFS (sdc2): Please umount the filesystem and rectify the problem(s)
[ 2487.810718] XFS (sdc3): xfs_log_force: error 5 returned.
[ 2487.810729] XFS (sdc3): xfs_do_force_shutdown(0x1) called from line 1031 of file fs/xfs/xfs_buf.c.  Return address = 0xffffffff8127c13a
[ 2487.812981] XFS (sdc2): xfs_log_force: error 5 returned.
[ 2487.812991] XFS (sdc2): xfs_do_force_shutdown(0x1) called from line 1031 of file fs/xfs/xfs_buf.c.  Return address = 0xffffffff8127c13a
[ 2489.420042] XFS (sdc3): Filesystem not writable / already shutdown.
[ 2490.955438] XFS (sdc2): Filesystem not writable / already shutdown.

Signed-off-by: Raghavendra D Prabhu <rprabhu@wnohang.net>
Tested-by: Raghavendra D Prabhu <rprabhu@wnohang.net>
---
 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;
+	}
+
 	if (!(mp->m_flags & XFS_MOUNT_RDONLY)) {
 		/* 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);
 }
-- 
1.7.10.1

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

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

[Raghavendra D Prabhu]

[-- Attachment #1.1.1: Type: text/plain, Size: 334 bytes --]

Hi,

Somehow git send-email sent with empty body, I have attached the 
patch.

* On Mon, May 07, 2012 at 02:44:07PM +0530, raghu.prabhu13@gmail.com <raghu.prabhu13@gmail.com> wrote:




Regards,
-- 
Raghavendra Prabhu
GPG Id : 0xD72BE977
Fingerprint: B93F EBCB 8E05 7039 CD3C A4B8 A616 DCA1 D72B E977
www: wnohang.net

[-- Attachment #1.1.2: 0001-PATCH-Stop-periodic-syncing-if-filesystem-is-already.patch --]
[-- Type: text/plain, Size: 3927 bytes --]

From a27cbce4f4b35c2a8aee1b58d88c22381fe70ccf Mon Sep 17 00:00:00 2001
Message-Id: <a27cbce4f4b35c2a8aee1b58d88c22381fe70ccf.1336378182.git.rprabhu@wnohang.net>
From: Raghavendra D Prabhu <rprabhu@wnohang.net>
Date: Mon, 7 May 2012 13:33:55 +0530
Subject: [PATCH] Stop periodic syncing if filesystem is already
 shutdown.

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.

==========================================
[ 3816.416570] XFS (sdb3): xfs_do_force_shutdown(0x1) called from line 1031 of file fs/xfs/xfs_buf.c.  Return address = 0xffffffff8127c13a
[ 3816.416586] XFS (sdb3): I/O Error Detected. Shutting down filesystem
[ 3816.416592] XFS (sdb3): Please umount the filesystem and rectify the problem(s)
[ 3842.941953] XFS (sdb3): xfs_log_force: error 5 returned.
[ 3873.009329] XFS (sdb3): xfs_log_force: error 5 returned.
[ 3878.913310] XFS (sdb3): xfs_log_force: error 5 returned.
[ 3878.913322] XFS (sdb3): xfs_do_force_shutdown(0x1) called from line 1031 of file fs/xfs/xfs_buf.c.  Return address = 0xffffffff8127c13a
[ 3878.913350] XFS (sdb3): xfs_log_force: error 5 returned.
[ 3878.913362] 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 simulate it, mount an xfs filesystem located on external disk, and then pull
the power to the disk (non-usb powered disk).

Tested it on latest linus tree. 
Now, the kernel log looks,

[ 2477.305280] XFS (sdc3): xfs_do_force_shutdown(0x1) called from line 1031 of file fs/xfs/xfs_buf.c.  Return address = 0xffffffff8127c13a
[ 2477.305295] XFS (sdc3): I/O Error Detected. Shutting down filesystem
[ 2477.305300] XFS (sdc3): Please umount the filesystem and rectify the problem(s)
[ 2477.305588] XFS (sdc2): xfs_do_force_shutdown(0x1) called from line 1031 of file fs/xfs/xfs_buf.c.  Return address = 0xffffffff8127c13a
[ 2477.305600] XFS (sdc2): I/O Error Detected. Shutting down filesystem
[ 2477.305604] XFS (sdc2): Please umount the filesystem and rectify the problem(s)
[ 2487.810718] XFS (sdc3): xfs_log_force: error 5 returned.
[ 2487.810729] XFS (sdc3): xfs_do_force_shutdown(0x1) called from line 1031 of file fs/xfs/xfs_buf.c.  Return address = 0xffffffff8127c13a
[ 2487.812981] XFS (sdc2): xfs_log_force: error 5 returned.
[ 2487.812991] XFS (sdc2): xfs_do_force_shutdown(0x1) called from line 1031 of file fs/xfs/xfs_buf.c.  Return address = 0xffffffff8127c13a
[ 2489.420042] XFS (sdc3): Filesystem not writable / already shutdown.
[ 2490.955438] XFS (sdc2): Filesystem not writable / already shutdown.

Signed-off-by: Raghavendra D Prabhu <rprabhu@wnohang.net>
Tested-by: Raghavendra D Prabhu <rprabhu@wnohang.net>
---
 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;
+	}
+
 	if (!(mp->m_flags & XFS_MOUNT_RDONLY)) {
 		/* 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);
 }
-- 
1.7.10.1


[-- Attachment #1.2: Type: application/pgp-signature, Size: 490 bytes --]

[-- Attachment #2: Type: text/plain, Size: 121 bytes --]

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

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

[Raghavendra D Prabhu]

[-- Attachment #1.1: Type: text/plain, Size: 7228 bytes --]

Hi,

Apparently calling xfs_syncd_stop from xfs_sync_worker doesn't 
work well; So I have moved it to xfs_bwrite; though with this I 
noticed a 2-3 log force errors at flush and umount. Here is the 
revised version. 


 From e202a7d76d8209fd3eb70158719a65034a409168 Mon Sep 17 00:00:00 2001
Message-Id: <e202a7d76d8209fd3eb70158719a65034a409168.1336390891.git.rprabhu@wnohang.net>
From: Raghavendra D Prabhu <rprabhu@wnohang.net>
Date: Mon, 7 May 2012 17:07:49 +0530
Subject: [PATCH] Stop periodic syncing if filesystem is already shutdown.

This is to prevent xfs_log_force from running ad-infinitum (due to xfs_sync) 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 simulate it, mount an xfs filesystem located on external disk, and then pull
the power to the disk (non-usb powered disk).

Tested it on latest linus tree.
Now, the kernel log 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)

Since, xfs_syncd_stop is already called in xfs_bwrite; the message is not
printed; also added checks in xfs_sync_worker and xfs_flush_worker to return in
this case (if they have already been entered).

Signed-off-by: Raghavendra D Prabhu <rprabhu@wnohang.net>
Tested-by: Raghavendra D Prabhu <rprabhu@wnohang.net>
  ---
---
  fs/xfs/xfs_buf.c  |    1 +
  fs/xfs/xfs_sync.c |   10 ++++++++++
  2 files changed, 11 insertions(+)

diff --git a/fs/xfs/xfs_buf.c b/fs/xfs/xfs_buf.c
index 6819b51..7bdf018 100644
--- a/fs/xfs/xfs_buf.c
+++ b/fs/xfs/xfs_buf.c
@@ -1029,6 +1029,7 @@ xfs_bwrite(
  	if (error) {
  		xfs_force_shutdown(bp->b_target->bt_mount,
  				   SHUTDOWN_META_IO_ERROR);
+		xfs_syncd_stop(bp->b_target->bt_mount);
  	}
  	return error;
  }
diff --git a/fs/xfs/xfs_sync.c b/fs/xfs/xfs_sync.c
index 205ebcb..79745a5 100644
--- a/fs/xfs/xfs_sync.c
+++ b/fs/xfs/xfs_sync.c
@@ -460,6 +460,11 @@ xfs_sync_worker(
  					struct xfs_mount, m_sync_work);
  	int		error;
  
+	if (!xfs_fs_writable(mp)) {
+		xfs_err(mp, "Filesystem not writable / already shutdown.");
+		return;
+	}
+
  	if (!(mp->m_flags & XFS_MOUNT_RDONLY)) {
  		/* dgc: errors ignored here */
  		if (mp->m_super->s_frozen == SB_UNFROZEN &&
@@ -551,6 +556,11 @@ 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.");
+		return;
+	}
+
  	xfs_sync_data(mp, SYNC_TRYLOCK);
  	xfs_sync_data(mp, SYNC_TRYLOCK | SYNC_WAIT);
  }
-- 
1.7.10.1



* On Mon, May 07, 2012 at 02:44:07PM +0530, raghu.prabhu13@gmail.com <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.
>
>==========================================
>[ 3816.416570] XFS (sdb3): xfs_do_force_shutdown(0x1) called from line 1031 of file fs/xfs/xfs_buf.c.  Return address = 0xffffffff8127c13a
>[ 3816.416586] XFS (sdb3): I/O Error Detected. Shutting down filesystem
>[ 3816.416592] XFS (sdb3): Please umount the filesystem and rectify the problem(s)
>[ 3842.941953] XFS (sdb3): xfs_log_force: error 5 returned.
>[ 3873.009329] XFS (sdb3): xfs_log_force: error 5 returned.
>[ 3878.913310] XFS (sdb3): xfs_log_force: error 5 returned.
>[ 3878.913322] XFS (sdb3): xfs_do_force_shutdown(0x1) called from line 1031 of file fs/xfs/xfs_buf.c.  Return address = 0xffffffff8127c13a
>[ 3878.913350] XFS (sdb3): xfs_log_force: error 5 returned.
>[ 3878.913362] 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 simulate it, mount an xfs filesystem located on external disk, and then pull
>the power to the disk (non-usb powered disk).
>
>Tested it on latest linus tree.
>Now, the kernel log looks,
>
>[ 2477.305280] XFS (sdc3): xfs_do_force_shutdown(0x1) called from line 1031 of file fs/xfs/xfs_buf.c.  Return address = 0xffffffff8127c13a
>[ 2477.305295] XFS (sdc3): I/O Error Detected. Shutting down filesystem
>[ 2477.305300] XFS (sdc3): Please umount the filesystem and rectify the problem(s)
>[ 2477.305588] XFS (sdc2): xfs_do_force_shutdown(0x1) called from line 1031 of file fs/xfs/xfs_buf.c.  Return address = 0xffffffff8127c13a
>[ 2477.305600] XFS (sdc2): I/O Error Detected. Shutting down filesystem
>[ 2477.305604] XFS (sdc2): Please umount the filesystem and rectify the problem(s)
>[ 2487.810718] XFS (sdc3): xfs_log_force: error 5 returned.
>[ 2487.810729] XFS (sdc3): xfs_do_force_shutdown(0x1) called from line 1031 of file fs/xfs/xfs_buf.c.  Return address = 0xffffffff8127c13a
>[ 2487.812981] XFS (sdc2): xfs_log_force: error 5 returned.
>[ 2487.812991] XFS (sdc2): xfs_do_force_shutdown(0x1) called from line 1031 of file fs/xfs/xfs_buf.c.  Return address = 0xffffffff8127c13a
>[ 2489.420042] XFS (sdc3): Filesystem not writable / already shutdown.
>[ 2490.955438] XFS (sdc2): Filesystem not writable / already shutdown.
>
>Signed-off-by: Raghavendra D Prabhu <rprabhu@wnohang.net>
>Tested-by: Raghavendra D Prabhu <rprabhu@wnohang.net>
>---
> 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;
>+	}
>+
> 	if (!(mp->m_flags & XFS_MOUNT_RDONLY)) {
> 		/* 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);
> }
>-- 
>1.7.10.1
>
>_______________________________________________
>xfs mailing list
>xfs@oss.sgi.com
>http://oss.sgi.com/mailman/listinfo/xfs




Regards,
-- 
Raghavendra Prabhu
GPG Id : 0xD72BE977
Fingerprint: B93F EBCB 8E05 7039 CD3C A4B8 A616 DCA1 D72B E977
www: wnohang.net

[-- Attachment #1.2: Type: application/pgp-signature, Size: 490 bytes --]

[-- Attachment #2: Type: text/plain, Size: 121 bytes --]

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

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

[Dave Chinner]

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

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

[Raghavendra D Prabhu]

[-- Attachment #1.1: Type: text/plain, Size: 3221 bytes --]

 
Hi,


* On Tue, May 08, 2012 at 09:53:21AM +1000, Dave Chinner <david@fromorbit.com> wrote:
>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.

Yes, in the next patch I had sent -- 
http://article.gmane.org/gmane.comp.file-systems.xfs.general/45568 
-- I removed xfs_syncd_stop from there and added it under 
xfs_bwrite. 

(PS: Adding xfs_syncd_stop in xfs_sync_worker was a very 
bad decision - it sent few of my kworkers to 'D' state and nearly 
corrupted that fs)


>
>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.
         
         I realized from your earlier comment that, calling 
         xfs_syncd_stop under xfs_bwrite won't be good either;
         so I was thinking of just checking for 

         XFS_FORCED_SHUTDOWN(mp) in xfs_log_force and bailing out 
         if already shutdown.

         which should take care of xfs_sync_worker, 
         xfs_flush_worker, xfs_iflush and xfs_unmountfs


     Does this sound good?

>
>>  		/* 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....
Yeah, I realized that (sent a patch after this 
http://article.gmane.org/gmane.comp.file-systems.xfs.general/45568).
>
>Cheers,
>
>Dave.
>-- 
>Dave Chinner
>david@fromorbit.com
>








Regards,
-- 
Raghavendra Prabhu
GPG Id : 0xD72BE977
Fingerprint: B93F EBCB 8E05 7039 CD3C A4B8 A616 DCA1 D72B E977
www: wnohang.net

[-- Attachment #1.2: Type: application/pgp-signature, Size: 490 bytes --]

[-- Attachment #2: Type: text/plain, Size: 121 bytes --]

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

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

[Raghavendra D Prabhu]

[-- Attachment #1.1: Type: text/plain, Size: 5313 bytes --]

Hi,

Resent with changes I had mentioned earlier.

================================================

[PATCH] This is to prevent xfs_log_force from running ad-infinitum (due to xfs_sync) till umount if the disk has been forcefully unplugged.

This is also 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 simulate it, mount an xfs filesystem located on external disk, and then pull
the power to the disk (non-usb powered disk).

Tested it on latest linus tree.
Now, the kernel log 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)

Signed-off-by: Raghavendra D Prabhu <rprabhu@wnohang.net>
Tested-by: Raghavendra D Prabhu <rprabhu@wnohang.net>
---
---
  fs/xfs/xfs_log.c |    7 +++++++
  1 file changed, 7 insertions(+)

diff --git a/fs/xfs/xfs_log.c b/fs/xfs/xfs_log.c
index 6db1fef..e4192b2 100644
--- a/fs/xfs/xfs_log.c
+++ b/fs/xfs/xfs_log.c
@@ -2932,6 +2932,13 @@ xfs_log_force(
  {
  	int	error;
  
+	/*
+	 * No need to printk here since xfs_bwrite already printks about xfs
+	 * shutdown if it has shutdown already.
+	 */
+	if (XFS_FORCED_SHUTDOWN(mp))
+		return;
+
  	error = _xfs_log_force(mp, flags, NULL);
  	if (error)
  		xfs_warn(mp, "%s: error %d returned.", __func__, error);
-- 
1.7.10.1


* On Thu, May 10, 2012 at 01:37:46AM +0530, Raghavendra D Prabhu <raghu.prabhu13@gmail.com> wrote:
>
>Hi,
>
>
>* On Tue, May 08, 2012 at 09:53:21AM +1000, Dave Chinner <david@fromorbit.com> wrote:
>>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.
>
>Yes, in the next patch I had sent -- 
>http://article.gmane.org/gmane.comp.file-systems.xfs.general/45568 -- 
>I removed xfs_syncd_stop from there and added it under xfs_bwrite.
>
>(PS: Adding xfs_syncd_stop in xfs_sync_worker was a very bad decision 
>- it sent few of my kworkers to 'D' state and nearly corrupted that 
>fs)
>
>
>>
>>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.
>                 I realized from your earlier comment that, calling         
>xfs_syncd_stop under xfs_bwrite won't be good either;
>        so I was thinking of just checking for
>
>        XFS_FORCED_SHUTDOWN(mp) in xfs_log_force and bailing out         
>if already shutdown.
>
>        which should take care of xfs_sync_worker,         
>xfs_flush_worker, xfs_iflush and xfs_unmountfs
>
>
>    Does this sound good?
>
>>
>>> 		/* 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....
>Yeah, I realized that (sent a patch after this 
>http://article.gmane.org/gmane.comp.file-systems.xfs.general/45568).
>>
>>Cheers,
>>
>>Dave.
>>-- 
>>Dave Chinner
>>david@fromorbit.com
>>
>
>
>
>
>
>
>
>
>Regards,
>-- 
>Raghavendra Prabhu
>GPG Id : 0xD72BE977
>Fingerprint: B93F EBCB 8E05 7039 CD3C A4B8 A616 DCA1 D72B E977
>www: wnohang.net






Regards,
-- 
Raghavendra Prabhu
GPG Id : 0xD72BE977
Fingerprint: B93F EBCB 8E05 7039 CD3C A4B8 A616 DCA1 D72B E977
www: wnohang.net

[-- Attachment #1.2: Type: application/pgp-signature, Size: 490 bytes --]

[-- Attachment #2: Type: text/plain, Size: 121 bytes --]

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