Hello Kent Overstreet,
The patch 502bed819cc3: "bcachefs: Kill bch2_fs_usage_scratch_get()"
from Apr 3, 2021 (linux-next), leads to the following Smatch static
checker warning:
fs/bcachefs/buckets.c:1352 bch2_trans_fs_usage_apply()
error: we previously assumed 'trans->disk_res' could be null (see line
1302)
fs/bcachefs/buckets.c
1296 int bch2_trans_fs_usage_apply(struct btree_trans *trans,
1297 struct replicas_delta_list *deltas)
1298 {
1299 struct bch_fs *c = trans->c;
1300 static int warned_disk_usage = 0;
1301 bool warn = false;
1302 unsigned disk_res_sectors = trans->disk_res ?
trans->disk_res->sectors : 0;
^^^^^^^^^^^^^^^
This code is quite subtle, but the static checker says that we check
for NULL here.
1303 struct replicas_delta *d = deltas->d, *d2;
1304 struct replicas_delta *top = (void *) deltas->d + deltas->used;
1305 struct bch_fs_usage *dst;
1306 s64 added = 0, should_not_have_added;
1307 unsigned i;
1308
1309 percpu_down_read(&c->mark_lock);
1310 preempt_disable();
1311 dst = fs_usage_ptr(c, trans->journal_res.seq, false);
1312
1313 for (d = deltas->d; d != top; d = replicas_delta_next(d)) {
1314 switch (d->r.data_type) {
1315 case BCH_DATA_btree:
1316 case BCH_DATA_user:
1317 case BCH_DATA_parity:
1318 added += d->delta;
And we set added to non-zero here so that doesn't seem related. Or the
relationship is not obvious.
1319 }
1320
1321 if (__update_replicas(c, dst, &d->r, d->delta))
1322 goto need_mark;
1323 }
1324
1325 dst->nr_inodes += deltas->nr_inodes;
1326
1327 for (i = 0; i < BCH_REPLICAS_MAX; i++) {
1328 added +=
deltas->persistent_reserved[i];
1329 dst->reserved +=
deltas->persistent_reserved[i];
1330 dst->persistent_reserved[i] +=
deltas->persistent_reserved[i];
1331 }
1332
1333 /*
1334 * Not allowed to reduce sectors_available except by getting a
1335 * reservation:
1336 */
1337 should_not_have_added = added - (s64) disk_res_sectors;
1338 if (unlikely(should_not_have_added > 0)) {
1339 u64 old, new, v = atomic64_read(&c->sectors_available);
1340
1341 do {
1342 old = v;
1343 new = max_t(s64, 0, old -
should_not_have_added);
1344 } while ((v = atomic64_cmpxchg(&c->sectors_available,
1345 old, new)) != old);
1346
1347 added -= should_not_have_added;
1348 warn = true;
1349 }
1350
1351 if (added > 0) {
--> 1352 trans->disk_res->sectors -= added;
^^^^^^^^^^^^^^^^^
Unchecked dereference.
1353 this_cpu_sub(*c->online_reserved, added);
1354 }
1355
1356 preempt_enable();
1357 percpu_up_read(&c->mark_lock);
1358
1359 if (unlikely(warn) && !xchg(&warned_disk_usage, 1))
1360 bch2_trans_inconsistent(trans,
1361 "disk usage increased %lli
more than %u sectors reserved)",
1362 should_not_have_added,
disk_res_sectors);
1363 return 0;
1364 need_mark:
1365 /* revert changes: */
1366 for (d2 = deltas->d; d2 != d; d2 = replicas_delta_next(d2))
1367 BUG_ON(__update_replicas(c, dst, &d2->r, -d2->delta));
1368
1369 preempt_enable();
1370 percpu_up_read(&c->mark_lock);
1371 return -1;
1372 }
regards,
dan carpenter
