On Fri, May 25, 2012 at 03:45:36PM -0500, Ben Myers wrote: > Hey Dave, > > On Thu, May 24, 2012 at 05:39:52PM -0500, Ben Myers wrote: > > Anyway, I'll make some time to work on this tomorrow so I can test it over > > the weekend. > > This is going to spin over the weekend. See what you think. > > ----------- > > xfs: shutdown xfs_sync_worker before the log > > Revert commit 1307bbd, which uses the s_umount semaphore to provide > exclusion between xfs_sync_worker and unmount, in favor of shutting down > the sync worker before freeing the log in xfs_log_unmount. This is a > cleaner way of resolving the race between xfs_sync_worker and unmount > than using s_umount. Looks fine to me. Reviewed-by: Dave Chinner <dchinner@xxxxxxxxxx> Once you commit this, I think I'll do a followup set of patches that remove all the problems caused by trying to start and stop unrelated functionality in the same place. I think starting by renaming the xfs-syncd workqueue to the xfs_mount_wq because there's nothing "sync" related about it's functionality any more. I'll then kill xfs_syncd_init/stop functions and open code the intialisation of the work structures and start them in the appropriate places for their functionality. e.g. reclaim work is demand started and stops when there's nothing more to do or at unmount, the flush work is demand started and we need to complete them all at unmount, and the xfssync work is really now "periodic log work" so should be started once we complete log recovery successfullly and stopped before we tear down the log.... Then I can move the xfs_sync_worker to xfs_log.c and rename it. If I then convert xfs_flush_worker to use the generic writeback code (writeback_inodes_sb_if_idle) the xfs_sync_data() can go away. That means the only user of xfs_inode_ag_iterator is the quotaoff code (xfs_qm_dqrele_all_inodes), so it could be moved elsewhere (like xfs_inode.c). Then xfs_quiesce_data() can be moved to xfs-super.c where it can sit alongside the two functions that call it, and the same can be done for xfs_quiesce_attr(). That will leave only inode cache reclaim functions in xfs_sync.c. These are closely aligned to the inode allocation, freeing and cache lookup functions in xfs_iget.c, so I'm thinking of merging the two into a single file named xfs_inode_cache.c so both xfs_sync.c and xfs_iget.c go away. Thoughts? Cheers, Dave. -- Dave Chinner david@xxxxxxxxxxxxx _______________________________________________ xfs mailing list xfs@xxxxxxxxxxx http://oss.sgi.com/mailman/listinfo/xfs
