Commit ccf7c23f authored by Dave Chinner's avatar Dave Chinner Committed by Alex Elder

xfs: Ensure inode allocation buffers are fully replayed

With delayed logging, we can get inode allocation buffers in the
same transaction inode unlink buffers. We don't currently mark inode
allocation buffers in the log, so inode unlink buffers take
precedence over allocation buffers.

The result is that when they are combined into the same checkpoint,
only the unlinked inode chain fields are replayed, resulting in
uninitialised inode buffers being detected when the next inode
modification is replayed.

To fix this, we need to ensure that we do not set the inode buffer
flag in the buffer log item format flags if the inode allocation has
not already hit the log. To avoid requiring a change to log
recovery, we really need to make this a modification that relies
only on in-memory sate.

We can do this by checking during buffer log formatting (while the
CIL cannot be flushed) if we are still in the same sequence when we
commit the unlink transaction as the inode allocation transaction.
If we are, then we do not add the inode buffer flag to the buffer
log format item flags. This means the entire buffer will be
replayed, not just the unlinked fields. We do this while
CIL flusheѕ are locked out to ensure that we don't race with the
sequence numbers changing and hence fail to put the inode buffer
flag in the buffer format flags when we really need to.
Signed-off-by: default avatarDave Chinner <dchinner@redhat.com>
Reviewed-by: default avatarChristoph Hellwig <hch@lst.de>
Signed-off-by: default avatarAlex Elder <aelder@sgi.com>
parent df806158
...@@ -254,6 +254,20 @@ xfs_buf_item_format( ...@@ -254,6 +254,20 @@ xfs_buf_item_format(
vecp++; vecp++;
nvecs = 1; nvecs = 1;
/*
* If it is an inode buffer, transfer the in-memory state to the
* format flags and clear the in-memory state. We do not transfer
* this state if the inode buffer allocation has not yet been committed
* to the log as setting the XFS_BLI_INODE_BUF flag will prevent
* correct replay of the inode allocation.
*/
if (bip->bli_flags & XFS_BLI_INODE_BUF) {
if (!((bip->bli_flags & XFS_BLI_INODE_ALLOC_BUF) &&
xfs_log_item_in_current_chkpt(&bip->bli_item)))
bip->bli_format.blf_flags |= XFS_BLF_INODE_BUF;
bip->bli_flags &= ~XFS_BLI_INODE_BUF;
}
if (bip->bli_flags & XFS_BLI_STALE) { if (bip->bli_flags & XFS_BLI_STALE) {
/* /*
* The buffer is stale, so all we need to log * The buffer is stale, so all we need to log
......
...@@ -69,6 +69,7 @@ typedef struct xfs_buf_log_format { ...@@ -69,6 +69,7 @@ typedef struct xfs_buf_log_format {
#define XFS_BLI_LOGGED 0x08 #define XFS_BLI_LOGGED 0x08
#define XFS_BLI_INODE_ALLOC_BUF 0x10 #define XFS_BLI_INODE_ALLOC_BUF 0x10
#define XFS_BLI_STALE_INODE 0x20 #define XFS_BLI_STALE_INODE 0x20
#define XFS_BLI_INODE_BUF 0x40
#define XFS_BLI_FLAGS \ #define XFS_BLI_FLAGS \
{ XFS_BLI_HOLD, "HOLD" }, \ { XFS_BLI_HOLD, "HOLD" }, \
...@@ -76,7 +77,8 @@ typedef struct xfs_buf_log_format { ...@@ -76,7 +77,8 @@ typedef struct xfs_buf_log_format {
{ XFS_BLI_STALE, "STALE" }, \ { XFS_BLI_STALE, "STALE" }, \
{ XFS_BLI_LOGGED, "LOGGED" }, \ { XFS_BLI_LOGGED, "LOGGED" }, \
{ XFS_BLI_INODE_ALLOC_BUF, "INODE_ALLOC" }, \ { XFS_BLI_INODE_ALLOC_BUF, "INODE_ALLOC" }, \
{ XFS_BLI_STALE_INODE, "STALE_INODE" } { XFS_BLI_STALE_INODE, "STALE_INODE" }, \
{ XFS_BLI_INODE_BUF, "INODE_BUF" }
#ifdef __KERNEL__ #ifdef __KERNEL__
......
...@@ -198,6 +198,7 @@ xlog_tid_t xfs_log_get_trans_ident(struct xfs_trans *tp); ...@@ -198,6 +198,7 @@ xlog_tid_t xfs_log_get_trans_ident(struct xfs_trans *tp);
int xfs_log_commit_cil(struct xfs_mount *mp, struct xfs_trans *tp, int xfs_log_commit_cil(struct xfs_mount *mp, struct xfs_trans *tp,
struct xfs_log_vec *log_vector, struct xfs_log_vec *log_vector,
xfs_lsn_t *commit_lsn, int flags); xfs_lsn_t *commit_lsn, int flags);
bool xfs_log_item_in_current_chkpt(struct xfs_log_item *lip);
#endif #endif
......
...@@ -199,6 +199,15 @@ xlog_cil_insert( ...@@ -199,6 +199,15 @@ xlog_cil_insert(
list_move_tail(&item->li_cil, &cil->xc_cil); list_move_tail(&item->li_cil, &cil->xc_cil);
ctx->nvecs += diff_iovecs; ctx->nvecs += diff_iovecs;
/*
* If this is the first time the item is being committed to the CIL,
* store the sequence number on the log item so we can tell
* in future commits whether this is the first checkpoint the item is
* being committed into.
*/
if (!item->li_seq)
item->li_seq = ctx->sequence;
/* /*
* Now transfer enough transaction reservation to the context ticket * Now transfer enough transaction reservation to the context ticket
* for the checkpoint. The context ticket is special - the unit * for the checkpoint. The context ticket is special - the unit
...@@ -325,6 +334,10 @@ xlog_cil_free_logvec( ...@@ -325,6 +334,10 @@ xlog_cil_free_logvec(
* For more specific information about the order of operations in * For more specific information about the order of operations in
* xfs_log_commit_cil() please refer to the comments in * xfs_log_commit_cil() please refer to the comments in
* xfs_trans_commit_iclog(). * xfs_trans_commit_iclog().
*
* Called with the context lock already held in read mode to lock out
* background commit, returns without it held once background commits are
* allowed again.
*/ */
int int
xfs_log_commit_cil( xfs_log_commit_cil(
...@@ -678,3 +691,35 @@ xlog_cil_push_lsn( ...@@ -678,3 +691,35 @@ xlog_cil_push_lsn(
spin_unlock(&cil->xc_cil_lock); spin_unlock(&cil->xc_cil_lock);
return commit_lsn; return commit_lsn;
} }
/*
* Check if the current log item was first committed in this sequence.
* We can't rely on just the log item being in the CIL, we have to check
* the recorded commit sequence number.
*
* Note: for this to be used in a non-racy manner, it has to be called with
* CIL flushing locked out. As a result, it should only be used during the
* transaction commit process when deciding what to format into the item.
*/
bool
xfs_log_item_in_current_chkpt(
struct xfs_log_item *lip)
{
struct xfs_cil_ctx *ctx;
if (!(lip->li_mountp->m_flags & XFS_MOUNT_DELAYLOG))
return false;
if (list_empty(&lip->li_cil))
return false;
ctx = lip->li_mountp->m_log->l_cilp->xc_ctx;
/*
* li_seq is written on the first commit of a log item to record the
* first checkpoint it is written to. Hence if it is different to the
* current sequence, we're in a new checkpoint.
*/
if (XFS_LSN_CMP(lip->li_seq, ctx->sequence) != 0)
return false;
return true;
}
...@@ -835,6 +835,7 @@ typedef struct xfs_log_item { ...@@ -835,6 +835,7 @@ typedef struct xfs_log_item {
/* delayed logging */ /* delayed logging */
struct list_head li_cil; /* CIL pointers */ struct list_head li_cil; /* CIL pointers */
struct xfs_log_vec *li_lv; /* active log vector */ struct xfs_log_vec *li_lv; /* active log vector */
xfs_lsn_t li_seq; /* CIL commit seq */
} xfs_log_item_t; } xfs_log_item_t;
#define XFS_LI_IN_AIL 0x1 #define XFS_LI_IN_AIL 0x1
......
...@@ -792,7 +792,7 @@ xfs_trans_binval( ...@@ -792,7 +792,7 @@ xfs_trans_binval(
XFS_BUF_UNDELAYWRITE(bp); XFS_BUF_UNDELAYWRITE(bp);
XFS_BUF_STALE(bp); XFS_BUF_STALE(bp);
bip->bli_flags |= XFS_BLI_STALE; bip->bli_flags |= XFS_BLI_STALE;
bip->bli_flags &= ~(XFS_BLI_LOGGED | XFS_BLI_DIRTY); bip->bli_flags &= ~(XFS_BLI_INODE_BUF | XFS_BLI_LOGGED | XFS_BLI_DIRTY);
bip->bli_format.blf_flags &= ~XFS_BLF_INODE_BUF; bip->bli_format.blf_flags &= ~XFS_BLF_INODE_BUF;
bip->bli_format.blf_flags |= XFS_BLF_CANCEL; bip->bli_format.blf_flags |= XFS_BLF_CANCEL;
memset((char *)(bip->bli_format.blf_data_map), 0, memset((char *)(bip->bli_format.blf_data_map), 0,
...@@ -802,16 +802,16 @@ xfs_trans_binval( ...@@ -802,16 +802,16 @@ xfs_trans_binval(
} }
/* /*
* This call is used to indicate that the buffer contains on-disk * This call is used to indicate that the buffer contains on-disk inodes which
* inodes which must be handled specially during recovery. They * must be handled specially during recovery. They require special handling
* require special handling because only the di_next_unlinked from * because only the di_next_unlinked from the inodes in the buffer should be
* the inodes in the buffer should be recovered. The rest of the * recovered. The rest of the data in the buffer is logged via the inodes
* data in the buffer is logged via the inodes themselves. * themselves.
* *
* All we do is set the XFS_BLI_INODE_BUF flag in the buffer's log * All we do is set the XFS_BLI_INODE_BUF flag in the items flags so it can be
* format structure so that we'll know what to do at recovery time. * transferred to the buffer's log format structure so that we'll know what to
* do at recovery time.
*/ */
/* ARGSUSED */
void void
xfs_trans_inode_buf( xfs_trans_inode_buf(
xfs_trans_t *tp, xfs_trans_t *tp,
...@@ -826,7 +826,7 @@ xfs_trans_inode_buf( ...@@ -826,7 +826,7 @@ xfs_trans_inode_buf(
bip = XFS_BUF_FSPRIVATE(bp, xfs_buf_log_item_t *); bip = XFS_BUF_FSPRIVATE(bp, xfs_buf_log_item_t *);
ASSERT(atomic_read(&bip->bli_refcount) > 0); ASSERT(atomic_read(&bip->bli_refcount) > 0);
bip->bli_format.blf_flags |= XFS_BLF_INODE_BUF; bip->bli_flags |= XFS_BLI_INODE_BUF;
} }
/* /*
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment