2020-05-01 23:00:45 +00:00
|
|
|
// SPDX-License-Identifier: GPL-2.0
|
|
|
|
/*
|
|
|
|
* Copyright (c) 2000-2006 Silicon Graphics, Inc.
|
|
|
|
* All Rights Reserved.
|
|
|
|
*/
|
|
|
|
#include "xfs.h"
|
|
|
|
#include "xfs_fs.h"
|
|
|
|
#include "xfs_shared.h"
|
|
|
|
#include "xfs_format.h"
|
|
|
|
#include "xfs_log_format.h"
|
|
|
|
#include "xfs_trans_resv.h"
|
|
|
|
#include "xfs_mount.h"
|
|
|
|
#include "xfs_inode.h"
|
|
|
|
#include "xfs_trans.h"
|
|
|
|
#include "xfs_inode_item.h"
|
|
|
|
#include "xfs_trace.h"
|
|
|
|
#include "xfs_trans_priv.h"
|
|
|
|
#include "xfs_buf_item.h"
|
|
|
|
#include "xfs_log.h"
|
|
|
|
#include "xfs_error.h"
|
|
|
|
#include "xfs_log_priv.h"
|
|
|
|
#include "xfs_log_recover.h"
|
2020-05-01 23:00:47 +00:00
|
|
|
#include "xfs_icache.h"
|
|
|
|
#include "xfs_bmap_btree.h"
|
2020-05-01 23:00:45 +00:00
|
|
|
|
2020-05-01 23:00:46 +00:00
|
|
|
STATIC void
|
|
|
|
xlog_recover_inode_ra_pass2(
|
|
|
|
struct xlog *log,
|
|
|
|
struct xlog_recover_item *item)
|
|
|
|
{
|
|
|
|
if (item->ri_buf[0].i_len == sizeof(struct xfs_inode_log_format)) {
|
|
|
|
struct xfs_inode_log_format *ilfp = item->ri_buf[0].i_addr;
|
|
|
|
|
|
|
|
xlog_buf_readahead(log, ilfp->ilf_blkno, ilfp->ilf_len,
|
|
|
|
&xfs_inode_buf_ra_ops);
|
|
|
|
} else {
|
|
|
|
struct xfs_inode_log_format_32 *ilfp = item->ri_buf[0].i_addr;
|
|
|
|
|
|
|
|
xlog_buf_readahead(log, ilfp->ilf_blkno, ilfp->ilf_len,
|
|
|
|
&xfs_inode_buf_ra_ops);
|
|
|
|
}
|
|
|
|
}
|
|
|
|
|
2020-05-01 23:00:47 +00:00
|
|
|
/*
|
|
|
|
* Inode fork owner changes
|
|
|
|
*
|
|
|
|
* If we have been told that we have to reparent the inode fork, it's because an
|
|
|
|
* extent swap operation on a CRC enabled filesystem has been done and we are
|
|
|
|
* replaying it. We need to walk the BMBT of the appropriate fork and change the
|
|
|
|
* owners of it.
|
|
|
|
*
|
|
|
|
* The complexity here is that we don't have an inode context to work with, so
|
|
|
|
* after we've replayed the inode we need to instantiate one. This is where the
|
|
|
|
* fun begins.
|
|
|
|
*
|
|
|
|
* We are in the middle of log recovery, so we can't run transactions. That
|
|
|
|
* means we cannot use cache coherent inode instantiation via xfs_iget(), as
|
|
|
|
* that will result in the corresponding iput() running the inode through
|
|
|
|
* xfs_inactive(). If we've just replayed an inode core that changes the link
|
|
|
|
* count to zero (i.e. it's been unlinked), then xfs_inactive() will run
|
|
|
|
* transactions (bad!).
|
|
|
|
*
|
|
|
|
* So, to avoid this, we instantiate an inode directly from the inode core we've
|
|
|
|
* just recovered. We have the buffer still locked, and all we really need to
|
|
|
|
* instantiate is the inode core and the forks being modified. We can do this
|
|
|
|
* manually, then run the inode btree owner change, and then tear down the
|
|
|
|
* xfs_inode without having to run any transactions at all.
|
|
|
|
*
|
|
|
|
* Also, because we don't have a transaction context available here but need to
|
|
|
|
* gather all the buffers we modify for writeback so we pass the buffer_list
|
|
|
|
* instead for the operation to use.
|
|
|
|
*/
|
|
|
|
|
|
|
|
STATIC int
|
|
|
|
xfs_recover_inode_owner_change(
|
|
|
|
struct xfs_mount *mp,
|
|
|
|
struct xfs_dinode *dip,
|
|
|
|
struct xfs_inode_log_format *in_f,
|
|
|
|
struct list_head *buffer_list)
|
|
|
|
{
|
|
|
|
struct xfs_inode *ip;
|
|
|
|
int error;
|
|
|
|
|
|
|
|
ASSERT(in_f->ilf_fields & (XFS_ILOG_DOWNER|XFS_ILOG_AOWNER));
|
|
|
|
|
|
|
|
ip = xfs_inode_alloc(mp, in_f->ilf_ino);
|
|
|
|
if (!ip)
|
|
|
|
return -ENOMEM;
|
|
|
|
|
|
|
|
/* instantiate the inode */
|
|
|
|
ASSERT(dip->di_version >= 3);
|
|
|
|
|
2020-05-14 21:00:02 +00:00
|
|
|
error = xfs_inode_from_disk(ip, dip);
|
2020-05-01 23:00:47 +00:00
|
|
|
if (error)
|
|
|
|
goto out_free_ip;
|
|
|
|
|
|
|
|
if (in_f->ilf_fields & XFS_ILOG_DOWNER) {
|
|
|
|
ASSERT(in_f->ilf_fields & XFS_ILOG_DBROOT);
|
|
|
|
error = xfs_bmbt_change_owner(NULL, ip, XFS_DATA_FORK,
|
|
|
|
ip->i_ino, buffer_list);
|
|
|
|
if (error)
|
|
|
|
goto out_free_ip;
|
|
|
|
}
|
|
|
|
|
|
|
|
if (in_f->ilf_fields & XFS_ILOG_AOWNER) {
|
|
|
|
ASSERT(in_f->ilf_fields & XFS_ILOG_ABROOT);
|
|
|
|
error = xfs_bmbt_change_owner(NULL, ip, XFS_ATTR_FORK,
|
|
|
|
ip->i_ino, buffer_list);
|
|
|
|
if (error)
|
|
|
|
goto out_free_ip;
|
|
|
|
}
|
|
|
|
|
|
|
|
out_free_ip:
|
|
|
|
xfs_inode_free(ip);
|
|
|
|
return error;
|
|
|
|
}
|
|
|
|
|
2020-08-17 16:59:07 +00:00
|
|
|
static inline bool xfs_log_dinode_has_bigtime(const struct xfs_log_dinode *ld)
|
|
|
|
{
|
|
|
|
return ld->di_version >= 3 &&
|
|
|
|
(ld->di_flags2 & XFS_DIFLAG2_BIGTIME);
|
|
|
|
}
|
|
|
|
|
2020-08-24 22:15:46 +00:00
|
|
|
/* Convert a log timestamp to an ondisk timestamp. */
|
|
|
|
static inline xfs_timestamp_t
|
|
|
|
xfs_log_dinode_to_disk_ts(
|
2020-08-17 16:59:07 +00:00
|
|
|
struct xfs_log_dinode *from,
|
2021-04-21 20:48:27 +00:00
|
|
|
const xfs_log_timestamp_t its)
|
2020-08-24 22:15:46 +00:00
|
|
|
{
|
|
|
|
struct xfs_legacy_timestamp *lts;
|
2021-04-21 20:48:27 +00:00
|
|
|
struct xfs_log_legacy_timestamp *lits;
|
2020-08-24 22:15:46 +00:00
|
|
|
xfs_timestamp_t ts;
|
|
|
|
|
2020-08-17 16:59:07 +00:00
|
|
|
if (xfs_log_dinode_has_bigtime(from))
|
|
|
|
return cpu_to_be64(its);
|
|
|
|
|
2020-08-24 22:15:46 +00:00
|
|
|
lts = (struct xfs_legacy_timestamp *)&ts;
|
2021-04-21 20:48:27 +00:00
|
|
|
lits = (struct xfs_log_legacy_timestamp *)&its;
|
2020-08-24 23:01:34 +00:00
|
|
|
lts->t_sec = cpu_to_be32(lits->t_sec);
|
|
|
|
lts->t_nsec = cpu_to_be32(lits->t_nsec);
|
2020-08-24 22:15:46 +00:00
|
|
|
|
|
|
|
return ts;
|
|
|
|
}
|
|
|
|
|
2021-11-16 09:04:43 +00:00
|
|
|
static inline bool xfs_log_dinode_has_large_extent_counts(
|
|
|
|
const struct xfs_log_dinode *ld)
|
|
|
|
{
|
|
|
|
return ld->di_version >= 3 &&
|
|
|
|
(ld->di_flags2 & XFS_DIFLAG2_NREXT64);
|
|
|
|
}
|
|
|
|
|
2022-03-08 09:34:28 +00:00
|
|
|
static inline void
|
|
|
|
xfs_log_dinode_to_disk_iext_counters(
|
|
|
|
struct xfs_log_dinode *from,
|
|
|
|
struct xfs_dinode *to)
|
|
|
|
{
|
|
|
|
if (xfs_log_dinode_has_large_extent_counts(from)) {
|
|
|
|
to->di_big_nextents = cpu_to_be64(from->di_big_nextents);
|
|
|
|
to->di_big_anextents = cpu_to_be32(from->di_big_anextents);
|
|
|
|
to->di_nrext64_pad = cpu_to_be16(from->di_nrext64_pad);
|
|
|
|
} else {
|
|
|
|
to->di_nextents = cpu_to_be32(from->di_nextents);
|
|
|
|
to->di_anextents = cpu_to_be16(from->di_anextents);
|
|
|
|
}
|
|
|
|
|
|
|
|
}
|
|
|
|
|
2020-08-17 16:58:43 +00:00
|
|
|
STATIC void
|
|
|
|
xfs_log_dinode_to_disk(
|
|
|
|
struct xfs_log_dinode *from,
|
xfs: logging the on disk inode LSN can make it go backwards
When we log an inode, we format the "log inode" core and set an LSN
in that inode core. We do that via xfs_inode_item_format_core(),
which calls:
xfs_inode_to_log_dinode(ip, dic, ip->i_itemp->ili_item.li_lsn);
to format the log inode. It writes the LSN from the inode item into
the log inode, and if recovery decides the inode item needs to be
replayed, it recovers the log inode LSN field and writes it into the
on disk inode LSN field.
Now this might seem like a reasonable thing to do, but it is wrong
on multiple levels. Firstly, if the item is not yet in the AIL,
item->li_lsn is zero. i.e. the first time the inode it is logged and
formatted, the LSN we write into the log inode will be zero. If we
only log it once, recovery will run and can write this zero LSN into
the inode.
This means that the next time the inode is logged and log recovery
runs, it will *always* replay changes to the inode regardless of
whether the inode is newer on disk than the version in the log and
that violates the entire purpose of recording the LSN in the inode
at writeback time (i.e. to stop it going backwards in time on disk
during recovery).
Secondly, if we commit the CIL to the journal so the inode item
moves to the AIL, and then relog the inode, the LSN that gets
stamped into the log inode will be the LSN of the inode's current
location in the AIL, not it's age on disk. And it's not the LSN that
will be associated with the current change. That means when log
recovery replays this inode item, the LSN that ends up on disk is
the LSN for the previous changes in the log, not the current
changes being replayed. IOWs, after recovery the LSN on disk is not
in sync with the LSN of the modifications that were replayed into
the inode. This, again, violates the recovery ordering semantics
that on-disk writeback LSNs provide.
Hence the inode LSN in the log dinode is -always- invalid.
Thirdly, recovery actually has the LSN of the log transaction it is
replaying right at hand - it uses it to determine if it should
replay the inode by comparing it to the on-disk inode's LSN. But it
doesn't use that LSN to stamp the LSN into the inode which will be
written back when the transaction is fully replayed. It uses the one
in the log dinode, which we know is always going to be incorrect.
Looking back at the change history, the inode logging was broken by
commit 93f958f9c41f ("xfs: cull unnecessary icdinode fields") way
back in 2016 by a stupid idiot who thought he knew how this code
worked. i.e. me. That commit replaced an in memory di_lsn field that
was updated only at inode writeback time from the inode item.li_lsn
value - and hence always contained the same LSN that appeared in the
on-disk inode - with a read of the inode item LSN at inode format
time. CLearly these are not the same thing.
Before 93f958f9c41f, the log recovery behaviour was irrelevant,
because the LSN in the log inode always matched the on-disk LSN at
the time the inode was logged, hence recovery of the transaction
would never make the on-disk LSN in the inode go backwards or get
out of sync.
A symptom of the problem is this, caught from a failure of
generic/482. Before log recovery, the inode has been allocated but
never used:
xfs_db> inode 393388
xfs_db> p
core.magic = 0x494e
core.mode = 0
....
v3.crc = 0x99126961 (correct)
v3.change_count = 0
v3.lsn = 0
v3.flags2 = 0
v3.cowextsize = 0
v3.crtime.sec = Thu Jan 1 10:00:00 1970
v3.crtime.nsec = 0
After log recovery:
xfs_db> p
core.magic = 0x494e
core.mode = 020444
....
v3.crc = 0x23e68f23 (correct)
v3.change_count = 2
v3.lsn = 0
v3.flags2 = 0
v3.cowextsize = 0
v3.crtime.sec = Thu Jul 22 17:03:03 2021
v3.crtime.nsec = 751000000
...
You can see that the LSN of the on-disk inode is 0, even though it
clearly has been written to disk. I point out this inode, because
the generic/482 failure occurred because several adjacent inodes in
this specific inode cluster were not replayed correctly and still
appeared to be zero on disk when all the other metadata (inobt,
finobt, directories, etc) indicated they should be allocated and
written back.
The fix for this is two-fold. The first is that we need to either
revert the LSN changes in 93f958f9c41f or stop logging the inode LSN
altogether. If we do the former, log recovery does not need to
change but we add 8 bytes of memory per inode to store what is
largely a write-only inode field. If we do the latter, log recovery
needs to stamp the on-disk inode in the same manner that inode
writeback does.
I prefer the latter, because we shouldn't really be trying to log
and replay changes to the on disk LSN as the on-disk value is the
canonical source of the on-disk version of the inode. It also
matches the way we recover buffer items - we create a buf_log_item
that carries the current recovery transaction LSN that gets stamped
into the buffer by the write verifier when it gets written back
when the transaction is fully recovered.
However, this might break log recovery on older kernels even more,
so I'm going to simply ignore the logged value in recovery and stamp
the on-disk inode with the LSN of the transaction being recovered
that will trigger writeback on transaction recovery completion. This
will ensure that the on-disk inode LSN always reflects the LSN of
the last change that was written to disk, regardless of whether it
comes from log recovery or runtime writeback.
Fixes: 93f958f9c41f ("xfs: cull unnecessary icdinode fields")
Signed-off-by: Dave Chinner <dchinner@redhat.com>
Reviewed-by: Darrick J. Wong <djwong@kernel.org>
Signed-off-by: Darrick J. Wong <djwong@kernel.org>
2021-07-27 23:23:49 +00:00
|
|
|
struct xfs_dinode *to,
|
|
|
|
xfs_lsn_t lsn)
|
2020-08-17 16:58:43 +00:00
|
|
|
{
|
|
|
|
to->di_magic = cpu_to_be16(from->di_magic);
|
|
|
|
to->di_mode = cpu_to_be16(from->di_mode);
|
|
|
|
to->di_version = from->di_version;
|
|
|
|
to->di_format = from->di_format;
|
|
|
|
to->di_onlink = 0;
|
|
|
|
to->di_uid = cpu_to_be32(from->di_uid);
|
|
|
|
to->di_gid = cpu_to_be32(from->di_gid);
|
|
|
|
to->di_nlink = cpu_to_be32(from->di_nlink);
|
|
|
|
to->di_projid_lo = cpu_to_be16(from->di_projid_lo);
|
|
|
|
to->di_projid_hi = cpu_to_be16(from->di_projid_hi);
|
|
|
|
|
2020-08-17 16:59:07 +00:00
|
|
|
to->di_atime = xfs_log_dinode_to_disk_ts(from, from->di_atime);
|
|
|
|
to->di_mtime = xfs_log_dinode_to_disk_ts(from, from->di_mtime);
|
|
|
|
to->di_ctime = xfs_log_dinode_to_disk_ts(from, from->di_ctime);
|
2020-08-17 16:58:43 +00:00
|
|
|
|
|
|
|
to->di_size = cpu_to_be64(from->di_size);
|
|
|
|
to->di_nblocks = cpu_to_be64(from->di_nblocks);
|
|
|
|
to->di_extsize = cpu_to_be32(from->di_extsize);
|
|
|
|
to->di_forkoff = from->di_forkoff;
|
|
|
|
to->di_aformat = from->di_aformat;
|
|
|
|
to->di_dmevmask = cpu_to_be32(from->di_dmevmask);
|
|
|
|
to->di_dmstate = cpu_to_be16(from->di_dmstate);
|
|
|
|
to->di_flags = cpu_to_be16(from->di_flags);
|
|
|
|
to->di_gen = cpu_to_be32(from->di_gen);
|
|
|
|
|
|
|
|
if (from->di_version == 3) {
|
|
|
|
to->di_changecount = cpu_to_be64(from->di_changecount);
|
2020-08-17 16:59:07 +00:00
|
|
|
to->di_crtime = xfs_log_dinode_to_disk_ts(from,
|
|
|
|
from->di_crtime);
|
2020-08-17 16:58:43 +00:00
|
|
|
to->di_flags2 = cpu_to_be64(from->di_flags2);
|
|
|
|
to->di_cowextsize = cpu_to_be32(from->di_cowextsize);
|
|
|
|
to->di_ino = cpu_to_be64(from->di_ino);
|
xfs: logging the on disk inode LSN can make it go backwards
When we log an inode, we format the "log inode" core and set an LSN
in that inode core. We do that via xfs_inode_item_format_core(),
which calls:
xfs_inode_to_log_dinode(ip, dic, ip->i_itemp->ili_item.li_lsn);
to format the log inode. It writes the LSN from the inode item into
the log inode, and if recovery decides the inode item needs to be
replayed, it recovers the log inode LSN field and writes it into the
on disk inode LSN field.
Now this might seem like a reasonable thing to do, but it is wrong
on multiple levels. Firstly, if the item is not yet in the AIL,
item->li_lsn is zero. i.e. the first time the inode it is logged and
formatted, the LSN we write into the log inode will be zero. If we
only log it once, recovery will run and can write this zero LSN into
the inode.
This means that the next time the inode is logged and log recovery
runs, it will *always* replay changes to the inode regardless of
whether the inode is newer on disk than the version in the log and
that violates the entire purpose of recording the LSN in the inode
at writeback time (i.e. to stop it going backwards in time on disk
during recovery).
Secondly, if we commit the CIL to the journal so the inode item
moves to the AIL, and then relog the inode, the LSN that gets
stamped into the log inode will be the LSN of the inode's current
location in the AIL, not it's age on disk. And it's not the LSN that
will be associated with the current change. That means when log
recovery replays this inode item, the LSN that ends up on disk is
the LSN for the previous changes in the log, not the current
changes being replayed. IOWs, after recovery the LSN on disk is not
in sync with the LSN of the modifications that were replayed into
the inode. This, again, violates the recovery ordering semantics
that on-disk writeback LSNs provide.
Hence the inode LSN in the log dinode is -always- invalid.
Thirdly, recovery actually has the LSN of the log transaction it is
replaying right at hand - it uses it to determine if it should
replay the inode by comparing it to the on-disk inode's LSN. But it
doesn't use that LSN to stamp the LSN into the inode which will be
written back when the transaction is fully replayed. It uses the one
in the log dinode, which we know is always going to be incorrect.
Looking back at the change history, the inode logging was broken by
commit 93f958f9c41f ("xfs: cull unnecessary icdinode fields") way
back in 2016 by a stupid idiot who thought he knew how this code
worked. i.e. me. That commit replaced an in memory di_lsn field that
was updated only at inode writeback time from the inode item.li_lsn
value - and hence always contained the same LSN that appeared in the
on-disk inode - with a read of the inode item LSN at inode format
time. CLearly these are not the same thing.
Before 93f958f9c41f, the log recovery behaviour was irrelevant,
because the LSN in the log inode always matched the on-disk LSN at
the time the inode was logged, hence recovery of the transaction
would never make the on-disk LSN in the inode go backwards or get
out of sync.
A symptom of the problem is this, caught from a failure of
generic/482. Before log recovery, the inode has been allocated but
never used:
xfs_db> inode 393388
xfs_db> p
core.magic = 0x494e
core.mode = 0
....
v3.crc = 0x99126961 (correct)
v3.change_count = 0
v3.lsn = 0
v3.flags2 = 0
v3.cowextsize = 0
v3.crtime.sec = Thu Jan 1 10:00:00 1970
v3.crtime.nsec = 0
After log recovery:
xfs_db> p
core.magic = 0x494e
core.mode = 020444
....
v3.crc = 0x23e68f23 (correct)
v3.change_count = 2
v3.lsn = 0
v3.flags2 = 0
v3.cowextsize = 0
v3.crtime.sec = Thu Jul 22 17:03:03 2021
v3.crtime.nsec = 751000000
...
You can see that the LSN of the on-disk inode is 0, even though it
clearly has been written to disk. I point out this inode, because
the generic/482 failure occurred because several adjacent inodes in
this specific inode cluster were not replayed correctly and still
appeared to be zero on disk when all the other metadata (inobt,
finobt, directories, etc) indicated they should be allocated and
written back.
The fix for this is two-fold. The first is that we need to either
revert the LSN changes in 93f958f9c41f or stop logging the inode LSN
altogether. If we do the former, log recovery does not need to
change but we add 8 bytes of memory per inode to store what is
largely a write-only inode field. If we do the latter, log recovery
needs to stamp the on-disk inode in the same manner that inode
writeback does.
I prefer the latter, because we shouldn't really be trying to log
and replay changes to the on disk LSN as the on-disk value is the
canonical source of the on-disk version of the inode. It also
matches the way we recover buffer items - we create a buf_log_item
that carries the current recovery transaction LSN that gets stamped
into the buffer by the write verifier when it gets written back
when the transaction is fully recovered.
However, this might break log recovery on older kernels even more,
so I'm going to simply ignore the logged value in recovery and stamp
the on-disk inode with the LSN of the transaction being recovered
that will trigger writeback on transaction recovery completion. This
will ensure that the on-disk inode LSN always reflects the LSN of
the last change that was written to disk, regardless of whether it
comes from log recovery or runtime writeback.
Fixes: 93f958f9c41f ("xfs: cull unnecessary icdinode fields")
Signed-off-by: Dave Chinner <dchinner@redhat.com>
Reviewed-by: Darrick J. Wong <djwong@kernel.org>
Signed-off-by: Darrick J. Wong <djwong@kernel.org>
2021-07-27 23:23:49 +00:00
|
|
|
to->di_lsn = cpu_to_be64(lsn);
|
2022-03-08 09:34:28 +00:00
|
|
|
memset(to->di_pad2, 0, sizeof(to->di_pad2));
|
2020-08-17 16:58:43 +00:00
|
|
|
uuid_copy(&to->di_uuid, &from->di_uuid);
|
2022-03-08 09:34:28 +00:00
|
|
|
to->di_v3_pad = 0;
|
2020-08-17 16:58:43 +00:00
|
|
|
} else {
|
|
|
|
to->di_flushiter = cpu_to_be16(from->di_flushiter);
|
2022-03-08 09:34:28 +00:00
|
|
|
memset(to->di_v2_pad, 0, sizeof(to->di_v2_pad));
|
|
|
|
}
|
|
|
|
|
|
|
|
xfs_log_dinode_to_disk_iext_counters(from, to);
|
|
|
|
}
|
|
|
|
|
|
|
|
STATIC int
|
|
|
|
xlog_dinode_verify_extent_counts(
|
|
|
|
struct xfs_mount *mp,
|
|
|
|
struct xfs_log_dinode *ldip)
|
|
|
|
{
|
|
|
|
xfs_extnum_t nextents;
|
|
|
|
xfs_aextnum_t anextents;
|
|
|
|
|
|
|
|
if (xfs_log_dinode_has_large_extent_counts(ldip)) {
|
|
|
|
if (!xfs_has_large_extent_counts(mp) ||
|
|
|
|
(ldip->di_nrext64_pad != 0)) {
|
|
|
|
XFS_CORRUPTION_ERROR(
|
|
|
|
"Bad log dinode large extent count format",
|
|
|
|
XFS_ERRLEVEL_LOW, mp, ldip, sizeof(*ldip));
|
|
|
|
xfs_alert(mp,
|
|
|
|
"Bad inode 0x%llx, large extent counts %d, padding 0x%x",
|
|
|
|
ldip->di_ino, xfs_has_large_extent_counts(mp),
|
|
|
|
ldip->di_nrext64_pad);
|
|
|
|
return -EFSCORRUPTED;
|
|
|
|
}
|
|
|
|
|
|
|
|
nextents = ldip->di_big_nextents;
|
|
|
|
anextents = ldip->di_big_anextents;
|
|
|
|
} else {
|
|
|
|
if (ldip->di_version == 3 && ldip->di_v3_pad != 0) {
|
|
|
|
XFS_CORRUPTION_ERROR(
|
|
|
|
"Bad log dinode di_v3_pad",
|
|
|
|
XFS_ERRLEVEL_LOW, mp, ldip, sizeof(*ldip));
|
|
|
|
xfs_alert(mp,
|
|
|
|
"Bad inode 0x%llx, di_v3_pad 0x%llx",
|
|
|
|
ldip->di_ino, ldip->di_v3_pad);
|
|
|
|
return -EFSCORRUPTED;
|
|
|
|
}
|
|
|
|
|
|
|
|
nextents = ldip->di_nextents;
|
|
|
|
anextents = ldip->di_anextents;
|
|
|
|
}
|
|
|
|
|
|
|
|
if (unlikely(nextents + anextents > ldip->di_nblocks)) {
|
|
|
|
XFS_CORRUPTION_ERROR("Bad log dinode extent counts",
|
|
|
|
XFS_ERRLEVEL_LOW, mp, ldip, sizeof(*ldip));
|
|
|
|
xfs_alert(mp,
|
|
|
|
"Bad inode 0x%llx, large extent counts %d, nextents 0x%llx, anextents 0x%x, nblocks 0x%llx",
|
|
|
|
ldip->di_ino, xfs_has_large_extent_counts(mp), nextents,
|
|
|
|
anextents, ldip->di_nblocks);
|
|
|
|
return -EFSCORRUPTED;
|
2020-08-17 16:58:43 +00:00
|
|
|
}
|
2022-03-08 09:34:28 +00:00
|
|
|
|
|
|
|
return 0;
|
2020-08-17 16:58:43 +00:00
|
|
|
}
|
|
|
|
|
2020-05-01 23:00:47 +00:00
|
|
|
STATIC int
|
|
|
|
xlog_recover_inode_commit_pass2(
|
|
|
|
struct xlog *log,
|
|
|
|
struct list_head *buffer_list,
|
|
|
|
struct xlog_recover_item *item,
|
|
|
|
xfs_lsn_t current_lsn)
|
|
|
|
{
|
|
|
|
struct xfs_inode_log_format *in_f;
|
|
|
|
struct xfs_mount *mp = log->l_mp;
|
|
|
|
struct xfs_buf *bp;
|
|
|
|
struct xfs_dinode *dip;
|
|
|
|
int len;
|
|
|
|
char *src;
|
|
|
|
char *dest;
|
|
|
|
int error;
|
|
|
|
int attr_index;
|
|
|
|
uint fields;
|
|
|
|
struct xfs_log_dinode *ldip;
|
|
|
|
uint isize;
|
|
|
|
int need_free = 0;
|
|
|
|
|
|
|
|
if (item->ri_buf[0].i_len == sizeof(struct xfs_inode_log_format)) {
|
|
|
|
in_f = item->ri_buf[0].i_addr;
|
|
|
|
} else {
|
|
|
|
in_f = kmem_alloc(sizeof(struct xfs_inode_log_format), 0);
|
|
|
|
need_free = 1;
|
|
|
|
error = xfs_inode_item_format_convert(&item->ri_buf[0], in_f);
|
|
|
|
if (error)
|
|
|
|
goto error;
|
|
|
|
}
|
|
|
|
|
|
|
|
/*
|
|
|
|
* Inode buffers can be freed, look out for it,
|
|
|
|
* and do not replay the inode.
|
|
|
|
*/
|
|
|
|
if (xlog_is_buffer_cancelled(log, in_f->ilf_blkno, in_f->ilf_len)) {
|
|
|
|
error = 0;
|
|
|
|
trace_xfs_log_recover_inode_cancel(log, in_f);
|
|
|
|
goto error;
|
|
|
|
}
|
|
|
|
trace_xfs_log_recover_inode_recover(log, in_f);
|
|
|
|
|
|
|
|
error = xfs_buf_read(mp->m_ddev_targp, in_f->ilf_blkno, in_f->ilf_len,
|
|
|
|
0, &bp, &xfs_inode_buf_ops);
|
|
|
|
if (error)
|
|
|
|
goto error;
|
|
|
|
ASSERT(in_f->ilf_fields & XFS_ILOG_CORE);
|
|
|
|
dip = xfs_buf_offset(bp, in_f->ilf_boffset);
|
|
|
|
|
|
|
|
/*
|
|
|
|
* Make sure the place we're flushing out to really looks
|
|
|
|
* like an inode!
|
|
|
|
*/
|
|
|
|
if (XFS_IS_CORRUPT(mp, !xfs_verify_magic16(bp, dip->di_magic))) {
|
|
|
|
xfs_alert(mp,
|
|
|
|
"%s: Bad inode magic number, dip = "PTR_FMT", dino bp = "PTR_FMT", ino = %Ld",
|
|
|
|
__func__, dip, bp, in_f->ilf_ino);
|
|
|
|
error = -EFSCORRUPTED;
|
|
|
|
goto out_release;
|
|
|
|
}
|
|
|
|
ldip = item->ri_buf[1].i_addr;
|
|
|
|
if (XFS_IS_CORRUPT(mp, ldip->di_magic != XFS_DINODE_MAGIC)) {
|
|
|
|
xfs_alert(mp,
|
|
|
|
"%s: Bad inode log record, rec ptr "PTR_FMT", ino %Ld",
|
|
|
|
__func__, item, in_f->ilf_ino);
|
|
|
|
error = -EFSCORRUPTED;
|
|
|
|
goto out_release;
|
|
|
|
}
|
|
|
|
|
|
|
|
/*
|
xfs: logging the on disk inode LSN can make it go backwards
When we log an inode, we format the "log inode" core and set an LSN
in that inode core. We do that via xfs_inode_item_format_core(),
which calls:
xfs_inode_to_log_dinode(ip, dic, ip->i_itemp->ili_item.li_lsn);
to format the log inode. It writes the LSN from the inode item into
the log inode, and if recovery decides the inode item needs to be
replayed, it recovers the log inode LSN field and writes it into the
on disk inode LSN field.
Now this might seem like a reasonable thing to do, but it is wrong
on multiple levels. Firstly, if the item is not yet in the AIL,
item->li_lsn is zero. i.e. the first time the inode it is logged and
formatted, the LSN we write into the log inode will be zero. If we
only log it once, recovery will run and can write this zero LSN into
the inode.
This means that the next time the inode is logged and log recovery
runs, it will *always* replay changes to the inode regardless of
whether the inode is newer on disk than the version in the log and
that violates the entire purpose of recording the LSN in the inode
at writeback time (i.e. to stop it going backwards in time on disk
during recovery).
Secondly, if we commit the CIL to the journal so the inode item
moves to the AIL, and then relog the inode, the LSN that gets
stamped into the log inode will be the LSN of the inode's current
location in the AIL, not it's age on disk. And it's not the LSN that
will be associated with the current change. That means when log
recovery replays this inode item, the LSN that ends up on disk is
the LSN for the previous changes in the log, not the current
changes being replayed. IOWs, after recovery the LSN on disk is not
in sync with the LSN of the modifications that were replayed into
the inode. This, again, violates the recovery ordering semantics
that on-disk writeback LSNs provide.
Hence the inode LSN in the log dinode is -always- invalid.
Thirdly, recovery actually has the LSN of the log transaction it is
replaying right at hand - it uses it to determine if it should
replay the inode by comparing it to the on-disk inode's LSN. But it
doesn't use that LSN to stamp the LSN into the inode which will be
written back when the transaction is fully replayed. It uses the one
in the log dinode, which we know is always going to be incorrect.
Looking back at the change history, the inode logging was broken by
commit 93f958f9c41f ("xfs: cull unnecessary icdinode fields") way
back in 2016 by a stupid idiot who thought he knew how this code
worked. i.e. me. That commit replaced an in memory di_lsn field that
was updated only at inode writeback time from the inode item.li_lsn
value - and hence always contained the same LSN that appeared in the
on-disk inode - with a read of the inode item LSN at inode format
time. CLearly these are not the same thing.
Before 93f958f9c41f, the log recovery behaviour was irrelevant,
because the LSN in the log inode always matched the on-disk LSN at
the time the inode was logged, hence recovery of the transaction
would never make the on-disk LSN in the inode go backwards or get
out of sync.
A symptom of the problem is this, caught from a failure of
generic/482. Before log recovery, the inode has been allocated but
never used:
xfs_db> inode 393388
xfs_db> p
core.magic = 0x494e
core.mode = 0
....
v3.crc = 0x99126961 (correct)
v3.change_count = 0
v3.lsn = 0
v3.flags2 = 0
v3.cowextsize = 0
v3.crtime.sec = Thu Jan 1 10:00:00 1970
v3.crtime.nsec = 0
After log recovery:
xfs_db> p
core.magic = 0x494e
core.mode = 020444
....
v3.crc = 0x23e68f23 (correct)
v3.change_count = 2
v3.lsn = 0
v3.flags2 = 0
v3.cowextsize = 0
v3.crtime.sec = Thu Jul 22 17:03:03 2021
v3.crtime.nsec = 751000000
...
You can see that the LSN of the on-disk inode is 0, even though it
clearly has been written to disk. I point out this inode, because
the generic/482 failure occurred because several adjacent inodes in
this specific inode cluster were not replayed correctly and still
appeared to be zero on disk when all the other metadata (inobt,
finobt, directories, etc) indicated they should be allocated and
written back.
The fix for this is two-fold. The first is that we need to either
revert the LSN changes in 93f958f9c41f or stop logging the inode LSN
altogether. If we do the former, log recovery does not need to
change but we add 8 bytes of memory per inode to store what is
largely a write-only inode field. If we do the latter, log recovery
needs to stamp the on-disk inode in the same manner that inode
writeback does.
I prefer the latter, because we shouldn't really be trying to log
and replay changes to the on disk LSN as the on-disk value is the
canonical source of the on-disk version of the inode. It also
matches the way we recover buffer items - we create a buf_log_item
that carries the current recovery transaction LSN that gets stamped
into the buffer by the write verifier when it gets written back
when the transaction is fully recovered.
However, this might break log recovery on older kernels even more,
so I'm going to simply ignore the logged value in recovery and stamp
the on-disk inode with the LSN of the transaction being recovered
that will trigger writeback on transaction recovery completion. This
will ensure that the on-disk inode LSN always reflects the LSN of
the last change that was written to disk, regardless of whether it
comes from log recovery or runtime writeback.
Fixes: 93f958f9c41f ("xfs: cull unnecessary icdinode fields")
Signed-off-by: Dave Chinner <dchinner@redhat.com>
Reviewed-by: Darrick J. Wong <djwong@kernel.org>
Signed-off-by: Darrick J. Wong <djwong@kernel.org>
2021-07-27 23:23:49 +00:00
|
|
|
* If the inode has an LSN in it, recover the inode only if the on-disk
|
|
|
|
* inode's LSN is older than the lsn of the transaction we are
|
|
|
|
* replaying. We can have multiple checkpoints with the same start LSN,
|
|
|
|
* so the current LSN being equal to the on-disk LSN doesn't necessarily
|
|
|
|
* mean that the on-disk inode is more recent than the change being
|
|
|
|
* replayed.
|
|
|
|
*
|
|
|
|
* We must check the current_lsn against the on-disk inode
|
|
|
|
* here because the we can't trust the log dinode to contain a valid LSN
|
|
|
|
* (see comment below before replaying the log dinode for details).
|
|
|
|
*
|
|
|
|
* Note: we still need to replay an owner change even though the inode
|
|
|
|
* is more recent than the transaction as there is no guarantee that all
|
|
|
|
* the btree blocks are more recent than this transaction, too.
|
2020-05-01 23:00:47 +00:00
|
|
|
*/
|
|
|
|
if (dip->di_version >= 3) {
|
|
|
|
xfs_lsn_t lsn = be64_to_cpu(dip->di_lsn);
|
|
|
|
|
xfs: logging the on disk inode LSN can make it go backwards
When we log an inode, we format the "log inode" core and set an LSN
in that inode core. We do that via xfs_inode_item_format_core(),
which calls:
xfs_inode_to_log_dinode(ip, dic, ip->i_itemp->ili_item.li_lsn);
to format the log inode. It writes the LSN from the inode item into
the log inode, and if recovery decides the inode item needs to be
replayed, it recovers the log inode LSN field and writes it into the
on disk inode LSN field.
Now this might seem like a reasonable thing to do, but it is wrong
on multiple levels. Firstly, if the item is not yet in the AIL,
item->li_lsn is zero. i.e. the first time the inode it is logged and
formatted, the LSN we write into the log inode will be zero. If we
only log it once, recovery will run and can write this zero LSN into
the inode.
This means that the next time the inode is logged and log recovery
runs, it will *always* replay changes to the inode regardless of
whether the inode is newer on disk than the version in the log and
that violates the entire purpose of recording the LSN in the inode
at writeback time (i.e. to stop it going backwards in time on disk
during recovery).
Secondly, if we commit the CIL to the journal so the inode item
moves to the AIL, and then relog the inode, the LSN that gets
stamped into the log inode will be the LSN of the inode's current
location in the AIL, not it's age on disk. And it's not the LSN that
will be associated with the current change. That means when log
recovery replays this inode item, the LSN that ends up on disk is
the LSN for the previous changes in the log, not the current
changes being replayed. IOWs, after recovery the LSN on disk is not
in sync with the LSN of the modifications that were replayed into
the inode. This, again, violates the recovery ordering semantics
that on-disk writeback LSNs provide.
Hence the inode LSN in the log dinode is -always- invalid.
Thirdly, recovery actually has the LSN of the log transaction it is
replaying right at hand - it uses it to determine if it should
replay the inode by comparing it to the on-disk inode's LSN. But it
doesn't use that LSN to stamp the LSN into the inode which will be
written back when the transaction is fully replayed. It uses the one
in the log dinode, which we know is always going to be incorrect.
Looking back at the change history, the inode logging was broken by
commit 93f958f9c41f ("xfs: cull unnecessary icdinode fields") way
back in 2016 by a stupid idiot who thought he knew how this code
worked. i.e. me. That commit replaced an in memory di_lsn field that
was updated only at inode writeback time from the inode item.li_lsn
value - and hence always contained the same LSN that appeared in the
on-disk inode - with a read of the inode item LSN at inode format
time. CLearly these are not the same thing.
Before 93f958f9c41f, the log recovery behaviour was irrelevant,
because the LSN in the log inode always matched the on-disk LSN at
the time the inode was logged, hence recovery of the transaction
would never make the on-disk LSN in the inode go backwards or get
out of sync.
A symptom of the problem is this, caught from a failure of
generic/482. Before log recovery, the inode has been allocated but
never used:
xfs_db> inode 393388
xfs_db> p
core.magic = 0x494e
core.mode = 0
....
v3.crc = 0x99126961 (correct)
v3.change_count = 0
v3.lsn = 0
v3.flags2 = 0
v3.cowextsize = 0
v3.crtime.sec = Thu Jan 1 10:00:00 1970
v3.crtime.nsec = 0
After log recovery:
xfs_db> p
core.magic = 0x494e
core.mode = 020444
....
v3.crc = 0x23e68f23 (correct)
v3.change_count = 2
v3.lsn = 0
v3.flags2 = 0
v3.cowextsize = 0
v3.crtime.sec = Thu Jul 22 17:03:03 2021
v3.crtime.nsec = 751000000
...
You can see that the LSN of the on-disk inode is 0, even though it
clearly has been written to disk. I point out this inode, because
the generic/482 failure occurred because several adjacent inodes in
this specific inode cluster were not replayed correctly and still
appeared to be zero on disk when all the other metadata (inobt,
finobt, directories, etc) indicated they should be allocated and
written back.
The fix for this is two-fold. The first is that we need to either
revert the LSN changes in 93f958f9c41f or stop logging the inode LSN
altogether. If we do the former, log recovery does not need to
change but we add 8 bytes of memory per inode to store what is
largely a write-only inode field. If we do the latter, log recovery
needs to stamp the on-disk inode in the same manner that inode
writeback does.
I prefer the latter, because we shouldn't really be trying to log
and replay changes to the on disk LSN as the on-disk value is the
canonical source of the on-disk version of the inode. It also
matches the way we recover buffer items - we create a buf_log_item
that carries the current recovery transaction LSN that gets stamped
into the buffer by the write verifier when it gets written back
when the transaction is fully recovered.
However, this might break log recovery on older kernels even more,
so I'm going to simply ignore the logged value in recovery and stamp
the on-disk inode with the LSN of the transaction being recovered
that will trigger writeback on transaction recovery completion. This
will ensure that the on-disk inode LSN always reflects the LSN of
the last change that was written to disk, regardless of whether it
comes from log recovery or runtime writeback.
Fixes: 93f958f9c41f ("xfs: cull unnecessary icdinode fields")
Signed-off-by: Dave Chinner <dchinner@redhat.com>
Reviewed-by: Darrick J. Wong <djwong@kernel.org>
Signed-off-by: Darrick J. Wong <djwong@kernel.org>
2021-07-27 23:23:49 +00:00
|
|
|
if (lsn && lsn != -1 && XFS_LSN_CMP(lsn, current_lsn) > 0) {
|
2020-05-01 23:00:47 +00:00
|
|
|
trace_xfs_log_recover_inode_skip(log, in_f);
|
|
|
|
error = 0;
|
|
|
|
goto out_owner_change;
|
|
|
|
}
|
|
|
|
}
|
|
|
|
|
|
|
|
/*
|
|
|
|
* di_flushiter is only valid for v1/2 inodes. All changes for v3 inodes
|
|
|
|
* are transactional and if ordering is necessary we can determine that
|
|
|
|
* more accurately by the LSN field in the V3 inode core. Don't trust
|
|
|
|
* the inode versions we might be changing them here - use the
|
|
|
|
* superblock flag to determine whether we need to look at di_flushiter
|
|
|
|
* to skip replay when the on disk inode is newer than the log one
|
|
|
|
*/
|
2021-08-19 01:46:37 +00:00
|
|
|
if (!xfs_has_v3inodes(mp) &&
|
2020-05-01 23:00:47 +00:00
|
|
|
ldip->di_flushiter < be16_to_cpu(dip->di_flushiter)) {
|
|
|
|
/*
|
|
|
|
* Deal with the wrap case, DI_MAX_FLUSH is less
|
|
|
|
* than smaller numbers
|
|
|
|
*/
|
|
|
|
if (be16_to_cpu(dip->di_flushiter) == DI_MAX_FLUSH &&
|
|
|
|
ldip->di_flushiter < (DI_MAX_FLUSH >> 1)) {
|
|
|
|
/* do nothing */
|
|
|
|
} else {
|
|
|
|
trace_xfs_log_recover_inode_skip(log, in_f);
|
|
|
|
error = 0;
|
|
|
|
goto out_release;
|
|
|
|
}
|
|
|
|
}
|
|
|
|
|
|
|
|
/* Take the opportunity to reset the flush iteration count */
|
|
|
|
ldip->di_flushiter = 0;
|
|
|
|
|
|
|
|
if (unlikely(S_ISREG(ldip->di_mode))) {
|
|
|
|
if ((ldip->di_format != XFS_DINODE_FMT_EXTENTS) &&
|
|
|
|
(ldip->di_format != XFS_DINODE_FMT_BTREE)) {
|
2022-03-08 09:19:46 +00:00
|
|
|
XFS_CORRUPTION_ERROR(
|
|
|
|
"Bad log dinode data fork format for regular file",
|
|
|
|
XFS_ERRLEVEL_LOW, mp, ldip, sizeof(*ldip));
|
2020-05-01 23:00:47 +00:00
|
|
|
xfs_alert(mp,
|
2022-03-08 09:19:46 +00:00
|
|
|
"Bad inode 0x%llx, data fork format 0x%x",
|
|
|
|
in_f->ilf_ino, ldip->di_format);
|
2020-05-01 23:00:47 +00:00
|
|
|
error = -EFSCORRUPTED;
|
|
|
|
goto out_release;
|
|
|
|
}
|
|
|
|
} else if (unlikely(S_ISDIR(ldip->di_mode))) {
|
|
|
|
if ((ldip->di_format != XFS_DINODE_FMT_EXTENTS) &&
|
|
|
|
(ldip->di_format != XFS_DINODE_FMT_BTREE) &&
|
|
|
|
(ldip->di_format != XFS_DINODE_FMT_LOCAL)) {
|
2022-03-08 09:19:46 +00:00
|
|
|
XFS_CORRUPTION_ERROR(
|
|
|
|
"Bad log dinode data fork format for directory",
|
|
|
|
XFS_ERRLEVEL_LOW, mp, ldip, sizeof(*ldip));
|
2020-05-01 23:00:47 +00:00
|
|
|
xfs_alert(mp,
|
2022-03-08 09:19:46 +00:00
|
|
|
"Bad inode 0x%llx, data fork format 0x%x",
|
|
|
|
in_f->ilf_ino, ldip->di_format);
|
2020-05-01 23:00:47 +00:00
|
|
|
error = -EFSCORRUPTED;
|
|
|
|
goto out_release;
|
|
|
|
}
|
|
|
|
}
|
2022-03-08 09:34:28 +00:00
|
|
|
|
|
|
|
error = xlog_dinode_verify_extent_counts(mp, ldip);
|
|
|
|
if (error)
|
2020-05-01 23:00:47 +00:00
|
|
|
goto out_release;
|
2022-03-08 09:34:28 +00:00
|
|
|
|
2020-05-01 23:00:47 +00:00
|
|
|
if (unlikely(ldip->di_forkoff > mp->m_sb.sb_inodesize)) {
|
2022-03-08 09:19:46 +00:00
|
|
|
XFS_CORRUPTION_ERROR("Bad log dinode fork offset",
|
|
|
|
XFS_ERRLEVEL_LOW, mp, ldip, sizeof(*ldip));
|
2020-05-01 23:00:47 +00:00
|
|
|
xfs_alert(mp,
|
2022-03-08 09:19:46 +00:00
|
|
|
"Bad inode 0x%llx, di_forkoff 0x%x",
|
|
|
|
in_f->ilf_ino, ldip->di_forkoff);
|
2020-05-01 23:00:47 +00:00
|
|
|
error = -EFSCORRUPTED;
|
|
|
|
goto out_release;
|
|
|
|
}
|
|
|
|
isize = xfs_log_dinode_size(mp);
|
|
|
|
if (unlikely(item->ri_buf[1].i_len > isize)) {
|
2022-03-08 09:19:46 +00:00
|
|
|
XFS_CORRUPTION_ERROR("Bad log dinode size", XFS_ERRLEVEL_LOW,
|
|
|
|
mp, ldip, sizeof(*ldip));
|
2020-05-01 23:00:47 +00:00
|
|
|
xfs_alert(mp,
|
2022-03-08 09:19:46 +00:00
|
|
|
"Bad inode 0x%llx log dinode size 0x%x",
|
|
|
|
in_f->ilf_ino, item->ri_buf[1].i_len);
|
2020-05-01 23:00:47 +00:00
|
|
|
error = -EFSCORRUPTED;
|
|
|
|
goto out_release;
|
|
|
|
}
|
|
|
|
|
xfs: logging the on disk inode LSN can make it go backwards
When we log an inode, we format the "log inode" core and set an LSN
in that inode core. We do that via xfs_inode_item_format_core(),
which calls:
xfs_inode_to_log_dinode(ip, dic, ip->i_itemp->ili_item.li_lsn);
to format the log inode. It writes the LSN from the inode item into
the log inode, and if recovery decides the inode item needs to be
replayed, it recovers the log inode LSN field and writes it into the
on disk inode LSN field.
Now this might seem like a reasonable thing to do, but it is wrong
on multiple levels. Firstly, if the item is not yet in the AIL,
item->li_lsn is zero. i.e. the first time the inode it is logged and
formatted, the LSN we write into the log inode will be zero. If we
only log it once, recovery will run and can write this zero LSN into
the inode.
This means that the next time the inode is logged and log recovery
runs, it will *always* replay changes to the inode regardless of
whether the inode is newer on disk than the version in the log and
that violates the entire purpose of recording the LSN in the inode
at writeback time (i.e. to stop it going backwards in time on disk
during recovery).
Secondly, if we commit the CIL to the journal so the inode item
moves to the AIL, and then relog the inode, the LSN that gets
stamped into the log inode will be the LSN of the inode's current
location in the AIL, not it's age on disk. And it's not the LSN that
will be associated with the current change. That means when log
recovery replays this inode item, the LSN that ends up on disk is
the LSN for the previous changes in the log, not the current
changes being replayed. IOWs, after recovery the LSN on disk is not
in sync with the LSN of the modifications that were replayed into
the inode. This, again, violates the recovery ordering semantics
that on-disk writeback LSNs provide.
Hence the inode LSN in the log dinode is -always- invalid.
Thirdly, recovery actually has the LSN of the log transaction it is
replaying right at hand - it uses it to determine if it should
replay the inode by comparing it to the on-disk inode's LSN. But it
doesn't use that LSN to stamp the LSN into the inode which will be
written back when the transaction is fully replayed. It uses the one
in the log dinode, which we know is always going to be incorrect.
Looking back at the change history, the inode logging was broken by
commit 93f958f9c41f ("xfs: cull unnecessary icdinode fields") way
back in 2016 by a stupid idiot who thought he knew how this code
worked. i.e. me. That commit replaced an in memory di_lsn field that
was updated only at inode writeback time from the inode item.li_lsn
value - and hence always contained the same LSN that appeared in the
on-disk inode - with a read of the inode item LSN at inode format
time. CLearly these are not the same thing.
Before 93f958f9c41f, the log recovery behaviour was irrelevant,
because the LSN in the log inode always matched the on-disk LSN at
the time the inode was logged, hence recovery of the transaction
would never make the on-disk LSN in the inode go backwards or get
out of sync.
A symptom of the problem is this, caught from a failure of
generic/482. Before log recovery, the inode has been allocated but
never used:
xfs_db> inode 393388
xfs_db> p
core.magic = 0x494e
core.mode = 0
....
v3.crc = 0x99126961 (correct)
v3.change_count = 0
v3.lsn = 0
v3.flags2 = 0
v3.cowextsize = 0
v3.crtime.sec = Thu Jan 1 10:00:00 1970
v3.crtime.nsec = 0
After log recovery:
xfs_db> p
core.magic = 0x494e
core.mode = 020444
....
v3.crc = 0x23e68f23 (correct)
v3.change_count = 2
v3.lsn = 0
v3.flags2 = 0
v3.cowextsize = 0
v3.crtime.sec = Thu Jul 22 17:03:03 2021
v3.crtime.nsec = 751000000
...
You can see that the LSN of the on-disk inode is 0, even though it
clearly has been written to disk. I point out this inode, because
the generic/482 failure occurred because several adjacent inodes in
this specific inode cluster were not replayed correctly and still
appeared to be zero on disk when all the other metadata (inobt,
finobt, directories, etc) indicated they should be allocated and
written back.
The fix for this is two-fold. The first is that we need to either
revert the LSN changes in 93f958f9c41f or stop logging the inode LSN
altogether. If we do the former, log recovery does not need to
change but we add 8 bytes of memory per inode to store what is
largely a write-only inode field. If we do the latter, log recovery
needs to stamp the on-disk inode in the same manner that inode
writeback does.
I prefer the latter, because we shouldn't really be trying to log
and replay changes to the on disk LSN as the on-disk value is the
canonical source of the on-disk version of the inode. It also
matches the way we recover buffer items - we create a buf_log_item
that carries the current recovery transaction LSN that gets stamped
into the buffer by the write verifier when it gets written back
when the transaction is fully recovered.
However, this might break log recovery on older kernels even more,
so I'm going to simply ignore the logged value in recovery and stamp
the on-disk inode with the LSN of the transaction being recovered
that will trigger writeback on transaction recovery completion. This
will ensure that the on-disk inode LSN always reflects the LSN of
the last change that was written to disk, regardless of whether it
comes from log recovery or runtime writeback.
Fixes: 93f958f9c41f ("xfs: cull unnecessary icdinode fields")
Signed-off-by: Dave Chinner <dchinner@redhat.com>
Reviewed-by: Darrick J. Wong <djwong@kernel.org>
Signed-off-by: Darrick J. Wong <djwong@kernel.org>
2021-07-27 23:23:49 +00:00
|
|
|
/*
|
|
|
|
* Recover the log dinode inode into the on disk inode.
|
|
|
|
*
|
|
|
|
* The LSN in the log dinode is garbage - it can be zero or reflect
|
|
|
|
* stale in-memory runtime state that isn't coherent with the changes
|
|
|
|
* logged in this transaction or the changes written to the on-disk
|
|
|
|
* inode. Hence we write the current lSN into the inode because that
|
|
|
|
* matches what xfs_iflush() would write inode the inode when flushing
|
|
|
|
* the changes in this transaction.
|
|
|
|
*/
|
|
|
|
xfs_log_dinode_to_disk(ldip, dip, current_lsn);
|
2020-05-01 23:00:47 +00:00
|
|
|
|
|
|
|
fields = in_f->ilf_fields;
|
|
|
|
if (fields & XFS_ILOG_DEV)
|
|
|
|
xfs_dinode_put_rdev(dip, in_f->ilf_u.ilfu_rdev);
|
|
|
|
|
|
|
|
if (in_f->ilf_size == 2)
|
|
|
|
goto out_owner_change;
|
|
|
|
len = item->ri_buf[2].i_len;
|
|
|
|
src = item->ri_buf[2].i_addr;
|
|
|
|
ASSERT(in_f->ilf_size <= 4);
|
|
|
|
ASSERT((in_f->ilf_size == 3) || (fields & XFS_ILOG_AFORK));
|
|
|
|
ASSERT(!(fields & XFS_ILOG_DFORK) ||
|
2022-05-04 01:45:50 +00:00
|
|
|
(len == xlog_calc_iovec_len(in_f->ilf_dsize)));
|
2020-05-01 23:00:47 +00:00
|
|
|
|
|
|
|
switch (fields & XFS_ILOG_DFORK) {
|
|
|
|
case XFS_ILOG_DDATA:
|
|
|
|
case XFS_ILOG_DEXT:
|
|
|
|
memcpy(XFS_DFORK_DPTR(dip), src, len);
|
|
|
|
break;
|
|
|
|
|
|
|
|
case XFS_ILOG_DBROOT:
|
|
|
|
xfs_bmbt_to_bmdr(mp, (struct xfs_btree_block *)src, len,
|
|
|
|
(struct xfs_bmdr_block *)XFS_DFORK_DPTR(dip),
|
|
|
|
XFS_DFORK_DSIZE(dip, mp));
|
|
|
|
break;
|
|
|
|
|
|
|
|
default:
|
|
|
|
/*
|
|
|
|
* There are no data fork flags set.
|
|
|
|
*/
|
|
|
|
ASSERT((fields & XFS_ILOG_DFORK) == 0);
|
|
|
|
break;
|
|
|
|
}
|
|
|
|
|
|
|
|
/*
|
|
|
|
* If we logged any attribute data, recover it. There may or
|
|
|
|
* may not have been any other non-core data logged in this
|
|
|
|
* transaction.
|
|
|
|
*/
|
|
|
|
if (in_f->ilf_fields & XFS_ILOG_AFORK) {
|
|
|
|
if (in_f->ilf_fields & XFS_ILOG_DFORK) {
|
|
|
|
attr_index = 3;
|
|
|
|
} else {
|
|
|
|
attr_index = 2;
|
|
|
|
}
|
|
|
|
len = item->ri_buf[attr_index].i_len;
|
|
|
|
src = item->ri_buf[attr_index].i_addr;
|
2022-05-04 01:45:50 +00:00
|
|
|
ASSERT(len == xlog_calc_iovec_len(in_f->ilf_asize));
|
2020-05-01 23:00:47 +00:00
|
|
|
|
|
|
|
switch (in_f->ilf_fields & XFS_ILOG_AFORK) {
|
|
|
|
case XFS_ILOG_ADATA:
|
|
|
|
case XFS_ILOG_AEXT:
|
|
|
|
dest = XFS_DFORK_APTR(dip);
|
|
|
|
ASSERT(len <= XFS_DFORK_ASIZE(dip, mp));
|
|
|
|
memcpy(dest, src, len);
|
|
|
|
break;
|
|
|
|
|
|
|
|
case XFS_ILOG_ABROOT:
|
|
|
|
dest = XFS_DFORK_APTR(dip);
|
|
|
|
xfs_bmbt_to_bmdr(mp, (struct xfs_btree_block *)src,
|
|
|
|
len, (struct xfs_bmdr_block *)dest,
|
|
|
|
XFS_DFORK_ASIZE(dip, mp));
|
|
|
|
break;
|
|
|
|
|
|
|
|
default:
|
|
|
|
xfs_warn(log->l_mp, "%s: Invalid flag", __func__);
|
|
|
|
ASSERT(0);
|
|
|
|
error = -EFSCORRUPTED;
|
|
|
|
goto out_release;
|
|
|
|
}
|
|
|
|
}
|
|
|
|
|
|
|
|
out_owner_change:
|
|
|
|
/* Recover the swapext owner change unless inode has been deleted */
|
|
|
|
if ((in_f->ilf_fields & (XFS_ILOG_DOWNER|XFS_ILOG_AOWNER)) &&
|
|
|
|
(dip->di_mode != 0))
|
|
|
|
error = xfs_recover_inode_owner_change(mp, dip, in_f,
|
|
|
|
buffer_list);
|
|
|
|
/* re-generate the checksum. */
|
|
|
|
xfs_dinode_calc_crc(log->l_mp, dip);
|
|
|
|
|
|
|
|
ASSERT(bp->b_mount == mp);
|
2020-06-29 21:48:47 +00:00
|
|
|
bp->b_flags |= _XBF_LOGRECOVERY;
|
2020-05-01 23:00:47 +00:00
|
|
|
xfs_buf_delwri_queue(bp, buffer_list);
|
|
|
|
|
|
|
|
out_release:
|
|
|
|
xfs_buf_relse(bp);
|
|
|
|
error:
|
|
|
|
if (need_free)
|
|
|
|
kmem_free(in_f);
|
|
|
|
return error;
|
|
|
|
}
|
|
|
|
|
2020-05-01 23:00:45 +00:00
|
|
|
const struct xlog_recover_item_ops xlog_inode_item_ops = {
|
|
|
|
.item_type = XFS_LI_INODE,
|
2020-05-01 23:00:46 +00:00
|
|
|
.ra_pass2 = xlog_recover_inode_ra_pass2,
|
2020-05-01 23:00:47 +00:00
|
|
|
.commit_pass2 = xlog_recover_inode_commit_pass2,
|
2020-05-01 23:00:45 +00:00
|
|
|
};
|