: xfs: remove the xfssyncd mess
v5 cover : xfs: remove the xfssyncd mess
Hopefully the final version.
Version 5:
- move initial queuing of log work so it isn't started on log mount error
- move xfs_flush_inodes() to xfs_super.c
- pass xfs_mount to xfs_flush_inodes()
- rewrite commit message for xfs_flush_inodes() changes
- added last_sync_lsn fix to series as the problem it fixes is reliably exposed by the changes in this series
Version 4:
- updated 'xfs: xfs_sync_data is redundant' with cleanups to the xfs_flush_inodes interface as per Christoph's request,
- updated 'xfs: xfs_sync_data is redundant', folding in changes from
http://oss.sgi.com/archives/xfs/2012-10/msg00036.html
- fixed a minor typo in xfs: 'syncd workqueue is no more', renaming the log worker from 'xfs-reclaim' to 'xfs-log'.
Version 3:
- per-mount log and reclaim workqueues instead of a generic mount workqueue.
- reordering of some of the patches as Christoph requested.
- fixed the writeback_inodes_sb_if_idle deadlock by moving it all the way back up the write stack to xfs_file_aio_buffered_write where we were just flushing the current file to avoid deadlocking on it anyway.
- reintroduced xfs_flush_inodes() as a wrapper for writeback_inodes_sb_if_idle().
- rebased on a current TOT.
Version 2:
- fix writeback_inodes_sb_if_idle call in xfs_create()
- refreshed patch 13 before sending.
Initial version:
- http://oss.sgi.com/archives/xfs/2012-06/msg00064.html
_______________________________________________
xfs mailing list
[email protected]
http://oss.sgi.com/mailman/listinfo/xfs
PATCH v5 1/14 xfs: xfs_syncd_stop must die
From: Dave Chinner <[email protected]>
xfs_syncd_start and xfs_syncd_stop tie a bunch of unrelated functionailty together that actually have different start and stop requirements. Kill these functions and open code the start/stop methods for each of the background functions.
Subsequent patches will move the start/stop functions around to the correct places to avoid races and shutdown issues.
--- fs/xfs/xfs_super.c | 25 ++++++++++++++++++------- fs/xfs/xfs_sync.c | 30 ++++-------------------------- fs/xfs/xfs_sync.h | 6 ++++-- 3 files changed, 26 insertions(+), 35 deletions(-)
fs/xfs/xfs_super.c +18 -7
@@ -1008,7 +1008,11 @@ xfs_fs_put_super(xfs_filestream_unmount (mp);cancel_delayed_work_sync (&mp->m_sync_work);xfs_unmountfs (mp); -xfs_syncd_stop (mp); + +cancel_delayed_work_sync (&mp->m_sync_work); +cancel_delayed_work_sync (&mp->m_reclaim_work); +cancel_work_sync (&mp->m_flush_work); +xfs_freesb (mp);xfs_icsb_destroy_counters (mp);xfs_destroy_mount_workqueues (mp);
@@ -1384,9 +1388,11 @@ xfs_fs_fill_super( sb->s_time_gran =1 ;set_posix_acl_flag (sb); - error =xfs_syncd_init (mp); -if (error) - goto out_filestream_unmount; +INIT_WORK (&mp->m_flush_work, xfs_flush_worker); +INIT_DELAYED_WORK (&mp->m_sync_work, xfs_sync_worker); +INIT_DELAYED_WORK (&mp->m_reclaim_work, xfs_reclaim_worker); + +xfs_syncd_queue_sync (mp); error =xfs_mountfs (mp);if (error)
@@ -1409,8 +1415,10 @@ xfs_fs_fill_super(return 0 ; out_syncd_stop: -xfs_syncd_stop (mp); - out_filestream_unmount: +cancel_delayed_work_sync (&mp->m_sync_work); +cancel_delayed_work_sync (&mp->m_reclaim_work); +cancel_work_sync (&mp->m_flush_work); +xfs_filestream_unmount (mp); out_free_sb:xfs_freesb (mp);
@@ -1429,7 +1437,10 @@ out_destroy_workqueues: out_unmount:xfs_filestream_unmount (mp);xfs_unmountfs (mp); -xfs_syncd_stop (mp); + +cancel_delayed_work_sync (&mp->m_sync_work); +cancel_delayed_work_sync (&mp->m_reclaim_work); +cancel_work_sync (&mp->m_flush_work); goto out_free_sb; }
fs/xfs/xfs_sync.c +4 -26
@@ -370,7 +370,7 @@ xfs_quiesce_attr(xfs_buf_unlock (mp->m_sb_bp); } -static void +void xfs_syncd_queue_sync (struct xfs_mount *mp) {
@@ -383,7 +383,7 @@ xfs_syncd_queue_sync(* disk quotas. We might need to cover the log to indicate that the * filesystem is idle and not frozen. */ -STATIC void +void xfs_sync_worker (struct work_struct *work) {
@@ -445,7 +445,7 @@ xfs_syncd_queue_reclaim(* goes low. It scans as quickly as possible avoiding locked inodes or those * already being flushed, and once done schedules a future pass. */ -STATIC void +void xfs_reclaim_worker (struct work_struct *work) {
@@ -478,7 +478,7 @@ xfs_flush_inodes(flush_work_sync (&mp->m_flush_work); } -STATIC void +void xfs_flush_worker (struct work_struct *work) {
@@ -489,28 +489,6 @@ xfs_flush_worker(xfs_sync_data (mp,SYNC_TRYLOCK |SYNC_WAIT ); } -int -xfs_syncd_init ( -struct xfs_mount *mp) -{ -INIT_WORK (&mp->m_flush_work, xfs_flush_worker); -INIT_DELAYED_WORK (&mp->m_sync_work, xfs_sync_worker); -INIT_DELAYED_WORK (&mp->m_reclaim_work, xfs_reclaim_worker); - -xfs_syncd_queue_sync (mp); - -return 0 ; -} - -void -xfs_syncd_stop ( -struct xfs_mount *mp) -{ -cancel_delayed_work_sync (&mp->m_sync_work); -cancel_delayed_work_sync (&mp->m_reclaim_work); -cancel_work_sync (&mp->m_flush_work); -} - void __xfs_inode_set_reclaim_tag( struct xfs_perag *pag,
fs/xfs/xfs_sync.h +4 -2
@@ -26,8 +26,10 @@ struct xfs_perag;extern struct workqueue_struct *xfs_syncd_wq;/* sync workqueue */ -int xfs_syncd_init (struct xfs_mount *mp); -void xfs_syncd_stop (struct xfs_mount *mp); +void xfs_syncd_queue_sync (struct xfs_mount *mp); +void xfs_sync_worker (struct work_struct *work); +void xfs_flush_worker (struct work_struct *work); +void xfs_reclaim_worker (struct work_struct *work);int xfs_quiesce_data (struct xfs_mount *mp);void xfs_quiesce_attr (struct xfs_mount *mp);
Signature · 6 lines
1.7.10 _______________________________________________ xfs mailing list [email protected] http://oss.sgi.com/mailman/listinfo/xfs
PATCH v5 2/14 xfs: rationalise xfs_mount_wq users
In reply to Dave ChinnerFrom: Dave Chinner <[email protected]>
Instead of starting and stopping background work on the xfs_mount_wq all at the same time, separate them to where they really are needed to start and stop.
The xfs_sync_worker, only needs to be started after all the mount processing has completed successfully, while it needs to be stopped before the log is unmounted.
The xfs_reclaim_worker is started on demand, and can be stopped before the unmount process does it's own inode reclaim pass.
The xfs_flush_inodes work is run on demand, and so we really only need to ensure that it has stopped running before we start processing an unmount, freeze or remount,ro.
--- fs/xfs/xfs_mount.c | 6 ++++-- fs/xfs/xfs_super.c | 34 ++++++++++++++-------------------- fs/xfs/xfs_sync.c | 21 +++++---------------- 3 files changed, 23 insertions(+), 38 deletions(-)
fs/xfs/xfs_mount.c +4 -2
@@ -1450,9 +1450,11 @@ xfs_unmountfs(/* * And reclaim all inodes. At this point there should be no dirty -* inode, and none should be pinned or locked, but use synchronous -* reclaim just to be sure. +* inodes and none should be pinned or locked, but use synchronous +* reclaim just to be sure. We can stop background inode reclaim +* here as well if it is still running. */ +cancel_delayed_work_sync (&mp->m_reclaim_work);xfs_reclaim_inodes (mp,SYNC_WAIT );xfs_qm_unmount (mp);
fs/xfs/xfs_super.c +14 -20
@@ -1005,14 +1005,12 @@ xfs_fs_put_super( {struct xfs_mount *mp =XFS_M (sb); -xfs_filestream_unmount (mp); -cancel_delayed_work_sync (&mp->m_sync_work); -xfs_unmountfs (mp); -cancel_delayed_work_sync (&mp->m_sync_work); -cancel_delayed_work_sync (&mp->m_reclaim_work);cancel_work_sync (&mp->m_flush_work); +xfs_filestream_unmount (mp); +xfs_unmountfs (mp); +xfs_freesb (mp);xfs_icsb_destroy_counters (mp);xfs_destroy_mount_workqueues (mp);
@@ -1325,6 +1323,9 @@ xfs_fs_fill_super(spin_lock_init (&mp->m_sb_lock);mutex_init (&mp->m_growlock);atomic_set (&mp->m_active_trans,0 ); +INIT_WORK (&mp->m_flush_work, xfs_flush_worker); +INIT_DELAYED_WORK (&mp->m_sync_work, xfs_sync_worker); +INIT_DELAYED_WORK (&mp->m_reclaim_work, xfs_reclaim_worker); mp->m_super = sb; sb->s_fs_info = mp;
@@ -1388,15 +1389,9 @@ xfs_fs_fill_super( sb->s_time_gran =1 ;set_posix_acl_flag (sb); -INIT_WORK (&mp->m_flush_work, xfs_flush_worker); -INIT_DELAYED_WORK (&mp->m_sync_work, xfs_sync_worker); -INIT_DELAYED_WORK (&mp->m_reclaim_work, xfs_reclaim_worker); - -xfs_syncd_queue_sync (mp); - error =xfs_mountfs (mp);if (error) - goto out_syncd_stop; + goto out_filestream_unmount; root =igrab (VFS_I (mp->m_rootip));if (!root) {
@@ -1413,12 +1408,15 @@ xfs_fs_fill_super( goto out_unmount; } +/* +* The filesystem is successfully mounted, so we can start background +* sync work now. +*/ +xfs_syncd_queue_sync (mp); +return 0 ; - out_syncd_stop: -cancel_delayed_work_sync (&mp->m_sync_work); -cancel_delayed_work_sync (&mp->m_reclaim_work); -cancel_work_sync (&mp->m_flush_work); + out_filestream_unmount:xfs_filestream_unmount (mp); out_free_sb:xfs_freesb (mp);
@@ -1437,10 +1435,6 @@ out_destroy_workqueues: out_unmount:xfs_filestream_unmount (mp);xfs_unmountfs (mp); - -cancel_delayed_work_sync (&mp->m_sync_work); -cancel_delayed_work_sync (&mp->m_reclaim_work); -cancel_work_sync (&mp->m_flush_work); goto out_free_sb; }
fs/xfs/xfs_sync.c +5 -16
@@ -379,9 +379,9 @@ xfs_syncd_queue_sync( }/* -* Every sync period we need to unpin all items, reclaim inodes and sync -* disk quotas. We might need to cover the log to indicate that the -* filesystem is idle and not frozen. +* Every sync period we need to unpin all items in the AIL and push them to +* disk. If there is nothing dirty, then we might need to cover the log to +* indicate that the filesystem is idle and not frozen. */ void xfs_sync_worker(
@@ -391,17 +391,7 @@ xfs_sync_worker( struct xfs_mount, m_sync_work);int error; -/* -* We shouldn't write/force the log if we are in the mount/unmount -* process or on a read only filesystem. The workqueue still needs to be -* active in both cases, however, because it is used for inode reclaim -* during these times. Use the MS_ACTIVE flag to avoid doing anything -* during mount. Doing work during unmount is avoided by calling -* cancel_delayed_work_sync on this work queue before tearing down -* the ail and the log in xfs_log_unmount. -*/ -if (!(mp->m_super->s_flags &MS_ACTIVE ) && - !(mp->m_flags &XFS_MOUNT_RDONLY )) { +if (!(mp->m_flags &XFS_MOUNT_RDONLY )) {/* dgc: errors ignored here */ if (mp->m_super->s_writers.frozen ==SB_UNFROZEN &&xfs_log_need_covered (mp))
@@ -409,8 +399,7 @@ xfs_sync_worker(else xfs_log_force (mp,0 ); -/* start pushing all the metadata that is currently -* dirty */ +/* start pushing all the metadata that is currently dirty */ xfs_ail_push_all (mp->m_ail); }
Signature · 6 lines
1.7.10 _______________________________________________ xfs mailing list [email protected] http://oss.sgi.com/mailman/listinfo/xfs
PATCH v5 3/14 xfs: don't run the sync work if the filesystem is read-only
In reply to Dave ChinnerFrom: Dave Chinner <[email protected]>
If the filesystem is mounted or remounted read-only, stop the sync worker that tries to flush or cover the log if the filesystem is dirty. It's read-only, so it isn't dirty. Restart it on a remount,rw as necessary. This avoids the need for RO checks in the work.
Similarly, stop the sync work when the filesystem is frozen, and start it again when the filesysetm is thawed. This avoids the need for special freeze checks in the work.
--- fs/xfs/xfs_super.c | 2 ++ fs/xfs/xfs_sync.c | 29 ++++++++++++++++------------- 2 files changed, 18 insertions(+), 13 deletions(-)
fs/xfs/xfs_super.c +2 -0
@@ -1200,6 +1200,7 @@ xfs_fs_remount(* value if it is non-zero, otherwise go with the default. */ xfs_restore_resvblks (mp); +xfs_syncd_queue_sync (mp); }/* rw -> ro */
@@ -1245,6 +1246,7 @@ xfs_fs_unfreeze(struct xfs_mount *mp =XFS_M (sb);xfs_restore_resvblks (mp); +xfs_syncd_queue_sync (mp);return 0 ; }
fs/xfs/xfs_sync.c +16 -13
@@ -323,6 +323,9 @@ xfs_quiesce_data(* Second stage of a quiesce. The data is already synced, now we have to take * care of the metadata. New transactions are already blocked, so we need to * wait for any remaining transactions to drain out before proceeding. +* +* Note: this stops background sync work - the callers must ensure it is started +* again when appropriate. */ void xfs_quiesce_attr(
@@ -341,6 +344,9 @@ xfs_quiesce_attr(/* flush all pending changes from the AIL */ xfs_ail_push_all_sync (mp->m_ail); +/* stop background sync work */ +cancel_delayed_work_sync (&mp->m_sync_work); +/* * Just warn here till VFS can correctly support * read-only remount without racing.
@@ -379,9 +385,8 @@ xfs_syncd_queue_sync( }/* -* Every sync period we need to unpin all items in the AIL and push them to -* disk. If there is nothing dirty, then we might need to cover the log to -* indicate that the filesystem is idle and not frozen. +* Every sync period we need to push dirty metadata and try to cover the log +* to indicate the filesystem is idle and not frozen. */ void xfs_sync_worker(
@@ -391,17 +396,15 @@ xfs_sync_worker( struct xfs_mount, m_sync_work);int error; -if (!(mp->m_flags &XFS_MOUNT_RDONLY )) { -/* dgc: errors ignored here */ -if (mp->m_super->s_writers.frozen ==SB_UNFROZEN && -xfs_log_need_covered (mp)) - error =xfs_fs_log_dummy (mp); -else -xfs_log_force (mp,0 ); +/* dgc: errors ignored here */ +if (mp->m_super->s_writers.frozen ==SB_UNFROZEN && +xfs_log_need_covered (mp)) + error =xfs_fs_log_dummy (mp); +else +xfs_log_force (mp,0 ); -/* start pushing all the metadata that is currently dirty */ -xfs_ail_push_all (mp->m_ail); - } +/* start pushing all the metadata that is currently dirty */ +xfs_ail_push_all (mp->m_ail);/* queue us up again */ xfs_syncd_queue_sync (mp);
Signature · 6 lines
1.7.10 _______________________________________________ xfs mailing list [email protected] http://oss.sgi.com/mailman/listinfo/xfs
PATCH v5 4/14 xfs: sync work is now only periodic log work
In reply to Dave ChinnerFrom: Dave Chinner <[email protected]>
The only thing the periodic sync work does now is flush the AIL and idle the log. These are really functions of the log code, so move the work to xfs_log.c and rename it appropriately.
The only wart that this leaves behind is the xfssyncd_centisecs sysctl, otherwise the xfssyncd is dead. Clean up any comments that related to xfssyncd to reflect it's passing.
--- fs/xfs/xfs_log.c | 61 +++++++++++++++++++++++++++++++++++++++++-------- fs/xfs/xfs_log.h | 3 +++ fs/xfs/xfs_log_priv.h | 1 + fs/xfs/xfs_mount.h | 1 - fs/xfs/xfs_super.c | 16 ++++--------- fs/xfs/xfs_sync.c | 39 +++---------------------------- fs/xfs/xfs_sync.h | 2 -- 7 files changed, 62 insertions(+), 61 deletions(-)
fs/xfs/xfs_log.c +51 -10
@@ -34,6 +34,7 @@#include "xfs_dinode.h" #include "xfs_inode.h" #include "xfs_trace.h" +#include "xfs_fsops.h" kmem_zone_t *xfs_log_ticket_zone;
@@ -679,25 +680,29 @@ out: }/* -* Finish the recovery of the file system. This is separate from -* the xfs_log_mount() call, because it depends on the code in -* xfs_mountfs() to read in the root and real-time bitmap inodes -* between calling xfs_log_mount() and here. +* Finish the recovery of the file system. This is separate from the +* xfs_log_mount() call, because it depends on the code in xfs_mountfs() to read +* in the root and real-time bitmap inodes between calling xfs_log_mount() and +* here. * -* mp - ubiquitous xfs mount point structure +* If we finish recovery successfully, start the background log work. If we are +* not doing recovery, then we have a RO filesystem and we don't need to start +* it. */ int xfs_log_mount_finish (xfs_mount_t *mp) { -int error; +int error =0 ; -if (!(mp->m_flags &XFS_MOUNT_NORECOVERY )) +if (!(mp->m_flags &XFS_MOUNT_NORECOVERY )) { error =xlog_recover_finish (mp->m_log); -else { - error =0 ; +if (!error) +xfs_log_work_queue (mp); + }else {ASSERT (mp->m_flags &XFS_MOUNT_RDONLY ); } +return error; }
@@ -858,7 +863,7 @@ xfs_log_unmount_write(xfs_mount_t *mp)void xfs_log_unmount (xfs_mount_t *mp) { -cancel_delayed_work_sync (&mp->m_sync_work); +cancel_delayed_work_sync (&mp->m_log->l_work);xfs_trans_ail_destroy (mp);xlog_dealloc_log (mp->m_log); }
@@ -1161,6 +1166,40 @@ done: }/* xlog_get_iclog_buffer_size */ +void +xfs_log_work_queue ( +struct xfs_mount *mp) +{ +queue_delayed_work (xfs_syncd_wq, &mp->m_log->l_work, +msecs_to_jiffies (xfs_syncd_centisecs *10 )); +} + +/* +* Every sync period we need to unpin all items in the AIL and push them to +* disk. If there is nothing dirty, then we might need to cover the log to +* indicate that the filesystem is idle. +*/ +void +xfs_log_worker ( +struct work_struct *work) +{ +struct xlog *log =container_of (to_delayed_work (work), + struct xlog, l_work); +struct xfs_mount *mp = log->l_mp; + +/* dgc: errors ignored - not fatal and nowhere to report them */ +if (xfs_log_need_covered (mp)) +xfs_fs_log_dummy (mp); +else +xfs_log_force (mp,0 ); + +/* start pushing all the metadata that is currently dirty */ +xfs_ail_push_all (mp->m_ail); + +/* queue us up again */ +xfs_log_work_queue (mp); +} +/* * This routine initializes some of the log structure for a given mount point. * Its primary purpose is to fill in enough, so recovery can occur. However,
@@ -1195,6 +1234,7 @@ xlog_alloc_log( log->l_logBBsize = num_bblks; log->l_covered_state =XLOG_STATE_COVER_IDLE ; log->l_flags |=XLOG_ACTIVE_RECOVERY ; +INIT_DELAYED_WORK (&log->l_work, xfs_log_worker); log->l_prev_block =-1 ;/* log->l_tail_lsn = 0x100000000LL; cycle = 1; current block = 0 */
@@ -3700,3 +3740,4 @@ xlog_iclogs_empty( }while (iclog != log->l_iclog);return 1 ; } +
fs/xfs/xfs_log.h +3 -0
@@ -181,5 +181,8 @@ int xfs_log_commit_cil(struct xfs_mount *mp, struct xfs_trans *tp, xfs_lsn_t *commit_lsn, int flags);bool xfs_log_item_in_current_chkpt (struct xfs_log_item *lip); +void xfs_log_work_queue (struct xfs_mount *mp); +void xfs_log_worker (struct work_struct *work); +#endif #endif /* __XFS_LOG_H__ */
fs/xfs/xfs_log_priv.h +1 -0
@@ -495,6 +495,7 @@ struct xlog {struct xfs_buf *l_xbuf;/* extra buffer for log * wrapping */ struct xfs_buftarg *l_targ;/* buftarg of log */ +struct delayed_work l_work;/* background flush work */ uint l_flags;uint l_quotaoffs_flag;/* XFS_DQ_*, for QUOTAOFFs */ struct list_head *l_buf_cancel_table;
fs/xfs/xfs_mount.h +0 -1
@@ -197,7 +197,6 @@ typedef struct xfs_mount {struct mutex m_icsb_mutex;/* balancer sync lock */ #endif struct xfs_mru_cache *m_filestream;/* per-mount filestream data */ -struct delayed_work m_sync_work;/* background sync work */ struct delayed_work m_reclaim_work;/* background inode reclaim */ struct work_struct m_flush_work;/* background inode flush */ __int64_t m_update_flags;/* sb flags we need to update
fs/xfs/xfs_super.c +4 -12
@@ -1005,7 +1005,6 @@ xfs_fs_put_super( {struct xfs_mount *mp =XFS_M (sb); -cancel_delayed_work_sync (&mp->m_sync_work);cancel_work_sync (&mp->m_flush_work);xfs_filestream_unmount (mp);
@@ -1040,10 +1039,10 @@ xfs_fs_sync_fs(if (laptop_mode) {/* * The disk must be active because we're syncing. -* We schedule xfssyncd now (now that the disk is +* We schedule log work now (now that the disk is * active) instead of later (when it might not be). */ -flush_delayed_work_sync (&mp->m_sync_work); +flush_delayed_work_sync (&mp->m_log->l_work); }return 0 ;
@@ -1200,7 +1199,7 @@ xfs_fs_remount(* value if it is non-zero, otherwise go with the default. */ xfs_restore_resvblks (mp); -xfs_syncd_queue_sync (mp); +xfs_log_work_queue (mp); }/* rw -> ro */
@@ -1246,7 +1245,7 @@ xfs_fs_unfreeze(struct xfs_mount *mp =XFS_M (sb);xfs_restore_resvblks (mp); -xfs_syncd_queue_sync (mp); +xfs_log_work_queue (mp);return 0 ; }
@@ -1326,7 +1325,6 @@ xfs_fs_fill_super(mutex_init (&mp->m_growlock);atomic_set (&mp->m_active_trans,0 );INIT_WORK (&mp->m_flush_work, xfs_flush_worker); -INIT_DELAYED_WORK (&mp->m_sync_work, xfs_sync_worker);INIT_DELAYED_WORK (&mp->m_reclaim_work, xfs_reclaim_worker); mp->m_super = sb;
@@ -1410,12 +1408,6 @@ xfs_fs_fill_super( goto out_unmount; } -/* -* The filesystem is successfully mounted, so we can start background -* sync work now. -*/ -xfs_syncd_queue_sync (mp); -return 0 ; out_filestream_unmount:
fs/xfs/xfs_sync.c +3 -36
@@ -19,6 +19,7 @@#include "xfs_fs.h" #include "xfs_types.h" #include "xfs_log.h" +#include "xfs_log_priv.h" #include "xfs_inum.h" #include "xfs_trans.h" #include "xfs_trans_priv.h"
@@ -344,8 +345,8 @@ xfs_quiesce_attr(/* flush all pending changes from the AIL */ xfs_ail_push_all_sync (mp->m_ail); -/* stop background sync work */ -cancel_delayed_work_sync (&mp->m_sync_work); +/* stop background log work */ +cancel_delayed_work_sync (&mp->m_log->l_work);/* * Just warn here till VFS can correctly support
@@ -376,40 +377,6 @@ xfs_quiesce_attr(xfs_buf_unlock (mp->m_sb_bp); } -void -xfs_syncd_queue_sync ( -struct xfs_mount *mp) -{ -queue_delayed_work (xfs_syncd_wq, &mp->m_sync_work, -msecs_to_jiffies (xfs_syncd_centisecs *10 )); -} - -/* -* Every sync period we need to push dirty metadata and try to cover the log -* to indicate the filesystem is idle and not frozen. -*/ -void -xfs_sync_worker ( -struct work_struct *work) -{ -struct xfs_mount *mp =container_of (to_delayed_work (work), - struct xfs_mount, m_sync_work); -int error; - -/* dgc: errors ignored here */ -if (mp->m_super->s_writers.frozen ==SB_UNFROZEN && -xfs_log_need_covered (mp)) - error =xfs_fs_log_dummy (mp); -else -xfs_log_force (mp,0 ); - -/* start pushing all the metadata that is currently dirty */ -xfs_ail_push_all (mp->m_ail); - -/* queue us up again */ -xfs_syncd_queue_sync (mp); -} -/* * Queue a new inode reclaim pass if there are reclaimable inodes and there * isn't a reclaim pass already in progress. By default it runs every 5s based
fs/xfs/xfs_sync.h +0 -2
@@ -26,8 +26,6 @@ struct xfs_perag;extern struct workqueue_struct *xfs_syncd_wq;/* sync workqueue */ -void xfs_syncd_queue_sync (struct xfs_mount *mp); -void xfs_sync_worker (struct work_struct *work);void xfs_flush_worker (struct work_struct *work);void xfs_reclaim_worker (struct work_struct *work);
Signature · 6 lines
1.7.10 _______________________________________________ xfs mailing list [email protected] http://oss.sgi.com/mailman/listinfo/xfs
PATCH v5 5/14 xfs: Bring some sanity to log unmounting
In reply to Dave ChinnerFrom: Dave Chinner <[email protected]>
When unmounting the filesystem, there are lots of operations that need to be done in a specific order, and they are spread across across a couple of functions. We have to drain the AIL before we write the unmount record, and we have to shut down the background log work before we do either of them.
But this is all split haphazardly across xfs_unmountfs() and xfs_log_unmount(). Move all the AIL flushing and log manipulations to xfs_log_unmount() so that the responisbilities of each function is clear and the operations they perform obvious.
--- fs/xfs/xfs_log.c | 29 ++++++++++++++++++++++++++--- fs/xfs/xfs_mount.c | 24 ------------------------ 2 files changed, 26 insertions(+), 27 deletions(-)
fs/xfs/xfs_log.c +26 -3
@@ -855,15 +855,38 @@ xfs_log_unmount_write(xfs_mount_t *mp) }/* xfs_log_unmount_write */ /* -* Deallocate log structures for unmount/relocation. +* Shut down and release the AIL and Log. * -* We need to stop the aild from running before we destroy -* and deallocate the log as the aild references the log. +* During unmount, we need to ensure we flush all the dirty metadata objects +* from the AIL so that the log is empty before we write the unmount record to +* the log. +* +* To do this, we first need to shut down the background log work so it is not +* trying to cover the log as we clean up. We then need to unpin all objects in +* the log so we can then flush them out. Once they have completed their IO and +* run the callbacks removing themselves from the AIL, we can write the unmount +* record, tear down the AIL and finally free the log. */ void xfs_log_unmount (xfs_mount_t *mp) {cancel_delayed_work_sync (&mp->m_log->l_work); +xfs_log_force (mp,XFS_LOG_SYNC ); + +/* +* The superblock buffer is uncached and while xfs_ail_push_all_sync() +* will push it, xfs_wait_buftarg() will not wait for it. Further, +* xfs_buf_iowait() cannot be used because it was pushed with the +* XBF_ASYNC flag set, so we need to use a lock/unlock pair to wait for +* the IO to complete. +*/ +xfs_ail_push_all_sync (mp->m_ail); +xfs_wait_buftarg (mp->m_ddev_targp); +xfs_buf_lock (mp->m_sb_bp); +xfs_buf_unlock (mp->m_sb_bp); + +xfs_log_unmount_write (mp); +xfs_trans_ail_destroy (mp);xlog_dealloc_log (mp->m_log); }
fs/xfs/xfs_mount.c +0 -24
@@ -1460,13 +1460,6 @@ xfs_unmountfs(xfs_qm_unmount (mp);/* -* Flush out the log synchronously so that we know for sure -* that nothing is pinned. This is important because bflush() -* will skip pinned buffers. -*/ -xfs_log_force (mp,XFS_LOG_SYNC ); - -/* * Unreserve any blocks we have so that when we unmount we don't account * the reserved free space as used. This is really only necessary for * lazy superblock counting because it trusts the incore superblock
@@ -1491,23 +1484,6 @@ xfs_unmountfs(xfs_warn (mp,"Unable to update superblock counters. " "Freespace may not be correct on next mount." ); -/* -* At this point we might have modified the superblock again and thus -* added an item to the AIL, thus flush it again. -*/ -xfs_ail_push_all_sync (mp->m_ail); -xfs_wait_buftarg (mp->m_ddev_targp); - -/* -* The superblock buffer is uncached and xfsaild_push() will lock and -* set the XBF_ASYNC flag on the buffer. We cannot do xfs_buf_iowait() -* here but a lock on the superblock buffer will block until iodone() -* has completed. -*/ -xfs_buf_lock (mp->m_sb_bp); -xfs_buf_unlock (mp->m_sb_bp); - -xfs_log_unmount_write (mp);xfs_log_unmount (mp);xfs_uuid_unmount (mp);
Signature · 6 lines
1.7.10 _______________________________________________ xfs mailing list [email protected] http://oss.sgi.com/mailman/listinfo/xfs
PATCH v5 6/14 xfs: xfs_sync_data is redundant.
In reply to Dave ChinnerFrom: Dave Chinner <[email protected]>
We don't do any data writeback from XFS any more - the VFS is completely responsible for that, including for freeze. We can replace the remaining caller with a VFS level function that achieves the same thing, but without conflicting with current writeback work.
This means we can remove the flush_work and xfs_flush_inodes() - the VFS functionality completely replaces the internal flush queue for doing this writeback work in a separate context to avoid stack overruns.
This does have one complication - it cannot be called with page locks held. Hence move the flushing of delalloc space when ENOSPC occurs back up into xfs_file_aio_buffered_write when we don't hold any locks that will stall writeback.
Unfortunately, writeback_inodes_sb_if_idle() is not sufficient to trigger delalloc conversion fast enough to prevent spurious ENOSPC whent here are hundreds of writers, thousands of small files and GBs of free RAM. Hence we need to use sync_sb_inodes() to block callers while we wait for writeback like the previous xfs_flush_inodes implementation did.
That means we have to hold the s_umount lock here, but because this call can nest inside i_mutex (the parent directory in the create case, held by the VFS), we have to use down_read_trylock() to avoid potential deadlocks. In practice, this trylock will succeed on almost every attempt as unmount/remount type operations are exceedingly rare.
Note: we always need to pass a count of zero to generic_file_buffered_write() as the previously written byte count. We only do this by accident before this patch by the virtue of ret always being zero when there are no errors. Make this explicit rather than needing to specifically zero ret in the ENOSPC retry case.
--- fs/xfs/xfs_file.c | 13 +++++---- fs/xfs/xfs_iomap.c | 23 +++++---------- fs/xfs/xfs_mount.h | 1 - fs/xfs/xfs_super.c | 21 +++++++++++-- fs/xfs/xfs_super.h | 1 + fs/xfs/xfs_sync.c | 78 ------------------------------------------------- fs/xfs/xfs_sync.h | 3 -- fs/xfs/xfs_vnodeops.c | 2 +- 8 files changed, 34 insertions(+), 108 deletions(-)
fs/xfs/xfs_file.c +7 -6
@@ -728,16 +728,17 @@ xfs_file_buffered_aio_write( write_retry:trace_xfs_file_buffered_write (ip, count, iocb->ki_pos,0 ); ret =generic_file_buffered_write (iocb, iovp, nr_segs, - pos, &iocb->ki_pos, count, ret); + pos, &iocb->ki_pos, count,0 ); +/* -* if we just got an ENOSPC, flush the inode now we aren't holding any -* page locks and retry *once* +* If we just got an ENOSPC, try to write back all dirty inodes to +* convert delalloc space to free up some of the excess reserved +* metadata space. */ if (ret == -ENOSPC && !enospc) { enospc =1 ; - ret = -xfs_flush_pages (ip,0 ,-1 ,0 ,FI_NONE ); -if (!ret) - goto write_retry; +xfs_flush_inodes (ip->i_mount); + goto write_retry; } current->backing_dev_info =NULL ;
fs/xfs/xfs_iomap.c +7 -16
@@ -373,7 +373,7 @@ xfs_iomap_write_delay(xfs_extlen_t extsz;int nimaps;xfs_bmbt_irec_t imap[XFS_WRITE_IMAPS ]; -int prealloc, flushed =0 ; +int prealloc;int error;ASSERT (xfs_isilocked (ip,XFS_ILOCK_EXCL ));
@@ -434,26 +434,17 @@ retry: }/* -* If bmapi returned us nothing, we got either ENOSPC or EDQUOT. For -* ENOSPC, * flush all other inodes with delalloc blocks to free up -* some of the excess reserved metadata space. For both cases, retry +* If bmapi returned us nothing, we got either ENOSPC or EDQUOT. Retry * without EOF preallocation. */ if (nimaps ==0 ) {trace_xfs_delalloc_enospc (ip, offset, count); -if (flushed) -return XFS_ERROR (error ? error :ENOSPC ); - -if (error ==ENOSPC ) { -xfs_iunlock (ip,XFS_ILOCK_EXCL ); -xfs_flush_inodes (ip); -xfs_ilock (ip,XFS_ILOCK_EXCL ); +if (prealloc) { + prealloc =0 ; + error =0 ; + goto retry; } - - flushed =1 ; - error =0 ; - prealloc =0 ; - goto retry; +return XFS_ERROR (error ? error :ENOSPC ); }if (!(imap[0 ].br_startblock ||XFS_IS_REALTIME_INODE (ip)))
fs/xfs/xfs_mount.h +0 -1
@@ -198,7 +198,6 @@ typedef struct xfs_mount {#endif struct xfs_mru_cache *m_filestream;/* per-mount filestream data */ struct delayed_work m_reclaim_work;/* background inode reclaim */ -struct work_struct m_flush_work;/* background inode flush */ __int64_t m_update_flags;/* sb flags we need to update on the next remount,rw */ struct shrinker m_inode_shrink;/* inode reclaim shrinker */
fs/xfs/xfs_super.c +18 -3
@@ -882,6 +882,24 @@ xfs_destroy_mount_workqueues(destroy_workqueue (mp->m_unwritten_workqueue); } +/* +* Flush all dirty data to disk. Must not be called while holding an XFS_ILOCK +* or a page lock. We use sync_inodes_sb() here to ensure we block while waiting +* for IO to complete so that we effectively throttle multiple callers to the +* rate at which IO is completing. +*/ +void +xfs_flush_inodes ( +struct xfs_mount *mp) +{ +struct super_block *sb = mp->m_super; + +if (down_read_trylock (&sb->s_umount)) { +sync_inodes_sb (sb); +up_read (&sb->s_umount); + } +} +/* Catch misguided souls that try to use this interface on XFS */ STATIC struct inode * xfs_fs_alloc_inode(
@@ -1005,8 +1023,6 @@ xfs_fs_put_super( {struct xfs_mount *mp =XFS_M (sb); -cancel_work_sync (&mp->m_flush_work); -xfs_filestream_unmount (mp);xfs_unmountfs (mp);
@@ -1324,7 +1340,6 @@ xfs_fs_fill_super(spin_lock_init (&mp->m_sb_lock);mutex_init (&mp->m_growlock);atomic_set (&mp->m_active_trans,0 ); -INIT_WORK (&mp->m_flush_work, xfs_flush_worker);INIT_DELAYED_WORK (&mp->m_reclaim_work, xfs_reclaim_worker); mp->m_super = sb;
fs/xfs/xfs_super.h +1 -0
@@ -74,6 +74,7 @@ struct block_device;extern __uint64_t xfs_max_file_offset (unsigned int ); +extern void xfs_flush_inodes (struct xfs_mount *mp);extern void xfs_blkdev_issue_flush (struct xfs_buftarg *);extern xfs_agnumber_t xfs_set_inode32 (struct xfs_mount *);extern xfs_agnumber_t xfs_set_inode64 (struct xfs_mount *);
fs/xfs/xfs_sync.c +0 -78
90 lines not on this page · raw message
fs/xfs/xfs_sync.h +0 -3
@@ -26,14 +26,11 @@ struct xfs_perag;extern struct workqueue_struct *xfs_syncd_wq;/* sync workqueue */ -void xfs_flush_worker (struct work_struct *work);void xfs_reclaim_worker (struct work_struct *work);int xfs_quiesce_data (struct xfs_mount *mp);void xfs_quiesce_attr (struct xfs_mount *mp); -void xfs_flush_inodes (struct xfs_inode *ip); -int xfs_reclaim_inodes (struct xfs_mount *mp,int mode);int xfs_reclaim_inodes_count (struct xfs_mount *mp);void xfs_reclaim_inodes_nr (struct xfs_mount *mp,int nr_to_scan);
fs/xfs/xfs_vnodeops.c +1 -1
@@ -777,7 +777,7 @@ xfs_create( XFS_TRANS_PERM_LOG_RES, log_count);if (error ==ENOSPC ) {/* flush outstanding delalloc blocks and retry */ -xfs_flush_inodes (dp); +xfs_flush_inodes (mp); error =xfs_trans_reserve (tp, resblks, log_res,0 ,XFS_TRANS_PERM_LOG_RES , log_count); }
Signature · 6 lines
1.7.10 _______________________________________________ xfs mailing list [email protected] http://oss.sgi.com/mailman/listinfo/xfs
Re: [PATCH 06/14] xfs: xfs_sync_data is redundant.
Hi Brain,
Given that you found the regression in the output of xfstest 273, could you do us a favor and review the fix and provide your Reviewed-by, and/or Tested-by?
Thanks much,
Ben
Quoted 330 lines · Dave Chinner
From: Dave Chinner <[email protected]>
We don't do any data writeback from XFS any more - the VFS is completely responsible for that, including for freeze. We can replace the remaining caller with a VFS level function that achieves the same thing, but without conflicting with current writeback work.
This means we can remove the
flush_workandxfs_flush_inodes()- the VFS functionality completely replaces the internal flush queue for doing this writeback work in a separate context to avoid stack overruns.This does have one complication - it cannot be called with page locks held. Hence move the flushing of delalloc space when ENOSPC occurs back up into
xfs_file_aio_buffered_writewhen we don't hold any locks that will stall writeback.Unfortunately,
writeback_inodes_sb_if_idle()is not sufficient to trigger delalloc conversion fast enough to prevent spurious ENOSPC whent here are hundreds of writers, thousands of small files and GBs of free RAM. Hence we need to usesync_sb_inodes()to block callers while we wait for writeback like the previousxfs_flush_inodesimplementation did.That means we have to hold the
s_umountlock here, but because this call can nest insidei_mutex(the parent directory in the create case, held by the VFS), we have to usedown_read_trylock()to avoid potential deadlocks. In practice, this trylock will succeed on almost every attempt as unmount/remount type operations are exceedingly rare.Note: we always need to pass a count of zero to
generic_file_buffered_write()as the previously written byte count. We only do this by accident before this patch by the virtue of ret always being zero when there are no errors. Make this explicit rather than needing to specifically zero ret in the ENOSPC retry case.Signed-off-by Dave Chinner <[email protected]>--- fs/xfs/xfs_file.c | 13 +++++---- fs/xfs/xfs_iomap.c | 23 +++++---------- fs/xfs/xfs_mount.h | 1 - fs/xfs/xfs_super.c | 21 +++++++++++-- fs/xfs/xfs_super.h | 1 + fs/xfs/xfs_sync.c | 78 ------------------------------------------------- fs/xfs/xfs_sync.h | 3 -- fs/xfs/xfs_vnodeops.c | 2 +- 8 files changed, 34 insertions(+), 108 deletions(-)fs/xfs/xfs_file.c +7
-6@@ -728,16 +728,17 @@ xfs_file_buffered_aio_write( write_retry:trace_xfs_file_buffered_write (ip, count, iocb->ki_pos,0 ); ret =generic_file_buffered_write (iocb, iovp, nr_segs, - pos, &iocb->ki_pos, count, ret); + pos, &iocb->ki_pos, count,0 ); +/* -* if we just got an ENOSPC, flush the inode now we aren't holding any -* page locks and retry *once* +* If we just got an ENOSPC, try to write back all dirty inodes to +* convert delalloc space to free up some of the excess reserved +* metadata space. */ if (ret == -ENOSPC && !enospc) { enospc =1 ; - ret = -xfs_flush_pages (ip,0 ,-1 ,0 ,FI_NONE ); -if (!ret) - goto write_retry; +xfs_flush_inodes (ip->i_mount); + goto write_retry; } current->backing_dev_info =NULL ;fs/xfs/xfs_iomap.c +7
-16@@ -373,7 +373,7 @@ xfs_iomap_write_delay(xfs_extlen_t extsz;int nimaps;xfs_bmbt_irec_t imap[XFS_WRITE_IMAPS ]; -int prealloc, flushed =0 ; +int prealloc;int error;ASSERT (xfs_isilocked (ip,XFS_ILOCK_EXCL ));@@ -434,26 +434,17 @@ retry: }/* -* If bmapi returned us nothing, we got either ENOSPC or EDQUOT. For -* ENOSPC, * flush all other inodes with delalloc blocks to free up -* some of the excess reserved metadata space. For both cases, retry +* If bmapi returned us nothing, we got either ENOSPC or EDQUOT. Retry * without EOF preallocation. */ if (nimaps ==0 ) {trace_xfs_delalloc_enospc (ip, offset, count); -if (flushed) -return XFS_ERROR (error ? error :ENOSPC ); - -if (error ==ENOSPC ) { -xfs_iunlock (ip,XFS_ILOCK_EXCL ); -xfs_flush_inodes (ip); -xfs_ilock (ip,XFS_ILOCK_EXCL ); +if (prealloc) { + prealloc =0 ; + error =0 ; + goto retry; } - - flushed =1 ; - error =0 ; - prealloc =0 ; - goto retry; +return XFS_ERROR (error ? error :ENOSPC ); }if (!(imap[0 ].br_startblock ||XFS_IS_REALTIME_INODE (ip)))fs/xfs/xfs_mount.h +0
-1@@ -198,7 +198,6 @@ typedef struct xfs_mount {#endif struct xfs_mru_cache *m_filestream;/* per-mount filestream data */ struct delayed_work m_reclaim_work;/* background inode reclaim */ -struct work_struct m_flush_work;/* background inode flush */ __int64_t m_update_flags;/* sb flags we need to update on the next remount,rw */ struct shrinker m_inode_shrink;/* inode reclaim shrinker */ fs/xfs/xfs_super.c +18
-3@@ -882,6 +882,24 @@ xfs_destroy_mount_workqueues(destroy_workqueue (mp->m_unwritten_workqueue); } +/* +* Flush all dirty data to disk. Must not be called while holding an XFS_ILOCK +* or a page lock. We use sync_inodes_sb() here to ensure we block while waiting +* for IO to complete so that we effectively throttle multiple callers to the +* rate at which IO is completing. +*/ +void +xfs_flush_inodes ( +struct xfs_mount *mp) +{ +struct super_block *sb = mp->m_super; + +if (down_read_trylock (&sb->s_umount)) { +sync_inodes_sb (sb); +up_read (&sb->s_umount); + } +} +/* Catch misguided souls that try to use this interface on XFS */ STATIC struct inode * xfs_fs_alloc_inode(@@ -1005,8 +1023,6 @@ xfs_fs_put_super( {struct xfs_mount *mp =XFS_M (sb); -cancel_work_sync (&mp->m_flush_work); -xfs_filestream_unmount (mp);xfs_unmountfs (mp);@@ -1324,7 +1340,6 @@ xfs_fs_fill_super(spin_lock_init (&mp->m_sb_lock);mutex_init (&mp->m_growlock);atomic_set (&mp->m_active_trans,0 ); -INIT_WORK (&mp->m_flush_work, xfs_flush_worker);INIT_DELAYED_WORK (&mp->m_reclaim_work, xfs_reclaim_worker); mp->m_super = sb;fs/xfs/xfs_super.h +1
-0@@ -74,6 +74,7 @@ struct block_device;extern __uint64_t xfs_max_file_offset (unsigned int ); +extern void xfs_flush_inodes (struct xfs_mount *mp);extern void xfs_blkdev_issue_flush (struct xfs_buftarg *);extern xfs_agnumber_t xfs_set_inode32 (struct xfs_mount *);extern xfs_agnumber_t xfs_set_inode64 (struct xfs_mount *);fs/xfs/xfs_sync.c +0
-78@@ -217,51 +217,6 @@ xfs_inode_ag_iterator( } STATIC int -xfs_sync_inode_data ( -struct xfs_inode *ip, -struct xfs_perag *pag, -int flags) -{ -struct inode *inode =VFS_I (ip); -struct address_space *mapping = inode->i_mapping; -int error =0 ; - -if (!mapping_tagged (mapping,PAGECACHE_TAG_DIRTY )) -return 0 ; - -if (!xfs_ilock_nowait (ip,XFS_IOLOCK_SHARED )) { -if (flags &SYNC_TRYLOCK ) -return 0 ; -xfs_ilock (ip,XFS_IOLOCK_SHARED ); - } - - error =xfs_flush_pages (ip,0 ,-1 , (flags &SYNC_WAIT ) ? -0 :XBF_ASYNC ,FI_NONE ); -xfs_iunlock (ip,XFS_IOLOCK_SHARED ); -return error; -} - -/* -* Write out pagecache data for the whole filesystem. -*/ -STATIC int -xfs_sync_data ( -struct xfs_mount *mp, -int flags) -{ -int error; - -ASSERT ((flags & ~(SYNC_TRYLOCK |SYNC_WAIT )) ==0 ); - - error =xfs_inode_ag_iterator (mp, xfs_sync_inode_data, flags); -if (error) -return XFS_ERROR (error); - -xfs_log_force (mp, (flags &SYNC_WAIT ) ?XFS_LOG_SYNC :0 ); -return 0 ; -} - -STATIC intxfs_sync_fsdata (struct xfs_mount *mp) {@@ -415,39 +370,6 @@ xfs_reclaim_worker(xfs_syncd_queue_reclaim (mp); } -/* -* Flush delayed allocate data, attempting to free up reserved space -* from existing allocations. At this point a new allocation attempt -* has failed with ENOSPC and we are in the process of scratching our -* heads, looking about for more room. -* -* Queue a new data flush if there isn't one already in progress and -* wait for completion of the flush. This means that we only ever have one -* inode flush in progress no matter how many ENOSPC events are occurring and -* so will prevent the system from bogging down due to every concurrent -* ENOSPC event scanning all the active inodes in the system for writeback. -*/ -void -xfs_flush_inodes ( -struct xfs_inode *ip) -{ -struct xfs_mount *mp = ip->i_mount; - -queue_work (xfs_syncd_wq, &mp->m_flush_work); -flush_work_sync (&mp->m_flush_work); -} - -void -xfs_flush_worker ( -struct work_struct *work) -{ -struct xfs_mount *mp =container_of (work, - struct xfs_mount, m_flush_work); - -xfs_sync_data (mp,SYNC_TRYLOCK ); -xfs_sync_data (mp,SYNC_TRYLOCK |SYNC_WAIT ); -} - void __xfs_inode_set_reclaim_tag( struct xfs_perag *pag,fs/xfs/xfs_sync.h +0
-3@@ -26,14 +26,11 @@ struct xfs_perag;extern struct workqueue_struct *xfs_syncd_wq;/* sync workqueue */ -void xfs_flush_worker (struct work_struct *work);void xfs_reclaim_worker (struct work_struct *work);int xfs_quiesce_data (struct xfs_mount *mp);void xfs_quiesce_attr (struct xfs_mount *mp); -void xfs_flush_inodes (struct xfs_inode *ip); -int xfs_reclaim_inodes (struct xfs_mount *mp,int mode);int xfs_reclaim_inodes_count (struct xfs_mount *mp);void xfs_reclaim_inodes_nr (struct xfs_mount *mp,int nr_to_scan);fs/xfs/xfs_vnodeops.c +1
-1@@ -777,7 +777,7 @@ xfs_create( XFS_TRANS_PERM_LOG_RES, log_count);if (error ==ENOSPC ) {/* flush outstanding delalloc blocks and retry */ -xfs_flush_inodes (dp); +xfs_flush_inodes (mp); error =xfs_trans_reserve (tp, resblks, log_res,0 ,XFS_TRANS_PERM_LOG_RES , log_count); }Signature · 6 lines
1.7.10 _______________________________________________ xfs mailing list [email protected] http://oss.sgi.com/mailman/listinfo/xfs
_______________________________________________
xfs mailing list
[email protected]
http://oss.sgi.com/mailman/listinfo/xfs
Re: [PATCH 06/14] xfs: xfs_sync_data is redundant.
Quoted 1 line · Ben Myers
Hi Brain,
[Pinky: "Gee, Brain, what do you want to do tonight?"
Brain: "The same thing we do every night, Pinky - try to take over
the world!"]
Quoted 2 lines
Given that you found the regression in the output of xfstest 273, could you do us a favor and review the fix and provide your Reviewed-by, and/or Tested-by?
Brian did test it, just as separate patches:
http://oss.sgi.com/archives/xfs/2012-10/msg00037.html
Cheers,
Dave.
Signature · 7 lines
Dave Chinner [email protected] _______________________________________________ xfs mailing list [email protected] http://oss.sgi.com/mailman/listinfo/xfs
Re: [PATCH 06/14] xfs: xfs_sync_data is redundant.
Hey,
Quoted 6 lines · Dave Chinner
Quoted 1 line · Ben Myers
Hi Brain,
[Pinky: "Gee, Brain, what do you want to do tonight?"
Brain: "The same thing we do every night, Pinky - try to take over
the world!"]
Wish I could say I called him 'brain' on purpose. Will you be playing the part of Pinky in today's show? ;)
Quoted 7 lines
Quoted 3 lines
Given that you found the regression in the output of xfstest 273, could you do us a favor and review the fix and provide your Reviewed-by, and/or Tested-by?
Brian did test it, just as separate patches:
Brian, would you mind extending your Tested-by?
Regards,
Ben
_______________________________________________
xfs mailing list
[email protected]
http://oss.sgi.com/mailman/listinfo/xfs
Brian Foster
Tested-byRe: [PATCH 06/14] xfs: xfs_sync_data is redundant.
Quoted 4 lines · Ben Myers
Hey,
Quoted 1 line · Dave Chinner
On Tue, Oct 09, 2012 at 04:01:24PM -0500, Ben Myers wrote:
Quoted 9 lines
Quoted 5 lines
Quoted 1 line
Hi Brain,
[Pinky: "Gee, Brain, what do you want to do tonight?"
Brain: "The same thing we do every night, Pinky - try to take over
the world!"]Wish I could say I called him 'brain' on purpose. Will you be playing the part of Pinky in today's show? ;)
Heh, it happens more than you would think. ;)
Quoted 10 lines
Quoted 7 lines
Quoted 3 lines
Given that you found the regression in the output of xfstest 273, could you do us a favor and review the fix and provide your Reviewed-by, and/or Tested-by?
Brian did test it, just as separate patches:
Brian, would you mind extending your Tested-by?
Sure, I tested Dave's change to xfs_flush_inodes() against 273 and ran another full test against the entire v3 set on that hardware, without any further noticeable issues.
Brian
Quoted 3 lines
Regards,
Ben
_______________________________________________
xfs mailing list
[email protected]
http://oss.sgi.com/mailman/listinfo/xfs
Re: [PATCH 06/14] xfs: xfs_sync_data is redundant.
In reply to Ben MyersQuoted 4 lines · Ben Myers
Hey,
Quoted 1 line · Dave Chinner
On Tue, Oct 09, 2012 at 04:01:24PM -0500, Ben Myers wrote:
Quoted 8 lines
Quoted 5 lines
Quoted 1 line
Hi Brain,
[Pinky: "Gee, Brain, what do you want to do tonight?"
Brain: "The same thing we do every night, Pinky - try to take over
the world!"]Wish I could say I called him 'brain' on purpose. Will you be playing the part of Pinky in today's show? ;)
I think so, Brain, but what if the badger won't hold the fork steady for long enough? :)
Cheers,
Dave.
PS: for those that don't get it, google AYPWIP....
Signature · 7 lines
Dave Chinner [email protected] _______________________________________________ xfs mailing list [email protected] http://oss.sgi.com/mailman/listinfo/xfs
PATCH v5 7/14 xfs: syncd workqueue is no more
In reply to Dave ChinnerFrom: Dave Chinner <[email protected]>
With the syncd functions moved to the log and/or removed, the syncd workqueue is the only remaining bit left. It is used by the log covering/ail pushing work, as well as by the inode reclaim work.
Given how cheap workqueues are these days, give the log and inode reclaim work their own work queues and kill the syncd work queue.
--- fs/xfs/xfs_log.c | 2 +- fs/xfs/xfs_mount.h | 2 ++ fs/xfs/xfs_super.c | 38 ++++++++++++++++++-------------------- fs/xfs/xfs_sync.c | 20 +++++++++----------- fs/xfs/xfs_sync.h | 2 -- 5 files changed, 30 insertions(+), 34 deletions(-)
fs/xfs/xfs_log.c +1 -1
@@ -1193,7 +1193,7 @@ voidxfs_log_work_queue ( struct xfs_mount *mp) { -queue_delayed_work (xfs_syncd_wq, &mp->m_log->l_work, +queue_delayed_work (mp->m_log_workqueue, &mp->m_log->l_work,msecs_to_jiffies (xfs_syncd_centisecs *10 )); }
fs/xfs/xfs_mount.h +2 -0
@@ -207,6 +207,8 @@ typedef struct xfs_mount {struct workqueue_struct *m_data_workqueue;struct workqueue_struct *m_unwritten_workqueue;struct workqueue_struct *m_cil_workqueue; +struct workqueue_struct *m_reclaim_workqueue; +struct workqueue_struct *m_log_workqueue; } xfs_mount_t;/*
fs/xfs/xfs_super.c +18 -20
@@ -863,8 +863,23 @@ xfs_init_mount_workqueues( WQ_MEM_RECLAIM, 0, mp->m_fsname);if (!mp->m_cil_workqueue) goto out_destroy_unwritten; + + mp->m_reclaim_workqueue =alloc_workqueue ("xfs-reclaim/%s" , +WQ_NON_REENTRANT ,0 , mp->m_fsname); +if (!mp->m_reclaim_workqueue) + goto out_destroy_cil; + + mp->m_log_workqueue =alloc_workqueue ("xfs-log/%s" , +WQ_NON_REENTRANT ,0 , mp->m_fsname); +if (!mp->m_log_workqueue) + goto out_destroy_reclaim; +return 0 ; +out_destroy_reclaim: +destroy_workqueue (mp->m_reclaim_workqueue); +out_destroy_cil: +destroy_workqueue (mp->m_cil_workqueue); out_destroy_unwritten:destroy_workqueue (mp->m_unwritten_workqueue); out_destroy_data_iodone_queue:
@@ -877,6 +892,8 @@ STATIC voidxfs_destroy_mount_workqueues ( struct xfs_mount *mp) { +destroy_workqueue (mp->m_log_workqueue); +destroy_workqueue (mp->m_reclaim_workqueue);destroy_workqueue (mp->m_cil_workqueue);destroy_workqueue (mp->m_data_workqueue);destroy_workqueue (mp->m_unwritten_workqueue);
@@ -1391,10 +1408,6 @@ xfs_fs_fill_super(/* * we must configure the block size in the superblock before we run the * full mount process as the mount process can lookup and cache inodes. -* For the same reason we must also initialise the syncd and register -* the inode cache shrinker so that inodes can be reclaimed during -* operations like a quotacheck that iterate all inodes in the -* filesystem. */ sb->s_magic =XFS_SB_MAGIC ; sb->s_blocksize = mp->m_sb.sb_blocksize;
@@ -1634,16 +1647,6 @@ STATIC int __initxfs_init_workqueues (void) {/* -* We never want to the same work item to run twice, reclaiming inodes -* or idling the log is not going to get any faster by multiple CPUs -* competing for ressources. Use the default large max_active value -* so that even lots of filesystems can perform these task in parallel. -*/ - xfs_syncd_wq =alloc_workqueue ("xfssyncd" ,WQ_NON_REENTRANT ,0 ); -if (!xfs_syncd_wq) -return -ENOMEM ; - -/* * The allocation workqueue can be used in memory reclaim situations * (writepage path), and parallelism is only limited by the number of * AGs in all the filesystems mounted. Hence use the default large
@@ -1651,20 +1654,15 @@ xfs_init_workqueues(void)*/ xfs_alloc_wq =alloc_workqueue ("xfsalloc" ,WQ_MEM_RECLAIM ,0 );if (!xfs_alloc_wq) - goto out_destroy_syncd; +return -ENOMEM ;return 0 ; - -out_destroy_syncd: -destroy_workqueue (xfs_syncd_wq); -return -ENOMEM ; } STATIC voidxfs_destroy_workqueues (void ) {destroy_workqueue (xfs_alloc_wq); -destroy_workqueue (xfs_syncd_wq); }STATIC int__init
fs/xfs/xfs_sync.c +9 -11
@@ -40,8 +40,6 @@#include <linux/kthread.h> #include <linux/freezer.h> -struct workqueue_struct *xfs_syncd_wq;/* sync workqueue */ -/* * The inode lookup is done in batches to keep the amount of lock traffic and * radix tree lookups to a minimum. The batch size is a trade off between
@@ -335,18 +333,18 @@ xfs_quiesce_attr(/* * Queue a new inode reclaim pass if there are reclaimable inodes and there * isn't a reclaim pass already in progress. By default it runs every 5s based -* on the xfs syncd work default of 30s. Perhaps this should have it's own +* on the xfs periodic sync default of 30s. Perhaps this should have it's own * tunable, but that can be done if this method proves to be ineffective or too * aggressive. */ static void -xfs_syncd_queue_reclaim ( +xfs_reclaim_work_queue (struct xfs_mount *mp) {rcu_read_lock ();if (radix_tree_tagged (&mp->m_perag_tree,XFS_ICI_RECLAIM_TAG )) { -queue_delayed_work (xfs_syncd_wq, &mp->m_reclaim_work, +queue_delayed_work (mp->m_reclaim_workqueue, &mp->m_reclaim_work,msecs_to_jiffies (xfs_syncd_centisecs /6 *10 )); }rcu_read_unlock ();
@@ -367,7 +365,7 @@ xfs_reclaim_worker( struct xfs_mount, m_reclaim_work);xfs_reclaim_inodes (mp,SYNC_TRYLOCK ); -xfs_syncd_queue_reclaim (mp); +xfs_reclaim_work_queue (mp); }void
@@ -388,7 +386,7 @@ __xfs_inode_set_reclaim_tag(spin_unlock (&ip->i_mount->m_perag_lock);/* schedule periodic background inode reclaim */ -xfs_syncd_queue_reclaim (ip->i_mount); +xfs_reclaim_work_queue (ip->i_mount);trace_xfs_perag_set_reclaim (ip->i_mount, pag->pag_agno,-1 , _RET_IP_);
@@ -646,9 +644,9 @@ out:/* * We could return EAGAIN here to make reclaim rescan the inode tree in * a short while. However, this just burns CPU time scanning the tree -* waiting for IO to complete and xfssyncd never goes back to the idle -* state. Instead, return 0 to let the next scheduled background reclaim -* attempt to reclaim the inode again. +* waiting for IO to complete and the reclaim work never goes back to +* the idle state. Instead, return 0 to let the next scheduled +* background reclaim attempt to reclaim the inode again. */ return 0 ; }
@@ -804,7 +802,7 @@ xfs_reclaim_inodes_nr( int nr_to_scan) {/* kick background reclaimer and push the AIL */ -xfs_syncd_queue_reclaim (mp); +xfs_reclaim_work_queue (mp);xfs_ail_push_all (mp->m_ail);xfs_reclaim_inodes_ag (mp,SYNC_TRYLOCK |SYNC_WAIT , &nr_to_scan);
fs/xfs/xfs_sync.h +0 -2
@@ -24,8 +24,6 @@ struct xfs_perag;#define SYNC_WAIT 0x0001/* wait for i/o to complete */ #define SYNC_TRYLOCK 0x0002/* only try to lock inodes */ -extern struct workqueue_struct *xfs_syncd_wq;/* sync workqueue */ -void xfs_reclaim_worker (struct work_struct *work);int xfs_quiesce_data (struct xfs_mount *mp);
Signature · 6 lines
1.7.10 _______________________________________________ xfs mailing list [email protected] http://oss.sgi.com/mailman/listinfo/xfs
PATCH v5 8/14 xfs: xfs_sync_fsdata is redundant
In reply to Dave ChinnerFrom: Dave Chinner <[email protected]>
Why do we need to write the superblock to disk once we've written all the data? We don't actually - the reasons for doing this are lost in the mists of time, and go back to the way Irix used to drive VFS flushing.
On linux, this code is only called from two contexts: remount and .sync_fs. In the remount case, the call is followed by a metadata sync, which unpins and writes the superblock. In the sync_fs case, we only need to force the log to disk to ensure that the superblock is correctly on disk, so we don't actually need to write it. Hence the functionality is either redundant or superfluous and thus can be removed.
Seeing as xfs_quiesce_data is essentially now just a log force, remove it as well and fold the code back into the two callers. Neither of them need the log covering check, either, as that is redundant for the remount case, and unnecessary for the .sync_fs case.
--- fs/xfs/xfs_super.c | 19 +++++---------- fs/xfs/xfs_sync.c | 67 +++++++--------------------------------------------- 2 files changed, 14 insertions(+), 72 deletions(-)
fs/xfs/xfs_super.c +6 -13
@@ -1057,7 +1057,6 @@ xfs_fs_sync_fs( int wait) {struct xfs_mount *mp =XFS_M (sb); -int error;/* * Doing anything during the async pass would be counterproductive.
@@ -1065,10 +1064,7 @@ xfs_fs_sync_fs(if (!wait)return 0 ; - error =xfs_quiesce_data (mp); -if (error) -return -error; - +xfs_log_force (mp,XFS_LOG_SYNC );if (laptop_mode) {/* * The disk must be active because we're syncing.
@@ -1238,15 +1234,12 @@ xfs_fs_remount(/* rw -> ro */ if (!(mp->m_flags &XFS_MOUNT_RDONLY ) && (*flags &MS_RDONLY )) {/* -* After we have synced the data but before we sync the -* metadata, we need to free up the reserve block pool so that -* the used block count in the superblock on disk is correct at -* the end of the remount. Stash the current reserve pool size -* so that if we get remounted rw, we can return it to the same -* size. +* Before we sync the metadata, we need to free up the reserve +* block pool so that the used block count in the superblock on +* disk is correct at the end of the remount. Stash the current +* reserve pool size so that if we get remounted rw, we can +* return it to the same size. */ - -xfs_quiesce_data (mp);xfs_save_resvblks (mp);xfs_quiesce_attr (mp); mp->m_flags |=XFS_MOUNT_RDONLY ;
fs/xfs/xfs_sync.c +8 -59
84 lines not on this page · raw message
Signature · 6 lines
1.7.10 _______________________________________________ xfs mailing list [email protected] http://oss.sgi.com/mailman/listinfo/xfs
PATCH v5 9/14 xfs: move xfs_quiesce_attr() into xfs_super.c
In reply to Dave ChinnerFrom: Dave Chinner <[email protected]>
Both callers of xfs_quiesce_attr() are in xfs_super.c, and there's nothing really sync-specific about this functionality so it doesn't really matter where it lives. Move it to benext to it's callers, so all the remount/sync_fs code is in the one place.
--- fs/xfs/xfs_super.c | 67 ++++++++++++++++++++++++++++++++++++++++++++++++++++ fs/xfs/xfs_sync.c | 65 -------------------------------------------------- fs/xfs/xfs_sync.h | 3 --- 3 files changed, 67 insertions(+), 68 deletions(-)
fs/xfs/xfs_super.c +67 -0
@@ -1148,6 +1148,73 @@ xfs_restore_resvblks(struct xfs_mount *mp)xfs_reserve_blocks (mp, &resblks,NULL ); } +/* +* Trigger writeback of all the dirty metadata in the file system. +* +* This ensures that the metadata is written to their location on disk rather +* than just existing in transactions in the log. This means after a quiesce +* there is no log replay required to write the inodes to disk (this is the main +* difference between a sync and a quiesce). +* +* This shoul deffectively mimic the code in xfs_unmountfs() and +* xfs_log_umount() but without tearing down any structures. +* XXX: bug fixes needed! +* +* Note: this stops background log work - the callers must ensure it is started +* again when appropriate. +*/ +void +xfs_quiesce_attr ( +struct xfs_mount *mp) +{ +int error =0 ; + +/* wait for all modifications to complete */ +while (atomic_read (&mp->m_active_trans) >0 ) +delay (100 ); + +/* force the log to unpin objects from the now complete transactions */ +xfs_log_force (mp,XFS_LOG_SYNC ); + +/* reclaim inodes to do any IO before the freeze completes */ +xfs_reclaim_inodes (mp,0 ); +xfs_reclaim_inodes (mp,SYNC_WAIT ); + +/* flush all pending changes from the AIL */ +xfs_ail_push_all_sync (mp->m_ail); + +/* stop background log work */ +cancel_delayed_work_sync (&mp->m_log->l_work); + +/* +* 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); +if (error) +xfs_warn (mp,"xfs_attr_quiesce: failed to log sb changes. " +"Frozen image may not be consistent." ); +xfs_log_unmount_write (mp); + +/* +* At this point we might have modified the superblock again and thus +* added an item to the AIL, thus flush it again. +*/ +xfs_ail_push_all_sync (mp->m_ail); + +/* +* The superblock buffer is uncached and xfsaild_push() will lock and +* set the XBF_ASYNC flag on the buffer. We cannot do xfs_buf_iowait() +* here but a lock on the superblock buffer will block until iodone() +* has completed. +*/ +xfs_buf_lock (mp->m_sb_bp); +xfs_buf_unlock (mp->m_sb_bp); +} + STATIC int xfs_fs_remount( struct super_block *sb,
fs/xfs/xfs_sync.c +0 -65
71 lines not on this page · raw message
fs/xfs/xfs_sync.h +0 -3
@@ -26,9 +26,6 @@ struct xfs_perag;void xfs_reclaim_worker (struct work_struct *work); -int xfs_quiesce_data (struct xfs_mount *mp); -void xfs_quiesce_attr (struct xfs_mount *mp); -int xfs_reclaim_inodes (struct xfs_mount *mp,int mode);int xfs_reclaim_inodes_count (struct xfs_mount *mp);void xfs_reclaim_inodes_nr (struct xfs_mount *mp,int nr_to_scan);
Signature · 6 lines
1.7.10 _______________________________________________ xfs mailing list [email protected] http://oss.sgi.com/mailman/listinfo/xfs
PATCH v5 10/14 xfs: xfs_quiesce_attr() should quiesce the log like unmount
In reply to Dave ChinnerFrom: Dave Chinner <[email protected]>
xfs_quiesce_attr() is supposed to leave the log empty with an unmount record written. Right now it does not wait for the AIL to be emptied before writing the unmount record, not does it wait for metadata IO completion, either. Fix it to use the same method and code as xfs_log_unmount().
--- fs/xfs/xfs_log.c | 25 ++++++++++++++++++------- fs/xfs/xfs_log.h | 1 + fs/xfs/xfs_super.c | 41 ++++++++--------------------------------- 3 files changed, 27 insertions(+), 40 deletions(-)
fs/xfs/xfs_log.c +18 -7
@@ -855,20 +855,17 @@ xfs_log_unmount_write(xfs_mount_t *mp) }/* xfs_log_unmount_write */ /* -* Shut down and release the AIL and Log. -* -* During unmount, we need to ensure we flush all the dirty metadata objects -* from the AIL so that the log is empty before we write the unmount record to -* the log. +* Empty the log for unmount/freeze. * * To do this, we first need to shut down the background log work so it is not * trying to cover the log as we clean up. We then need to unpin all objects in * the log so we can then flush them out. Once they have completed their IO and * run the callbacks removing themselves from the AIL, we can write the unmount -* record, tear down the AIL and finally free the log. +* record. */ void -xfs_log_unmount (xfs_mount_t *mp) +xfs_log_quiesce ( +struct xfs_mount *mp) {cancel_delayed_work_sync (&mp->m_log->l_work);xfs_log_force (mp,XFS_LOG_SYNC );
@@ -886,6 +883,20 @@ xfs_log_unmount(xfs_mount_t *mp)xfs_buf_unlock (mp->m_sb_bp);xfs_log_unmount_write (mp); +} + +/* +* Shut down and release the AIL and Log. +* +* During unmount, we need to ensure we flush all the dirty metadata objects +* from the AIL so that the log is empty before we write the unmount record to +* the log. Once this is done, we can tear down the AIL and the log. +*/ +void +xfs_log_unmount ( +struct xfs_mount *mp) +{ +xfs_log_quiesce (mp);xfs_trans_ail_destroy (mp);xlog_dealloc_log (mp->m_log);
fs/xfs/xfs_log.h +1 -0
@@ -183,6 +183,7 @@ bool xfs_log_item_in_current_chkpt(struct xfs_log_item *lip);void xfs_log_work_queue (struct xfs_mount *mp);void xfs_log_worker (struct work_struct *work); +void xfs_log_quiesce (struct xfs_mount *mp);#endif #endif /* __XFS_LOG_H__ */
fs/xfs/xfs_super.c +8 -33
62 lines not on this page · raw message
Signature · 6 lines
1.7.10 _______________________________________________ xfs mailing list [email protected] http://oss.sgi.com/mailman/listinfo/xfs
PATCH v5 11/14 xfs: rename xfs_sync.[ch] to xfs_icache.[ch]
In reply to Dave ChinnerFrom: Dave Chinner <[email protected]>
xfs_sync.c now only contains inode reclaim functions and inode cache iteration functions. It is not related to sync operations anymore. Rename to xfs_icache.c to reflect it's contents and prepare for consolidation with the other inode cache file that exists (xfs_iget.c).
--- fs/xfs/Makefile | 2 +- fs/xfs/xfs_icache.c | 715 ++++++++++++++++++++++++++++++++++++++++++++++ fs/xfs/xfs_icache.h | 43 +++ fs/xfs/xfs_iget.c | 1 + fs/xfs/xfs_mount.c | 1 + fs/xfs/xfs_mount.h | 2 - fs/xfs/xfs_qm_syscalls.c | 1 + fs/xfs/xfs_super.c | 2 +- fs/xfs/xfs_sync.c | 714 --------------------------------------------- fs/xfs/xfs_sync.h | 43 --- 10 files changed, 763 insertions(+), 761 deletions(-) create mode 100644 fs/xfs/xfs_icache.c create mode 100644 fs/xfs/xfs_icache.h delete mode 100644 fs/xfs/xfs_sync.c delete mode 100644 fs/xfs/xfs_sync.h
fs/xfs/Makefile +1 -1
@@ -39,6 +39,7 @@ xfs-y += xfs_aops.o \ xfs_fsops.o \ xfs_fs_subr.o \ xfs_globals.o \ + xfs_icache.o \ xfs_iget.o \ xfs_ioctl.o \ xfs_iomap.o \
@@ -47,7 +48,6 @@ xfs-y += xfs_aops.o \ xfs_message.o \ xfs_mru_cache.o \ xfs_super.o \ - xfs_sync.o \ xfs_xattr.o \ xfs_rename.o \ xfs_utils.o \
fs/xfs/xfs_icache.c new file +715 -0
715 lines not on this page · raw message
fs/xfs/xfs_icache.h new file +43 -0
43 lines not on this page · raw message
fs/xfs/xfs_iget.c +1 -0
@@ -38,6 +38,7 @@#include "xfs_inode_item.h" #include "xfs_bmap.h" #include "xfs_trace.h" +#include "xfs_icache.h" /*
fs/xfs/xfs_mount.c +1 -0
@@ -42,6 +42,7 @@#include "xfs_fsops.h" #include "xfs_utils.h" #include "xfs_trace.h" +#include "xfs_icache.h" #ifdef HAVE_PERCPU_SB
fs/xfs/xfs_mount.h +0 -2
@@ -51,8 +51,6 @@ typedef struct xfs_trans_reservations {#else /* __KERNEL__ */ -#include "xfs_sync.h" -struct xlog ;struct xfs_inode ;struct xfs_mru_cache ;
fs/xfs/xfs_qm_syscalls.c +1 -0
@@ -40,6 +40,7 @@#include "xfs_utils.h" #include "xfs_qm.h" #include "xfs_trace.h" +#include "xfs_icache.h" STATIC int xfs_qm_log_quotaoff(xfs_mount_t *, xfs_qoff_logitem_t **, uint); STATIC int xfs_qm_log_quotaoff_end(xfs_mount_t *, xfs_qoff_logitem_t *,
fs/xfs/xfs_super.c +1 -1
@@ -49,7 +49,7 @@#include "xfs_extfree_item.h" #include "xfs_mru_cache.h" #include "xfs_inode_item.h" -#include "xfs_sync.h" +#include "xfs_icache.h" #include "xfs_trace.h" #include <linux/namei.h>
fs/xfs/xfs_sync.c deleted +0 -714
714 lines not on this page · raw message
fs/xfs/xfs_sync.h deleted +0 -43
43 lines not on this page · raw message
Signature · 6 lines
1.7.10 _______________________________________________ xfs mailing list [email protected] http://oss.sgi.com/mailman/listinfo/xfs
PATCH v5 12/14 xfs: move inode locking functions to xfs_inode.c
In reply to Dave ChinnerFrom: Dave Chinner <[email protected]>
xfs_ilock() and friends really aren't related to the inode cache in any way, so move them to xfs_inode.c with all the other inode related functionality.
While doing this move, move the xfs_ilock() tracepoints to *before* the lock is taken so that when a hang on a lock occurs we have events to indicate which process and what inode we were trying to lock when the hang occurred. This is much better than the current silence we get on a hang...
--- fs/xfs/xfs_iget.c | 251 ---------------------------------------------------- fs/xfs/xfs_inode.c | 250 +++++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 250 insertions(+), 251 deletions(-)
fs/xfs/xfs_iget.c +0 -251
@@ -453,254 +453,3 @@ out_error_or_again:return error; } -/* -* This is a wrapper routine around the xfs_ilock() routine -* used to centralize some grungy code. It is used in places -* that wish to lock the inode solely for reading the extents. -* The reason these places can't just call xfs_ilock(SHARED) -* is that the inode lock also guards to bringing in of the -* extents from disk for a file in b-tree format. If the inode -* is in b-tree format, then we need to lock the inode exclusively -* until the extents are read in. Locking it exclusively all -* the time would limit our parallelism unnecessarily, though. -* What we do instead is check to see if the extents have been -* read in yet, and only lock the inode exclusively if they -* have not. -* -* The function returns a value which should be given to the -* corresponding xfs_iunlock_map_shared(). This value is -* the mode in which the lock was actually taken. -*/ -uint -xfs_ilock_map_shared ( -xfs_inode_t *ip) -{ -uint lock_mode; - -if ((ip->i_d.di_format ==XFS_DINODE_FMT_BTREE ) && - ((ip->i_df.if_flags &XFS_IFEXTENTS ) ==0 )) { - lock_mode =XFS_ILOCK_EXCL ; - }else { - lock_mode =XFS_ILOCK_SHARED ; - } - -xfs_ilock (ip, lock_mode); - -return lock_mode; -} - -/* -* This is simply the unlock routine to go with xfs_ilock_map_shared(). -* All it does is call xfs_iunlock() with the given lock_mode. -*/ -void -xfs_iunlock_map_shared ( -xfs_inode_t *ip, -unsigned int lock_mode) -{ -xfs_iunlock (ip, lock_mode); -} - -/* -* The xfs inode contains 2 locks: a multi-reader lock called the -* i_iolock and a multi-reader lock called the i_lock. This routine -* allows either or both of the locks to be obtained. -* -* The 2 locks should always be ordered so that the IO lock is -* obtained first in order to prevent deadlock. -* -* ip -- the inode being locked -* lock_flags -- this parameter indicates the inode's locks -* to be locked. It can be: -* XFS_IOLOCK_SHARED, -* XFS_IOLOCK_EXCL, -* XFS_ILOCK_SHARED, -* XFS_ILOCK_EXCL, -* XFS_IOLOCK_SHARED | XFS_ILOCK_SHARED, -* XFS_IOLOCK_SHARED | XFS_ILOCK_EXCL, -* XFS_IOLOCK_EXCL | XFS_ILOCK_SHARED, -* XFS_IOLOCK_EXCL | XFS_ILOCK_EXCL -*/ -void -xfs_ilock ( -xfs_inode_t *ip, -uint lock_flags) -{ -/* -* You can't set both SHARED and EXCL for the same lock, -* and only XFS_IOLOCK_SHARED, XFS_IOLOCK_EXCL, XFS_ILOCK_SHARED, -* and XFS_ILOCK_EXCL are valid values to set in lock_flags. -*/ -ASSERT ((lock_flags & (XFS_IOLOCK_SHARED |XFS_IOLOCK_EXCL )) != - (XFS_IOLOCK_SHARED |XFS_IOLOCK_EXCL )); -ASSERT ((lock_flags & (XFS_ILOCK_SHARED |XFS_ILOCK_EXCL )) != - (XFS_ILOCK_SHARED |XFS_ILOCK_EXCL )); -ASSERT ((lock_flags & ~(XFS_LOCK_MASK |XFS_LOCK_DEP_MASK )) ==0 ); - -if (lock_flags &XFS_IOLOCK_EXCL ) -mrupdate_nested (&ip->i_iolock,XFS_IOLOCK_DEP (lock_flags)); -else if (lock_flags &XFS_IOLOCK_SHARED ) -mraccess_nested (&ip->i_iolock,XFS_IOLOCK_DEP (lock_flags)); - -if (lock_flags &XFS_ILOCK_EXCL ) -mrupdate_nested (&ip->i_lock,XFS_ILOCK_DEP (lock_flags)); -else if (lock_flags &XFS_ILOCK_SHARED ) -mraccess_nested (&ip->i_lock,XFS_ILOCK_DEP (lock_flags)); - -trace_xfs_ilock (ip, lock_flags, _RET_IP_); -} - -/* -* This is just like xfs_ilock(), except that the caller -* is guaranteed not to sleep. It returns 1 if it gets -* the requested locks and 0 otherwise. If the IO lock is -* obtained but the inode lock cannot be, then the IO lock -* is dropped before returning. -* -* ip -- the inode being locked -* lock_flags -- this parameter indicates the inode's locks to be -* to be locked. See the comment for xfs_ilock() for a list -* of valid values. -*/ -int -xfs_ilock_nowait ( -xfs_inode_t *ip, -uint lock_flags) -{ -/* -* You can't set both SHARED and EXCL for the same lock, -* and only XFS_IOLOCK_SHARED, XFS_IOLOCK_EXCL, XFS_ILOCK_SHARED, -* and XFS_ILOCK_EXCL are valid values to set in lock_flags. -*/ -ASSERT ((lock_flags & (XFS_IOLOCK_SHARED |XFS_IOLOCK_EXCL )) != - (XFS_IOLOCK_SHARED |XFS_IOLOCK_EXCL )); -ASSERT ((lock_flags & (XFS_ILOCK_SHARED |XFS_ILOCK_EXCL )) != - (XFS_ILOCK_SHARED |XFS_ILOCK_EXCL )); -ASSERT ((lock_flags & ~(XFS_LOCK_MASK |XFS_LOCK_DEP_MASK )) ==0 ); - -if (lock_flags &XFS_IOLOCK_EXCL ) { -if (!mrtryupdate (&ip->i_iolock)) - goto out; - }else if (lock_flags &XFS_IOLOCK_SHARED ) { -if (!mrtryaccess (&ip->i_iolock)) - goto out; - } -if (lock_flags &XFS_ILOCK_EXCL ) { -if (!mrtryupdate (&ip->i_lock)) - goto out_undo_iolock; - }else if (lock_flags &XFS_ILOCK_SHARED ) { -if (!mrtryaccess (&ip->i_lock)) - goto out_undo_iolock; - } -trace_xfs_ilock_nowait (ip, lock_flags, _RET_IP_); -return 1 ; - - out_undo_iolock: -if (lock_flags &XFS_IOLOCK_EXCL ) -mrunlock_excl (&ip->i_iolock); -else if (lock_flags &XFS_IOLOCK_SHARED ) -mrunlock_shared (&ip->i_iolock); - out: -return 0 ; -} - -/* -* xfs_iunlock() is used to drop the inode locks acquired with -* xfs_ilock() and xfs_ilock_nowait(). The caller must pass -* in the flags given to xfs_ilock() or xfs_ilock_nowait() so -* that we know which locks to drop. -* -* ip -- the inode being unlocked -* lock_flags -- this parameter indicates the inode's locks to be -* to be unlocked. See the comment for xfs_ilock() for a list -* of valid values for this parameter. -* -*/ -void -xfs_iunlock ( -xfs_inode_t *ip, -uint lock_flags) -{ -/* -* You can't set both SHARED and EXCL for the same lock, -* and only XFS_IOLOCK_SHARED, XFS_IOLOCK_EXCL, XFS_ILOCK_SHARED, -* and XFS_ILOCK_EXCL are valid values to set in lock_flags. -*/ -ASSERT ((lock_flags & (XFS_IOLOCK_SHARED |XFS_IOLOCK_EXCL )) != - (XFS_IOLOCK_SHARED |XFS_IOLOCK_EXCL )); -ASSERT ((lock_flags & (XFS_ILOCK_SHARED |XFS_ILOCK_EXCL )) != - (XFS_ILOCK_SHARED |XFS_ILOCK_EXCL )); -ASSERT ((lock_flags & ~(XFS_LOCK_MASK |XFS_LOCK_DEP_MASK )) ==0 ); -ASSERT (lock_flags !=0 ); - -if (lock_flags &XFS_IOLOCK_EXCL ) -mrunlock_excl (&ip->i_iolock); -else if (lock_flags &XFS_IOLOCK_SHARED ) -mrunlock_shared (&ip->i_iolock); - -if (lock_flags &XFS_ILOCK_EXCL ) -mrunlock_excl (&ip->i_lock); -else if (lock_flags &XFS_ILOCK_SHARED ) -mrunlock_shared (&ip->i_lock); - -trace_xfs_iunlock (ip, lock_flags, _RET_IP_); -} - -/* -* give up write locks. the i/o lock cannot be held nested -* if it is being demoted. -*/ -void -xfs_ilock_demote ( -xfs_inode_t *ip, -uint lock_flags) -{ -ASSERT (lock_flags & (XFS_IOLOCK_EXCL |XFS_ILOCK_EXCL )); -ASSERT ((lock_flags & ~(XFS_IOLOCK_EXCL |XFS_ILOCK_EXCL )) ==0 ); - -if (lock_flags &XFS_ILOCK_EXCL ) -mrdemote (&ip->i_lock); -if (lock_flags &XFS_IOLOCK_EXCL ) -mrdemote (&ip->i_iolock); - -trace_xfs_ilock_demote (ip, lock_flags, _RET_IP_); -} - -#ifdef DEBUG -int -xfs_isilocked ( -xfs_inode_t *ip, -uint lock_flags) -{ -if (lock_flags & (XFS_ILOCK_EXCL |XFS_ILOCK_SHARED )) { -if (!(lock_flags &XFS_ILOCK_SHARED )) -return !!ip->i_lock.mr_writer; -return rwsem_is_locked (&ip->i_lock.mr_lock); - } - -if (lock_flags & (XFS_IOLOCK_EXCL |XFS_IOLOCK_SHARED )) { -if (!(lock_flags &XFS_IOLOCK_SHARED )) -return !!ip->i_iolock.mr_writer; -return rwsem_is_locked (&ip->i_iolock.mr_lock); - } - -ASSERT (0 ); -return 0 ; -} -#endif - -void -__xfs_iflock ( -struct xfs_inode *ip) -{ -wait_queue_head_t *wq =bit_waitqueue (&ip->i_flags, __XFS_IFLOCK_BIT); -DEFINE_WAIT_BIT (wait, &ip->i_flags, __XFS_IFLOCK_BIT); - -do { -prepare_to_wait_exclusive (wq, &wait.wait,TASK_UNINTERRUPTIBLE ); -if (xfs_isiflocked (ip)) -io_schedule (); - }while (!xfs_iflock_nowait (ip)); - -finish_wait (wq, &wait.wait); -}
fs/xfs/xfs_inode.c +250 -0
256 lines not on this page · raw message
Signature · 6 lines
1.7.10 _______________________________________________ xfs mailing list [email protected] http://oss.sgi.com/mailman/listinfo/xfs
PATCH v5 13/14 xfs: remove xfs_iget.c
In reply to Dave ChinnerFrom: Dave Chinner <[email protected]>
The inode cache functions remaining in xfs_iget.c can be moved to xfs_icache.c along with the other inode cache functions. This removes all functionality from xfs_iget.c, so the file can simply be removed.
This move results in various functions now only having the scope of a single file (e.g. xfs_inode_free()), so clean up all the definitions and exported prototypes in xfs_icache.[ch] and xfs_inode.h appropriately.
--- fs/xfs/Makefile | 1 - fs/xfs/xfs_export.c | 1 + fs/xfs/xfs_icache.c | 421 +++++++++++++++++++++++++++++++++++++++++- fs/xfs/xfs_icache.h | 6 +- fs/xfs/xfs_iget.c | 455 ---------------------------------------------- fs/xfs/xfs_inode.c | 1 + fs/xfs/xfs_inode.h | 10 +- fs/xfs/xfs_itable.c | 1 + fs/xfs/xfs_log_recover.c | 1 + fs/xfs/xfs_qm.c | 1 + fs/xfs/xfs_rtalloc.c | 1 + fs/xfs/xfs_vnodeops.c | 1 + 12 files changed, 430 insertions(+), 470 deletions(-) delete mode 100644 fs/xfs/xfs_iget.c
fs/xfs/Makefile +0 -1
@@ -40,7 +40,6 @@ xfs-y += xfs_aops.o \ xfs_fs_subr.o \ xfs_globals.o \ xfs_icache.o \ - xfs_iget.o \ xfs_ioctl.o \ xfs_iomap.o \ xfs_iops.o \
fs/xfs/xfs_export.c +1 -0
@@ -29,6 +29,7 @@#include "xfs_inode.h" #include "xfs_inode_item.h" #include "xfs_trace.h" +#include "xfs_icache.h" /* * Note that we only accept fileids which are long enough rather than allow
fs/xfs/xfs_icache.c +418 -3
445 lines not on this page · raw message
fs/xfs/xfs_icache.h +3 -3
@@ -24,6 +24,9 @@ struct xfs_perag;#define SYNC_WAIT 0x0001/* wait for i/o to complete */ #define SYNC_TRYLOCK 0x0002/* only try to lock inodes */ +int xfs_iget (struct xfs_mount *mp,struct xfs_trans *tp,xfs_ino_t ino, +uint flags,uint lock_flags,xfs_inode_t **ipp); +void xfs_reclaim_worker (struct work_struct *work);int xfs_reclaim_inodes (struct xfs_mount *mp,int mode);
@@ -31,9 +34,6 @@ int xfs_reclaim_inodes_count(struct xfs_mount *mp);void xfs_reclaim_inodes_nr (struct xfs_mount *mp,int nr_to_scan);void xfs_inode_set_reclaim_tag (struct xfs_inode *ip); -void __xfs_inode_set_reclaim_tag (struct xfs_perag *pag,struct xfs_inode *ip); -void __xfs_inode_clear_reclaim_tag (struct xfs_mount *mp,struct xfs_perag *pag, -struct xfs_inode *ip);int xfs_sync_inode_grab (struct xfs_inode *ip); int xfs_inode_ag_iterator(struct xfs_mount *mp,
fs/xfs/xfs_iget.c deleted +0 -455
455 lines not on this page · raw message
fs/xfs/xfs_inode.c +1 -0
@@ -45,6 +45,7 @@#include "xfs_filestream.h" #include "xfs_vnodeops.h" #include "xfs_trace.h" +#include "xfs_icache.h" kmem_zone_t *xfs_ifork_zone;kmem_zone_t *xfs_inode_zone;
fs/xfs/xfs_inode.h +2 -8
@@ -496,11 +496,10 @@ static inline int xfs_isiflocked(struct xfs_inode *ip) (((pip)->i_mount->m_flags &XFS_MOUNT_GRPID ) || \ ((pip)->i_d.di_mode &S_ISGID )) +/* -* xfs_iget.c prototypes. +* xfs_inode.c prototypes. */ -int xfs_iget (struct xfs_mount *,struct xfs_trans *,xfs_ino_t , -uint ,uint ,xfs_inode_t **);void xfs_ilock (xfs_inode_t *,uint );int xfs_ilock_nowait (xfs_inode_t *,uint );void xfs_iunlock (xfs_inode_t *,uint );
@@ -508,11 +507,6 @@ void xfs_ilock_demote(xfs_inode_t *, uint);int xfs_isilocked (xfs_inode_t *,uint );uint xfs_ilock_map_shared (xfs_inode_t *);void xfs_iunlock_map_shared (xfs_inode_t *,uint ); -void xfs_inode_free (struct xfs_inode *ip); - -/* -* xfs_inode.c prototypes. -*/ int xfs_ialloc (struct xfs_trans *,xfs_inode_t *,umode_t ,xfs_nlink_t ,xfs_dev_t ,prid_t ,int ,struct xfs_buf **,xfs_inode_t **);
fs/xfs/xfs_itable.c +1 -0
@@ -34,6 +34,7 @@#include "xfs_error.h" #include "xfs_btree.h" #include "xfs_trace.h" +#include "xfs_icache.h" STATIC int xfs_internal_inum(
fs/xfs/xfs_log_recover.c +1 -0
@@ -42,6 +42,7 @@#include "xfs_quota.h" #include "xfs_utils.h" #include "xfs_trace.h" +#include "xfs_icache.h" STATIC int xlog_find_zeroed(
fs/xfs/xfs_qm.c +1 -0
@@ -40,6 +40,7 @@#include "xfs_utils.h" #include "xfs_qm.h" #include "xfs_trace.h" +#include "xfs_icache.h" /* * The global quota manager. There is only one of these for the entire
fs/xfs/xfs_rtalloc.c +1 -0
@@ -38,6 +38,7 @@#include "xfs_utils.h" #include "xfs_trace.h" #include "xfs_buf.h" +#include "xfs_icache.h" /*
fs/xfs/xfs_vnodeops.c +1 -0
@@ -47,6 +47,7 @@#include "xfs_filestream.h" #include "xfs_vnodeops.h" #include "xfs_trace.h" +#include "xfs_icache.h" /* * The maximum pathlen is 1024 bytes. Since the minimum file system
Signature · 6 lines
1.7.10 _______________________________________________ xfs mailing list [email protected] http://oss.sgi.com/mailman/listinfo/xfs
PATCH v5 14/14 xfs: only update the last_sync_lsn when a transaction completes
In reply to Dave ChinnerFrom: Dave Chinner <[email protected]>
The log write code stamps each iclog with the current tail LSN in the iclog header so that recovery knows where to find the tail of thelog once it has found the head. Normally this is taken from the first item on the AIL - the log item that corresponds to the oldest active item in the log.
The problem is that when the AIL is empty, the tail lsn is dervied from the the l_last_sync_lsn, which is the LSN of the last iclog to be written to the log. In most cases this doesn't happen, because the AIL is rarely empty on an active filesystem. However, when it does, it opens up an interesting case when the transaction being committed to the iclog spans multiple iclogs.
That is, the first iclog is stamped with the l_last_sync_lsn, and IO is issued. Then the next iclog is setup, the changes copied into the iclog (takes some time), and then the l_last_sync_lsn is stamped into the header and IO is issued. This is still the same transaction, so the tail lsn of both iclogs must be the same for log recovery to find the entire transaction to be able to replay it.
The problem arises in that the iclog buffer IO completion updates the l_last_sync_lsn with it's own LSN. Therefore, If the first iclog completes it's IO before the second iclog is filled and has the tail lsn stamped in it, it will stamp the LSN of the first iclog into it's tail lsn field. If the system fails at this point, log recovery will not see a complete transaction, so the transaction will no be replayed.
The fix is simple - the l_last_sync_lsn is updated when a iclog buffer IO completes, and this is incorrect. The l_last_sync_lsn shoul dbe updated when a transaction is completed by a iclog buffer IO. That is, only iclog buffers that have transaction commit callbacks attached to them should update the l_last_sync_lsn. This means that the last_sync_lsn will only move forward when a commit record it written, not in the middle of a large transaction that is rolling through multiple iclog buffers.
--- fs/xfs/xfs_log.c | 19 ++++++++++++++++--- 1 file changed, 16 insertions(+), 3 deletions(-)
fs/xfs/xfs_log.c +16 -3
@@ -2461,14 +2461,27 @@ xlog_state_do_callback(/* -* update the last_sync_lsn before we drop the +* Completion of a iclog IO does not imply that +* a transaction has completed, as transactions +* can be large enough to span many iclogs. We +* cannot change the tail of the log half way +* through a transaction as this may be the only +* transaction in the log and moving th etail to +* point to the middle of it will prevent +* recovery from finding the start of the +* transaction. Hence we should only update the +* last_sync_lsn if this iclog contains +* transaction completion callbacks on it. +* +* We have to do this before we drop the * icloglock to ensure we are the only one that * can update it. */ ASSERT (XFS_LSN_CMP (atomic64_read (&log->l_last_sync_lsn),be64_to_cpu (iclog->ic_header.h_lsn)) <=0 ); -atomic64_set (&log->l_last_sync_lsn, -be64_to_cpu (iclog->ic_header.h_lsn)); +if (iclog->ic_callback) +atomic64_set (&log->l_last_sync_lsn, +be64_to_cpu (iclog->ic_header.h_lsn)); }else ioerrors++;
Signature · 6 lines
1.7.10 _______________________________________________ xfs mailing list [email protected] http://oss.sgi.com/mailman/listinfo/xfs
Re: [PATCH 14/14] xfs: only update the last_sync_lsn when a transaction completes
Hey Dave,
Quoted 39 lines · Dave Chinner
From: Dave Chinner <[email protected]>
The log write code stamps each iclog with the current tail LSN in the iclog header so that recovery knows where to find the tail of thelog once it has found the head. Normally this is taken from the first item on the AIL - the log item that corresponds to the oldest active item in the log.
The problem is that when the AIL is empty, the tail lsn is dervied from the the
l_last_sync_lsn, which is the LSN of the last iclog to be written to the log. In most cases this doesn't happen, because the AIL is rarely empty on an active filesystem. However, when it does, it opens up an interesting case when the transaction being committed to the iclog spans multiple iclogs.That is, the first iclog is stamped with the
l_last_sync_lsn, and IO is issued. Then the next iclog is setup, the changes copied into the iclog (takes some time), and then thel_last_sync_lsnis stamped into the header and IO is issued. This is still the same transaction, so the tail lsn of both iclogs must be the same for log recovery to find the entire transaction to be able to replay it.The problem arises in that the iclog buffer IO completion updates the
l_last_sync_lsnwith it's own LSN. Therefore, If the first iclog completes it's IO before the second iclog is filled and has the tail lsn stamped in it, it will stamp the LSN of the first iclog into it's tail lsn field. If the system fails at this point, log recovery will not see a complete transaction, so the transaction will no be replayed.The fix is simple - the
l_last_sync_lsnis updated when a iclog buffer IO completes, and this is incorrect. Thel_last_sync_lsnshoul dbe updated when a transaction is completed by a iclog buffer IO. That is, only iclog buffers that have transaction commit callbacks attached to them should update thel_last_sync_lsn. This means that thelast_sync_lsnwill only move forward when a commit record it written, not in the middle of a large transaction that is rolling through multiple iclog buffers.
Quoted 2 lines
Signed-off-by Dave Chinner <[email protected]>Reviewed-by Mark Tinguely <[email protected]>
I think this one is appropriate for the 3.7 release. What say you?
-Ben
_______________________________________________
xfs mailing list
[email protected]
http://oss.sgi.com/mailman/listinfo/xfs
Re: [PATCH 14/14] xfs: only update the last_sync_lsn when a transaction completes
Quoted 42 lines · Ben Myers
Hey Dave,
Quoted 39 lines · Dave Chinner
From: Dave Chinner <[email protected]>
The log write code stamps each iclog with the current tail LSN in the iclog header so that recovery knows where to find the tail of thelog once it has found the head. Normally this is taken from the first item on the AIL - the log item that corresponds to the oldest active item in the log.
The problem is that when the AIL is empty, the tail lsn is dervied from the the
l_last_sync_lsn, which is the LSN of the last iclog to be written to the log. In most cases this doesn't happen, because the AIL is rarely empty on an active filesystem. However, when it does, it opens up an interesting case when the transaction being committed to the iclog spans multiple iclogs.That is, the first iclog is stamped with the
l_last_sync_lsn, and IO is issued. Then the next iclog is setup, the changes copied into the iclog (takes some time), and then thel_last_sync_lsnis stamped into the header and IO is issued. This is still the same transaction, so the tail lsn of both iclogs must be the same for log recovery to find the entire transaction to be able to replay it.The problem arises in that the iclog buffer IO completion updates the
l_last_sync_lsnwith it's own LSN. Therefore, If the first iclog completes it's IO before the second iclog is filled and has the tail lsn stamped in it, it will stamp the LSN of the first iclog into it's tail lsn field. If the system fails at this point, log recovery will not see a complete transaction, so the transaction will no be replayed.The fix is simple - the
l_last_sync_lsnis updated when a iclog buffer IO completes, and this is incorrect. Thel_last_sync_lsnshoul dbe updated when a transaction is completed by a iclog buffer IO. That is, only iclog buffers that have transaction commit callbacks attached to them should update thel_last_sync_lsn. This means that thelast_sync_lsnwill only move forward when a commit record it written, not in the middle of a large transaction that is rolling through multiple iclog buffers.
Quoted 4 lines
Quoted 2 lines
Signed-off-by Dave Chinner <[email protected]>Reviewed-by Mark Tinguely <[email protected]>I think this one is appropriate for the 3.7 release. What say you?
Yes, if you want. It's not a common or high risk problem without the preceeding changes, though, so I'm not concerned if it stays with this patchset....
Cheers,
Dave.
Signature · 7 lines
Dave Chinner [email protected] _______________________________________________ xfs mailing list [email protected] http://oss.sgi.com/mailman/listinfo/xfs
Hi Dave,
Quoted 1 line · Dave Chinner
Hopefully the final version.
I am testing this new rev now... My v4 run over the weekend crashed but unfortunately I wasn't able to get a stack trace. We'll see what shakes out.
Regards,
Ben
Quoted 33 lines
Version 5:
- move initial queuing of log work so it isn't started on log mount error
- movexfs_flush_inodes()toxfs_super.c
- passxfs_mounttoxfs_flush_inodes()
- rewrite commit message forxfs_flush_inodes()changes
- addedlast_sync_lsnfix to series as the problem it fixes is reliably exposed by the changes in this seriesVersion 4:
- updated 'xfs:xfs_sync_datais redundant' with cleanups to thexfs_flush_inodesinterface as per Christoph's request,
- updated 'xfs:xfs_sync_datais redundant', folding in changes from
http://oss.sgi.com/archives/xfs/2012-10/msg00036.html
- fixed a minor typo in xfs: 'syncd workqueue is no more', renaming the log worker from 'xfs-reclaim' to 'xfs-log'.Version 3:
- per-mount log and reclaim workqueues instead of a generic mount workqueue.
- reordering of some of the patches as Christoph requested.
- fixed thewriteback_inodes_sb_if_idledeadlock by moving it all the way back up the write stack toxfs_file_aio_buffered_writewhere we were just flushing the current file to avoid deadlocking on it anyway.
- reintroducedxfs_flush_inodes()as a wrapper forwriteback_inodes_sb_if_idle().
- rebased on a current TOT.Version 2:
- fixwriteback_inodes_sb_if_idlecall inxfs_create()
- refreshed patch 13 before sending.
Quoted 7 lines
Initial version:
- http://oss.sgi.com/archives/xfs/2012-06/msg00064.html_______________________________________________
xfs mailing list
[email protected]
http://oss.sgi.com/mailman/listinfo/xfs
_______________________________________________
xfs mailing list
[email protected]
http://oss.sgi.com/mailman/listinfo/xfs
Hey Dave,
Quoted 5 lines · Ben Myers
Quoted 1 line · Dave Chinner
Hopefully the final version.
I am testing this new rev now... My v4 run over the weekend crashed but unfortunately I wasn't able to get a stack trace. We'll see what shakes out.
I'm still not sure what happened with the v4 run I mentioned. Subsequent testing has shown this series to be solid with the new changes. It is ready to go.
However, pulling this in right now will result in a merge commit from the upstream tree to ours later in the rc1 merge. My understanding is that if Linus were to subsequently pull from our tree, the merge commit would cause some ugliness in upstream commit history. See:
http://oss.sgi.com/archives/xfs/2009-04/msg00014.html
I haven't found a way around this, so that's why we're waiting until after the -rc1 fast-forward to pull this in.
Regards,
Ben
_______________________________________________
xfs mailing list
[email protected]
http://oss.sgi.com/mailman/listinfo/xfs
Quoted 13 lines · Ben Myers
Hey Dave,
Quoted 5 lines · Ben Myers
Quoted 1 line · Dave Chinner
Hopefully the final version.
I am testing this new rev now... My v4 run over the weekend crashed but unfortunately I wasn't able to get a stack trace. We'll see what shakes out.
I'm still not sure what happened with the v4 run I mentioned. Subsequent testing has shown this series to be solid with the new changes. It is ready to go.
Quoted 6 lines
However, pulling this in right now will result in a merge commit from the upstream tree to ours later in the rc1 merge. My understanding is that if Linus were to subsequently pull from our tree, the merge commit would cause some ugliness in upstream commit history. See:
I'm not sure that's relevant. The problem ther was this sort of behaviour:
- merge mainline into XFS tree.
- commit XFS stuff
- merge mainline into XFS tree
- commit XFS stuff
.....
- merge mainline into XFS tree
- commit XFS stuff
- pull request
- merge mainline into XFS tree
- commit XFS stuff
<loop>
And so the changes in the XFS tree where not easy to discern from the changes pull in from mainline. Indeed, look at the pull request to see exactly waht Linus was complaining about:
http://oss.sgi.com/archives/xfs/2009-04/msg00009.html
Felix Blyakher (25):
Merge branch 'master' of git+ssh://oss.sgi.com/oss/git/xfs/xfs [XFS] Warn on transaction in flight on read-only remount Merge branch 'master' of git://git.kernel.org/.../torvalds/ linux-2.6 Merge branch 'master' of git://git.kernel.org/.../torvalds/ linux-2.6 Merge branch 'master' of git://git.kernel.org/.../torvalds/ linux-2.6
xfs: Update maintainers
Merge branch 'master' of git://git.kernel.org/.../torvalds/ linux-2.6 Merge branch 'master' of git://git.kernel.org/pub/scm/fs/xfs/xfs
Merge branch 'master' of git://git.kernel.org/pub/scm/fs/xfs/xfs
Merge branch 'master' of git://git.kernel.org/.../torvalds/ linux-2.6 Merge branch 'master' of git://git.kernel.org/pub/scm/fs/xfs/xfs
Merge branch 'master' of git://git.kernel.org/.../torvalds/ linux-2.6 Revert "[XFS] use scalable vmap API"
Revert "[XFS] remove old vmap cache"
Merge branch 'master' of git://git.kernel.org/.../torvalds/ linux-2.6 Merge branch 'master' of git://git.kernel.org/.../torvalds/ linux-2.6 Merge branch 'master' of git://git.kernel.org/.../torvalds/ linux-2.6 Fix xfs debug build breakage by pushing xfs_error.h after Merge branch 'master' of git://git.kernel.org/.../torvalds/ linux-2.6 Merge branch 'master' of git://git.kernel.org/pub/scm/fs/xfs/xfs
Merge branch 'master' of git://git.kernel.org/.../torvalds/ linux-2.6
xfs: increase the maximum number of supported ACL entries Merge branch 'master' of git://git.kernel.org/.../torvalds/ linux-2.6 Merge branch 'master' of git://git.kernel.org/pub/scm/fs/xfs/xfs
Revert "xfs: increase the maximum number of supported ACL entries"
This is the mess that Linus was complaining about - not about a single merge per release that is used to keep the tree tracking mainlinei having a conflict in it.
Having a dev tree history something like:
- commit XFS stuff
- commit XFS stuff
- merge -rc1 from mainline
- commit XFS stuff
- commit XFS stuff
- commit XFS stuff
......
- commit XFS stuff
- pull request
- commit XFS stuff
- commit XFS stuff
<loop per release>
Isn't really the problem that Linus was talking about. Indeed, one mainline merge per release is pretty much as expected, especially if you have a tree that you do not rebase that is consistently committed to....
Quoted 2 lines
I haven't found a way around this, so that's why we're waiting until after the -rc1 fast-forward to pull this in.
Merges by themselves aren't bad - it's excessive, unnecessary use of them that causes problems. :/
Cheers,
Dave.
Signature · 7 lines
Dave Chinner [email protected] _______________________________________________ xfs mailing list [email protected] http://oss.sgi.com/mailman/listinfo/xfs
Hey Dave,
Quoted 68 lines · Dave Chinner
Quoted 11 lines · Ben Myers
Quoted 5 lines · Ben Myers
Quoted 1 line · Dave Chinner
Hopefully the final version.
I am testing this new rev now... My v4 run over the weekend crashed but unfortunately I wasn't able to get a stack trace. We'll see what shakes out.
I'm still not sure what happened with the v4 run I mentioned. Subsequent testing has shown this series to be solid with the new changes. It is ready to go.
Quoted 6 lines
However, pulling this in right now will result in a merge commit from the upstream tree to ours later in the rc1 merge. My understanding is that if Linus were to subsequently pull from our tree, the merge commit would cause some ugliness in upstream commit history. See:
I'm not sure that's relevant. The problem ther was this sort of behaviour:
- merge mainline into XFS tree.
- commit XFS stuff
- merge mainline into XFS tree
- commit XFS stuff
.....
- merge mainline into XFS tree
- commit XFS stuff
- pull request
- merge mainline into XFS tree
- commit XFS stuff
<loop>And so the changes in the XFS tree where not easy to discern from the changes pull in from mainline. Indeed, look at the pull request to see exactly waht Linus was complaining about:
http://oss.sgi.com/archives/xfs/2009-04/msg00009.html
Felix Blyakher (25):
Merge branch 'master' of git+ssh://oss.sgi.com/oss/git/xfs/xfs [XFS] Warn on transaction in flight on read-only remount Merge branch 'master' ofgit://git.kernel.org/.../torvalds/linux-2.6 Merge branch 'master' ofgit://git.kernel.org/.../torvalds/linux-2.6 Merge branch 'master' ofgit://git.kernel.org/.../torvalds/linux-2.6
xfs: Update maintainers
Merge branch 'master' ofgit://git.kernel.org/.../torvalds/linux-2.6 Merge branch 'master' ofgit://git.kernel.org/pub/scm/fs/xfs/xfs
Merge branch 'master' ofgit://git.kernel.org/pub/scm/fs/xfs/xfs
Merge branch 'master' ofgit://git.kernel.org/.../torvalds/linux-2.6 Merge branch 'master' ofgit://git.kernel.org/pub/scm/fs/xfs/xfs
Merge branch 'master' ofgit://git.kernel.org/.../torvalds/linux-2.6 Revert "[XFS] use scalable vmap API"
Revert "[XFS] remove old vmap cache"
Merge branch 'master' ofgit://git.kernel.org/.../torvalds/linux-2.6 Merge branch 'master' ofgit://git.kernel.org/.../torvalds/linux-2.6 Merge branch 'master' ofgit://git.kernel.org/.../torvalds/linux-2.6 Fix xfs debug build breakage by pushingxfs_error.hafter Merge branch 'master' ofgit://git.kernel.org/.../torvalds/linux-2.6 Merge branch 'master' ofgit://git.kernel.org/pub/scm/fs/xfs/xfs
Merge branch 'master' ofgit://git.kernel.org/.../torvalds/linux-2.6
xfs: increase the maximum number of supported ACL entries Merge branch 'master' ofgit://git.kernel.org/.../torvalds/linux-2.6 Merge branch 'master' ofgit://git.kernel.org/pub/scm/fs/xfs/xfs
Revert "xfs: increase the maximum number of supported ACL entries"
Quoted 3 lines
This is the mess that Linus was complaining about - not about a single merge per release that is used to keep the tree tracking mainlinei having a conflict in it.
Yowza. That is ugly.
Quoted 15 lines
Having a dev tree history something like:
- commit XFS stuff
- commit XFS stuff
- merge -rc1 from mainline
- commit XFS stuff
- commit XFS stuff
- commit XFS stuff
......
- commit XFS stuff
- pull request
- commit XFS stuff
- commit XFS stuff
<loop per release>
Quoted 4 lines
Isn't really the problem that Linus was talking about. Indeed, one mainline merge per release is pretty much as expected, especially if you have a tree that you do not rebase that is consistently committed to....
I see what you mean. The current situation where we always fast-forward to -rc1 is still much cleaner than that in terms of history. The problem could also be solved with a topic branch for the duration of the merge window.
There is some more insteresting discussion here:
http://lwn.net/Articles/328438/
I'll quote from Linus's email:
"And, in fact, preferably you don't pull my tree at ALL, since nothing
in my tree should be relevant to the development work _you_ do."
If we can abide by that by using topic branches instead of pulling work into the master branch during the merge window I think it would be clearer in terms of history. I'd really like to sync up on -rc1 without making a mess each time. More topic branches would probably be good for us anyway. Maybe that is something we can experiment with.
This link was also kind of interesting reading:
http://yarchive.net/comp/linux/git_merges_from_upstream.html
Quoted 5 lines
Quoted 2 lines
I haven't found a way around this, so that's why we're waiting until after the -rc1 fast-forward to pull this in.
Merges by themselves aren't bad - it's excessive, unnecessary use of them that causes problems. :/
Yep, I gather that I misunderstood the issue with that pull request.
Thanks,
Ben
_______________________________________________
xfs mailing list
[email protected]
http://oss.sgi.com/mailman/listinfo/xfs
This version looks good to me, and I agree that the last_sync_lsn fix probably is a 3.7 candidate.
Feel free to add a:
to the patches that don't have it yet.
_______________________________________________
xfs mailing list
[email protected]
http://oss.sgi.com/mailman/listinfo/xfs
Hey Christoph,
Quoted 8 lines · Christoph Hellwig
This version looks good to me, and I agree that the
last_sync_lsnfix probably is a 3.7 candidate.Feel free to add a:
Reviewed-by Christoph Hellwig <[email protected]>to the patches that don't have it yet.
Will do.
Regards,
Ben
_______________________________________________
xfs mailing list
[email protected]
http://oss.sgi.com/mailman/listinfo/xfs
Hey Dave,
Quoted 1 line · Dave Chinner
Hopefully the final version.
I'm giving this the smoke test on v3.7-rc1 before pulling it in. Hopefully we can do that tonight or tomorrow.
"xfs: xfs_sync_data is redundant" and "xfs: sync work is now only periodic log work" had trivial conflicts due to
commit 43829731dd372d04d6706c51052b9dabab9ca356
Author: Tejun Heo <[email protected]> Date: Mon Aug 20 14:51:24 2012 -0700
workqueue: deprecate flush[_delayed]_work_sync()
flush[_delayed]_work_sync() are now spurious. Mark them deprecated and convert all users to flush[_delayed]_work().
If you're cc'd and wondering what's going on: Now all workqueues are non-reentrant and the regular flushes guarantee that the work item is not pending or running on any CPU on return, so there's no reason to use the sync flushes at all and they're going away.
This patch doesn't make any functional difference.
Regards,
Ben
Quoted 33 lines
Version 5:
- move initial queuing of log work so it isn't started on log mount error
- movexfs_flush_inodes()toxfs_super.c
- passxfs_mounttoxfs_flush_inodes()
- rewrite commit message forxfs_flush_inodes()changes
- addedlast_sync_lsnfix to series as the problem it fixes is reliably exposed by the changes in this seriesVersion 4:
- updated 'xfs:xfs_sync_datais redundant' with cleanups to thexfs_flush_inodesinterface as per Christoph's request,
- updated 'xfs:xfs_sync_datais redundant', folding in changes from
http://oss.sgi.com/archives/xfs/2012-10/msg00036.html
- fixed a minor typo in xfs: 'syncd workqueue is no more', renaming the log worker from 'xfs-reclaim' to 'xfs-log'.Version 3:
- per-mount log and reclaim workqueues instead of a generic mount workqueue.
- reordering of some of the patches as Christoph requested.
- fixed thewriteback_inodes_sb_if_idledeadlock by moving it all the way back up the write stack toxfs_file_aio_buffered_writewhere we were just flushing the current file to avoid deadlocking on it anyway.
- reintroducedxfs_flush_inodes()as a wrapper forwriteback_inodes_sb_if_idle().
- rebased on a current TOT.Version 2:
- fixwriteback_inodes_sb_if_idlecall inxfs_create()
- refreshed patch 13 before sending.
Quoted 7 lines
Initial version:
- http://oss.sgi.com/archives/xfs/2012-06/msg00064.html_______________________________________________
xfs mailing list
[email protected]
http://oss.sgi.com/mailman/listinfo/xfs
_______________________________________________
xfs mailing list
[email protected]
http://oss.sgi.com/mailman/listinfo/xfs
Hi Dave,
Quoted 1 line · Dave Chinner
Hopefully the final version.
This series has been committed to git://oss.sgi.com/xfs/xfs.git, master and for-next branches.
Regards,
Ben
_______________________________________________
xfs mailing list
[email protected]
http://oss.sgi.com/mailman/listinfo/xfs