* [bug report] btrfs: refactor __btrfs_run_delayed_refs loop
@ 2026-05-21 12:53 Dan Carpenter
2026-05-21 14:44 ` Filipe Manana
0 siblings, 1 reply; 2+ messages in thread
From: Dan Carpenter @ 2026-05-21 12:53 UTC (permalink / raw)
To: Nikolay Borisov; +Cc: linux-btrfs
[ 8 year old code and no one has complained... It's surprising no
one has hit this in real life. - dan ]
Hello Nikolay Borisov,
Commit 0110a4c43451 ("btrfs: refactor __btrfs_run_delayed_refs loop")
from Aug 15, 2018 (linux-next), leads to the following Smatch static
checker warning:
fs/btrfs/extent-tree.c:2131 __btrfs_run_delayed_refs()
error: 'locked_ref' dereferencing possible ERR_PTR()
fs/btrfs/extent-tree.c
2084 static noinline int __btrfs_run_delayed_refs(struct btrfs_trans_handle *trans,
2085 u64 min_bytes)
2086 {
2087 struct btrfs_fs_info *fs_info = trans->fs_info;
2088 struct btrfs_delayed_ref_root *delayed_refs;
2089 struct btrfs_delayed_ref_head *locked_ref = NULL;
2090 int ret;
2091 unsigned long count = 0;
2092 unsigned long max_count = 0;
2093 u64 bytes_processed = 0;
2094
2095 delayed_refs = &trans->transaction->delayed_refs;
2096 if (min_bytes == 0) {
2097 /*
2098 * We may be subject to a harmless race if some task is
2099 * concurrently adding or removing a delayed ref, so silence
2100 * KCSAN and similar tools.
2101 */
2102 max_count = data_race(delayed_refs->num_heads_ready);
2103 min_bytes = U64_MAX;
2104 }
2105
2106 do {
2107 if (!locked_ref) {
2108 locked_ref = btrfs_select_ref_head(fs_info, delayed_refs);
2109 if (IS_ERR_OR_NULL(locked_ref)) {
2110 if (PTR_ERR(locked_ref) == -EAGAIN) {
2111 continue;
In the old we set "locked_ref = NULL" on this path and I think we
probably should restore that line. Now it looks like a crash.
2112 } else {
2113 break;
2114 }
2115 }
2116 count++;
2117 }
2118 /*
2119 * We need to try and merge add/drops of the same ref since we
2120 * can run into issues with relocate dropping the implicit ref
2121 * and then it being added back again before the drop can
2122 * finish. If we merged anything we need to re-loop so we can
2123 * get a good ref.
2124 * Or we can get node references of the same type that weren't
2125 * merged when created due to bumps in the tree mod seq, and
2126 * we need to merge them to prevent adding an inline extent
2127 * backref before dropping it (triggering a BUG_ON at
2128 * insert_inline_extent_backref()).
2129 */
2130 spin_lock(&locked_ref->lock);
^^^^^^^^^^^^^^^^
On the next iteration &locked_ref will be ERR_PTR(-EAGAIN).
--> 2131 btrfs_merge_delayed_refs(fs_info, delayed_refs, locked_ref);
2132
2133 ret = btrfs_run_delayed_refs_for_head(trans, locked_ref, &bytes_processed);
2134 if (ret < 0 && ret != -EAGAIN) {
2135 /*
2136 * Error, btrfs_run_delayed_refs_for_head already
2137 * unlocked everything so just bail out
2138 */
2139 return ret;
2140 } else if (!ret) {
2141 /*
2142 * Success, perform the usual cleanup of a processed
2143 * head
2144 */
2145 ret = cleanup_ref_head(trans, locked_ref, &bytes_processed);
2146 if (ret > 0 ) {
2147 /* We dropped our lock, we need to loop. */
2148 ret = 0;
2149 continue;
2150 } else if (ret) {
2151 return ret;
2152 }
2153 }
2154
2155 /*
2156 * Either success case or btrfs_run_delayed_refs_for_head
2157 * returned -EAGAIN, meaning we need to select another head
2158 */
2159
2160 locked_ref = NULL;
2161 cond_resched();
2162 } while ((min_bytes != U64_MAX && bytes_processed < min_bytes) ||
2163 (max_count > 0 && count < max_count) ||
2164 locked_ref);
^^^^^^^^^^
We continue down to here and loop back since locked_ref is -EAGAIN.
2165
2166 return 0;
2167 }
This email is a free service from the Smatch-CI project [smatch.sf.net].
regards,
dan carpenter
^ permalink raw reply [flat|nested] 2+ messages in thread
* Re: [bug report] btrfs: refactor __btrfs_run_delayed_refs loop
2026-05-21 12:53 [bug report] btrfs: refactor __btrfs_run_delayed_refs loop Dan Carpenter
@ 2026-05-21 14:44 ` Filipe Manana
0 siblings, 0 replies; 2+ messages in thread
From: Filipe Manana @ 2026-05-21 14:44 UTC (permalink / raw)
To: Dan Carpenter; +Cc: Nikolay Borisov, linux-btrfs
On Thu, May 21, 2026 at 2:02 PM Dan Carpenter <error27@gmail.com> wrote:
>
> [ 8 year old code and no one has complained... It's surprising no
> one has hit this in real life. - dan ]
>
> Hello Nikolay Borisov,
>
> Commit 0110a4c43451 ("btrfs: refactor __btrfs_run_delayed_refs loop")
> from Aug 15, 2018 (linux-next), leads to the following Smatch static
> checker warning:
>
> fs/btrfs/extent-tree.c:2131 __btrfs_run_delayed_refs()
> error: 'locked_ref' dereferencing possible ERR_PTR()
>
> fs/btrfs/extent-tree.c
> 2084 static noinline int __btrfs_run_delayed_refs(struct btrfs_trans_handle *trans,
> 2085 u64 min_bytes)
> 2086 {
> 2087 struct btrfs_fs_info *fs_info = trans->fs_info;
> 2088 struct btrfs_delayed_ref_root *delayed_refs;
> 2089 struct btrfs_delayed_ref_head *locked_ref = NULL;
> 2090 int ret;
> 2091 unsigned long count = 0;
> 2092 unsigned long max_count = 0;
> 2093 u64 bytes_processed = 0;
> 2094
> 2095 delayed_refs = &trans->transaction->delayed_refs;
> 2096 if (min_bytes == 0) {
> 2097 /*
> 2098 * We may be subject to a harmless race if some task is
> 2099 * concurrently adding or removing a delayed ref, so silence
> 2100 * KCSAN and similar tools.
> 2101 */
> 2102 max_count = data_race(delayed_refs->num_heads_ready);
> 2103 min_bytes = U64_MAX;
> 2104 }
> 2105
> 2106 do {
> 2107 if (!locked_ref) {
> 2108 locked_ref = btrfs_select_ref_head(fs_info, delayed_refs);
> 2109 if (IS_ERR_OR_NULL(locked_ref)) {
> 2110 if (PTR_ERR(locked_ref) == -EAGAIN) {
> 2111 continue;
>
> In the old we set "locked_ref = NULL" on this path and I think we
> probably should restore that line. Now it looks like a crash.
>
> 2112 } else {
> 2113 break;
> 2114 }
> 2115 }
> 2116 count++;
> 2117 }
> 2118 /*
> 2119 * We need to try and merge add/drops of the same ref since we
> 2120 * can run into issues with relocate dropping the implicit ref
> 2121 * and then it being added back again before the drop can
> 2122 * finish. If we merged anything we need to re-loop so we can
> 2123 * get a good ref.
> 2124 * Or we can get node references of the same type that weren't
> 2125 * merged when created due to bumps in the tree mod seq, and
> 2126 * we need to merge them to prevent adding an inline extent
> 2127 * backref before dropping it (triggering a BUG_ON at
> 2128 * insert_inline_extent_backref()).
> 2129 */
> 2130 spin_lock(&locked_ref->lock);
> ^^^^^^^^^^^^^^^^
> On the next iteration &locked_ref will be ERR_PTR(-EAGAIN).
Indeed.
Nikolay is no longer active in btrfs, so I've sent a fix for this:
https://lore.kernel.org/linux-btrfs/f33f040f3ced4b3813dba4ddb0b5377ab5aeac29.1779374308.git.fdmanana@suse.com/
Thanks.
>
> --> 2131 btrfs_merge_delayed_refs(fs_info, delayed_refs, locked_ref);
> 2132
> 2133 ret = btrfs_run_delayed_refs_for_head(trans, locked_ref, &bytes_processed);
> 2134 if (ret < 0 && ret != -EAGAIN) {
> 2135 /*
> 2136 * Error, btrfs_run_delayed_refs_for_head already
> 2137 * unlocked everything so just bail out
> 2138 */
> 2139 return ret;
> 2140 } else if (!ret) {
> 2141 /*
> 2142 * Success, perform the usual cleanup of a processed
> 2143 * head
> 2144 */
> 2145 ret = cleanup_ref_head(trans, locked_ref, &bytes_processed);
> 2146 if (ret > 0 ) {
> 2147 /* We dropped our lock, we need to loop. */
> 2148 ret = 0;
> 2149 continue;
> 2150 } else if (ret) {
> 2151 return ret;
> 2152 }
> 2153 }
> 2154
> 2155 /*
> 2156 * Either success case or btrfs_run_delayed_refs_for_head
> 2157 * returned -EAGAIN, meaning we need to select another head
> 2158 */
> 2159
> 2160 locked_ref = NULL;
> 2161 cond_resched();
> 2162 } while ((min_bytes != U64_MAX && bytes_processed < min_bytes) ||
> 2163 (max_count > 0 && count < max_count) ||
> 2164 locked_ref);
> ^^^^^^^^^^
> We continue down to here and loop back since locked_ref is -EAGAIN.
>
> 2165
> 2166 return 0;
> 2167 }
>
> This email is a free service from the Smatch-CI project [smatch.sf.net].
>
> regards,
> dan carpenter
>
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2026-05-21 14:45 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-05-21 12:53 [bug report] btrfs: refactor __btrfs_run_delayed_refs loop Dan Carpenter
2026-05-21 14:44 ` Filipe Manana
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.