Re: [PATCH] [XFS] Warn on transaction in flight on read-only remount

Re: [PATCH] [XFS] Warn on transaction in flight on read-only remount

[Felix Blyakher]

[resending after fixing xfs mailing list on oss]

Just wanted to point out to discussion on this topic:

http://oss.sgi.com/archives/xfs/2008-12/msg00019.html

I merely took a suggestion from it, and put it in a patch.

Felix

On Jan 21, 2009, at 4:57 PM, Felix Blyakher wrote:

> Till VFS can correctly support read-only remount without racing,
> use WARN_ON instead of BUG_ON on detecting transaction in flight
> after quiescing filesystem.
>
> Signed-off-by: Felix Blyakher <felixb@sgi.com>
> ---
>  fs/xfs/linux-2.6/xfs_sync.c |    5 ++++-
>  1 files changed, 4 insertions(+), 1 deletions(-)
>
> diff --git a/fs/xfs/linux-2.6/xfs_sync.c b/fs/xfs/linux-2.6/xfs_sync.c
> index 2ed0353..d8373ee 100644
> --- a/fs/xfs/linux-2.6/xfs_sync.c
> +++ b/fs/xfs/linux-2.6/xfs_sync.c
> @@ -371,7 +371,10 @@ xfs_quiesce_attr(
>  	/* flush inodes and push all remaining buffers out to disk */
>  	xfs_quiesce_fs(mp);
>
> -	ASSERT_ALWAYS(atomic_read(&mp->m_active_trans) == 0);
> +	/* Just warn here till VFS can correctly support
> +	 * read-only remount without racing.
> +	 */
> +	WARN_ON(atomic_read(&mp->m_active_trans) == 0);
>
>  	/* Push the superblock and write an unmount record */
>  	error = xfs_log_sbcount(mp, 1);
> -- 
> 1.6.1
>

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

Re: [PATCH] [XFS] Warn on transaction in flight on read-only remount

[Christoph Hellwig]

On Thu, Jan 22, 2009 at 12:10:15AM -0600, Felix Blyakher wrote:
> [resending after fixing xfs mailing list on oss]
>
> Just wanted to point out to discussion on this topic:
>
> http://oss.sgi.com/archives/xfs/2008-12/msg00019.html
>
> I merely took a suggestion from it, and put it in a patch.

Did you actually test it?  WARN_ON has an inverted sense over
ASSERT/ASSERT_ALWAYS, so this would trigger all the time.

>> +	/* Just warn here till VFS can correctly support
>> +	 * read-only remount without racing.
>> +	 */

Also this should be:

	/*
	 * ...

I also have a VFS -level patch somewhere to prevent further writers to
occur during a remount in a rather hacky way, but I'd rather have a
testcase to reproduce this reliably before sending it to Al.

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

Re: [PATCH] [XFS] Warn on transaction in flight on read-only remount

[Felix Blyakher]

On Jan 22, 2009, at 12:27 AM, Christoph Hellwig wrote:

> On Thu, Jan 22, 2009 at 12:10:15AM -0600, Felix Blyakher wrote:
>> [resending after fixing xfs mailing list on oss]
>>
>> Just wanted to point out to discussion on this topic:
>>
>> http://oss.sgi.com/archives/xfs/2008-12/msg00019.html
>>
>> I merely took a suggestion from it, and put it in a patch.
>
> Did you actually test it?  WARN_ON has an inverted sense over
> ASSERT/ASSERT_ALWAYS, so this would trigger all the time.

Oops. Sure. Thanks for noticing it.

>
>>> +	/* Just warn here till VFS can correctly support
>>> +	 * read-only remount without racing.
>>> +	 */
>
> Also this should be:
>
> 	/*
> 	 * ...

Noted.

>
> I also have a VFS -level patch somewhere to prevent further writers to
> occur during a remount in a rather hacky way, but I'd rather have a
> testcase to reproduce this reliably before sending it to Al.

Sorry, I don't have a reproducible test case.
Just thought to prevent panic for now.

I'll repost the patch tomorrow.

Felix

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

[PATCH] [XFS] Warn on transaction in flight on read-only remount

[Felix Blyakher]

Till VFS can correctly support read-only remount without racing,
use WARN_ON instead of BUG_ON on detecting transaction in flight
after quiescing filesystem.

Signed-off-by: Felix Blyakher <felixb@sgi.com>
---
 fs/xfs/linux-2.6/xfs_sync.c |    6 +++++-
 1 files changed, 5 insertions(+), 1 deletions(-)

diff --git a/fs/xfs/linux-2.6/xfs_sync.c b/fs/xfs/linux-2.6/xfs_sync.c
index 2ed0353..a608e72 100644
--- a/fs/xfs/linux-2.6/xfs_sync.c
+++ b/fs/xfs/linux-2.6/xfs_sync.c
@@ -371,7 +371,11 @@ xfs_quiesce_attr(
 	/* flush inodes and push all remaining buffers out to disk */
 	xfs_quiesce_fs(mp);
 
-	ASSERT_ALWAYS(atomic_read(&mp->m_active_trans) == 0);
+	/*
+	 * Just warn here till VFS can correctly support
+	 * read-only remount without racing.
+	 */
+	WARN_ON(atomic_read(&mp->m_active_trans) != 0);
 
 	/* Push the superblock and write an unmount record */
 	error = xfs_log_sbcount(mp, 1);
-- 
1.6.1

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

Re: [PATCH] [XFS] Warn on transaction in flight on read-only remount

[Christoph Hellwig]

On Fri, Jan 23, 2009 at 12:14:30AM -0600, Felix Blyakher wrote:
> Till VFS can correctly support read-only remount without racing,
> use WARN_ON instead of BUG_ON on detecting transaction in flight
> after quiescing filesystem.
> 
> Signed-off-by: Felix Blyakher <felixb@sgi.com>

Looks good to me.

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

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