Kill async inode writeback V2
cover Kill async inode writeback V2
Currently we do background inode writeback on demand from many different places - xfssyncd, xfsbufd, xfsaild and the bdi writeback threads. The result is that inodes can be pushed at any time and there is little to no locality to the IO patterns results from such writeback. Indeed, we can have completing writebacks occurring which only serves to slow down writeback.
The idea behind this series is to make metadata buffers get written from xfsbufd via the delayed write queue rather than than from all these other places. All the other places do is make the buffers delayed write so that the xfsbufd can issue them.
This means that inode flushes can no longer happen asynchronously, but we still need a method for ensuring timely dispatch of buffers that we may be waiting for IO completion on. To do this, we allow delayed write buffers to be "promoted" in the delayed write queue. This effectively short-cuts the aging of the buffers, and combined with a demand flush of the xfsbufd we push all aged and promoted buffers out at the same time.
Combine this with sorting the delayed write buffers to be written into disk offset order before dispatch, and we vastly improve the IO patterns for metadata writeback. IO is issued from one place and in a disk/elevator friendly order.
Version 2:
- use generic list sort function
- when unmounting, push the delwri buffers first, then do sync inode reclaim so that reclaim doesn't block for 15 seconds waiting for delwri inode buffers to be aged and written before the inodes can be reclaimed.
Perf results (average of 3 runs) on a debug XFS build (means allocation patterns are randomly varied, so runtimes are also a bit variable):
Untar 2.6.32 kernel tarball, sync, then remove:
Untar+sync rm -rf
xfs-dev: 25.2s 13.0s xfs-dev-delwri-1: 22.5s 9.1s xfs-dev-delwri-2: 21.9s 8.4s
4 processes each creating 100,000, five byte files in separate directories concurrently, then 4 processes removing a directory each concurrently.
create rm -rf
xfs-dev: 8m32s 4m10s xfs-dev-delwri-1: 4m55s 3m42s xfs-dev-delwri-2: 4m56s 3m33s
_______________________________________________
xfs mailing list
[email protected]
http://oss.sgi.com/mailman/listinfo/xfs
PATCH 1/3 xfs: Use delayed write for inodes rather than async
We currently do background inode flush asynchronously, resulting in inodes being written in whatever order the background writeback issues them.
Make the inode flush delayed write instead of asynchronous which will push the inode into the backing buffer but not issue any IO. The buffer will then sit in cache to be flushed by either an AIL push or the xfsbufd timing out the buffer. This will allow accumulation of dirty inode buffers in memory and allow optimisation of inode cluster writeback at the xfsbufd level where we have much greater queue depths than the block layer elevators.
This effectively means that any inode that is written back by background writeback will be seen as flush locked during AIL pushing, and will result in the buffers being pushed from there. A future path will address this non-optimal form of writeback, too.
A side effect of this delayed write mechanism is that background inode reclaim will no longer directly flush inodes, nor can it wait on the flush lock. The result is that inode reclaim must leave the inode in the reclaimable state until it is clean. Hence attempts to reclaim a dirty inode in the background will simply skip the inode until it is clean and this allows other mechanisms (i.e. xfsbufd) to do more optimal writeback of the dirty buffers.
--- fs/xfs/linux-2.6/xfs_super.c | 2 +- fs/xfs/linux-2.6/xfs_sync.c | 52 +++++++++++++++++++++++++++++------------- fs/xfs/xfs_inode.c | 31 ++++--------------------- fs/xfs/xfs_inode.h | 8 ++---- fs/xfs/xfs_inode_item.c | 10 +++++-- fs/xfs/xfs_mount.c | 9 ++++++- 6 files changed, 60 insertions(+), 52 deletions(-)
fs/xfs/linux-2.6/xfs_super.c +1 -1
@@ -1074,7 +1074,7 @@ xfs_fs_write_inode(if (xfs_ipincount (ip) || !xfs_iflock_nowait (ip)) goto out_unlock; - error =xfs_iflush (ip,XFS_IFLUSH_ASYNC_NOBLOCK ); + error =xfs_iflush (ip,XFS_IFLUSH_DELWRI_NOBLOCK ); } out_unlock:
fs/xfs/linux-2.6/xfs_sync.c +36 -16
90 lines not on this page · raw message
fs/xfs/xfs_inode.c +5 -26
@@ -2832,7 +2832,7 @@ xfs_iflush(xfs_dinode_t *dip;xfs_mount_t *mp;int error; -int noblock = (flags ==XFS_IFLUSH_ASYNC_NOBLOCK ); +int noblock = (flags ==XFS_IFLUSH_DELWRI_NOBLOCK );enum {INT_DELWRI = (1 <<0 ),INT_ASYNC = (1 <<1 ) };XFS_STATS_INC (xs_iflush_count);
@@ -2905,12 +2905,8 @@ xfs_iflush(case XFS_IFLUSH_DELWRI_ELSE_SYNC : flags =0 ;break ; -case XFS_IFLUSH_ASYNC_NOBLOCK : -case XFS_IFLUSH_ASYNC : -case XFS_IFLUSH_DELWRI_ELSE_ASYNC : - flags =INT_ASYNC ; -break ;case XFS_IFLUSH_DELWRI : +case XFS_IFLUSH_DELWRI_NOBLOCK : flags =INT_DELWRI ;break ;default :
@@ -2920,15 +2916,11 @@ xfs_iflush( } }else {switch (flags) { +case XFS_IFLUSH_DELWRI_NOBLOCK :case XFS_IFLUSH_DELWRI_ELSE_SYNC : -case XFS_IFLUSH_DELWRI_ELSE_ASYNC :case XFS_IFLUSH_DELWRI : flags =INT_DELWRI ;break ; -case XFS_IFLUSH_ASYNC_NOBLOCK : -case XFS_IFLUSH_ASYNC : - flags =INT_ASYNC ; -break ;case XFS_IFLUSH_SYNC : flags =0 ;break ;
@@ -2971,13 +2963,10 @@ xfs_iflush(if (error) goto cluster_corrupt_out; -if (flags &INT_DELWRI ) { +if (flags &INT_DELWRI )xfs_bdwrite (mp, bp); - }else if (flags &INT_ASYNC ) { - error =xfs_bawrite (mp, bp); - }else { +else error =xfs_bwrite (mp, bp); - }return error; corrupt_out:
@@ -3012,16 +3001,6 @@ xfs_iflush_int( iip = ip->i_itemp; mp = ip->i_mount; - -/* -* If the inode isn't dirty, then just release the inode -* flush lock and do nothing. -*/ -if (xfs_inode_clean (ip)) { -xfs_ifunlock (ip); -return 0 ; - } -/* set *dip = inode's place in the buffer */ dip = (xfs_dinode_t *)xfs_buf_offset (bp, ip->i_imap.im_boffset);
fs/xfs/xfs_inode.h +3 -5
@@ -423,11 +423,9 @@ static inline void xfs_ifunlock(xfs_inode_t *ip)* Flags for xfs_iflush() */ #define XFS_IFLUSH_DELWRI_ELSE_SYNC 1 -#define XFS_IFLUSH_DELWRI_ELSE_ASYNC 2 -#define XFS_IFLUSH_SYNC 3 -#define XFS_IFLUSH_ASYNC 4 -#define XFS_IFLUSH_DELWRI 5 -#define XFS_IFLUSH_ASYNC_NOBLOCK 6 +#define XFS_IFLUSH_SYNC 2 +#define XFS_IFLUSH_DELWRI 3 +#define XFS_IFLUSH_DELWRI_NOBLOCK 4/* * Flags for xfs_itruncate_start().
fs/xfs/xfs_inode_item.c +7 -3
@@ -867,10 +867,14 @@ xfs_inode_item_push( iip->ili_format.ilf_fields != 0);/* -* Write out the inode. The completion routine ('iflush_done') will -* pull it from the AIL, mark it clean, unlock the flush lock. +* Push the inode to it's backing buffer. This will not remove +* the inode from the AIL - a further push will be required to trigger +* a buffer push. However, this allows all the dirty inodes to be pushed to +* the buffer before it is pushed to disk. THe buffer IO completion +* will pull th einode from the AIL, mark it clean and unlock the flush +* lock. */ - (void )xfs_iflush (ip,XFS_IFLUSH_ASYNC ); + (void )xfs_iflush (ip,XFS_IFLUSH_DELWRI );xfs_iunlock (ip,XFS_ILOCK_SHARED );return ;
fs/xfs/xfs_mount.c +8 -1
@@ -1444,7 +1444,14 @@ xfs_unmountfs(* need to force the log first. */ xfs_log_force (mp, (xfs_lsn_t )0 ,XFS_LOG_FORCE |XFS_LOG_SYNC ); -xfs_reclaim_inodes (mp,XFS_IFLUSH_ASYNC ); + +/* +* flush the delwri buffers before the reclaim so that it doesn't +* block for a long time waiting to reclaim inodes that are already +* in the delwri state. +*/ +XFS_bflush (mp->m_ddev_targp); +xfs_reclaim_inodes (mp,XFS_IFLUSH_SYNC );xfs_qm_unmount (mp);
Signature · 6 lines
1.6.5 _______________________________________________ xfs mailing list [email protected] http://oss.sgi.com/mailman/listinfo/xfs
Re: [PATCH 1/3] xfs: Use delayed write for inodes rather than async
fs/xfs/linux-2.6/xfs_sync.c xfs_quiesce_fs( · line 464
Quoted 8 lines
+++
b/fs/xfs/linux-2.6/xfs_sync.cfs/xfs/linux-2.6/xfs_sync.c +1
-1@@ -460,8 +460,8 @@ xfs_quiesce_fs( {int count =0 , pincount; +xfs_reclaim_inodes (mp,XFS_IFLUSH_DELWRI );xfs_flush_buftarg (mp->m_ddev_targp,0 ); -xfs_reclaim_inodes (mp,XFS_IFLUSH_DELWRI_ELSE_ASYNC );
Hmm. I think the current code here is simply wrong. We do need to flush all delwri buffers after the inode reclaim. Maybe we should get this hunk in for .33?
fs/xfs/linux-2.6/xfs_sync.c xfs_reclaim_inode( · line 690
Quoted 11 lines
fs/xfs/linux-2.6/xfs_sync.c +1
-1-xfs_reclaim_inodes (mp,XFS_IFLUSH_DELWRI_ELSE_ASYNC ); +xfs_reclaim_inodes (mp,XFS_IFLUSH_DELWRI );/* dgc: errors ignored here */ error =xfs_qm_sync (mp,SYNC_TRYLOCK );fs/xfs/linux-2.6/xfs_sync.c +1
-1error =xfs_sync_fsdata (mp,SYNC_TRYLOCK );@@ -687,7 +687,7 @@ xfs_reclaim_inode(spin_unlock (&ip->i_flags_lock);write_unlock (&pag->pag_ici_lock);xfs_perag_put (pag); -return -EAGAIN ; +return EAGAIN ;
Unrelated bug in the upsteam code. But your inode direct reclaim changes should sort this out already.
Quoted 4 lines
fs/xfs/xfs_inode.c +0
-0@@ -3012,16 +3001,6 @@ xfs_iflush_int( iip = ip->i_itemp; mp = ip->i_mount;
fs/xfs/xfs_inode.c xfs_iflush_int( · line 3024
Quoted 10 lines
fs/xfs/xfs_inode.c +0
-10- -/* -* If the inode isn't dirty, then just release the inode -* flush lock and do nothing. -*/ -if (xfs_inode_clean (ip)) { -xfs_ifunlock (ip); -return 0 ; - } -
while we now have this check in xfs_reclaim_inode there still are various other callers. Did you audit them all to make sure we don't need the check here anymore?
Quoted 6 lines
index 223d9c3..16c4654 100644
fs/xfs/xfs_mount.c +0
-0@@ -1444,7 +1444,14 @@ xfs_unmountfs(* need to force the log first. */
fs/xfs/xfs_mount.c xfs_unmountfs( · line 1454
Quoted 10 lines
fs/xfs/xfs_mount.c +8
-1xfs_log_force (mp, (xfs_lsn_t )0 ,XFS_LOG_FORCE |XFS_LOG_SYNC ); -xfs_reclaim_inodes (mp,XFS_IFLUSH_ASYNC ); + +/* +* flush the delwri buffers before the reclaim so that it doesn't +* block for a long time waiting to reclaim inodes that are already +* in the delwri state. +*/ +XFS_bflush (mp->m_ddev_targp); +xfs_reclaim_inodes (mp,XFS_IFLUSH_SYNC );
Wouldn't it be more efficient to also write them out delwri and then flush out the delwri queue again?
Either way the current code seems fishy to me with an async writeout here.
_______________________________________________
xfs mailing list
[email protected]
http://oss.sgi.com/mailman/listinfo/xfs
Re: [PATCH 1/3] xfs: Use delayed write for inodes rather than async
Quoted 5 lines · Christoph Hellwig
Quoted 5 lines
+++
b/fs/xfs/linux-2.6/xfs_sync.cfs/xfs/linux-2.6/xfs_sync.c +0
-0@@ -460,8 +460,8 @@ xfs_quiesce_fs( {int count =0 , pincount;
Quoted 7 lines
fs/xfs/linux-2.6/xfs_sync.c xfs_quiesce_fs( · line 464
Quoted 3 lines
fs/xfs/linux-2.6/xfs_sync.c +1
-1+xfs_reclaim_inodes (mp,XFS_IFLUSH_DELWRI );xfs_flush_buftarg (mp->m_ddev_targp,0 ); -xfs_reclaim_inodes (mp,XFS_IFLUSH_DELWRI_ELSE_ASYNC );Hmm. I think the current code here is simply wrong. We do need to flush all delwri buffers after the inode reclaim. Maybe we should get this hunk in for .33?
I don't think it really matters for the existing code as we do the xfs_flush_buftarg(SYNC_WAIT) in the loop below which will push out inodes flushed during reclaim.
Hmmm - given that xfs_reclaim_inodes(mp, XFS_IFLUSH_DELWRI) can skip inodes, there probably should be a sync reclaim done in the flush loop to ensure we've caught them.
Quoted 6 lines
Quoted 6 lines
fs/xfs/linux-2.6/xfs_sync.c +1
-1-xfs_reclaim_inodes (mp,XFS_IFLUSH_DELWRI_ELSE_ASYNC ); +xfs_reclaim_inodes (mp,XFS_IFLUSH_DELWRI );/* dgc: errors ignored here */ error =xfs_qm_sync (mp,SYNC_TRYLOCK );fs/xfs/linux-2.6/xfs_sync.c +0
-0error =xfs_sync_fsdata (mp,SYNC_TRYLOCK );@@ -687,7 +687,7 @@ xfs_reclaim_inode(
Quoted 8 lines
fs/xfs/linux-2.6/xfs_sync.c xfs_reclaim_inode( · line 690
Quoted 5 lines
fs/xfs/linux-2.6/xfs_sync.c +1
-1spin_unlock (&ip->i_flags_lock);write_unlock (&pag->pag_ici_lock);xfs_perag_put (pag); -return -EAGAIN ; +return EAGAIN ;Unrelated bug in the upsteam code. But your inode direct reclaim changes should sort this out already.
*nod*
Quoted 4 lines
Quoted 4 lines
fs/xfs/xfs_inode.c +0
-0@@ -3012,16 +3001,6 @@ xfs_iflush_int( iip = ip->i_itemp; mp = ip->i_mount;
Quoted 14 lines
fs/xfs/xfs_inode.c xfs_iflush_int( · line 3024
Quoted 10 lines
fs/xfs/xfs_inode.c +0
-10- -/* -* If the inode isn't dirty, then just release the inode -* flush lock and do nothing. -*/ -if (xfs_inode_clean (ip)) { -xfs_ifunlock (ip); -return 0 ; - } -while we now have this check in
xfs_reclaim_inodethere still are various other callers. Did you audit them all to make sure we don't need the check here anymore?
Yes - xfs_iflush_int() gets called only from xfs_iflush() and xfs_iflush_cluster() and both check first.
Quoted 11 lines
Quoted 11 lines
index 223d9c3..16c4654 100644
fs/xfs/xfs_mount.c +3
-1@@ -1444,7 +1444,14 @@ xfs_unmountfs(* need to force the log first. */ xfs_log_force (mp, (xfs_lsn_t )0 ,XFS_LOG_FORCE |XFS_LOG_SYNC ); -xfs_reclaim_inodes (mp,XFS_IFLUSH_ASYNC ); + +/* +* flush the delwri buffers before the reclaim so that it doesn't
Quoted 8 lines
fs/xfs/xfs_mount.c xfs_unmountfs( · line 1454
Quoted 5 lines
fs/xfs/xfs_mount.c +5
-0+* block for a long time waiting to reclaim inodes that are already +* in the delwri state. +*/ +XFS_bflush (mp->m_ddev_targp); +xfs_reclaim_inodes (mp,XFS_IFLUSH_SYNC );Wouldn't it be more efficient to also write them out delwri and then flush out the delwri queue again?
The delayed write flush can skip inodes, so we need to do a sync flush to guarantee that we reclaim all dirty inodes. The flush is done first so the sync flush doesn't block on the flush locks for too long for inodes that are already locked for delwri flushing. Perhaps a:
xfs_reclaim_inodes(mp, XFS_IFLUSH_DELWRI); XFS_bflush(mp->m_ddev_targp); xfs_reclaim_inodes(mp, XFS_IFLUSH_SYNC);
sequence would be better here?
Quoted 2 lines
Either way the current code seems fishy to me with an async writeout here.
Agreed.
Cheers,
Dave.
Signature · 7 lines
Dave Chinner [email protected] _______________________________________________ xfs mailing list [email protected] http://oss.sgi.com/mailman/listinfo/xfs
Re: [PATCH 1/3] xfs: Use delayed write for inodes rather than async
Quoted 3 lines · Dave Chinner
I don't think it really matters for the existing code as we do the
xfs_flush_buftarg(SYNC_WAIT)in the loop below which will push out inodes flushed during reclaim.
True.
Quoted 3 lines
Hmmm - given that
xfs_reclaim_inodes(mp, XFS_IFLUSH_DELWRI)can skip inodes, there probably should be a sync reclaim done in the flush loop to ensure we've caught them.
Indeed, the skipping behaviour is rather confusing and needs to be taken care off.
Quoted 2 lines
Yes -
xfs_iflush_int()gets called only fromxfs_iflush()andxfs_iflush_cluster()and both check first.
Ok.
Quoted 11 lines
The delayed write flush can skip inodes, so we need to do a sync flush to guarantee that we reclaim all dirty inodes. The flush is done first so the sync flush doesn't block on the flush locks for too long for inodes that are already locked for delwri flushing. Perhaps a:
xfs_reclaim_inodes(mp, XFS_IFLUSH_DELWRI); XFS_bflush(mp->m_ddev_targp); xfs_reclaim_inodes(mp, XFS_IFLUSH_SYNC);sequence would be better here?
I guess that would be optimal. Maybe with a little comment explaining why we do it.
_______________________________________________
xfs mailing list
[email protected]
http://oss.sgi.com/mailman/listinfo/xfs
PATCH 2/3 xfs: Don't issue buffer IO direct from AIL push
In reply to Dave ChinnerAll buffers logged into the AIL are marked as delayed write. When the AIL needs to push the buffer out, it issues an async write of the buffer. This means that IO patterns are dependent on the order of buffers in the AIL.
Instead of flushing the buffer, promote the buffer in the delayed write list so that the next time the xfsbufd is run the buffer will be flushed by the xfsbufd. Return the state to the xfsaild that the buffer was promoted so that the xfsaild knows that it needs to cause the xfsbufd to run to flush the buffers that were promoted.
Using the xfsbufd for issuing the IO allows us to dispatch all buffer IO from the one queue. This means that we can make much more enlightened decisions on what order to flush buffers to disk as we don't have multiple places issuing IO. Optimisations to xfsbufd will be in a future patch.
--- fs/xfs/linux-2.6/xfs_buf.c | 22 +++++++++ fs/xfs/linux-2.6/xfs_buf.h | 11 +++++ fs/xfs/linux-2.6/xfs_trace.h | 1 + fs/xfs/quota/xfs_dquot_item.c | 87 +++++------------------------------ fs/xfs/quota/xfs_dquot_item.h | 4 -- fs/xfs/xfs_buf_item.c | 64 ++++++++++++++------------ fs/xfs/xfs_inode_item.c | 99 ++++++---------------------------------- fs/xfs/xfs_inode_item.h | 6 --- fs/xfs/xfs_trans_ail.c | 7 +++ 9 files changed, 104 insertions(+), 197 deletions(-)
fs/xfs/linux-2.6/xfs_buf.c +22 -0
@@ -1631,6 +1631,28 @@ xfs_buf_delwri_dequeue(trace_xfs_buf_delwri_dequeue (bp, _RET_IP_); } +/* +* If a delwri buffer needs to be pushed before it has aged out, then +* promote it to the head of the delwri queue so that it will be flushed +* on the next xfsbufd run. +*/ +void +xfs_buf_delwri_promote ( +xfs_buf_t *bp) +{ +struct list_head *dwq = &bp->b_target->bt_delwrite_queue; +spinlock_t *dwlk = &bp->b_target->bt_delwrite_lock; +long age = xfs_buf_age_centisecs *msecs_to_jiffies (10 ) +1 ; + +spin_lock (dwlk); +ASSERT (bp->b_flags &XBF_DELWRI ); +ASSERT (bp->b_flags & _XBF_DELWRI_Q); +list_del (&bp->b_list); +list_add (&bp->b_list, dwq); + bp->b_queuetime = jiffies - age; +spin_unlock (dwlk); +} + STATIC voidxfs_buf_runall_queues (struct workqueue_struct *queue)
fs/xfs/linux-2.6/xfs_buf.h +11 -0
@@ -261,6 +261,7 @@ extern int xfs_buf_ispin(xfs_buf_t *);/* Delayed Write Buffer Routines */ extern void xfs_buf_delwri_dequeue (xfs_buf_t *); +extern void xfs_buf_delwri_promote (xfs_buf_t *);/* Buffer Daemon Setup Routines */ extern int xfs_buf_init (void );
@@ -424,6 +425,16 @@ extern void xfs_free_buftarg(struct xfs_mount *, struct xfs_buftarg *);extern void xfs_wait_buftarg (xfs_buftarg_t *);extern int xfs_setsize_buftarg (xfs_buftarg_t *,unsigned int ,unsigned int );extern int xfs_flush_buftarg (xfs_buftarg_t *,int ); + +/* +* run the xfsbufd on demand to age buffers. Use in combination with +* xfs_buf_delwri_promote() to flus delayed write buffers efficiently. +*/ +static inline void xfs_flush_buftarg_delwri (xfs_buftarg_t *btp) +{ +wake_up_process (btp->bt_task); +} +#ifdef CONFIG_KDB_MODULES extern struct list_head *xfs_get_buftarg_list (void );#endif
fs/xfs/linux-2.6/xfs_trace.h +1 -0
@@ -464,6 +464,7 @@ DEFINE_BUF_ITEM_EVENT(xfs_buf_item_unlock);DEFINE_BUF_ITEM_EVENT (xfs_buf_item_unlock_stale);DEFINE_BUF_ITEM_EVENT (xfs_buf_item_committed);DEFINE_BUF_ITEM_EVENT (xfs_buf_item_push); +DEFINE_BUF_ITEM_EVENT (xfs_buf_item_pushbuf);DEFINE_BUF_ITEM_EVENT (xfs_trans_get_buf);DEFINE_BUF_ITEM_EVENT (xfs_trans_get_buf_recur);DEFINE_BUF_ITEM_EVENT (xfs_trans_getsb);
fs/xfs/quota/xfs_dquot_item.c +13 -74
131 lines not on this page · raw message
fs/xfs/quota/xfs_dquot_item.h +0 -4
@@ -27,10 +27,6 @@ typedef struct xfs_dq_logitem {xfs_log_item_t qli_item;/* common portion */ struct xfs_dquot *qli_dquot;/* dquot ptr */ xfs_lsn_t qli_flush_lsn;/* lsn at last flush */ -unsigned short qli_pushbuf_flag;/* 1 bit used in push_ail */ -#ifdef DEBUG -uint64_t qli_push_owner; -#endif xfs_dq_logformat_t qli_format;/* logged structure */ } xfs_dq_logitem_t;
fs/xfs/xfs_buf_item.c +35 -29
102 lines not on this page · raw message
fs/xfs/xfs_inode_item.c +15 -84
146 lines not on this page · raw message
fs/xfs/xfs_inode_item.h +0 -6
@@ -144,12 +144,6 @@ typedef struct xfs_inode_log_item {data exts */ struct xfs_bmbt_rec *ili_aextents_buf;/* array of logged attr exts */ -unsigned int ili_pushbuf_flag;/* one bit used in push_ail */ - -#ifdef DEBUG -uint64_t ili_push_owner;/* one who sets pushbuf_flag -above gets to push the buf */ -#endif #ifdef XFS_TRANS_DEBUG int ili_root_size;char *ili_orig_root;
fs/xfs/xfs_trans_ail.c +7 -0
@@ -253,6 +253,7 @@ xfsaild_push(int flush_log, count, stuck;xfs_mount_t *mp = ailp->xa_mount;struct xfs_ail_cursor *cur = &ailp->xa_cursors; +int push_xfsbufd =0 ;spin_lock (&ailp->xa_lock);xfs_trans_ail_cursor_init (ailp, cur);
@@ -308,6 +309,7 @@ xfsaild_push(XFS_STATS_INC (xs_push_ail_pushbuf);IOP_PUSHBUF (lip); last_pushed_lsn = lsn; + push_xfsbufd =1 ;break ;case XFS_ITEM_PINNED :
@@ -374,6 +376,11 @@ xfsaild_push(xfs_log_force (mp, (xfs_lsn_t )0 ,XFS_LOG_FORCE ); } +if (push_xfsbufd) { +/* we've got delayed write buffers to flush */ +xfs_flush_buftarg_delwri (mp->m_ddev_targp); + } +if (!count) {/* We're past our target or empty, so idle */ last_pushed_lsn =0 ;
Signature · 6 lines
1.6.5 _______________________________________________ xfs mailing list [email protected] http://oss.sgi.com/mailman/listinfo/xfs
Re: [PATCH 2/3] xfs: Don't issue buffer IO direct from AIL push
Quoted 10 lines
fs/xfs/linux-2.6/xfs_buf.c +10
-0+/* +* If a delwri buffer needs to be pushed before it has aged out, then +* promote it to the head of the delwri queue so that it will be flushed +* on the next xfsbufd run. +*/ +void +xfs_buf_delwri_promote ( +xfs_buf_t *bp) +{ +struct list_head *dwq = &bp->b_target->bt_delwrite_queue;
fs/xfs/linux-2.6/xfs_buf.c xfs_buf_delwri_dequeue( · line 1653
Quoted 10 lines
fs/xfs/linux-2.6/xfs_buf.c +10
-0+spinlock_t *dwlk = &bp->b_target->bt_delwrite_lock; +long age = xfs_buf_age_centisecs *msecs_to_jiffies (10 ) +1 ; + +spin_lock (dwlk); +ASSERT (bp->b_flags &XBF_DELWRI ); +ASSERT (bp->b_flags & _XBF_DELWRI_Q); +list_del (&bp->b_list); +list_add (&bp->b_list, dwq); + bp->b_queuetime = jiffies - age; +spin_unlock (dwlk);
Sorry for the nitpicking, but:
a) can you use the struct types instead of the typedefs where possible?
b) second the pointer to spinlock style used here like in some other buf code is rather odd. What about this instead:
void
xfs_buf_delwri_promote(
struct xfs_buf *bp)
{
struct xfs_buftarg *target = bp->b_target;
spin_lock(&target->bt_delwrite_lock);
ASSERT(bp->b_flags & XBF_DELWRI);
ASSERT(bp->b_flags & _XBF_DELWRI_Q);
list_move(&bp->b_list, &target->bt_delwrite_queue);
bp->b_queuetime = jiffies -
xfs_buf_age_centisecs * msecs_to_jiffies(10) - 1;
spin_unlock(&target->bt_delwrite_lock);
}Also the queuetime calculation could use some comments.
fs/xfs/linux-2.6/xfs_buf.h extern void xfs_free_buftarg(struct xfs_mount *, struct xfs_buftarg *); · line 436
Quoted 12 lines
extern void
xfs_wait_buftarg(xfs_buftarg_t *);
extern intxfs_setsize_buftarg(xfs_buftarg_t *, unsigned int, unsigned int); extern intxfs_flush_buftarg(xfs_buftarg_t *, int);fs/xfs/linux-2.6/xfs_buf.h +9
-0+ +/* +* run the xfsbufd on demand to age buffers. Use in combination with +* xfs_buf_delwri_promote() to flus delayed write buffers efficiently. +*/ +static inline void xfs_flush_buftarg_delwri (xfs_buftarg_t *btp) +{ +wake_up_process (btp->bt_task); +}
The function name is extremly misleading. It's an xfsbufd wakeup, so it should be named like that. In doubt I'd just opencode the wake_up_process call instead.
The changes to the various log items look good, especially as we bring some more commonality into the various items.
You removed the only call to trace_xfs_inode_item_push, so you might aswell remove the trace point declaration, too.
_______________________________________________
xfs mailing list
[email protected]
http://oss.sgi.com/mailman/listinfo/xfs
Re: [PATCH 2/3] xfs: Don't issue buffer IO direct from AIL push
Quoted 23 lines · Christoph Hellwig
Quoted 10 lines
fs/xfs/linux-2.6/xfs_buf.c +10
-0+/* +* If a delwri buffer needs to be pushed before it has aged out, then +* promote it to the head of the delwri queue so that it will be flushed +* on the next xfsbufd run. +*/ +void +xfs_buf_delwri_promote ( +xfs_buf_t *bp) +{ +struct list_head *dwq = &bp->b_target->bt_delwrite_queue;fs/xfs/linux-2.6/xfs_buf.c xfs_buf_delwri_dequeue( · line 1653
Quoted 10 lines
fs/xfs/linux-2.6/xfs_buf.c +10
-0+spinlock_t *dwlk = &bp->b_target->bt_delwrite_lock; +long age = xfs_buf_age_centisecs *msecs_to_jiffies (10 ) +1 ; + +spin_lock (dwlk); +ASSERT (bp->b_flags &XBF_DELWRI ); +ASSERT (bp->b_flags & _XBF_DELWRI_Q); +list_del (&bp->b_list); +list_add (&bp->b_list, dwq); + bp->b_queuetime = jiffies - age; +spin_unlock (dwlk);Sorry for the nitpicking, but:
Quoted 1 line
a) can you use the struct types instead of the typedefs where possible?
Sure - I missed that one when I went back over this patch after a c'n'p to create the function.
Quoted 6 lines
b) second the pointer to spinlock style used here like in some other buf code is rather odd. What about this instead:
void
xfs_buf_delwri_promote(
structxfs_buf*bp)
Quoted 12 lines
{
struct xfs_buftarg *target = bp->b_target;
spin_lock(&target->bt_delwrite_lock);
ASSERT(bp->b_flags & XBF_DELWRI);
ASSERT(bp->b_flags & _XBF_DELWRI_Q);
list_move(&bp->b_list, &target->bt_delwrite_queue);
bp->b_queuetime = jiffies -
xfs_buf_age_centisecs * msecs_to_jiffies(10) - 1;
spin_unlock(&target->bt_delwrite_lock);
}Yup, will change.
Quoted 1 line
Also the queuetime calculation could use some comments.
OK, will do.
Quoted 8 lines
Quoted 8 lines
extern void
xfs_wait_buftarg(xfs_buftarg_t *);
extern intxfs_setsize_buftarg(xfs_buftarg_t *, unsigned int, unsigned int); extern intxfs_flush_buftarg(xfs_buftarg_t *, int);fs/xfs/linux-2.6/xfs_buf.h +5
-0+ +/* +* run the xfsbufd on demand to age buffers. Use in combination with +* xfs_buf_delwri_promote() to flus delayed write buffers efficiently. +*/
Quoted 8 lines
fs/xfs/linux-2.6/xfs_buf.h extern void xfs_free_buftarg(struct xfs_mount *, struct xfs_buftarg *); · line 436
Quoted 4 lines
fs/xfs/linux-2.6/xfs_buf.h +4
-0+static inline void xfs_flush_buftarg_delwri (xfs_buftarg_t *btp) +{ +wake_up_process (btp->bt_task); +}The function name is extremly misleading. It's an xfsbufd wakeup, so it should be named like that. In doubt I'd just opencode the
wake_up_processcall instead.
OK, I just didn't want to be digging deep into the buftarg structure in places that don't really know about it.
Quoted 6 lines
The changes to the various log items look good, especially as we bring some more commonality into the various items.
You removed the only call to
trace_xfs_inode_item_push, so you might aswell remove the trace point declaration, too.
Will do.
Cheers,
Dave.
Signature · 7 lines
Dave Chinner [email protected] _______________________________________________ xfs mailing list [email protected] http://oss.sgi.com/mailman/listinfo/xfs
PATCH 3/3 xfs: Sort delayed write buffers before dispatch
In reply to Dave ChinnerCurrently when the xfsbufd writes delayed write buffers, it pushes them to disk in the order they come off the delayed write list. If there are lots of buffers ѕpread widely over the disk, this results in overwhelming the elevator sort queues in the block layer and we end up losing the posibility of merging adjacent buffers to minimise the number of IOs.
Use the new generic list_sort function to sort the delwri dispatch queue before issue to ensure that the buffers are pushed in the most friendly order possible to the lower layers.
--- fs/xfs/linux-2.6/xfs_buf.c | 90 +++++++++++++++++++++++++++++++------------ 1 files changed, 65 insertions(+), 25 deletions(-)
fs/xfs/linux-2.6/xfs_buf.c +65 -25
@@ -33,6 +33,7 @@#include <linux/migrate.h> #include <linux/backing-dev.h> #include <linux/freezer.h> +#include <linux/sort.h> #include "xfs_sb.h" #include "xfs_inum.h"
@@ -1644,12 +1645,18 @@ xfs_buf_delwri_promote(spinlock_t *dwlk = &bp->b_target->bt_delwrite_lock;long age = xfs_buf_age_centisecs *msecs_to_jiffies (10 ) +1 ; -spin_lock (dwlk);ASSERT (bp->b_flags &XBF_DELWRI );ASSERT (bp->b_flags & _XBF_DELWRI_Q); -list_del (&bp->b_list); -list_add (&bp->b_list, dwq); + +/* +* Check the buffer age before locking the delayed write queue as we +* don't need to promote buffers that are already past the flush age. +*/ +if (bp->b_queuetime < jiffies - age) +return ; bp->b_queuetime = jiffies - age; +spin_lock (dwlk); +list_move (&bp->b_list, dwq);spin_unlock (dwlk); }
@@ -1723,14 +1730,42 @@ xfs_buf_delwri_split( } +/* +* Compare function is more complex than it needs to be because +* the return value is only 32 bits and we are doing comparisons +* on 64 bit values +*/ +int +xfs_buf_cmp ( +void *priv, +struct list_head *a, +struct list_head *b) +{ + struct xfs_buf *ap = container_of(a, struct xfs_buf, b_list); + struct xfs_buf *bp = container_of(b, struct xfs_buf, b_list); +xfs_daddr_t diff; + + diff = ap->b_bn - bp->b_bn; +if (diff <0 ) +return -1 ; +if (diff >0 ) +return 1 ; +return 0 ; +} + +void +xfs_buf_delwri_sort ( +xfs_buftarg_t *target, +struct list_head *list) +{ +list_sort (NULL , list, xfs_buf_cmp); +} + STATIC intxfsbufd (void *data) { -struct list_head tmp; -xfs_buftarg_t *target = (xfs_buftarg_t *)data; -int count; -xfs_buf_t *bp; +xfs_buftarg_t *target = (xfs_buftarg_t *)data; current->flags |=PF_MEMALLOC ;
@@ -1739,6 +1774,8 @@ xfsbufd( do { long age = xfs_buf_age_centisecs * msecs_to_jiffies(10); long tout = age; + int count = 0; + struct list_head tmp; if (unlikely(freezing(current))) { set_bit(XBT_FORCE_SLEEP, &target->bt_flags);
@@ -1753,11 +1790,10 @@ xfsbufd(schedule_timeout_interruptible (tout);xfs_buf_delwri_split (target, &tmp, age); - count =0 ; +xfs_buf_delwri_sort (target, &tmp);while (!list_empty (&tmp)) { - bp =list_entry (tmp.next, xfs_buf_t, b_list); -ASSERT (target == bp->b_target); - +struct xfs_buf *bp; + bp = list_first_entry(&tmp, struct xfs_buf, b_list);list_del_init (&bp->b_list);xfs_buf_iostrategy (bp); count++;
@@ -1783,38 +1819,42 @@ xfs_flush_buftarg( xfs_buftarg_t *target, int wait) { -struct list_head tmp; -xfs_buf_t *bp, *n; +xfs_buf_t *bp;int pincount =0 ; +LIST_HEAD (tmp_list); +LIST_HEAD (wait_list);xfs_buf_runall_queues (xfsconvertd_workqueue);xfs_buf_runall_queues (xfsdatad_workqueue);xfs_buf_runall_queues (xfslogd_workqueue);set_bit (XBT_FORCE_FLUSH , &target->bt_flags); - pincount =xfs_buf_delwri_split (target, &tmp,0 ); + pincount =xfs_buf_delwri_split (target, &tmp_list,0 );/* -* Dropped the delayed write list lock, now walk the temporary list +* Dropped the delayed write list lock, now walk the temporary list. +* All I/O is issued async and then if we need to wait for completion +* we do that after issuing all the IO. */ -list_for_each_entry_safe (bp, n, &tmp, b_list) { +xfs_buf_delwri_sort (target, &tmp_list); +while (!list_empty (&tmp_list)) { +struct xfs_buf *bp; + bp = list_first_entry(&tmp_list, struct xfs_buf, b_list);ASSERT (target == bp->b_target); -if (wait) +list_del_init (&bp->b_list); +if (wait) { bp->b_flags &= ~XBF_ASYNC ; -else -list_del_init (&bp->b_list); - +list_add (&bp->b_list, &wait_list); + }xfs_buf_iostrategy (bp); }if (wait)blk_run_address_space (target->bt_mapping); -/* -* Remaining list items must be flushed before returning -*/ -while (!list_empty (&tmp)) { - bp =list_entry (tmp.next, xfs_buf_t, b_list); +/* Now wait for IO to complete if required. */ +while (!list_empty (&wait_list)) { + bp = list_first_entry(&wait_list, struct xfs_buf, b_list);list_del_init (&bp->b_list);xfs_iowait (bp);
Signature · 6 lines
1.6.5 _______________________________________________ xfs mailing list [email protected] http://oss.sgi.com/mailman/listinfo/xfs
Re: [PATCH 3/3] xfs: Sort delayed write buffers before dispatch
fs/xfs/linux-2.6/xfs_buf.c xfs_buf_delwri_split( · line 1739
Quoted 7 lines
fs/xfs/linux-2.6/xfs_buf.c +7
-0+/* +* Compare function is more complex than it needs to be because +* the return value is only 32 bits and we are doing comparisons +* on 64 bit values +*/ +int +xfs_buf_cmp(
Should be marked static.
fs/xfs/linux-2.6/xfs_buf.c xfs_buf_delwri_split( · line 1762
Quoted 7 lines
fs/xfs/linux-2.6/xfs_buf.c +7
-0+void +xfs_buf_delwri_sort ( +xfs_buftarg_t *target, +struct list_head *list) +{ +list_sort (NULL , list, xfs_buf_cmp); +}
Same here. Not sure I would even bother with the wrapper. Also the first argument is entirely unused.
Quoted 29 lines
STATIC int
xfsbufd(fs/xfs/linux-2.6/xfs_buf.c +6
-0void *data) { +xfs_buftarg_t *target = (xfs_buftarg_t *)data; current->flags |=PF_MEMALLOC ;@@ -1739,6 +1774,8 @@ xfsbufd( do { long age = xfs_buf_age_centisecs * msecs_to_jiffies(10); long tout = age; + int count = 0; + struct list_head tmp; if (unlikely(freezing(current))) { set_bit(XBT_FORCE_SLEEP, &target->bt_flags);@@ -1753,11 +1790,10 @@ xfsbufd(schedule_timeout_interruptible (tout);xfs_buf_delwri_split (target, &tmp, age); +xfs_buf_delwri_sort (target, &tmp);while (!list_empty (&tmp)) { +struct xfs_buf *bp; + bp = list_first_entry(&tmp, struct xfs_buf, b_list);list_del_init (&bp->b_list);xfs_buf_iostrategy (bp); count++;
fs/xfs/linux-2.6/xfs_buf.c xfsbufd(
Quoted 10 lines
fs/xfs/linux-2.6/xfs_buf.c +3
-0if (wait)blk_run_address_space (target->bt_mapping); +/* Now wait for IO to complete if required. */ +while (!list_empty (&wait_list)) { + bp = list_first_entry(&wait_list, struct xfs_buf, b_list);list_del_init (&bp->b_list);xfs_iowait (bp);
As a tiny optimization you might want to move this into the if (wait) block
_______________________________________________
xfs mailing list
[email protected]
http://oss.sgi.com/mailman/listinfo/xfs
Re: [PATCH 3/3] xfs: Sort delayed write buffers before dispatch
Quoted 58 lines · Christoph Hellwig
fs/xfs/linux-2.6/xfs_buf.c xfs_buf_delwri_split( · line 1739
Quoted 7 lines
fs/xfs/linux-2.6/xfs_buf.c +7
-0+/* +* Compare function is more complex than it needs to be because +* the return value is only 32 bits and we are doing comparisons +* on 64 bit values +*/ +int +xfs_buf_cmp(Should be marked static.
fs/xfs/linux-2.6/xfs_buf.c xfs_buf_delwri_split( · line 1762
Quoted 7 lines
fs/xfs/linux-2.6/xfs_buf.c +7
-0+void +xfs_buf_delwri_sort ( +xfs_buftarg_t *target, +struct list_head *list) +{ +list_sort (NULL , list, xfs_buf_cmp); +}Same here. Not sure I would even bother with the wrapper. Also the first argument is entirely unused.
Quoted 37 lines
STATIC int
xfsbufd(fs/xfs/linux-2.6/xfs_buf.c +9
-0void *data) { +xfs_buftarg_t *target = (xfs_buftarg_t *)data; current->flags |=PF_MEMALLOC ;@@ -1739,6 +1774,8 @@ xfsbufd( do { long age = xfs_buf_age_centisecs * msecs_to_jiffies(10); long tout = age; + int count = 0; + struct list_head tmp; if (unlikely(freezing(current))) { set_bit(XBT_FORCE_SLEEP, &target->bt_flags);@@ -1753,11 +1790,10 @@ xfsbufd(schedule_timeout_interruptible (tout);xfs_buf_delwri_split (target, &tmp, age); +xfs_buf_delwri_sort (target, &tmp);while (!list_empty (&tmp)) { +struct xfs_buf *bp; + bp = list_first_entry(&tmp, struct xfs_buf, b_list);list_del_init (&bp->b_list);xfs_buf_iostrategy (bp); count++;if (wait)blk_run_address_space (target->bt_mapping); +/* Now wait for IO to complete if required. */ +while (!list_empty (&wait_list)) { + bp = list_first_entry(&wait_list, struct xfs_buf, b_list);
Quoted 5 lines
fs/xfs/linux-2.6/xfs_buf.c xfs_flush_buftarg( · line 1860
Quoted 2 lines
fs/xfs/linux-2.6/xfs_buf.c +0
-0list_del_init (&bp->b_list);xfs_iowait (bp);As a tiny optimization you might want to move this into the if (wait) block
All makes sense, will update.
Cheers,
Dave.
Signature · 7 lines
Dave Chinner [email protected] _______________________________________________ xfs mailing list [email protected] http://oss.sgi.com/mailman/listinfo/xfs
Btw, after this series XFS_IFLUSH_DELWRI_ELSE_SYNC is also unused, might be worth to throw something like the patch below in to clean up xfs_iflush:
I'm also not sure we do enough of the noblock calls either with or without your series. There seem to be a lot more non-blocking sync calls than iflush calls.
fs/xfs/xfs_inode.c +6 -49
@@ -2827,8 +2827,6 @@ xfs_iflush(xfs_dinode_t *dip;xfs_mount_t *mp;int error; -int noblock = (flags ==XFS_IFLUSH_DELWRI_NOBLOCK ); -enum {INT_DELWRI = (1 <<0 ),INT_ASYNC = (1 <<1 ) };XFS_STATS_INC (xs_iflush_count);
@@ -2860,7 +2858,7 @@ xfs_iflush(* in the same cluster are dirty, they will probably write the inode * out for us if they occur after the log force completes. */ -if (noblock &&xfs_ipincount (ip)) { +if ((flags &XFS_IFLUSH_NOBLOCK ) &&xfs_ipincount (ip)) {xfs_iunpin_nowait (ip);xfs_ifunlock (ip);return EAGAIN ;
@@ -2881,52 +2879,11 @@ xfs_iflush( }/* -* Decide how buffer will be flushed out. This is done before -* the call to xfs_iflush_int because this field is zeroed by it. -*/ -if (iip !=NULL && iip->ili_format.ilf_fields !=0 ) { -/* -* Flush out the inode buffer according to the directions -* of the caller. In the cases where the caller has given -* us a choice choose the non-delwri case. This is because -* the inode is in the AIL and we need to get it out soon. -*/ -switch (flags) { -case XFS_IFLUSH_SYNC : -case XFS_IFLUSH_DELWRI_ELSE_SYNC : - flags =0 ; -break ; -case XFS_IFLUSH_DELWRI : -case XFS_IFLUSH_DELWRI_NOBLOCK : - flags =INT_DELWRI ; -break ; -default : -ASSERT (0 ); - flags =0 ; -break ; - } - }else { -switch (flags) { -case XFS_IFLUSH_DELWRI_NOBLOCK : -case XFS_IFLUSH_DELWRI_ELSE_SYNC : -case XFS_IFLUSH_DELWRI : - flags =INT_DELWRI ; -break ; -case XFS_IFLUSH_SYNC : - flags =0 ; -break ; -default : -ASSERT (0 ); - flags =0 ; -break ; - } - } - -/* * Get the buffer containing the on-disk inode. */ error =xfs_itobp (mp,NULL , ip, &dip, &bp, - noblock ?XFS_BUF_TRYLOCK :XFS_BUF_LOCK ); + (flags &XFS_IFLUSH_NOBLOCK ) ? +XFS_BUF_TRYLOCK :XFS_BUF_LOCK );if (error || !bp) {xfs_ifunlock (ip);return error;
@@ -2954,10 +2911,10 @@ xfs_iflush(if (error) goto cluster_corrupt_out; -if (flags &INT_DELWRI ) -xfs_bdwrite (mp, bp); -else +if (flags &XFS_IFLUSH_SYNC ) error =xfs_bwrite (mp, bp); +else +xfs_bdwrite (mp, bp);return error; corrupt_out:
fs/xfs/xfs_inode.h +2 -4
@@ -422,10 +422,8 @@ static inline void xfs_ifunlock(xfs_inod/* * Flags for xfs_iflush() */ -#define XFS_IFLUSH_DELWRI_ELSE_SYNC 1 -#define XFS_IFLUSH_SYNC 2 -#define XFS_IFLUSH_DELWRI 3 -#define XFS_IFLUSH_DELWRI_NOBLOCK 4 +#define XFS_IFLUSH_SYNC 1 +#define XFS_IFLUSH_NOBLOCK 2/* * Flags for xfs_itruncate_start().
fs/xfs/linux-2.6/xfs_super.c +1 -1
@@ -1075,7 +1075,7 @@ xfs_fs_write_inode(if (xfs_ipincount (ip) || !xfs_iflock_nowait (ip)) goto out_unlock; - error =xfs_iflush (ip,XFS_IFLUSH_DELWRI_NOBLOCK ); + error =xfs_iflush (ip,XFS_IFLUSH_NOBLOCK ); } out_unlock:
fs/xfs/linux-2.6/xfs_sync.c +4 -5
@@ -260,8 +260,7 @@ xfs_sync_inode_attr( goto out_unlock; } - error =xfs_iflush (ip, (flags &SYNC_WAIT ) ? -XFS_IFLUSH_SYNC :XFS_IFLUSH_DELWRI ); + error =xfs_iflush (ip, (flags &SYNC_WAIT ) ?XFS_IFLUSH_SYNC :0 ); out_unlock:xfs_iunlock (ip,XFS_ILOCK_SHARED );
@@ -460,7 +459,7 @@ xfs_quiesce_fs( {int count =0 , pincount; -xfs_reclaim_inodes (mp,XFS_IFLUSH_DELWRI ); +xfs_reclaim_inodes (mp,0 );xfs_flush_buftarg (mp->m_ddev_targp,0 );/*
@@ -585,7 +584,7 @@ xfs_sync_worker(if (!(mp->m_flags &XFS_MOUNT_RDONLY )) {xfs_log_force (mp, (xfs_lsn_t )0 ,XFS_LOG_FORCE ); -xfs_reclaim_inodes (mp,XFS_IFLUSH_DELWRI ); +xfs_reclaim_inodes (mp,0 );/* dgc: errors ignored here */ error =xfs_qm_sync (mp,SYNC_TRYLOCK ); error =xfs_sync_fsdata (mp,SYNC_TRYLOCK );
@@ -718,7 +717,7 @@ xfs_reclaim_inode(* xfs_iflush_done by locking and unlocking the flush lock. */ if (xfs_iflush (ip, sync_mode) ==0 ) { -if (sync_mode ==XFS_IFLUSH_DELWRI ) +if (sync_mode ==0 ) goto unlock_and_requeue;xfs_iflock (ip);xfs_ifunlock (ip);
fs/xfs/xfs_inode_item.c +1 -1
@@ -806,7 +806,7 @@ xfs_inode_item_push(* will pull th einode from the AIL, mark it clean and unlock the flush * lock. */ - (void )xfs_iflush (ip,XFS_IFLUSH_DELWRI ); + (void )xfs_iflush (ip,0 );xfs_iunlock (ip,XFS_ILOCK_SHARED );return ;
fs/xfs/xfs_mount.c +1 -1
@@ -1373,7 +1373,7 @@ xfs_unmountfs(* need to force the log first. */ xfs_log_force (mp, (xfs_lsn_t )0 ,XFS_LOG_FORCE |XFS_LOG_SYNC ); -xfs_reclaim_inodes (mp,XFS_IFLUSH_DELWRI ); +xfs_reclaim_inodes (mp,0 );XFS_bflush (mp->m_ddev_targp);xfs_qm_unmount (mp);
_______________________________________________
xfs mailing list
[email protected]
http://oss.sgi.com/mailman/listinfo/xfs
Quoted 3 lines · Christoph Hellwig
Btw, after this series
XFS_IFLUSH_DELWRI_ELSE_SYNCis also unused, might be worth to throw something like the patch below in to clean upxfs_iflush:
Yes, makes sense. I'll add the patch to my QA series after updating it for the slight changes to the unmount reclaim I ahd in the second posting of the patch. Can I get a signoff from you for this?
Quoted 3 lines
I'm also not sure we do enough of the noblock calls either with or without your series. There seem to be a lot more non-blocking sync calls than iflush calls.
I don't quite follow - inode flushes from the bdi threads should be the majority of flushes (i.e. from xfs_fs_write_inode()) and they are non-blocking. the xfssyncd does delwri writeback (maybe that should be non-blocking and then we can get rid of that flag, too), so the only sync inode writeback path is from xfs_fs_write_inode() for sync flushing, as well as the unmount reclaim path....
Cheers,
Dave.
Signature · 7 lines
Dave Chinner [email protected] _______________________________________________ xfs mailing list [email protected] http://oss.sgi.com/mailman/listinfo/xfs
Quoted 8 lines · Dave Chinner
Quoted 3 lines · Christoph Hellwig
Btw, after this series
XFS_IFLUSH_DELWRI_ELSE_SYNCis also unused, might be worth to throw something like the patch below in to clean upxfs_iflush:Yes, makes sense. I'll add the patch to my QA series after updating it for the slight changes to the unmount reclaim I ahd in the second posting of the patch. Can I get a signoff from you for this?
Quoted 11 lines
Quoted 4 lines
I'm also not sure we do enough of the noblock calls either with or without your series. There seem to be a lot more non-blocking sync calls than iflush calls.
I don't quite follow - inode flushes from the bdi threads should be the majority of flushes (i.e. from
xfs_fs_write_inode()) and they are non-blocking. the xfssyncd does delwri writeback (maybe that should be non-blocking and then we can get rid of that flag, too), so the only sync inode writeback path is fromxfs_fs_write_inode()for sync flushing, as well as the unmount reclaim path....
Sorry, I mean non-blocking delwri calls above. xfs_sync_worker should certainly be non-blocking as the whole daemon is operating that way. And possibly xfs_sync_attr as well.
_______________________________________________
xfs mailing list
[email protected]
http://oss.sgi.com/mailman/listinfo/xfs