Assertion failed: atomic_read(&mp->m_active_trans)

Assertion failed: atomic_read(&mp->m_active_trans)

[Donald Douwsma]

[-- Attachment #1: Type: text/plain, Size: 1634 bytes --]

We still occasionally see transactions in flight after remounting
read-only. This has come up a few times in the past, but we never
seem to have gotten to the bottom of it.

http://www.gossamer-threads.com/lists/linux/kernel/868139

Most recently we've seen this on 2.6.27, when unmounting the root
filesystem during shutdown/reboot.

Stack traceback for pid 13170
0xffff81024dcd9080    13170    12901  1    1   R  0xffff81024dcd93c0 *mount
rsp                rip                Function (args)
0xffff8101fb977d18 0xffffffff803b8acd assfail+0x1a (invalid, invalid, invalid)
0xffff8101fb977d50 0xffffffff803a57e4 xfs_attr_quiesce+0x4a (0xffff8102211e4b20)
0xffff8101fb977d70 0xffffffff803a589b xfs_mntupdate+0x7c (0xffff8102211e4b20, invalid, invalid)
0xffff8101fb977d90 0xffffffff803b7cf6 xfs_fs_remount+0x49 (invalid, 0xffff8101fb977dd4, invalid)
0xffff8101fb977dc0 0xffffffff802830fe do_remount_sb+0xe9 (0xffff81025c804670, invalid, 0xffff8101ee490000, invalid)
0xffff8101fb977e00 0xffffffff8029698d do_remount+0x7d (0xffff8101fb977e58, invalid, invalid, 0xffff8101ee490000)
0xffff8101fb977e40 0xffffffff802974fd do_mount+0x13b (0xffff8102079c2000, 0xffff8102004ea000, 0xffff810219cb0000, invalid, 0xffff8101ee490000)
0xffff8101fb977f20 0xffffffff8029761a sys_mount+0x89 (0x523d90, invalid, invalid, 0xffffffffc0ed0021, 0x523e30)
0xffff8101fb977f80 0xffffffff8020b18b system_call_after_swapgs+0x7b (invalid, invalid, invalid, invalid, invalid, invalid)

Previously we've discussed changing the ASSERT_ALWAYS to a normal
ASSERT to lessen the impact for users. Any objections to doing this
until we fix the underlying problem?

Don



[-- Attachment #2: Dont-assert-if-transactions-are-in-flight-for-release-builds --]
[-- Type: text/plain, Size: 492 bytes --]

Dont assert if transactions are in flight for release builds. 

--- a/fs/xfs/linux-2.6/xfs_sync.c
+++ b/fs/xfs/linux-2.6/xfs_sync.c
@@ -371,7 +371,7 @@ 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);
+       ASSERT(atomic_read(&mp->m_active_trans) == 0);
 
        /* Push the superblock and write an unmount record */
        error = xfs_log_sbcount(mp, 1);


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

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

Re: Assertion failed: atomic_read(&mp->m_active_trans)

[Lachlan McIlroy]

That looks fine to me.  Just make sure the PV doesn't get closed when
you check in the changes since the real problem is still unresolved.

Donald Douwsma wrote:
> We still occasionally see transactions in flight after remounting
> read-only. This has come up a few times in the past, but we never
> seem to have gotten to the bottom of it.
> 
> http://www.gossamer-threads.com/lists/linux/kernel/868139
> 
> Most recently we've seen this on 2.6.27, when unmounting the root
> filesystem during shutdown/reboot.
> 
> Stack traceback for pid 13170
> 0xffff81024dcd9080    13170    12901  1    1   R  0xffff81024dcd93c0 *mount
> rsp                rip                Function (args)
> 0xffff8101fb977d18 0xffffffff803b8acd assfail+0x1a (invalid, invalid, invalid)
> 0xffff8101fb977d50 0xffffffff803a57e4 xfs_attr_quiesce+0x4a (0xffff8102211e4b20)
> 0xffff8101fb977d70 0xffffffff803a589b xfs_mntupdate+0x7c (0xffff8102211e4b20, invalid, invalid)
> 0xffff8101fb977d90 0xffffffff803b7cf6 xfs_fs_remount+0x49 (invalid, 0xffff8101fb977dd4, invalid)
> 0xffff8101fb977dc0 0xffffffff802830fe do_remount_sb+0xe9 (0xffff81025c804670, invalid, 0xffff8101ee490000, invalid)
> 0xffff8101fb977e00 0xffffffff8029698d do_remount+0x7d (0xffff8101fb977e58, invalid, invalid, 0xffff8101ee490000)
> 0xffff8101fb977e40 0xffffffff802974fd do_mount+0x13b (0xffff8102079c2000, 0xffff8102004ea000, 0xffff810219cb0000, invalid, 0xffff8101ee490000)
> 0xffff8101fb977f20 0xffffffff8029761a sys_mount+0x89 (0x523d90, invalid, invalid, 0xffffffffc0ed0021, 0x523e30)
> 0xffff8101fb977f80 0xffffffff8020b18b system_call_after_swapgs+0x7b (invalid, invalid, invalid, invalid, invalid, invalid)
> 
> Previously we've discussed changing the ASSERT_ALWAYS to a normal
> ASSERT to lessen the impact for users. Any objections to doing this
> until we fix the underlying problem?
> 
> Don
> 
> 
> 
> 
> ------------------------------------------------------------------------
> 
> _______________________________________________
> xfs mailing list
> xfs@oss.sgi.com
> http://oss.sgi.com/mailman/listinfo/xfs

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

Re: Assertion failed: atomic_read(&mp->m_active_trans)

[Christoph Hellwig]

I'd rather fix it properly.  Do you guys have a somewhat reliable
testcase hitting it?

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

Re: Assertion failed: atomic_read(&mp->m_active_trans)

[Dave Chinner]

On Wed, Dec 03, 2008 at 05:48:49AM -0500, Christoph Hellwig wrote:
> I'd rather fix it properly.

Sure, but in the mean time, I'd suggest changing it to a WARN_ON()
rather than an ASSERT(). That way we'll continue to have ppl bug us
about it until the VFS can support read-only remounts without racing
correctly.

Has that work been dropped on the floor, Christoph? We've
been holding off removing this ASSERT or adding the hack
I did to work around the common case of the assert triggering
based on the fact that the problem in the VFS would be fixed
in the next release. That was the case each release since
2.6.25 and there doesn't seem to be much progress...

> Do you guys have a somewhat reliable
> testcase hitting it?

I used to have one of the xfsqa tests hit it every so often,
but not what you'd call reliably....

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

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

Re: Assertion failed: atomic_read(&mp->m_active_trans)

[Donald Douwsma]

[-- Attachment #1: Type: text/plain, Size: 1321 bytes --]

Dave Chinner wrote:
> On Wed, Dec 03, 2008 at 05:48:49AM -0500, Christoph Hellwig wrote:
>> I'd rather fix it properly.
> 
> Sure, but in the mean time, I'd suggest changing it to a WARN_ON()
> rather than an ASSERT(). That way we'll continue to have ppl bug us
> about it until the VFS can support read-only remounts without racing
> correctly.

That sounds like a much better idea.

Unfortunately we wont get feedback when people hit this on the root fs
unless they have serial consoles (since /var/log/... has gone away by then).

> Has that work been dropped on the floor, Christoph? We'vecat 
> been holding off removing this ASSERT or adding the hack
> I did to work around the common case of the assert triggering
> based on the fact that the problem in the VFS would be fixed
> in the next release. That was the case each release since
> 2.6.25 and there doesn't seem to be much progress...
> 
>> Do you guys have a somewhat reliable
>> testcase hitting it?
> 
> I used to have one of the xfsqa tests hit it every so often,
> but not what you'd call reliably....

Indeed, we dont hit this in regular qa. I guess most qa scripts dont
specifically exercise remount readonly wile stressing the filesystem.
The few occasions we have hit it were when rebooting in between test
runs when the root fs was xfs.

Don






[-- Attachment #2: warn-if-transactions-are-in-flight-on-remount-ro --]
[-- Type: text/plain, Size: 430 bytes --]


--- a/fs/xfs/linux-2.6/xfs_sync.c
+++ b/fs/xfs/linux-2.6/xfs_sync.c
@@ -371,7 +371,7 @@ 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);
+       WARN_ON(atomic_read(&mp->m_active_trans) == 0);
 
        /* Push the superblock and write an unmount record */
        error = xfs_log_sbcount(mp, 1);


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

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

Re: Assertion failed: atomic_read(&mp->m_active_trans)

[Christoph Hellwig]

On Thu, Dec 04, 2008 at 08:39:50AM +1100, Dave Chinner wrote:
> On Wed, Dec 03, 2008 at 05:48:49AM -0500, Christoph Hellwig wrote:
> > I'd rather fix it properly.
> 
> Sure, but in the mean time, I'd suggest changing it to a WARN_ON()
> rather than an ASSERT(). That way we'll continue to have ppl bug us
> about it until the VFS can support read-only remounts without racing
> correctly.

Makes sense.

> Has that work been dropped on the floor, Christoph? We've
> been holding off removing this ASSERT or adding the hack
> I did to work around the common case of the assert triggering
> based on the fact that the problem in the VFS would be fixed
> in the next release. That was the case each release since
> 2.6.25 and there doesn't seem to be much progress...

Yeah, once we got the r/o bind mounts which introduces the
infrastructure to deal with people dropped that ball and we never fixed
it.  But I just heard from Al that he's looking into some major surgery
for the remount path, which should include this in the second or third
batch.

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