David Chinner & Tim wrote:
And xfs_log_move_tail is called by:
* xfs_trans_update_ail, xfs_trans_delete_ail,
(xfs_trans_unlocked_item and xlog_ungrant_log_space who call
xfs_log_move_tail call it with param 1 which doesn't modify
l_tail_lsn)
I would have thought update_ail and delete_ail would cover the
changes to the ail and hence what the new min item in the ail list
is and hence the change in the tail.
Right - the tail gets moved by (re)moving the tail object in the AIL
In the case of an empty AIL, I guess it needs to use l_last_sync_lsn
which is what xlog_assign_tail_lsn gives you that xfs_log_move_tail
doesn't.
No, if a value of zero is passed to xfs_log_move_tail(), the lsn is
grabbed from l_last_sync_lsn, so when we remove the last object from
the AIL from xfs_trans_delete_ail() we set the l_tail_lsn to l_last_sync_lsn,
same as xlog_assign_tail_lsn does.
Yep, sorry missed that bit (we do the same in both).
IOWs, xlog_assign_tail_lsn() could really be considered redundant as
the I/O completion keeps the l_tail_lsn up to date via the AIL manipulation
functions. I decided to split the difference and ensure that what is
written to the iclog does not change by calling it just before the
tail lsn is written into the header....
IOWs, I don't think calling xlog_assign_tail_lsn() in
xlog_state_release_iclog() ever changes the l_tail_lsn because
it is always kept up to date via the notifications in the AIL
code.....
Exactly.
When looking, I just couldn't see what calling xlog_assign_tail_lsn() is
going to do for us considering every time we change the AIL (from
metadata io completions etc) we update the l_tail_lsn immediately.
Which led me to look at what differences there could be (and found
a wrong difference ;-).
So do we really need to call xlog_assign_tail_lsn() then?
Or are we just being conservative in case we missed something?
--Tim
|