Assertion failed: atomic_read(&mp->m_active_trans)

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

Dont assert if transactions are in flight for release builds.

@@ -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);

_______________________________________________
xfs mailing list
[email protected]
http://oss.sgi.com/mailman/listinfo/xfs

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.

_______________________________________________
xfs mailing list
[email protected]
http://oss.sgi.com/mailman/listinfo/xfs

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

_______________________________________________
xfs mailing list
[email protected]
http://oss.sgi.com/mailman/listinfo/xfs

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...

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
[email protected]

_______________________________________________
xfs mailing list
[email protected]
http://oss.sgi.com/mailman/listinfo/xfs

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).

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

@@ -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);

_______________________________________________
xfs mailing list
[email protected]
http://oss.sgi.com/mailman/listinfo/xfs

Makes sense.

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
[email protected]
http://oss.sgi.com/mailman/listinfo/xfs