From: Miao Xie <miaox@cn.fujitsu.com>
To: Linux Btrfs <linux-btrfs@vger.kernel.org>
Subject: [PATCH V2 01/10] Btrfs: use atomic for btrfs_fs_info->generation
Date: Tue, 29 Jan 2013 17:59:14 +0800 [thread overview]
Message-ID: <51079D72.7000301@cn.fujitsu.com> (raw)
fs_info->generation is a 64bit variant, and it can be accessed by
multi-task, if there is no lock or other methods to protect it, we
might get the wrong number, especially on 32bit machine.
For example, Assuming ->generation is 0x0000 0000 ffff ffff at the
beginning, then we increase it by 1, ->generation will be
0x0000 0001 0000 0000, but it is in the registers, then we store it
into the memory. If some task accesses it at this time, just like
this:
Task0 Task1
set low 32 bits
load low 32 bits
load high 32 bits
set high 32 bits
The task will get 0, it is a wrong number.
We fix this problem by the atomic operation.
Signed-off-by: Zhao Lei <zhaolei@cn.fujitsu.com>
Signed-off-by: Miao Xie <miaox@cn.fujitsu.com>
---
Changelog v1 -> v2:
- modify the changelog and make it more clear and stringency.
---
fs/btrfs/ctree.c | 7 ++++---
fs/btrfs/ctree.h | 2 +-
fs/btrfs/disk-io.c | 8 ++++----
fs/btrfs/file.c | 6 ++++--
fs/btrfs/inode.c | 5 +++--
fs/btrfs/qgroup.c | 2 +-
fs/btrfs/transaction.c | 4 ++--
include/trace/events/btrfs.h | 3 ++-
8 files changed, 21 insertions(+), 16 deletions(-)
diff --git a/fs/btrfs/ctree.c b/fs/btrfs/ctree.c
index eea5da7..4a36c03 100644
--- a/fs/btrfs/ctree.c
+++ b/fs/btrfs/ctree.c
@@ -1365,10 +1365,11 @@ noinline int btrfs_cow_block(struct btrfs_trans_handle *trans,
(unsigned long long)
root->fs_info->running_transaction->transid);
- if (trans->transid != root->fs_info->generation)
+ if (trans->transid != atomic64_read(&root->fs_info->generation))
WARN(1, KERN_CRIT "trans %llu running %llu\n",
(unsigned long long)trans->transid,
- (unsigned long long)root->fs_info->generation);
+ (unsigned long long)atomic64_read(
+ &root->fs_info->generation));
if (!should_cow_block(trans, root, buf)) {
*cow_ret = buf;
@@ -1465,7 +1466,7 @@ int btrfs_realloc_node(struct btrfs_trans_handle *trans,
return 0;
WARN_ON(trans->transaction != root->fs_info->running_transaction);
- WARN_ON(trans->transid != root->fs_info->generation);
+ WARN_ON(trans->transid != atomic64_read(&root->fs_info->generation));
parent_nritems = btrfs_header_nritems(parent);
blocksize = btrfs_level_size(root, parent_level - 1);
diff --git a/fs/btrfs/ctree.h b/fs/btrfs/ctree.h
index 547b7b0..c3edb22 100644
--- a/fs/btrfs/ctree.h
+++ b/fs/btrfs/ctree.h
@@ -1278,7 +1278,7 @@ struct btrfs_fs_info {
struct btrfs_block_rsv empty_block_rsv;
- u64 generation;
+ atomic64_t generation;
u64 last_trans_committed;
/*
diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c
index 65f0367..f03aebc 100644
--- a/fs/btrfs/disk-io.c
+++ b/fs/btrfs/disk-io.c
@@ -1200,7 +1200,7 @@ static void __setup_root(u32 nodesize, u32 leafsize, u32 sectorsize,
memset(&root->root_item, 0, sizeof(root->root_item));
memset(&root->defrag_progress, 0, sizeof(root->defrag_progress));
memset(&root->root_kobj, 0, sizeof(root->root_kobj));
- root->defrag_trans_start = fs_info->generation;
+ root->defrag_trans_start = atomic64_read(&fs_info->generation);
init_completion(&root->kobj_unregister);
root->defrag_running = 0;
root->root_key.objectid = objectid;
@@ -2501,7 +2501,7 @@ retry_root_backup:
fs_info->pending_quota_state = 1;
}
- fs_info->generation = generation;
+ atomic64_set(&fs_info->generation, generation);
fs_info->last_trans_committed = generation;
ret = btrfs_recover_balance(fs_info);
@@ -3436,12 +3436,12 @@ void btrfs_mark_buffer_dirty(struct extent_buffer *buf)
int was_dirty;
btrfs_assert_tree_locked(buf);
- if (transid != root->fs_info->generation)
+ if (transid != atomic64_read(&root->fs_info->generation))
WARN(1, KERN_CRIT "btrfs transid mismatch buffer %llu, "
"found %llu running %llu\n",
(unsigned long long)buf->start,
(unsigned long long)transid,
- (unsigned long long)root->fs_info->generation);
+ (u64)atomic64_read(&root->fs_info->generation));
was_dirty = set_extent_buffer_dirty(buf);
if (!was_dirty) {
spin_lock(&root->fs_info->delalloc_lock);
diff --git a/fs/btrfs/file.c b/fs/btrfs/file.c
index 841cfe3..02409b6 100644
--- a/fs/btrfs/file.c
+++ b/fs/btrfs/file.c
@@ -1588,7 +1588,8 @@ static ssize_t btrfs_file_aio_write(struct kiocb *iocb,
* otherwise subsequent syncs to a file that's been synced in this
* transaction will appear to have already occured.
*/
- BTRFS_I(inode)->last_trans = root->fs_info->generation + 1;
+ BTRFS_I(inode)->last_trans = atomic64_read(&root->fs_info->generation) +
+ 1;
BTRFS_I(inode)->last_sub_trans = root->log_transid;
if (num_written > 0 || num_written == -EIOCBQUEUED) {
err = generic_write_sync(file, pos, num_written);
@@ -1679,7 +1680,8 @@ int btrfs_sync_file(struct file *file, loff_t start, loff_t end, int datasync)
* syncing
*/
smp_mb();
- if (btrfs_inode_in_log(inode, root->fs_info->generation) ||
+ if (btrfs_inode_in_log(inode,
+ atomic64_read(&root->fs_info->generation)) ||
BTRFS_I(inode)->last_trans <=
root->fs_info->last_trans_committed) {
BTRFS_I(inode)->last_trans = 0;
diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
index ca7ace7..35c4dda 100644
--- a/fs/btrfs/inode.c
+++ b/fs/btrfs/inode.c
@@ -2644,7 +2644,8 @@ static void btrfs_read_locked_inode(struct inode *inode)
* idea about which extents were modified before we were evicted from
* cache.
*/
- if (BTRFS_I(inode)->last_trans == root->fs_info->generation)
+ if (BTRFS_I(inode)->last_trans ==
+ atomic64_read(&root->fs_info->generation))
set_bit(BTRFS_INODE_NEEDS_FULL_SYNC,
&BTRFS_I(inode)->runtime_flags);
@@ -6903,7 +6904,7 @@ again:
set_page_dirty(page);
SetPageUptodate(page);
- BTRFS_I(inode)->last_trans = root->fs_info->generation;
+ BTRFS_I(inode)->last_trans = atomic64_read(&root->fs_info->generation);
BTRFS_I(inode)->last_sub_trans = BTRFS_I(inode)->root->log_transid;
BTRFS_I(inode)->last_log_commit = BTRFS_I(inode)->root->last_log_commit;
diff --git a/fs/btrfs/qgroup.c b/fs/btrfs/qgroup.c
index a5c8562..dd002ae 100644
--- a/fs/btrfs/qgroup.c
+++ b/fs/btrfs/qgroup.c
@@ -290,7 +290,7 @@ int btrfs_read_qgroup_config(struct btrfs_fs_info *fs_info)
goto out;
}
if (btrfs_qgroup_status_generation(l, ptr) !=
- fs_info->generation) {
+ atomic64_read(&fs_info->generation)) {
flags |= BTRFS_QGROUP_STATUS_FLAG_INCONSISTENT;
printk(KERN_ERR
"btrfs: qgroup generation mismatch, "
diff --git a/fs/btrfs/transaction.c b/fs/btrfs/transaction.c
index f154946..105d642 100644
--- a/fs/btrfs/transaction.c
+++ b/fs/btrfs/transaction.c
@@ -161,8 +161,8 @@ loop:
list_add_tail(&cur_trans->list, &fs_info->trans_list);
extent_io_tree_init(&cur_trans->dirty_pages,
fs_info->btree_inode->i_mapping);
- fs_info->generation++;
- cur_trans->transid = fs_info->generation;
+ atomic64_inc(&fs_info->generation);
+ cur_trans->transid = atomic64_read(&fs_info->generation);
fs_info->running_transaction = cur_trans;
cur_trans->aborted = 0;
spin_unlock(&fs_info->trans_lock);
diff --git a/include/trace/events/btrfs.h b/include/trace/events/btrfs.h
index ea546a4..09dc72f 100644
--- a/include/trace/events/btrfs.h
+++ b/include/trace/events/btrfs.h
@@ -71,7 +71,8 @@ TRACE_EVENT(btrfs_transaction_commit,
),
TP_fast_assign(
- __entry->generation = root->fs_info->generation;
+ __entry->generation =
+ atomic64_read(&root->fs_info->generation);
__entry->root_objectid = root->root_key.objectid;
),
--
1.7.11.7
next reply other threads:[~2013-01-29 9:58 UTC|newest]
Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top
2013-01-29 9:59 Miao Xie [this message]
2013-01-29 10:00 ` [PATCH V2 02/10] Btrfs: use atomic for fs_info->last_trans_committed Miao Xie
2013-01-29 10:03 ` [PATCH V2 03/10] Btrfs: use atomic for fs_info->last_trans_log_full_commit Miao Xie
2013-01-29 10:05 ` [PATCH V2 04/10] Btrfs: add a comment for fs_info->max_inline Miao Xie
2013-01-29 10:07 ` [PATCH V2 05/10] Btrfs: protect fs_info->alloc_start Miao Xie
2013-01-29 10:09 ` [PATCH V2 06/10] Btrfs: use percpu counter for dirty metadata count Miao Xie
2013-01-29 10:10 ` [PATCH V2 07/10] Btrfs: use percpu counter for fs_info->delalloc_bytes Miao Xie
2013-01-29 10:11 ` [PATCH V2 08/10] Btrfs: use the inode own lock to protect its delalloc_bytes Miao Xie
2013-01-29 10:13 ` [PATCH V2 09/10] Btrfs: use seqlock to protect fs_info->avail_{data, metadata, system}_alloc_bits Miao Xie
2013-01-29 10:14 ` [PATCH V2 10/10] Btrfs: use bit operation for ->fs_state Miao Xie
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=51079D72.7000301@cn.fujitsu.com \
--to=miaox@cn.fujitsu.com \
--cc=linux-btrfs@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).