btrfs: some optimizations and cleanups for log syncing
Subsystems (2)
cover btrfs: some optimizations and cleanups for log syncing
From: Filipe Manana <[email protected]>
Some optimizations for log syncing, specially for non-sdd mounts, and some cleanups. Details in the changelogs of each patch.
Filipe Manana (7):
btrfs: stop sleeping for one jiffy in non-ssd mounts during log commit
btrfs: remove log batch counter use for fsync
btrfs: do an initial lockless committed log transaction check during log syncing
btrfs: check for log writers count first in wait_for_writer()
btrfs: move condition for log commit wait into wait_log_commit()
btrfs: check for exit condition after waking in wait_log_commit()
btrfs: use simple booleans for log_commit field in struct btrfs_root
fs/btrfs/ctree.h | 6 +-- fs/btrfs/disk-io.c | 3 -- fs/btrfs/file.c | 4 -- fs/btrfs/transaction.c | 8 +-- fs/btrfs/tree-log.c | 98 ++++++++++++++---------------------- include/trace/events/btrfs.h | 4 +- 6 files changed, 42 insertions(+), 81 deletions(-)
Signature · 1 line
2.47.2
PATCH 1/7 btrfs: stop sleeping for one jiffy in non-ssd mounts during log commit
From: Filipe Manana <[email protected]>
Joining/starting a log transaction tracks if we ever had more than one task concurrently logging by setting the flag BTRFS_ROOT_MULTI_LOG_TASKS in the respective root. Once set, this flag remains for the rest of the lifetime of the transaction, only cleared when we don't have a log root and need to create a new one (transaction commits drop log roots).
During log commit, if we are not on a ssd mount (or use the -o nossd mount option) and the BTRFS_ROOT_MULTI_LOG_TASKS flag is set, we sleep for one jiffy with the excuse to allow future log writers to join and log inodes and then commit a larger log transaction to reduce overall IO. However this is extremely inefficient because:
1) If at some point we had multiple tasks logging concurrently but now we have only one task at a time, we force it to wait for 1 jiffy;
2) One jiffy can vary between 1ms to 10ms, depending on the kernel config option CONFIG_HZ, which by default has a value of 250HZ and that corresponds to 4ms - that is a lot.
This massively reduces the latency of fsyncs for non-ssd mounts, even on consumer grade spinning disks.
Remove this mechanism to track if we have (or ever had) multiple tasks logging and wait for 1 jiffy.
The following fio test was used to benchmark:
$ cat fio-buffered-fsync.sh DEV=/dev/sdj MNT=/mnt/sdj MOUNT_OPTIONS="" MKFS_OPTIONS=""
if [ $# -ne 6 ]; then
echo "Use $0 NUM_JOBS FILE_SIZE IO_SIZE FSYNC_FREQ BLOCK_SIZE [write|randwrite]" exit 1
fi
NUM_JOBS=$1
FILE_SIZE=$2
IO_SIZE=$3
FSYNC_FREQ=$4
BLOCK_SIZE=$5
WRITE_MODE=$6
if [ "$WRITE_MODE" != "write" ] && [ "$WRITE_MODE" != "randwrite" ]; then
echo "Invalid WRITE_MODE, must be 'write' or 'randwrite'" exit 1
fi
cat <<EOF > /tmp/fio-job.ini
[writers]
rw=$WRITE_MODE
fsync=$FSYNC_FREQ
fallocate=none
group_reporting=1
direct=0
bs=$BLOCK_SIZE
ioengine=psync
filesize=$FILE_SIZE
io_size=$IO_SIZE
directory=$MNT
numjobs=$NUM_JOBS
EOF
echo
echo "Using config:"
echo
cat /tmp/fio-job.ini
echo
umount $MNT &> /dev/null
mkfs.btrfs -f $MKFS_OPTIONS $DEV
mount $MOUNT_OPTIONS $DEV $MNT
fio /tmp/fio-job.ini
umount $MNT
Running the script as: ./fio-buffered-fsync.sh 8 64M 64M 1 4K randwrite
Before patch:
WRITE: bw=2647KiB/s (2711kB/s), 2647KiB/s-2647KiB/s (2711kB/s-2711kB/s), io=512MiB (537MB), run=198055-198055msec
After patch:
WRITE: bw=14.9MiB/s (15.6MB/s), 14.9MiB/s-14.9MiB/s (15.6MB/s-15.6MB/s), io=512MiB (537MB), run=34471-34471msec
That's about 5.7 times faster.
--- fs/btrfs/ctree.h | 2 -- fs/btrfs/tree-log.c | 18 +----------------- 2 files changed, 1 insertion(+), 19 deletions(-)
fs/btrfs/ctree.h +0 -2
@@ -131,7 +131,6 @@ enum {BTRFS_ROOT_ORPHAN_ITEM_INSERTED ,BTRFS_ROOT_DEFRAG_RUNNING ,BTRFS_ROOT_FORCE_COW , -BTRFS_ROOT_MULTI_LOG_TASKS ,BTRFS_ROOT_DIRTY , BTRFS_ROOT_DELETING,
@@ -216,7 +215,6 @@ struct btrfs_root {* to access this field. */ int last_log_commit; -pid_t log_start_pid;u64 last_trans;
fs/btrfs/tree-log.c +1 -17
@@ -316,13 +316,6 @@ static int start_log_trans(struct btrfs_trans_handle *trans,wait_log_commit (root, root->log_transid -1 ); goto again; } - -if (!root->log_start_pid) { -clear_bit (BTRFS_ROOT_MULTI_LOG_TASKS , &root->state); - root->log_start_pid = current->pid; - }else if (root->log_start_pid != current->pid) { -set_bit (BTRFS_ROOT_MULTI_LOG_TASKS , &root->state); - } }else {/* * This means fs_info->log_root_tree was already created
@@ -340,8 +333,6 @@ static int start_log_trans(struct btrfs_trans_handle *trans, goto out;set_bit (BTRFS_ROOT_HAS_LOG_TREE , &root->state); -clear_bit (BTRFS_ROOT_MULTI_LOG_TASKS , &root->state); - root->log_start_pid = current->pid; }atomic_inc (&root->log_writers);
@@ -3347,13 +3338,7 @@ int btrfs_sync_log(struct btrfs_trans_handle *trans,while (1 ) {int batch =atomic_read (&root->log_batch); -/* when we're on an ssd, just kick the log commit out */ -if (!btrfs_test_opt (fs_info,SSD ) && -test_bit (BTRFS_ROOT_MULTI_LOG_TASKS , &root->state)) { -mutex_unlock (&root->log_mutex); -schedule_timeout_uninterruptible (1 ); -mutex_lock (&root->log_mutex); - } +wait_for_writer (root);if (batch ==atomic_read (&root->log_batch))break ;
@@ -3414,7 +3399,6 @@ int btrfs_sync_log(struct btrfs_trans_handle *trans,btrfs_set_root_log_transid (root, root->log_transid +1 ); log->log_transid = root->log_transid; - root->log_start_pid =0 ;/* * IO has been started, blocks of the log tree have WRITTEN flag set * in their headers. new modifications of the log will be written to
Signature · 1 line
2.47.2
PATCH 2/7 btrfs: remove log batch counter use for fsync
In reply to [email protected]From: Filipe Manana <[email protected]>
We have the log batch counter defined per root which is now useless after the previous patch (titled: "btrfs: stop sleeping for one jiffy in non-ssd mounts during log commit"). The counter is incremented early in the fsync path, before and after flushing dellaloc and waiting for writeback, and then the counter is read during the log sync path. The goal was to wait for tasks that are about to join a log transaction, so that we could reduce the amount of IO and log syncing (flush all log tree extent buffers and write super blocks), but that mechanism does not work since if there are currently no log writers, btrfs_sync_log() does not unlock the root's log_mutex, so no new log writers can join the log transaction. Having concurrent fsync tasks increasing the log_batch counter only makes us loop unncessarily in btrfs_sync_log() - that is always true since the previous patch mentioned above and was true before that patch only when not using the "-o ssd" mount option (which is activated by default if the filesystem does not have rotational devices).
So remove the log batch counter. No performance changes were observed after removing it.
--- fs/btrfs/ctree.h | 2 -- fs/btrfs/disk-io.c | 1 - fs/btrfs/file.c | 4 ---- fs/btrfs/tree-log.c | 8 +------- 4 files changed, 1 insertion(+), 14 deletions(-)
fs/btrfs/ctree.h +0 -2
@@ -196,8 +196,6 @@ struct btrfs_root {/* Used only for log trees of subvolumes, not for the log root tree */ atomic_t log_writers;atomic_t log_commit[2 ]; -/* Used only for log trees of subvolumes, not for the log root tree */ -atomic_t log_batch;/* * Protected by the 'log_mutex' lock but can be read without holding * that lock to avoid unnecessary lock contention, in which case it
fs/btrfs/disk-io.c +0 -1
@@ -669,7 +669,6 @@ static struct btrfs_root *btrfs_alloc_root(struct btrfs_fs_info *fs_info,atomic_set (&root->log_commit[0 ],0 );atomic_set (&root->log_commit[1 ],0 );atomic_set (&root->log_writers,0 ); -atomic_set (&root->log_batch,0 );refcount_set (&root->refs,1 );atomic_set (&root->snapshot_force_cow,0 );atomic_set (&root->nr_swapfiles,0 );
fs/btrfs/file.c +0 -4
@@ -1573,8 +1573,6 @@ int btrfs_sync_file(struct file *file, loff_t start, loff_t end, int datasync)else btrfs_inode_lock (inode,BTRFS_ILOCK_MMAP ); -atomic_inc (&root->log_batch); -/* * Before we acquired the inode's lock and the mmap lock, someone may * have dirtied more pages in the target range. We need to make sure
@@ -1657,8 +1655,6 @@ int btrfs_sync_file(struct file *file, loff_t start, loff_t end, int datasync)if (ret) goto out_release_extents; -atomic_inc (&root->log_batch); -if (skip_inode_logging (&ctx)) {/* * We've had everything committed since the last time we were
fs/btrfs/tree-log.c +1 -7
@@ -3336,13 +3336,7 @@ int btrfs_sync_log(struct btrfs_trans_handle *trans,if (atomic_read (&root->log_commit[(index1 +1 ) %2 ]))wait_log_commit (root, log_transid -1 ); -while (1 ) { -int batch =atomic_read (&root->log_batch); - -wait_for_writer (root); -if (batch ==atomic_read (&root->log_batch)) -break ; - } +wait_for_writer (root);/* bail out if we need to do a full commit */ if (btrfs_need_log_full_commit (trans)) {
Signature · 1 line
2.47.2
PATCH 3/7 btrfs: do an initial lockless committed log transaction check during log syncing
In reply to [email protected]From: Filipe Manana <[email protected]>
When calling btrfs_sync_log() one of the things we do first is to check if the log transaction the given context belongs to was already committed and return immediately if it was committed before. However we take the root's log_mutex before doing the check, which adds lock contention in case the log transaction was already committed, as this is a heavily used mutex in case we have multiple tasks doing fsyncs or renaming and adding hard links to files that were fsynced before in the current transaction.
So add an initial lockless check in btrfs_sync_log() to reduce lock contention.
The following fio test was used to benchmark:
$ cat fio-buffered-fsync.sh DEV=/dev/sdj MNT=/mnt/sdj MOUNT_OPTIONS="" MKFS_OPTIONS=""
if [ $# -ne 6 ]; then
echo "Use $0 NUM_JOBS FILE_SIZE IO_SIZE FSYNC_FREQ BLOCK_SIZE [write|randwrite]" exit 1
fi
NUM_JOBS=$1
FILE_SIZE=$2
IO_SIZE=$3
FSYNC_FREQ=$4
BLOCK_SIZE=$5
WRITE_MODE=$6
if [ "$WRITE_MODE" != "write" ] && [ "$WRITE_MODE" != "randwrite" ]; then
echo "Invalid WRITE_MODE, must be 'write' or 'randwrite'" exit 1
fi
cat <<EOF > /tmp/fio-job.ini
[writers]
rw=$WRITE_MODE
fsync=$FSYNC_FREQ
fallocate=none
group_reporting=1
direct=0
bs=$BLOCK_SIZE
ioengine=psync
filesize=$FILE_SIZE
io_size=$IO_SIZE
directory=$MNT
numjobs=$NUM_JOBS
EOF
echo
echo "Using config:"
echo
cat /tmp/fio-job.ini
echo
umount $MNT &> /dev/null
mkfs.btrfs -f $MKFS_OPTIONS $DEV
mount $MOUNT_OPTIONS $DEV $MNT
fio /tmp/fio-job.ini
umount $MNT
Running the script as: ./fio-buffered-fsync.sh 12 64M 64M 1 4K randwrite
Before patch:
WRITE: bw=16.6MiB/s (17.4MB/s), 16.6MiB/s-16.6MiB/s (17.4MB/s-17.4MB/s), io=768MiB (805MB), run=46235-46235msec
After patch:
WRITE: bw=18.2MiB/s (19.0MB/s), 18.2MiB/s-18.2MiB/s (19.0MB/s-19.0MB/s), io=768MiB (805MB), run=42277-42277msec
--- fs/btrfs/tree-log.c | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-)
fs/btrfs/tree-log.c +8 -2
@@ -3306,15 +3306,21 @@ int btrfs_sync_log(struct btrfs_trans_handle *trans,struct btrfs_root *log = root->log_root;struct btrfs_root *log_root_tree = fs_info->log_root_tree;struct btrfs_root_item new_root_item; -int log_transid =0 ; +int log_transid = ctx->log_transid;struct btrfs_log_ctx root_log_ctx;struct blk_plug plug;u64 log_root_start;u64 log_root_level; +/* Avoid fist taking root->log_mutex, reduce lock contention. */ +if (data_race (root->log_transid_committed) >= log_transid) { +trace_btrfs_sync_log_enter (trans, root, ctx); +trace_btrfs_sync_log_exit (trans, root, ctx, ctx->log_ret); +return ctx->log_ret; + } +mutex_lock (&root->log_mutex);trace_btrfs_sync_log_enter (trans, root, ctx); - log_transid = ctx->log_transid;if (root->log_transid_committed >= log_transid) {trace_btrfs_sync_log_exit (trans, root, ctx, ctx->log_ret);mutex_unlock (&root->log_mutex);
Signature · 1 line
2.47.2
PATCH 4/7 btrfs: check for log writers count first in wait_for_writer()
In reply to [email protected]From: Filipe Manana <[email protected]>
In wait_for_writer() we first add the wait queue entry to the wait queue and then check for the number of log writers. In case where right in the first iteration of the loop we have no log writers, we end up wasting time adding the wait queue entry to the wait queue (function call, locking the queue with irq disable and enable on unlock, etc). So check first for the number of log writers before adding to the wait queue.
--- fs/btrfs/tree-log.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)
fs/btrfs/tree-log.c +3 -3
@@ -3202,11 +3202,11 @@ static void wait_for_writer(struct btrfs_root *root)DEFINE_WAIT (wait);for (;;) { -prepare_to_wait (&root->log_writer_wait, &wait, -TASK_UNINTERRUPTIBLE ); -if (!atomic_read (&root->log_writers)) +if (atomic_read (&root->log_writers) ==0 )break ; +prepare_to_wait (&root->log_writer_wait, &wait, +TASK_UNINTERRUPTIBLE );mutex_unlock (&root->log_mutex);schedule ();mutex_lock (&root->log_mutex);
Signature · 1 line
2.47.2
PATCH 5/7 btrfs: move condition for log commit wait into wait_log_commit()
In reply to [email protected]From: Filipe Manana <[email protected]>
Instead of having every caller check for root->log_commit[] being non-zero and then call wait_log_commit(), move the check into wait_log_commit() and have the callers call it unconditionally.
--- fs/btrfs/tree-log.c | 38 +++++++++++++++----------------------- 1 file changed, 15 insertions(+), 23 deletions(-)
fs/btrfs/tree-log.c +15 -23
@@ -221,7 +221,7 @@ static int btrfs_log_inode(struct btrfs_trans_handle *trans,static int link_to_fixup_dir (struct walk_control *wc,u64 objectid); static noinline int replay_dir_deletes(struct walk_control *wc,u64 dirid,bool del_all); -static void wait_log_commit (struct btrfs_root *root,int transid); +static bool wait_log_commit (struct btrfs_root *root,int transid);/* * tree logging is a special write ahead log used to make sure that
@@ -305,17 +305,13 @@ static int start_log_trans(struct btrfs_trans_handle *trans, again:if (root->log_root) { -int index = (root->log_transid +1 ) %2 ; -if (btrfs_need_log_full_commit (trans)) { ret =BTRFS_LOG_FORCE_COMMIT ; goto out; } -if (zoned &&atomic_read (&root->log_commit[index])) { -wait_log_commit (root, root->log_transid -1 ); +if (zoned &&wait_log_commit (root, root->log_transid -1 )) goto again; - } }else {/* * This means fs_info->log_root_tree was already created
@@ -363,13 +359,9 @@ static int join_running_log_trans(struct btrfs_root *root)mutex_lock (&root->log_mutex); again:if (root->log_root) { -int index = (root->log_transid +1 ) %2 ; - ret =0 ; -if (zoned &&atomic_read (&root->log_commit[index])) { -wait_log_commit (root, root->log_transid -1 ); +if (zoned &&wait_log_commit (root, root->log_transid -1 )) goto again; - }atomic_inc (&root->log_writers); }mutex_unlock (&root->log_mutex);
@@ -3172,11 +3164,15 @@ static int update_log_root(struct btrfs_trans_handle *trans,return ret; } -static void wait_log_commit (struct btrfs_root *root,int transid) +/* Returns true if we had to wait, false otherwise. */ +static bool wait_log_commit (struct btrfs_root *root,int transid) {DEFINE_WAIT (wait);int index = transid %2 ; +if (atomic_read (&root->log_commit[index]) ==0 ) +return false; +/* * we only allow two pending log transactions at a time, * so we know that if ours is more than 2 older than the
@@ -3195,6 +3191,8 @@ static void wait_log_commit(struct btrfs_root *root, int transid)mutex_lock (&root->log_mutex); }finish_wait (&root->log_commit_wait[index], &wait); + +return true; }static void wait_for_writer (struct btrfs_root *root)
@@ -3298,8 +3296,6 @@ static inline void btrfs_remove_all_log_ctxs(struct btrfs_root *root,int btrfs_sync_log (struct btrfs_trans_handle *trans,struct btrfs_root *root,struct btrfs_log_ctx *ctx) { -int index1; -int index2;int mark;int ret;struct btrfs_fs_info *fs_info = root->fs_info;
@@ -3307,6 +3303,8 @@ int btrfs_sync_log(struct btrfs_trans_handle *trans,struct btrfs_root *log_root_tree = fs_info->log_root_tree;struct btrfs_root_item new_root_item;int log_transid = ctx->log_transid; +int index1 = log_transid %2 ; +int index2;struct btrfs_log_ctx root_log_ctx;struct blk_plug plug;u64 log_root_start;
@@ -3327,9 +3325,7 @@ int btrfs_sync_log(struct btrfs_trans_handle *trans,return ctx->log_ret; } - index1 = log_transid %2 ; -if (atomic_read (&root->log_commit[index1])) { -wait_log_commit (root, log_transid); +if (wait_log_commit (root, log_transid)) {trace_btrfs_sync_log_exit (trans, root, ctx, ctx->log_ret);mutex_unlock (&root->log_mutex);return ctx->log_ret;
@@ -3339,8 +3335,7 @@ int btrfs_sync_log(struct btrfs_trans_handle *trans,atomic_set (&root->log_commit[index1],1 );/* wait for previous tree log sync to complete */ -if (atomic_read (&root->log_commit[(index1 +1 ) %2 ])) -wait_log_commit (root, log_transid -1 ); +wait_log_commit (root, log_transid -1 );wait_for_writer (root);
@@ -3473,10 +3468,7 @@ int btrfs_sync_log(struct btrfs_trans_handle *trans, root_log_ctx.log_transid, log_root_tree->log_transid);atomic_set (&log_root_tree->log_commit[index2],1 ); -if (atomic_read (&log_root_tree->log_commit[(index2 +1 ) %2 ])) { -wait_log_commit (log_root_tree, - root_log_ctx.log_transid -1 ); - } +wait_log_commit (log_root_tree, root_log_ctx.log_transid -1 );/* * now that we've moved on to the tree of log tree roots,
Signature · 1 line
2.47.2
PATCH 6/7 btrfs: check for exit condition after waking in wait_log_commit()
In reply to [email protected]From: Filipe Manana <[email protected]>
We check for the exit condition after we add ourselves to the wait queue and before we unlock the root's log_mutex, sleep and lock again log_mutex. This is not incorrect, but it's not optimal since in the first iteration this is pointless because we already know that root->log_commit[index] is not zero, so we should check the exit condition only after unlocking log_mutex, sleeping, waking up and locking again the log_mutex.
So move the check for the exit condition to bottom of the loop, after we were woken and locked log_mutex again.
--- fs/btrfs/tree-log.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-)
fs/btrfs/tree-log.c +4 -4
@@ -3182,13 +3182,13 @@ static bool wait_log_commit(struct btrfs_root *root, int transid)prepare_to_wait (&root->log_commit_wait[index], &wait,TASK_UNINTERRUPTIBLE ); -if (!(root->log_transid_committed < transid && -atomic_read (&root->log_commit[index]))) -break ; -mutex_unlock (&root->log_mutex);schedule ();mutex_lock (&root->log_mutex); + +if (!(root->log_transid_committed < transid && +atomic_read (&root->log_commit[index]) !=0 )) +break ; }finish_wait (&root->log_commit_wait[index], &wait);
Signature · 1 line
2.47.2
PATCH 7/7 btrfs: use simple booleans for log_commit field in struct btrfs_root
In reply to [email protected]From: Filipe Manana <[email protected]>
We are using atomic types for the log_commit array of struct btrfs_root but all we need is simple booleans. The log_commit array elements are always protected by the root's log_mutex, both for writes and reads, so we can use a simple boolean. The use of atomics if from the very early days of the log tree code where the access to the fields was not protected by any lock.
So switch to simple booleans, which results in cheaper code and slightly reduces the object size too.
--- fs/btrfs/ctree.h | 2 +- fs/btrfs/disk-io.c | 2 -- fs/btrfs/transaction.c | 8 ++------ fs/btrfs/tree-log.c | 16 ++++++++-------- include/trace/events/btrfs.h | 4 ++-- 5 files changed, 13 insertions(+), 19 deletions(-)
fs/btrfs/ctree.h +1 -1
@@ -195,7 +195,7 @@ struct btrfs_root {struct list_head log_ctxs[2 ];/* Used only for log trees of subvolumes, not for the log root tree */ atomic_t log_writers; -atomic_t log_commit[2 ]; +bool log_commit[2 ];/* * Protected by the 'log_mutex' lock but can be read without holding * that lock to avoid unnecessary lock contention, in which case it
fs/btrfs/disk-io.c +0 -2
@@ -666,8 +666,6 @@ static struct btrfs_root *btrfs_alloc_root(struct btrfs_fs_info *fs_info,init_waitqueue_head (&root->log_commit_wait[1 ]);INIT_LIST_HEAD (&root->log_ctxs[0 ]);INIT_LIST_HEAD (&root->log_ctxs[1 ]); -atomic_set (&root->log_commit[0 ],0 ); -atomic_set (&root->log_commit[1 ],0 );atomic_set (&root->log_writers,0 );refcount_set (&root->refs,1 );atomic_set (&root->snapshot_force_cow,0 );
fs/btrfs/transaction.c +2 -6
@@ -1517,12 +1517,8 @@ static noinline int commit_fs_roots(struct btrfs_trans_handle *trans)ASSERT (atomic_read (&root->log_writers) ==0 ,"atomic_read(&root->log_writers)=%d" ,atomic_read (&root->log_writers)); -ASSERT (atomic_read (&root->log_commit[0 ]) ==0 , -"atomic_read(&root->log_commit[0])=%d" , -atomic_read (&root->log_commit[0 ])); -ASSERT (atomic_read (&root->log_commit[1 ]) ==0 , -"atomic_read(&root->log_commit[1])=%d" , -atomic_read (&root->log_commit[1 ])); +ASSERT (!root->log_commit[0 ]); +ASSERT (!root->log_commit[1 ]); radix_tree_tag_clear(&fs_info->fs_roots_radix, (unsigned long)btrfs_root_id(root),
fs/btrfs/tree-log.c +8 -8
@@ -3170,7 +3170,7 @@ static bool wait_log_commit(struct btrfs_root *root, int transid)DEFINE_WAIT (wait);int index = transid %2 ; -if (atomic_read (&root->log_commit[index]) ==0 ) +if (!root->log_commit[index])return false;/*
@@ -3187,7 +3187,7 @@ static bool wait_log_commit(struct btrfs_root *root, int transid)mutex_lock (&root->log_mutex);if (!(root->log_transid_committed < transid && -atomic_read (&root->log_commit[index]) !=0 )) + root->log_commit[index]))break ; }finish_wait (&root->log_commit_wait[index], &wait);
@@ -3332,7 +3332,7 @@ int btrfs_sync_log(struct btrfs_trans_handle *trans, }ASSERT (log_transid == root->log_transid,"log_transid=%d root->log_transid=%d" , log_transid, root->log_transid); -atomic_set (&root->log_commit[index1],1 ); + root->log_commit[index1] = true;/* wait for previous tree log sync to complete */ wait_log_commit (root, log_transid -1 );
@@ -3452,7 +3452,7 @@ int btrfs_sync_log(struct btrfs_trans_handle *trans, goto out; } -if (atomic_read (&log_root_tree->log_commit[index2])) { +if (log_root_tree->log_commit[index2]) {blk_finish_plug (&plug); ret =btrfs_wait_tree_log_extents (log, mark); wait_log_commit(log_root_tree,
@@ -3466,7 +3466,7 @@ int btrfs_sync_log(struct btrfs_trans_handle *trans,ASSERT (root_log_ctx.log_transid == log_root_tree->log_transid,"root_log_ctx.log_transid=%d log_root_tree->log_transid=%d" , root_log_ctx.log_transid, log_root_tree->log_transid); -atomic_set (&log_root_tree->log_commit[index2],1 ); + log_root_tree->log_commit[index2] = true;wait_log_commit (log_root_tree, root_log_ctx.log_transid -1 );
@@ -3566,7 +3566,7 @@ int btrfs_sync_log(struct btrfs_trans_handle *trans,/* * We know there can only be one task here, since we have not yet set -* root->log_commit[index1] to 0 and any task attempting to sync the +* root->log_commit[index1] to false and any task attempting to sync the * log must wait for the previous log transaction to commit if it's * still in progress or wait for the current log transaction commit if * someone else already started it. We use <= and not < because the
@@ -3582,7 +3582,7 @@ int btrfs_sync_log(struct btrfs_trans_handle *trans,btrfs_remove_all_log_ctxs (log_root_tree, index2, ret); log_root_tree->log_transid_committed++; -atomic_set (&log_root_tree->log_commit[index2],0 ); + log_root_tree->log_commit[index2] = false;mutex_unlock (&log_root_tree->log_mutex);/*
@@ -3595,7 +3595,7 @@ int btrfs_sync_log(struct btrfs_trans_handle *trans,mutex_lock (&root->log_mutex);btrfs_remove_all_log_ctxs (root, index1, ret); root->log_transid_committed++; -atomic_set (&root->log_commit[index1],0 ); + root->log_commit[index1] = false;mutex_unlock (&root->log_mutex);/*
include/trace/events/btrfs.h +2 -2
@@ -1578,9 +1578,9 @@ TRACE_EVENT(btrfs_sync_log_enter, __entry->log_transid_committed =data_race (root->log_transid_committed); __entry->log_committing = -atomic_read (&root->log_commit[ctx->log_transid %2 ]); + root->log_commit[ctx->log_transid %2 ]; __entry->log_committing_prev = -atomic_read (&root->log_commit[(ctx->log_transid +1 ) %2 ]); + root->log_commit[(ctx->log_transid +1 ) %2 ]; __entry->log_writers =atomic_read (&root->log_writers); ),
Signature · 1 line
2.47.2
Quoted 4 lines · On Wed, Jul 22, 2026 at 01:41:36PM +0100, [email protected] wrote:
From: Filipe Manana <[email protected]>
Some optimizations for log syncing, specially for non-sdd mounts, and some cleanups. Details in the changelogs of each patch.
Quoted 21 lines
Filipe Manana (7):
btrfs: stop sleeping for one jiffy in non-ssd mounts during log commit
btrfs: remove log batch counter use for fsync
btrfs: do an initial lockless committed log transaction check during log syncing
btrfs: check for log writers count first inwait_for_writer()
btrfs: move condition for log commit wait intowait_log_commit()
btrfs: check for exit condition after waking inwait_log_commit()
btrfs: use simple booleans forlog_commitfield in structbtrfs_rootfs/btrfs/ctree.h | 6 +-- fs/btrfs/disk-io.c | 3 -- fs/btrfs/file.c | 4 -- fs/btrfs/transaction.c | 8 +-- fs/btrfs/tree-log.c | 98 ++++++++++++++---------------------- include/trace/events/btrfs.h | 4 +- 6 files changed, 42 insertions(+), 81 deletions(-)Signature · 1 line
2.47.2
v2 cover btrfs: some optimizations and cleanups for log syncing
In reply to [email protected]From: Filipe Manana <[email protected]>
Some optimizations for log syncing, specially for non-sdd mounts, and some cleanups. Details in the changelogs of each patch.
V2: Removed two patches which sashiko found problematic, the benefits of
fixing them up versus the complexity required to fix them is not worth it. Updated last patch to add the data_race() annotation in the trace events.
Filipe Manana (5):
btrfs: stop sleeping for one jiffy in non-ssd mounts during log commit
btrfs: remove log batch counter use for fsync
btrfs: move condition for log commit wait into wait_log_commit()
btrfs: check for exit condition after waking in wait_log_commit()
btrfs: use simple booleans for log_commit field in struct btrfs_root
fs/btrfs/ctree.h | 6 +-- fs/btrfs/disk-io.c | 3 -- fs/btrfs/file.c | 4 -- fs/btrfs/transaction.c | 8 +--- fs/btrfs/tree-log.c | 85 ++++++++++++------------------------ include/trace/events/btrfs.h | 4 +- 6 files changed, 32 insertions(+), 78 deletions(-)
Signature · 1 line
2.47.2
PATCH v2 1/5 btrfs: stop sleeping for one jiffy in non-ssd mounts during log commit
From: Filipe Manana <[email protected]>
Joining/starting a log transaction tracks if we ever had more than one task concurrently logging by setting the flag BTRFS_ROOT_MULTI_LOG_TASKS in the respective root. Once set, this flag remains for the rest of the lifetime of the transaction, only cleared when we don't have a log root and need to create a new one (transaction commits drop log roots).
During log commit, if we are not on a ssd mount (or use the -o nossd mount option) and the BTRFS_ROOT_MULTI_LOG_TASKS flag is set, we sleep for one jiffy with the excuse to allow future log writers to join and log inodes and then commit a larger log transaction to reduce overall IO. However this is extremely inefficient because:
1) If at some point we had multiple tasks logging concurrently but now we have only one task at a time, we force it to wait for 1 jiffy;
2) One jiffy can vary between 1ms to 10ms, depending on the kernel config option CONFIG_HZ, which by default has a value of 250HZ and that corresponds to 4ms - that is a lot.
This massively reduces the latency of fsyncs for non-ssd mounts, even on consumer grade spinning disks.
Remove this mechanism to track if we have (or ever had) multiple tasks logging and wait for 1 jiffy.
The following fio test was used to benchmark:
$ cat fio-buffered-fsync.sh DEV=/dev/sdj MNT=/mnt/sdj MOUNT_OPTIONS="" MKFS_OPTIONS=""
if [ $# -ne 6 ]; then
echo "Use $0 NUM_JOBS FILE_SIZE IO_SIZE FSYNC_FREQ BLOCK_SIZE [write|randwrite]" exit 1
fi
NUM_JOBS=$1
FILE_SIZE=$2
IO_SIZE=$3
FSYNC_FREQ=$4
BLOCK_SIZE=$5
WRITE_MODE=$6
if [ "$WRITE_MODE" != "write" ] && [ "$WRITE_MODE" != "randwrite" ]; then
echo "Invalid WRITE_MODE, must be 'write' or 'randwrite'" exit 1
fi
cat <<EOF > /tmp/fio-job.ini
[writers]
rw=$WRITE_MODE
fsync=$FSYNC_FREQ
fallocate=none
group_reporting=1
direct=0
bs=$BLOCK_SIZE
ioengine=psync
filesize=$FILE_SIZE
io_size=$IO_SIZE
directory=$MNT
numjobs=$NUM_JOBS
EOF
echo
echo "Using config:"
echo
cat /tmp/fio-job.ini
echo
umount $MNT &> /dev/null
mkfs.btrfs -f $MKFS_OPTIONS $DEV
mount $MOUNT_OPTIONS $DEV $MNT
fio /tmp/fio-job.ini
umount $MNT
Running the script as: ./fio-buffered-fsync.sh 8 64M 64M 1 4K randwrite
Before patch:
WRITE: bw=2647KiB/s (2711kB/s), 2647KiB/s-2647KiB/s (2711kB/s-2711kB/s), io=512MiB (537MB), run=198055-198055msec
After patch:
WRITE: bw=14.9MiB/s (15.6MB/s), 14.9MiB/s-14.9MiB/s (15.6MB/s-15.6MB/s), io=512MiB (537MB), run=34471-34471msec
That's about 5.7 times faster.
--- fs/btrfs/ctree.h | 2 -- fs/btrfs/tree-log.c | 18 +----------------- 2 files changed, 1 insertion(+), 19 deletions(-)
fs/btrfs/ctree.h +0 -2
@@ -131,7 +131,6 @@ enum {BTRFS_ROOT_ORPHAN_ITEM_INSERTED ,BTRFS_ROOT_DEFRAG_RUNNING ,BTRFS_ROOT_FORCE_COW , -BTRFS_ROOT_MULTI_LOG_TASKS ,BTRFS_ROOT_DIRTY , BTRFS_ROOT_DELETING,
@@ -216,7 +215,6 @@ struct btrfs_root {* to access this field. */ int last_log_commit; -pid_t log_start_pid;u64 last_trans;
fs/btrfs/tree-log.c +1 -17
@@ -316,13 +316,6 @@ static int start_log_trans(struct btrfs_trans_handle *trans,wait_log_commit (root, root->log_transid -1 ); goto again; } - -if (!root->log_start_pid) { -clear_bit (BTRFS_ROOT_MULTI_LOG_TASKS , &root->state); - root->log_start_pid = current->pid; - }else if (root->log_start_pid != current->pid) { -set_bit (BTRFS_ROOT_MULTI_LOG_TASKS , &root->state); - } }else {/* * This means fs_info->log_root_tree was already created
@@ -340,8 +333,6 @@ static int start_log_trans(struct btrfs_trans_handle *trans, goto out;set_bit (BTRFS_ROOT_HAS_LOG_TREE , &root->state); -clear_bit (BTRFS_ROOT_MULTI_LOG_TASKS , &root->state); - root->log_start_pid = current->pid; }atomic_inc (&root->log_writers);
@@ -3347,13 +3338,7 @@ int btrfs_sync_log(struct btrfs_trans_handle *trans,while (1 ) {int batch =atomic_read (&root->log_batch); -/* when we're on an ssd, just kick the log commit out */ -if (!btrfs_test_opt (fs_info,SSD ) && -test_bit (BTRFS_ROOT_MULTI_LOG_TASKS , &root->state)) { -mutex_unlock (&root->log_mutex); -schedule_timeout_uninterruptible (1 ); -mutex_lock (&root->log_mutex); - } +wait_for_writer (root);if (batch ==atomic_read (&root->log_batch))break ;
@@ -3414,7 +3399,6 @@ int btrfs_sync_log(struct btrfs_trans_handle *trans,btrfs_set_root_log_transid (root, root->log_transid +1 ); log->log_transid = root->log_transid; - root->log_start_pid =0 ;/* * IO has been started, blocks of the log tree have WRITTEN flag set * in their headers. new modifications of the log will be written to
Signature · 1 line
2.47.2
PATCH v2 2/5 btrfs: remove log batch counter use for fsync
In reply to [email protected]From: Filipe Manana <[email protected]>
We have the log batch counter defined per root which is now useless after the previous patch (titled: "btrfs: stop sleeping for one jiffy in non-ssd mounts during log commit"). The counter is incremented early in the fsync path, before and after flushing dellaloc and waiting for writeback, and then the counter is read during the log sync path. The goal was to wait for tasks that are about to join a log transaction, so that we could reduce the amount of IO and log syncing (flush all log tree extent buffers and write super blocks), but that mechanism does not work since if there are currently no log writers, btrfs_sync_log() does not unlock the root's log_mutex, so no new log writers can join the log transaction. Having concurrent fsync tasks increasing the log_batch counter only makes us loop unncessarily in btrfs_sync_log() - that is always true since the previous patch mentioned above and was true before that patch only when not using the "-o ssd" mount option (which is activated by default if the filesystem does not have rotational devices).
So remove the log batch counter. No performance changes were observed after removing it.
--- fs/btrfs/ctree.h | 2 -- fs/btrfs/disk-io.c | 1 - fs/btrfs/file.c | 4 ---- fs/btrfs/tree-log.c | 8 +------- 4 files changed, 1 insertion(+), 14 deletions(-)
fs/btrfs/ctree.h +0 -2
@@ -196,8 +196,6 @@ struct btrfs_root {/* Used only for log trees of subvolumes, not for the log root tree */ atomic_t log_writers;atomic_t log_commit[2 ]; -/* Used only for log trees of subvolumes, not for the log root tree */ -atomic_t log_batch;/* * Protected by the 'log_mutex' lock but can be read without holding * that lock to avoid unnecessary lock contention, in which case it
fs/btrfs/disk-io.c +0 -1
@@ -669,7 +669,6 @@ static struct btrfs_root *btrfs_alloc_root(struct btrfs_fs_info *fs_info,atomic_set (&root->log_commit[0 ],0 );atomic_set (&root->log_commit[1 ],0 );atomic_set (&root->log_writers,0 ); -atomic_set (&root->log_batch,0 );refcount_set (&root->refs,1 );atomic_set (&root->snapshot_force_cow,0 );atomic_set (&root->nr_swapfiles,0 );
fs/btrfs/file.c +0 -4
@@ -1573,8 +1573,6 @@ int btrfs_sync_file(struct file *file, loff_t start, loff_t end, int datasync)else btrfs_inode_lock (inode,BTRFS_ILOCK_MMAP ); -atomic_inc (&root->log_batch); -/* * Before we acquired the inode's lock and the mmap lock, someone may * have dirtied more pages in the target range. We need to make sure
@@ -1657,8 +1655,6 @@ int btrfs_sync_file(struct file *file, loff_t start, loff_t end, int datasync)if (ret) goto out_release_extents; -atomic_inc (&root->log_batch); -if (skip_inode_logging (&ctx)) {/* * We've had everything committed since the last time we were
fs/btrfs/tree-log.c +1 -7
@@ -3336,13 +3336,7 @@ int btrfs_sync_log(struct btrfs_trans_handle *trans,if (atomic_read (&root->log_commit[(index1 +1 ) %2 ]))wait_log_commit (root, log_transid -1 ); -while (1 ) { -int batch =atomic_read (&root->log_batch); - -wait_for_writer (root); -if (batch ==atomic_read (&root->log_batch)) -break ; - } +wait_for_writer (root);/* bail out if we need to do a full commit */ if (btrfs_need_log_full_commit (trans)) {
Signature · 1 line
2.47.2
PATCH v2 3/5 btrfs: move condition for log commit wait into wait_log_commit()
In reply to [email protected]From: Filipe Manana <[email protected]>
Instead of having every caller check for root->log_commit[] being non-zero and then call wait_log_commit(), move the check into wait_log_commit() and have the callers call it unconditionally.
--- fs/btrfs/tree-log.c | 41 ++++++++++++++++------------------------- 1 file changed, 16 insertions(+), 25 deletions(-)
fs/btrfs/tree-log.c +16 -25
@@ -221,7 +221,7 @@ static int btrfs_log_inode(struct btrfs_trans_handle *trans,static int link_to_fixup_dir (struct walk_control *wc,u64 objectid); static noinline int replay_dir_deletes(struct walk_control *wc,u64 dirid,bool del_all); -static void wait_log_commit (struct btrfs_root *root,int transid); +static bool wait_log_commit (struct btrfs_root *root,int transid);/* * tree logging is a special write ahead log used to make sure that
@@ -305,17 +305,13 @@ static int start_log_trans(struct btrfs_trans_handle *trans, again:if (root->log_root) { -int index = (root->log_transid +1 ) %2 ; -if (btrfs_need_log_full_commit (trans)) { ret =BTRFS_LOG_FORCE_COMMIT ; goto out; } -if (zoned &&atomic_read (&root->log_commit[index])) { -wait_log_commit (root, root->log_transid -1 ); +if (zoned &&wait_log_commit (root, root->log_transid -1 )) goto again; - } }else {/* * This means fs_info->log_root_tree was already created
@@ -363,13 +359,9 @@ static int join_running_log_trans(struct btrfs_root *root)mutex_lock (&root->log_mutex); again:if (root->log_root) { -int index = (root->log_transid +1 ) %2 ; - ret =0 ; -if (zoned &&atomic_read (&root->log_commit[index])) { -wait_log_commit (root, root->log_transid -1 ); +if (zoned &&wait_log_commit (root, root->log_transid -1 )) goto again; - }atomic_inc (&root->log_writers); }mutex_unlock (&root->log_mutex);
@@ -3172,11 +3164,15 @@ static int update_log_root(struct btrfs_trans_handle *trans,return ret; } -static void wait_log_commit (struct btrfs_root *root,int transid) +/* Returns true if we had to wait, false otherwise. */ +static bool wait_log_commit (struct btrfs_root *root,int transid) {DEFINE_WAIT (wait);int index = transid %2 ; +if (atomic_read (&root->log_commit[index]) ==0 ) +return false; +/* * we only allow two pending log transactions at a time, * so we know that if ours is more than 2 older than the
@@ -3195,6 +3191,8 @@ static void wait_log_commit(struct btrfs_root *root, int transid)mutex_lock (&root->log_mutex); }finish_wait (&root->log_commit_wait[index], &wait); + +return true; }static void wait_for_writer (struct btrfs_root *root)
@@ -3298,15 +3296,15 @@ static inline void btrfs_remove_all_log_ctxs(struct btrfs_root *root,int btrfs_sync_log (struct btrfs_trans_handle *trans,struct btrfs_root *root,struct btrfs_log_ctx *ctx) { -int index1; -int index2;int mark;int ret;struct btrfs_fs_info *fs_info = root->fs_info;struct btrfs_root *log = root->log_root;struct btrfs_root *log_root_tree = fs_info->log_root_tree;struct btrfs_root_item new_root_item; -int log_transid =0 ; +int log_transid = ctx->log_transid; +int index1 = log_transid %2 ; +int index2;struct btrfs_log_ctx root_log_ctx;struct blk_plug plug;u64 log_root_start;
@@ -3314,16 +3312,13 @@ int btrfs_sync_log(struct btrfs_trans_handle *trans,mutex_lock (&root->log_mutex);trace_btrfs_sync_log_enter (trans, root, ctx); - log_transid = ctx->log_transid;if (root->log_transid_committed >= log_transid) {trace_btrfs_sync_log_exit (trans, root, ctx, ctx->log_ret);mutex_unlock (&root->log_mutex);return ctx->log_ret; } - index1 = log_transid %2 ; -if (atomic_read (&root->log_commit[index1])) { -wait_log_commit (root, log_transid); +if (wait_log_commit (root, log_transid)) {trace_btrfs_sync_log_exit (trans, root, ctx, ctx->log_ret);mutex_unlock (&root->log_mutex);return ctx->log_ret;
@@ -3333,8 +3328,7 @@ int btrfs_sync_log(struct btrfs_trans_handle *trans,atomic_set (&root->log_commit[index1],1 );/* wait for previous tree log sync to complete */ -if (atomic_read (&root->log_commit[(index1 +1 ) %2 ])) -wait_log_commit (root, log_transid -1 ); +wait_log_commit (root, log_transid -1 );wait_for_writer (root);
@@ -3467,10 +3461,7 @@ int btrfs_sync_log(struct btrfs_trans_handle *trans, root_log_ctx.log_transid, log_root_tree->log_transid);atomic_set (&log_root_tree->log_commit[index2],1 ); -if (atomic_read (&log_root_tree->log_commit[(index2 +1 ) %2 ])) { -wait_log_commit (log_root_tree, - root_log_ctx.log_transid -1 ); - } +wait_log_commit (log_root_tree, root_log_ctx.log_transid -1 );/* * now that we've moved on to the tree of log tree roots,
Signature · 1 line
2.47.2
kernel test robot
build botRe: [PATCH v2 3/5] btrfs: move condition for log commit wait into wait_log_commit()
kernel test robot noticed "UBSAN:array-index-out-of-bounds_in_btrfs/tree-log.c" on:
Hello,
kernel test robot noticed "UBSAN:array-index-out-of-bounds_in_btrfs/tree-log.c" on:
commit: a99687b4edeead900d714fa0839a302a5a56d7aa ("[PATCH v2 3/5] btrfs: move condition for log commit wait into wait_log_commit()")
url: https://github.com/intel-lab-lkp/linux/commits/fdmanana-kernel-org/btrfs-stop-sleeping-for-one-jiffy-in-non-ssd-mounts-during-log-commit/20260722-235417
patch link: https://lore.kernel.org/all/842ef70c9ca3ddf17f8660ad24f63f6272621afa.1784733396.git.fdmanana@suse.com/
patch subject: [PATCH v2 3/5] btrfs: move condition for log commit wait into wait_log_commit()
in testcase: ltp
version:
with following parameters:
disk: 1SSD
fs: btrfs
test: syscalls-01
config: x86_64-rhel-9.4-ltp
compiler: gcc-14
test machine: 8 threads Intel(R) Core(TM) i7-6700 CPU @ 3.40GHz (Skylake) with 16G memory
(please refer to attached dmesg/kmsg for entire log/backtrace)
If you fix the issue in a separate patch/commit (i.e. not just a new version of the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <[email protected]>
| Closes: https://lore.kernel.org/oe-lkp/[email protected]
kern :warn : [ 104.466941] [ T5568] ------------[ cut here ]------------ kern :err : [ 104.467469] [ T5568] UBSAN: array-index-out-of-bounds in btrfs/tree-log.c:3173:35 kern :err : [ 104.467989] [ T5568] index -1 is out of range for type 'atomic_t [2]' kern :warn : [ 104.468507] [ T5568] CPU: 4 UID: 0 PID: 5568 Comm: fdatasync03 Not tainted 7.2.0-rc4+ #1 PREEMPT(lazy) kern :warn : [ 104.468512] [ T5568] Hardware name: System manufacturer System Product Name/Z170I PRO GAMING, BIOS 3805 05/16/2018 kern :warn : [ 104.468514] [ T5568] Call Trace: kern :warn : [ 104.468516] [ T5568] <TASK> kern :warn : [ 104.468519] [ T5568] dump_stack_lvl (dump_stack.c:94 dump_stack.c:120) kern :warn : [ 104.468528] [ T5568] ubsan_epilogue (ubsan.c:233) kern :warn : [ 104.468533] [ T5568] __ubsan_handle_out_of_bounds.cold (ubsan.c:455 ubsan.c:442) kern :warn : [ 104.468539] [ T5568] ? __pfx_stack_trace_consume_entry (usercopy_64.c:?) kern :warn : [ 104.468544] [ T5568] wait_log_commit (btrfs/tree-log.c:3173) btrfs kern :warn : [ 104.468751] [ T5568] ? kernel_text_address (extable.c:125 extable.c:94) kern :warn : [ 104.468757] [ T5568] ? __pfx_wait_log_commit (btrfs/tree-log.c:4984) btrfs kern :warn : [ 104.468950] [ T5568] ? arch_stack_walk (x86/kernel/stacktrace.c:26) kern :warn : [ 104.468955] [ T5568] ? __pfx_autoremove_wake_function (linux/list.h:441 (discriminator 4)) kern :warn : [ 104.468961] [ T5568] ? mutex_lock (linux/instrumented.h:55 linux/atomic/atomic-instrumented.h:4457 locking/mutex.c:161 locking/mutex.c:318) kern :warn : [ 104.468966] [ T5568] ? __pfx_mutex_lock (locking/mutex.c:1164 (discriminator 1)) kern :warn : [ 104.468973] [ T5568] btrfs_sync_log (btrfs/tree-log.c:3331) btrfs kern :warn : [ 104.469161] [ T5568] ? __printk_deferred_exit (printk/printk_safe.c:55 (discriminator 1)) kern :warn : [ 104.469164] [ T5568] ? stack_depot_save_flags (stackdepot.c:728) kern :warn : [ 104.469168] [ T5568] ? kasan_save_stack (kasan/common.c:58) kern :warn : [ 104.469171] [ T5568] ? kasan_save_stack (kasan/common.c:57) kern :warn : [ 104.469174] [ T5568] ? __pfx_btrfs_sync_log (btrfs/tree-log.c:3254) btrfs kern :warn : [ 104.469285] [ T5568] ? do_fsync (sync.c:186 sync.c:200 sync.c:211) kern :warn : [ 104.469288] [ T5568] ? __x64_sys_fdatasync (sync.c:221 sync.c:219 sync.c:219) kern :warn : [ 104.469291] [ T5568] ? do_syscall_64 (x86/entry/syscall_64.c:63 x86/entry/syscall_64.c:94) kern :warn : [ 104.469294] [ T5568] ? entry_SYSCALL_64_after_hwframe (x86/entry/entry_64.S:121) kern :warn : [ 104.469298] [ T5568] ? __pfx_free_extent_buffer_clear_reading (btrfs/extent_io.c:3746) btrfs kern :warn : [ 104.469405] [ T5568] ? __pfx_copy_inode_items_to_log (linux/list.h:161) btrfs kern :warn : [ 104.469512] [ T5568] ? kasan_save_track (kasan/common.c:78) kern :warn : [ 104.469516] [ T5568] ? kmem_cache_free (linux/kasan.h:235 slub.c:2705 slub.c:6405 slub.c:6532) kern :warn : [ 104.469519] [ T5568] ? btrfs_release_path (btrfs/ctree.c:173 (discriminator 1)) btrfs kern :warn : [ 104.469618] [ T5568] ? free_frozen_page_commit (linux/instrumented.h:82 asm-generic/bitops/instrumented-non-atomic.h:141 page_alloc.c:2809 page_alloc.c:2881) kern :warn : [ 104.469622] [ T5568] ? __free_frozen_pages (linux/spinlock.h:390 page_alloc.c:2980) kern :warn : [ 104.469626] [ T5568] ? __page_cache_release (linux/instrumented.h:82 asm-generic/bitops/instrumented-non-atomic.h:141 linux/page-flags.h:552 swap.c:76) kern :warn : [ 104.469629] [ T5568] ? __virt_addr_valid (linux/mmzone.h:2187 (discriminator 1) linux/mmzone.h:2269 (discriminator 1) x86/mm/physaddr.c:54 (discriminator 1)) kern :warn : [ 104.469632] [ T5568] ? rcu_segcblist_enqueue (linux/instrumented.h:112 linux/atomic/atomic-instrumented.h:3260 rcu/rcu_segcblist.c:214 rcu/rcu_segcblist.c:231 rcu/rcu_segcblist.c:332) kern :warn : [ 104.469635] [ T5568] ? __call_rcu_common+0x341/0x970 kern :warn : [ 104.469639] [ T5568] ? __pfx___call_rcu_common+0x10/0x10 kern :warn : [ 104.469642] [ T5568] ? _raw_spin_lock_irq (linux/instrumented.h:55 linux/atomic/atomic-instrumented.h:1301 asm-generic/qspinlock.h:111 linux/spinlock.h:187 linux/spinlock_api_smp.h:143 locking/spinlock.c:174) kern :warn : [ 104.469645] [ T5568] ? detach_extent_buffer_folio (linux/mm.h:2124 linux/pagemap.h:636 btrfs/extent_io.c:2976) btrfs kern :warn : [ 104.469753] [ T5568] ? release_extent_buffer (btrfs/extent_io.c:3758 btrfs/extent_io.c:3721) btrfs kern :warn : [ 104.469860] [ T5568] ? free_extent_buffer_clear_reading (btrfs/extent_io.c:3821) btrfs kern :warn : [ 104.469966] [ T5568] ? __pfx_free_extent_buffer_clear_reading (btrfs/extent_io.c:3746) btrfs kern :warn : [ 104.470073] [ T5568] btrfs_sync_file (btrfs/file.c:1726) btrfs kern :warn : [ 104.470181] [ T5568] ? __pfx_btrfs_sync_file (btrfs/file.c:1489) btrfs kern :warn : [ 104.470296] [ T5568] ? fdget (linux/instrumented.h:82 linux/atomic/atomic-instrumented.h:49 file.c:1194 file.c:1208) kern :warn : [ 104.470300] [ T5568] do_fsync (sync.c:186 sync.c:200 sync.c:211) kern :warn : [ 104.470303] [ T5568] __x64_sys_fdatasync (sync.c:221 sync.c:219 sync.c:219) kern :warn : [ 104.470306] [ T5568] do_syscall_64 (x86/entry/syscall_64.c:63 x86/entry/syscall_64.c:94) kern :warn : [ 104.470310] [ T5568] ? do_syscall_64 (linux/irq-entry-common.h:210 linux/irq-entry-common.h:230 linux/entry-common.h:318 x86/entry/syscall_64.c:100) kern :warn : [ 104.470313] [ T5568] ? __pfx_vfs_write (linux/fs.h:2733 (discriminator 1)) kern :warn : [ 104.470317] [ T5568] ? __x64_sys_munmap (mmap.c:1079 mmap.c:1076 mmap.c:1076) kern :warn : [ 104.470320] [ T5568] ? do_syscall_64 (linux/irq-entry-common.h:210 linux/irq-entry-common.h:230 linux/entry-common.h:318 x86/entry/syscall_64.c:100) kern :warn : [ 104.470324] [ T5568] ? fdget_pos (linux/instrumented.h:82 linux/atomic/atomic-instrumented.h:3188 linux/file_ref.h:215 file.c:1231 file.c:1257) kern :warn : [ 104.470327] [ T5568] ? ksys_write (read_write.c:739) kern :warn : [ 104.470330] [ T5568] ? __pfx_ksys_write (read_write.c:723) kern :warn : [ 104.470333] [ T5568] ? __pfx_ksys_write (read_write.c:723) kern :warn : [ 104.470337] [ T5568] ? do_syscall_64 (linux/irq-entry-common.h:210 linux/irq-entry-common.h:230 linux/entry-common.h:318 x86/entry/syscall_64.c:100) kern :warn : [ 104.470340] [ T5568] ? do_syscall_64 (linux/irq-entry-common.h:210 linux/irq-entry-common.h:230 linux/entry-common.h:318 x86/entry/syscall_64.c:100) kern :warn : [ 104.470344] [ T5568] ? do_syscall_64 (linux/randomize_kstack.h:57 x86/entry/syscall_64.c:92) kern :warn : [ 104.470347] [ T5568] entry_SYSCALL_64_after_hwframe (x86/entry/entry_64.S:121) kern :warn : [ 104.470350] [ T5568] RIP: 0033:0x7f28a60b5687 kern :warn : [ 104.470353] [ T5568] Code: 48 89 fa 4c 89 df e8 58 b3 00 00 8b 93 08 03 00 00 59 5e 48 83 f8 fc 74 1a 5b c3 0f 1f 84 00 00 00 00 00 48 8b 44 24 10 0f 05 <5b> c3 0f 1f 80 00 00 00 00 83 e2 39 83 fa 08 75 de e8 23 ff ff ff All code ======== 0: 48 89 fa mov %rdi,%rdx 3: 4c 89 df mov %r11,%rdi 6: e8 58 b3 00 00 call 0xb363 b: 8b 93 08 03 00 00 mov 0x308(%rbx),%edx 11: 59 pop %rcx 12: 5e pop %rsi 13: 48 83 f8 fc cmp $0xfffffffffffffffc,%rax 17: 74 1a je 0x33 19: 5b pop %rbx 1a: c3 ret 1b: 0f 1f 84 00 00 00 00 nopl 0x0(%rax,%rax,1) 22: 00 23: 48 8b 44 24 10 mov 0x10(%rsp),%rax 28: 0f 05 syscall 2a:* 5b pop %rbx <-- trapping instruction 2b: c3 ret 2c: 0f 1f 80 00 00 00 00 nopl 0x0(%rax) 33: 83 e2 39 and $0x39,%edx 36: 83 fa 08 cmp $0x8,%edx 39: 75 de jne 0x19 3b: e8 23 ff ff ff call 0xffffffffffffff63
Code starting with the faulting instruction
===========================================
0: 5b pop %rbx
1: c3 ret
2: 0f 1f 80 00 00 00 00 nopl 0x0(%rax) 9: 83 e2 39 and $0x39,%edx c: 83 fa 08 cmp $0x8,%edx f: 75 de jne 0xffffffffffffffef 11: e8 23 ff ff ff call 0xffffffffffffff39 kern :warn : [ 104.470356] [ T5568] RSP: 002b:00007ffcb7666460 EFLAGS: 00000202 ORIG_RAX: 000000000000004b kern :warn : [ 104.470360] [ T5568] RAX: ffffffffffffffda RBX: 00007f28a6023780 RCX: 00007f28a60b5687 kern :warn : [ 104.470362] [ T5568] RDX: 0000000000000000 RSI: 0000000000000000 RDI: 0000000000000003 kern :warn : [ 104.470363] [ T5568] RBP: 00007f28a6023720 R08: 0000000000000000 R09: 0000000000000000 kern :warn : [ 104.470365] [ T5568] R10: 0000000000000000 R11: 0000000000000202 R12: 000056382db4c047 kern :warn : [ 104.470367] [ T5568] R13: 0000000000000004 R14: 0000000000000000 R15: 000056382db4d260 kern :warn : [ 104.470370] [ T5568] </TASK> kern :warn : [ 104.504778] [ T5568] ---[ end trace ]---
The kernel config and materials to reproduce are available at:
https://download.01.org/0day-ci/archive/20260730/[email protected]
Signature · 2 lines
0-DAY CI Kernel Test Service https://github.com/intel/lkp-tests/wiki
Re: [PATCH v2 3/5] btrfs: move condition for log commit wait into wait_log_commit()
Quoted 6 lines · kernel test robot
Hello,
kernel test robot noticed "UBSAN:array-index-out-of-bounds_in_btrfs/tree-log.c" on:
Yes, this was actually fixed before committing to the for-next branch on github, but missed the mailing list.
The incremental diff is:
fs/btrfs/tree-log.c +0 -0
@@ -3168,7 +3168,7 @@ static int update_log_root(struct
btrfs_trans_handle *trans,
static bool wait_log_commit(struct btrfs_root *root, int transid)
{
DEFINE_WAIT(wait);- int index = transid % 2;
+ const int index = (transid >= 0 ? transid % 2 : -transid % 2);
if (!root->log_commit[index])
return false;Thanks.
Quoted 149 lines
commit:
a99687b4edeead900d714fa0839a302a5a56d7aa("[PATCH v2 3/5] btrfs: move condition for log commit wait intowait_log_commit()")
url: https://github.com/intel-lab-lkp/linux/commits/fdmanana-kernel-org/btrfs-stop-sleeping-for-one-jiffy-in-non-ssd-mounts-during-log-commit/20260722-235417
patch link: https://lore.kernel.org/all/842ef70c9ca3ddf17f8660ad24f63f6272621afa.1784733396.git.fdmanana@suse.com/
patch subject: [PATCH v2 3/5] btrfs: move condition for log commit wait intowait_log_commit()in testcase: ltp
version:
with following parameters:disk: 1SSD
fs: btrfs
test: syscalls-01config: x86_64-rhel-9.4-ltp
compiler: gcc-14
test machine: 8 threads Intel(R) Core(TM) i7-6700 CPU @ 3.40GHz (Skylake) with 16G memory(please refer to attached dmesg/kmsg for entire log/backtrace)
If you fix the issue in a separate patch/commit (i.e. not just a new version of the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <[email protected]>
| Closes: https://lore.kernel.org/oe-lkp/[email protected]kern :warn : [ 104.466941] [ T5568] ------------[ cut here ]------------ kern :err : [ 104.467469] [ T5568] UBSAN: array-index-out-of-bounds in btrfs/tree-log.c:3173:35 kern :err : [ 104.467989] [ T5568] index -1 is out of range for type 'atomic_t [2]' kern :warn : [ 104.468507] [ T5568] CPU: 4 UID: 0 PID: 5568 Comm: fdatasync03 Not tainted 7.2.0-rc4+ #1 PREEMPT(lazy) kern :warn : [ 104.468512] [ T5568] Hardware name: System manufacturer System Product Name/Z170I PRO GAMING, BIOS 3805 05/16/2018 kern :warn : [ 104.468514] [ T5568] Call Trace: kern :warn : [ 104.468516] [ T5568] <TASK> kern :warn : [ 104.468519] [ T5568] dump_stack_lvl (dump_stack.c:94 dump_stack.c:120) kern :warn : [ 104.468528] [ T5568] ubsan_epilogue (ubsan.c:233) kern :warn : [ 104.468533] [ T5568] __ubsan_handle_out_of_bounds.cold (ubsan.c:455 ubsan.c:442) kern :warn : [ 104.468539] [ T5568] ? __pfx_stack_trace_consume_entry (usercopy_64.c:?) kern :warn : [ 104.468544] [ T5568] wait_log_commit (btrfs/tree-log.c:3173) btrfs kern :warn : [ 104.468751] [ T5568] ? kernel_text_address (extable.c:125 extable.c:94) kern :warn : [ 104.468757] [ T5568] ? __pfx_wait_log_commit (btrfs/tree-log.c:4984) btrfs kern :warn : [ 104.468950] [ T5568] ? arch_stack_walk (x86/kernel/stacktrace.c:26) kern :warn : [ 104.468955] [ T5568] ? __pfx_autoremove_wake_function (linux/list.h:441 (discriminator 4)) kern :warn : [ 104.468961] [ T5568] ? mutex_lock (linux/instrumented.h:55 linux/atomic/atomic-instrumented.h:4457 locking/mutex.c:161 locking/mutex.c:318) kern :warn : [ 104.468966] [ T5568] ? __pfx_mutex_lock (locking/mutex.c:1164 (discriminator 1)) kern :warn : [ 104.468973] [ T5568] btrfs_sync_log (btrfs/tree-log.c:3331) btrfs kern :warn : [ 104.469161] [ T5568] ? __printk_deferred_exit (printk/printk_safe.c:55 (discriminator 1)) kern :warn : [ 104.469164] [ T5568] ? stack_depot_save_flags (stackdepot.c:728) kern :warn : [ 104.469168] [ T5568] ? kasan_save_stack (kasan/common.c:58) kern :warn : [ 104.469171] [ T5568] ? kasan_save_stack (kasan/common.c:57) kern :warn : [ 104.469174] [ T5568] ? __pfx_btrfs_sync_log (btrfs/tree-log.c:3254) btrfs kern :warn : [ 104.469285] [ T5568] ? do_fsync (sync.c:186 sync.c:200 sync.c:211) kern :warn : [ 104.469288] [ T5568] ? __x64_sys_fdatasync (sync.c:221 sync.c:219 sync.c:219) kern :warn : [ 104.469291] [ T5568] ? do_syscall_64 (x86/entry/syscall_64.c:63 x86/entry/syscall_64.c:94) kern :warn : [ 104.469294] [ T5568] ? entry_SYSCALL_64_after_hwframe (x86/entry/entry_64.S:121) kern :warn : [ 104.469298] [ T5568] ? __pfx_free_extent_buffer_clear_reading (btrfs/extent_io.c:3746) btrfs kern :warn : [ 104.469405] [ T5568] ? __pfx_copy_inode_items_to_log (linux/list.h:161) btrfs kern :warn : [ 104.469512] [ T5568] ? kasan_save_track (kasan/common.c:78) kern :warn : [ 104.469516] [ T5568] ? kmem_cache_free (linux/kasan.h:235 slub.c:2705 slub.c:6405 slub.c:6532) kern :warn : [ 104.469519] [ T5568] ? btrfs_release_path (btrfs/ctree.c:173 (discriminator 1)) btrfs kern :warn : [ 104.469618] [ T5568] ? free_frozen_page_commit (linux/instrumented.h:82 asm-generic/bitops/instrumented-non-atomic.h:141 page_alloc.c:2809 page_alloc.c:2881) kern :warn : [ 104.469622] [ T5568] ? __free_frozen_pages (linux/spinlock.h:390 page_alloc.c:2980) kern :warn : [ 104.469626] [ T5568] ? __page_cache_release (linux/instrumented.h:82 asm-generic/bitops/instrumented-non-atomic.h:141 linux/page-flags.h:552 swap.c:76) kern :warn : [ 104.469629] [ T5568] ? __virt_addr_valid (linux/mmzone.h:2187 (discriminator 1) linux/mmzone.h:2269 (discriminator 1) x86/mm/physaddr.c:54 (discriminator 1)) kern :warn : [ 104.469632] [ T5568] ? rcu_segcblist_enqueue (linux/instrumented.h:112 linux/atomic/atomic-instrumented.h:3260 rcu/rcu_segcblist.c:214 rcu/rcu_segcblist.c:231 rcu/rcu_segcblist.c:332) kern :warn : [ 104.469635] [ T5568] ? __call_rcu_common+0x341/0x970 kern :warn : [ 104.469639] [ T5568] ? __pfx___call_rcu_common+0x10/0x10 kern :warn : [ 104.469642] [ T5568] ? _raw_spin_lock_irq (linux/instrumented.h:55 linux/atomic/atomic-instrumented.h:1301 asm-generic/qspinlock.h:111 linux/spinlock.h:187 linux/spinlock_api_smp.h:143 locking/spinlock.c:174) kern :warn : [ 104.469645] [ T5568] ? detach_extent_buffer_folio (linux/mm.h:2124 linux/pagemap.h:636 btrfs/extent_io.c:2976) btrfs kern :warn : [ 104.469753] [ T5568] ? release_extent_buffer (btrfs/extent_io.c:3758 btrfs/extent_io.c:3721) btrfs kern :warn : [ 104.469860] [ T5568] ? free_extent_buffer_clear_reading (btrfs/extent_io.c:3821) btrfs kern :warn : [ 104.469966] [ T5568] ? __pfx_free_extent_buffer_clear_reading (btrfs/extent_io.c:3746) btrfs kern :warn : [ 104.470073] [ T5568] btrfs_sync_file (btrfs/file.c:1726) btrfs kern :warn : [ 104.470181] [ T5568] ? __pfx_btrfs_sync_file (btrfs/file.c:1489) btrfs kern :warn : [ 104.470296] [ T5568] ? fdget (linux/instrumented.h:82 linux/atomic/atomic-instrumented.h:49 file.c:1194 file.c:1208) kern :warn : [ 104.470300] [ T5568] do_fsync (sync.c:186 sync.c:200 sync.c:211) kern :warn : [ 104.470303] [ T5568] __x64_sys_fdatasync (sync.c:221 sync.c:219 sync.c:219) kern :warn : [ 104.470306] [ T5568] do_syscall_64 (x86/entry/syscall_64.c:63 x86/entry/syscall_64.c:94) kern :warn : [ 104.470310] [ T5568] ? do_syscall_64 (linux/irq-entry-common.h:210 linux/irq-entry-common.h:230 linux/entry-common.h:318 x86/entry/syscall_64.c:100) kern :warn : [ 104.470313] [ T5568] ? __pfx_vfs_write (linux/fs.h:2733 (discriminator 1)) kern :warn : [ 104.470317] [ T5568] ? __x64_sys_munmap (mmap.c:1079 mmap.c:1076 mmap.c:1076) kern :warn : [ 104.470320] [ T5568] ? do_syscall_64 (linux/irq-entry-common.h:210 linux/irq-entry-common.h:230 linux/entry-common.h:318 x86/entry/syscall_64.c:100) kern :warn : [ 104.470324] [ T5568] ? fdget_pos (linux/instrumented.h:82 linux/atomic/atomic-instrumented.h:3188 linux/file_ref.h:215 file.c:1231 file.c:1257) kern :warn : [ 104.470327] [ T5568] ? ksys_write (read_write.c:739) kern :warn : [ 104.470330] [ T5568] ? __pfx_ksys_write (read_write.c:723) kern :warn : [ 104.470333] [ T5568] ? __pfx_ksys_write (read_write.c:723) kern :warn : [ 104.470337] [ T5568] ? do_syscall_64 (linux/irq-entry-common.h:210 linux/irq-entry-common.h:230 linux/entry-common.h:318 x86/entry/syscall_64.c:100) kern :warn : [ 104.470340] [ T5568] ? do_syscall_64 (linux/irq-entry-common.h:210 linux/irq-entry-common.h:230 linux/entry-common.h:318 x86/entry/syscall_64.c:100) kern :warn : [ 104.470344] [ T5568] ? do_syscall_64 (linux/randomize_kstack.h:57 x86/entry/syscall_64.c:92) kern :warn : [ 104.470347] [ T5568] entry_SYSCALL_64_after_hwframe (x86/entry/entry_64.S:121) kern :warn : [ 104.470350] [ T5568] RIP: 0033:0x7f28a60b5687 kern :warn : [ 104.470353] [ T5568] Code: 48 89 fa 4c 89 df e8 58 b3 00 00 8b 93 08 03 00 00 59 5e 48 83 f8 fc 74 1a 5b c3 0f 1f 84 00 00 00 00 00 48 8b 44 24 10 0f 05 <5b> c3 0f 1f 80 00 00 00 00 83 e2 39 83 fa 08 75 de e8 23 ff ff ff All code ======== 0: 48 89 fa mov %rdi,%rdx 3: 4c 89 df mov %r11,%rdi 6: e8 58 b3 00 00 call 0xb363 b: 8b 93 08 03 00 00 mov 0x308(%rbx),%edx 11: 59 pop %rcx 12: 5e pop %rsi 13: 48 83 f8 fc cmp $0xfffffffffffffffc,%rax 17: 74 1a je 0x33 19: 5b pop %rbx 1a: c3 ret 1b: 0f 1f 84 00 00 00 00 nopl 0x0(%rax,%rax,1) 22: 00 23: 48 8b 44 24 10 mov 0x10(%rsp),%rax 28: 0f 05 syscall 2a:* 5b pop %rbx <-- trapping instruction 2b: c3 ret 2c: 0f 1f 80 00 00 00 00 nopl 0x0(%rax) 33: 83 e2 39 and $0x39,%edx 36: 83 fa 08 cmp $0x8,%edx 39: 75 de jne 0x19 3b: e8 23 ff ff ff call 0xffffffffffffff63Code starting with the faulting instruction
===========================================0: 5b pop %rbx 1: c3 ret 2: 0f 1f 80 00 00 00 00 nopl 0x0(%rax) 9: 83 e2 39 and $0x39,%edx c: 83 fa 08 cmp $0x8,%edx f: 75 de jne 0xffffffffffffffef 11: e8 23 ff ff ff call 0xffffffffffffff39 kern :warn : [ 104.470356] [ T5568] RSP: 002b:00007ffcb7666460 EFLAGS: 00000202 ORIG_RAX: 000000000000004b kern :warn : [ 104.470360] [ T5568] RAX: ffffffffffffffda RBX: 00007f28a6023780 RCX: 00007f28a60b5687 kern :warn : [ 104.470362] [ T5568] RDX: 0000000000000000 RSI: 0000000000000000 RDI: 0000000000000003 kern :warn : [ 104.470363] [ T5568] RBP: 00007f28a6023720 R08: 0000000000000000 R09: 0000000000000000 kern :warn : [ 104.470365] [ T5568] R10: 0000000000000000 R11: 0000000000000202 R12: 000056382db4c047 kern :warn : [ 104.470367] [ T5568] R13: 0000000000000004 R14: 0000000000000000 R15: 000056382db4d260 kern :warn : [ 104.470370] [ T5568] </TASK> kern :warn : [ 104.504778] [ T5568] ---[ end trace ]---The kernel config and materials to reproduce are available at:
https://download.01.org/0day-ci/archive/20260730/[email protected]--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
Re: [PATCH v2 3/5] btrfs: move condition for log commit wait into wait_log_commit()
hi, Filipe Manana,
Quoted 10 lines · Filipe Manana
Quoted 6 lines · kernel test robot
Hello,
kernel test robot noticed "UBSAN:array-index-out-of-bounds_in_btrfs/tree-log.c" on:
Yes, this was actually fixed before committing to the for-next branch on github, but missed the mailing list.
thanks a lot for information!
Quoted 168 lines
The incremental diff is:
fs/btrfs/tree-log.c +1
-1@@ -3168,7 +3168,7 @@ static int update_log_root(structbtrfs_trans_handle *trans, static bool wait_log_commit(struct btrfs_root *root, int transid) {DEFINE_WAIT (wait ); -int index = transid %2 ; +const int index = (transid >=0 ? transid %2 : -transid %2 );if (!root->log_commit[index])return false;Thanks.
Quoted 149 lines
commit:
a99687b4edeead900d714fa0839a302a5a56d7aa("[PATCH v2 3/5] btrfs: move condition for log commit wait intowait_log_commit()")
url: https://github.com/intel-lab-lkp/linux/commits/fdmanana-kernel-org/btrfs-stop-sleeping-for-one-jiffy-in-non-ssd-mounts-during-log-commit/20260722-235417
patch link: https://lore.kernel.org/all/842ef70c9ca3ddf17f8660ad24f63f6272621afa.1784733396.git.fdmanana@suse.com/
patch subject: [PATCH v2 3/5] btrfs: move condition for log commit wait intowait_log_commit()in testcase: ltp
version:
with following parameters:disk: 1SSD
fs: btrfs
test: syscalls-01config: x86_64-rhel-9.4-ltp
compiler: gcc-14
test machine: 8 threads Intel(R) Core(TM) i7-6700 CPU @ 3.40GHz (Skylake) with 16G memory(please refer to attached dmesg/kmsg for entire log/backtrace)
If you fix the issue in a separate patch/commit (i.e. not just a new version of the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <[email protected]>
| Closes: https://lore.kernel.org/oe-lkp/[email protected]kern :warn : [ 104.466941] [ T5568] ------------[ cut here ]------------ kern :err : [ 104.467469] [ T5568] UBSAN: array-index-out-of-bounds in btrfs/tree-log.c:3173:35 kern :err : [ 104.467989] [ T5568] index -1 is out of range for type 'atomic_t [2]' kern :warn : [ 104.468507] [ T5568] CPU: 4 UID: 0 PID: 5568 Comm: fdatasync03 Not tainted 7.2.0-rc4+ #1 PREEMPT(lazy) kern :warn : [ 104.468512] [ T5568] Hardware name: System manufacturer System Product Name/Z170I PRO GAMING, BIOS 3805 05/16/2018 kern :warn : [ 104.468514] [ T5568] Call Trace: kern :warn : [ 104.468516] [ T5568] <TASK> kern :warn : [ 104.468519] [ T5568] dump_stack_lvl (dump_stack.c:94 dump_stack.c:120) kern :warn : [ 104.468528] [ T5568] ubsan_epilogue (ubsan.c:233) kern :warn : [ 104.468533] [ T5568] __ubsan_handle_out_of_bounds.cold (ubsan.c:455 ubsan.c:442) kern :warn : [ 104.468539] [ T5568] ? __pfx_stack_trace_consume_entry (usercopy_64.c:?) kern :warn : [ 104.468544] [ T5568] wait_log_commit (btrfs/tree-log.c:3173) btrfs kern :warn : [ 104.468751] [ T5568] ? kernel_text_address (extable.c:125 extable.c:94) kern :warn : [ 104.468757] [ T5568] ? __pfx_wait_log_commit (btrfs/tree-log.c:4984) btrfs kern :warn : [ 104.468950] [ T5568] ? arch_stack_walk (x86/kernel/stacktrace.c:26) kern :warn : [ 104.468955] [ T5568] ? __pfx_autoremove_wake_function (linux/list.h:441 (discriminator 4)) kern :warn : [ 104.468961] [ T5568] ? mutex_lock (linux/instrumented.h:55 linux/atomic/atomic-instrumented.h:4457 locking/mutex.c:161 locking/mutex.c:318) kern :warn : [ 104.468966] [ T5568] ? __pfx_mutex_lock (locking/mutex.c:1164 (discriminator 1)) kern :warn : [ 104.468973] [ T5568] btrfs_sync_log (btrfs/tree-log.c:3331) btrfs kern :warn : [ 104.469161] [ T5568] ? __printk_deferred_exit (printk/printk_safe.c:55 (discriminator 1)) kern :warn : [ 104.469164] [ T5568] ? stack_depot_save_flags (stackdepot.c:728) kern :warn : [ 104.469168] [ T5568] ? kasan_save_stack (kasan/common.c:58) kern :warn : [ 104.469171] [ T5568] ? kasan_save_stack (kasan/common.c:57) kern :warn : [ 104.469174] [ T5568] ? __pfx_btrfs_sync_log (btrfs/tree-log.c:3254) btrfs kern :warn : [ 104.469285] [ T5568] ? do_fsync (sync.c:186 sync.c:200 sync.c:211) kern :warn : [ 104.469288] [ T5568] ? __x64_sys_fdatasync (sync.c:221 sync.c:219 sync.c:219) kern :warn : [ 104.469291] [ T5568] ? do_syscall_64 (x86/entry/syscall_64.c:63 x86/entry/syscall_64.c:94) kern :warn : [ 104.469294] [ T5568] ? entry_SYSCALL_64_after_hwframe (x86/entry/entry_64.S:121) kern :warn : [ 104.469298] [ T5568] ? __pfx_free_extent_buffer_clear_reading (btrfs/extent_io.c:3746) btrfs kern :warn : [ 104.469405] [ T5568] ? __pfx_copy_inode_items_to_log (linux/list.h:161) btrfs kern :warn : [ 104.469512] [ T5568] ? kasan_save_track (kasan/common.c:78) kern :warn : [ 104.469516] [ T5568] ? kmem_cache_free (linux/kasan.h:235 slub.c:2705 slub.c:6405 slub.c:6532) kern :warn : [ 104.469519] [ T5568] ? btrfs_release_path (btrfs/ctree.c:173 (discriminator 1)) btrfs kern :warn : [ 104.469618] [ T5568] ? free_frozen_page_commit (linux/instrumented.h:82 asm-generic/bitops/instrumented-non-atomic.h:141 page_alloc.c:2809 page_alloc.c:2881) kern :warn : [ 104.469622] [ T5568] ? __free_frozen_pages (linux/spinlock.h:390 page_alloc.c:2980) kern :warn : [ 104.469626] [ T5568] ? __page_cache_release (linux/instrumented.h:82 asm-generic/bitops/instrumented-non-atomic.h:141 linux/page-flags.h:552 swap.c:76) kern :warn : [ 104.469629] [ T5568] ? __virt_addr_valid (linux/mmzone.h:2187 (discriminator 1) linux/mmzone.h:2269 (discriminator 1) x86/mm/physaddr.c:54 (discriminator 1)) kern :warn : [ 104.469632] [ T5568] ? rcu_segcblist_enqueue (linux/instrumented.h:112 linux/atomic/atomic-instrumented.h:3260 rcu/rcu_segcblist.c:214 rcu/rcu_segcblist.c:231 rcu/rcu_segcblist.c:332) kern :warn : [ 104.469635] [ T5568] ? __call_rcu_common+0x341/0x970 kern :warn : [ 104.469639] [ T5568] ? __pfx___call_rcu_common+0x10/0x10 kern :warn : [ 104.469642] [ T5568] ? _raw_spin_lock_irq (linux/instrumented.h:55 linux/atomic/atomic-instrumented.h:1301 asm-generic/qspinlock.h:111 linux/spinlock.h:187 linux/spinlock_api_smp.h:143 locking/spinlock.c:174) kern :warn : [ 104.469645] [ T5568] ? detach_extent_buffer_folio (linux/mm.h:2124 linux/pagemap.h:636 btrfs/extent_io.c:2976) btrfs kern :warn : [ 104.469753] [ T5568] ? release_extent_buffer (btrfs/extent_io.c:3758 btrfs/extent_io.c:3721) btrfs kern :warn : [ 104.469860] [ T5568] ? free_extent_buffer_clear_reading (btrfs/extent_io.c:3821) btrfs kern :warn : [ 104.469966] [ T5568] ? __pfx_free_extent_buffer_clear_reading (btrfs/extent_io.c:3746) btrfs kern :warn : [ 104.470073] [ T5568] btrfs_sync_file (btrfs/file.c:1726) btrfs kern :warn : [ 104.470181] [ T5568] ? __pfx_btrfs_sync_file (btrfs/file.c:1489) btrfs kern :warn : [ 104.470296] [ T5568] ? fdget (linux/instrumented.h:82 linux/atomic/atomic-instrumented.h:49 file.c:1194 file.c:1208) kern :warn : [ 104.470300] [ T5568] do_fsync (sync.c:186 sync.c:200 sync.c:211) kern :warn : [ 104.470303] [ T5568] __x64_sys_fdatasync (sync.c:221 sync.c:219 sync.c:219) kern :warn : [ 104.470306] [ T5568] do_syscall_64 (x86/entry/syscall_64.c:63 x86/entry/syscall_64.c:94) kern :warn : [ 104.470310] [ T5568] ? do_syscall_64 (linux/irq-entry-common.h:210 linux/irq-entry-common.h:230 linux/entry-common.h:318 x86/entry/syscall_64.c:100) kern :warn : [ 104.470313] [ T5568] ? __pfx_vfs_write (linux/fs.h:2733 (discriminator 1)) kern :warn : [ 104.470317] [ T5568] ? __x64_sys_munmap (mmap.c:1079 mmap.c:1076 mmap.c:1076) kern :warn : [ 104.470320] [ T5568] ? do_syscall_64 (linux/irq-entry-common.h:210 linux/irq-entry-common.h:230 linux/entry-common.h:318 x86/entry/syscall_64.c:100) kern :warn : [ 104.470324] [ T5568] ? fdget_pos (linux/instrumented.h:82 linux/atomic/atomic-instrumented.h:3188 linux/file_ref.h:215 file.c:1231 file.c:1257) kern :warn : [ 104.470327] [ T5568] ? ksys_write (read_write.c:739) kern :warn : [ 104.470330] [ T5568] ? __pfx_ksys_write (read_write.c:723) kern :warn : [ 104.470333] [ T5568] ? __pfx_ksys_write (read_write.c:723) kern :warn : [ 104.470337] [ T5568] ? do_syscall_64 (linux/irq-entry-common.h:210 linux/irq-entry-common.h:230 linux/entry-common.h:318 x86/entry/syscall_64.c:100) kern :warn : [ 104.470340] [ T5568] ? do_syscall_64 (linux/irq-entry-common.h:210 linux/irq-entry-common.h:230 linux/entry-common.h:318 x86/entry/syscall_64.c:100) kern :warn : [ 104.470344] [ T5568] ? do_syscall_64 (linux/randomize_kstack.h:57 x86/entry/syscall_64.c:92) kern :warn : [ 104.470347] [ T5568] entry_SYSCALL_64_after_hwframe (x86/entry/entry_64.S:121) kern :warn : [ 104.470350] [ T5568] RIP: 0033:0x7f28a60b5687 kern :warn : [ 104.470353] [ T5568] Code: 48 89 fa 4c 89 df e8 58 b3 00 00 8b 93 08 03 00 00 59 5e 48 83 f8 fc 74 1a 5b c3 0f 1f 84 00 00 00 00 00 48 8b 44 24 10 0f 05 <5b> c3 0f 1f 80 00 00 00 00 83 e2 39 83 fa 08 75 de e8 23 ff ff ff All code ======== 0: 48 89 fa mov %rdi,%rdx 3: 4c 89 df mov %r11,%rdi 6: e8 58 b3 00 00 call 0xb363 b: 8b 93 08 03 00 00 mov 0x308(%rbx),%edx 11: 59 pop %rcx 12: 5e pop %rsi 13: 48 83 f8 fc cmp $0xfffffffffffffffc,%rax 17: 74 1a je 0x33 19: 5b pop %rbx 1a: c3 ret 1b: 0f 1f 84 00 00 00 00 nopl 0x0(%rax,%rax,1) 22: 00 23: 48 8b 44 24 10 mov 0x10(%rsp),%rax 28: 0f 05 syscall 2a:* 5b pop %rbx <-- trapping instruction 2b: c3 ret 2c: 0f 1f 80 00 00 00 00 nopl 0x0(%rax) 33: 83 e2 39 and $0x39,%edx 36: 83 fa 08 cmp $0x8,%edx 39: 75 de jne 0x19 3b: e8 23 ff ff ff call 0xffffffffffffff63Code starting with the faulting instruction
===========================================0: 5b pop %rbx 1: c3 ret 2: 0f 1f 80 00 00 00 00 nopl 0x0(%rax) 9: 83 e2 39 and $0x39,%edx c: 83 fa 08 cmp $0x8,%edx f: 75 de jne 0xffffffffffffffef 11: e8 23 ff ff ff call 0xffffffffffffff39 kern :warn : [ 104.470356] [ T5568] RSP: 002b:00007ffcb7666460 EFLAGS: 00000202 ORIG_RAX: 000000000000004b kern :warn : [ 104.470360] [ T5568] RAX: ffffffffffffffda RBX: 00007f28a6023780 RCX: 00007f28a60b5687 kern :warn : [ 104.470362] [ T5568] RDX: 0000000000000000 RSI: 0000000000000000 RDI: 0000000000000003 kern :warn : [ 104.470363] [ T5568] RBP: 00007f28a6023720 R08: 0000000000000000 R09: 0000000000000000 kern :warn : [ 104.470365] [ T5568] R10: 0000000000000000 R11: 0000000000000202 R12: 000056382db4c047 kern :warn : [ 104.470367] [ T5568] R13: 0000000000000004 R14: 0000000000000000 R15: 000056382db4d260 kern :warn : [ 104.470370] [ T5568] </TASK> kern :warn : [ 104.504778] [ T5568] ---[ end trace ]---The kernel config and materials to reproduce are available at:
https://download.01.org/0day-ci/archive/20260730/[email protected]--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
PATCH v2 4/5 btrfs: check for exit condition after waking in wait_log_commit()
In reply to [email protected]From: Filipe Manana <[email protected]>
We check for the exit condition after we add ourselves to the wait queue and before we unlock the root's log_mutex, sleep and lock again log_mutex. This is not incorrect, but it's not optimal since in the first iteration this is pointless because we already know that root->log_commit[index] is not zero, so we should check the exit condition only after unlocking log_mutex, sleeping, waking up and locking again the log_mutex.
So move the check for the exit condition to bottom of the loop, after we were woken and locked log_mutex again.
--- fs/btrfs/tree-log.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-)
fs/btrfs/tree-log.c +4 -4
@@ -3182,13 +3182,13 @@ static bool wait_log_commit(struct btrfs_root *root, int transid)prepare_to_wait (&root->log_commit_wait[index], &wait,TASK_UNINTERRUPTIBLE ); -if (!(root->log_transid_committed < transid && -atomic_read (&root->log_commit[index]))) -break ; -mutex_unlock (&root->log_mutex);schedule ();mutex_lock (&root->log_mutex); + +if (!(root->log_transid_committed < transid && +atomic_read (&root->log_commit[index]) !=0 )) +break ; }finish_wait (&root->log_commit_wait[index], &wait);
Signature · 1 line
2.47.2
PATCH v2 5/5 btrfs: use simple booleans for log_commit field in struct btrfs_root
In reply to [email protected]From: Filipe Manana <[email protected]>
We are using atomic types for the log_commit array of struct btrfs_root but all we need is simple booleans. The log_commit array elements are always protected by the root's log_mutex, both for writes and reads, so we can use a simple boolean. The use of atomics if from the very early days of the log tree code where the access to the fields was not protected by any lock.
So switch to simple booleans, which results in cheaper code and slightly reduces the object size too.
--- fs/btrfs/ctree.h | 2 +- fs/btrfs/disk-io.c | 2 -- fs/btrfs/transaction.c | 8 ++------ fs/btrfs/tree-log.c | 16 ++++++++-------- include/trace/events/btrfs.h | 4 ++-- 5 files changed, 13 insertions(+), 19 deletions(-)
fs/btrfs/ctree.h +1 -1
@@ -195,7 +195,7 @@ struct btrfs_root {struct list_head log_ctxs[2 ];/* Used only for log trees of subvolumes, not for the log root tree */ atomic_t log_writers; -atomic_t log_commit[2 ]; +bool log_commit[2 ];/* * Protected by the 'log_mutex' lock but can be read without holding * that lock to avoid unnecessary lock contention, in which case it
fs/btrfs/disk-io.c +0 -2
@@ -666,8 +666,6 @@ static struct btrfs_root *btrfs_alloc_root(struct btrfs_fs_info *fs_info,init_waitqueue_head (&root->log_commit_wait[1 ]);INIT_LIST_HEAD (&root->log_ctxs[0 ]);INIT_LIST_HEAD (&root->log_ctxs[1 ]); -atomic_set (&root->log_commit[0 ],0 ); -atomic_set (&root->log_commit[1 ],0 );atomic_set (&root->log_writers,0 );refcount_set (&root->refs,1 );atomic_set (&root->snapshot_force_cow,0 );
fs/btrfs/transaction.c +2 -6
@@ -1517,12 +1517,8 @@ static noinline int commit_fs_roots(struct btrfs_trans_handle *trans)ASSERT (atomic_read (&root->log_writers) ==0 ,"atomic_read(&root->log_writers)=%d" ,atomic_read (&root->log_writers)); -ASSERT (atomic_read (&root->log_commit[0 ]) ==0 , -"atomic_read(&root->log_commit[0])=%d" , -atomic_read (&root->log_commit[0 ])); -ASSERT (atomic_read (&root->log_commit[1 ]) ==0 , -"atomic_read(&root->log_commit[1])=%d" , -atomic_read (&root->log_commit[1 ])); +ASSERT (!root->log_commit[0 ]); +ASSERT (!root->log_commit[1 ]); radix_tree_tag_clear(&fs_info->fs_roots_radix, (unsigned long)btrfs_root_id(root),
fs/btrfs/tree-log.c +8 -8
@@ -3170,7 +3170,7 @@ static bool wait_log_commit(struct btrfs_root *root, int transid)DEFINE_WAIT (wait);int index = transid %2 ; -if (atomic_read (&root->log_commit[index]) ==0 ) +if (!root->log_commit[index])return false;/*
@@ -3187,7 +3187,7 @@ static bool wait_log_commit(struct btrfs_root *root, int transid)mutex_lock (&root->log_mutex);if (!(root->log_transid_committed < transid && -atomic_read (&root->log_commit[index]) !=0 )) + root->log_commit[index]))break ; }finish_wait (&root->log_commit_wait[index], &wait);
@@ -3325,7 +3325,7 @@ int btrfs_sync_log(struct btrfs_trans_handle *trans, }ASSERT (log_transid == root->log_transid,"log_transid=%d root->log_transid=%d" , log_transid, root->log_transid); -atomic_set (&root->log_commit[index1],1 ); + root->log_commit[index1] = true;/* wait for previous tree log sync to complete */ wait_log_commit (root, log_transid -1 );
@@ -3445,7 +3445,7 @@ int btrfs_sync_log(struct btrfs_trans_handle *trans, goto out; } -if (atomic_read (&log_root_tree->log_commit[index2])) { +if (log_root_tree->log_commit[index2]) {blk_finish_plug (&plug); ret =btrfs_wait_tree_log_extents (log, mark); wait_log_commit(log_root_tree,
@@ -3459,7 +3459,7 @@ int btrfs_sync_log(struct btrfs_trans_handle *trans,ASSERT (root_log_ctx.log_transid == log_root_tree->log_transid,"root_log_ctx.log_transid=%d log_root_tree->log_transid=%d" , root_log_ctx.log_transid, log_root_tree->log_transid); -atomic_set (&log_root_tree->log_commit[index2],1 ); + log_root_tree->log_commit[index2] = true;wait_log_commit (log_root_tree, root_log_ctx.log_transid -1 );
@@ -3559,7 +3559,7 @@ int btrfs_sync_log(struct btrfs_trans_handle *trans,/* * We know there can only be one task here, since we have not yet set -* root->log_commit[index1] to 0 and any task attempting to sync the +* root->log_commit[index1] to false and any task attempting to sync the * log must wait for the previous log transaction to commit if it's * still in progress or wait for the current log transaction commit if * someone else already started it. We use <= and not < because the
@@ -3575,7 +3575,7 @@ int btrfs_sync_log(struct btrfs_trans_handle *trans,btrfs_remove_all_log_ctxs (log_root_tree, index2, ret); log_root_tree->log_transid_committed++; -atomic_set (&log_root_tree->log_commit[index2],0 ); + log_root_tree->log_commit[index2] = false;mutex_unlock (&log_root_tree->log_mutex);/*
@@ -3588,7 +3588,7 @@ int btrfs_sync_log(struct btrfs_trans_handle *trans,mutex_lock (&root->log_mutex);btrfs_remove_all_log_ctxs (root, index1, ret); root->log_transid_committed++; -atomic_set (&root->log_commit[index1],0 ); + root->log_commit[index1] = false;mutex_unlock (&root->log_mutex);/*
include/trace/events/btrfs.h +2 -2
@@ -1578,9 +1578,9 @@ TRACE_EVENT(btrfs_sync_log_enter, __entry->log_transid_committed =data_race (root->log_transid_committed); __entry->log_committing = -atomic_read (&root->log_commit[ctx->log_transid %2 ]); +data_race (root->log_commit[ctx->log_transid %2 ]); __entry->log_committing_prev = -atomic_read (&root->log_commit[(ctx->log_transid +1 ) %2 ]); +data_race (root->log_commit[(ctx->log_transid +1 ) %2 ]); __entry->log_writers =atomic_read (&root->log_writers); ),
Signature · 1 line
2.47.2
Quoted 17 lines · On Wed, 2026-07-22 at 16:24 +0100, [email protected] wrote:
From: Filipe Manana <[email protected]>
Some optimizations for log syncing, specially for non-sdd mounts, and some cleanups. Details in the changelogs of each patch.
V2: Removed two patches which sashiko found problematic, the benefits of
fixing them up versus the complexity required to fix them is not worth it. Updated last patch to add thedata_race()annotation in the trace events.Filipe Manana (5):
btrfs: stop sleeping for one jiffy in non-ssd mounts during log commit
btrfs: remove log batch counter use for fsync
btrfs: move condition for log commit wait intowait_log_commit()
btrfs: check for exit condition after waking inwait_log_commit()
btrfs: use simple booleans forlog_commitfield in structbtrfs_root
Quoted 7 lines
fs/btrfs/ctree.h | 6 +-- fs/btrfs/disk-io.c | 3 -- fs/btrfs/file.c | 4 -- fs/btrfs/transaction.c | 8 +--- fs/btrfs/tree-log.c | 85 ++++++++++++------------------------ include/trace/events/btrfs.h | 4 +- 6 files changed, 32 insertions(+), 78 deletions(-)
These all look like good cleanups, and the diffstat is nice!