rust: Fix ptr casting in Fs::open()

Signed-off-by: Kent Overstreet <kent.overstreet@linux.dev>

10 files changed, 136 insertions, 83 deletions

diff --git a/.bcachefs_revision b/.bcachefs_revision
index afaf4c13..033b6af2 100644
--- a/.bcachefs_revision
+++ b/.bcachefs_revision
@@ -1 +1 @@
-2272c5f5b76a5dc0c925064b3682110218a3e53b
+c28937622fbd373f152df01f29efa2d79af99633
diff --git a/libbcachefs/alloc_foreground.c b/libbcachefs/alloc_foreground.c
index af1c1e2b..b2755c1e 100644
--- a/libbcachefs/alloc_foreground.c
+++ b/libbcachefs/alloc_foreground.c
@@ -754,6 +754,7 @@ static int bucket_alloc_from_stripe(struct btree_trans *trans,
 			 unsigned nr_replicas,
 			 unsigned *nr_effective,
 			 bool *have_cache,
+			 enum alloc_reserve reserve,
 			 unsigned flags,
 			 struct closure *cl)
 {
@@ -771,9 +772,7 @@ static int bucket_alloc_from_stripe(struct btree_trans *trans,
 	if (ec_open_bucket(c, ptrs))
 		return 0;
 
-	h = bch2_ec_stripe_head_get(trans, target, 0, nr_replicas - 1,
-				    wp == &c->copygc_write_point,
-				    cl);
+	h = bch2_ec_stripe_head_get(trans, target, 0, nr_replicas - 1, reserve, cl);
 	if (IS_ERR(h))
 		return PTR_ERR(h);
 	if (!h)
@@ -958,7 +957,8 @@ static int __open_bucket_add_buckets(struct btree_trans *trans,
 		ret = bucket_alloc_from_stripe(trans, ptrs, wp, &devs,
 					 target,
 					 nr_replicas, nr_effective,
-					 have_cache, flags, _cl);
+					 have_cache,
+					 reserve, flags, _cl);
 	} else {
 retry_blocking:
 		/*
diff --git a/libbcachefs/alloc_types.h b/libbcachefs/alloc_types.h
index 8bfd4e6a..cd0c50aa 100644
--- a/libbcachefs/alloc_types.h
+++ b/libbcachefs/alloc_types.h
@@ -22,7 +22,8 @@ struct ec_bucket_buf;
 	x(btree_movinggc)		\
 	x(btree)			\
 	x(movinggc)			\
-	x(none)
+	x(none)				\
+	x(stripe)
 
 enum alloc_reserve {
 #define x(name)	RESERVE_##name,
diff --git a/libbcachefs/btree_gc.c b/libbcachefs/btree_gc.c
index 5712e5cc..65fda70d 100644
--- a/libbcachefs/btree_gc.c
+++ b/libbcachefs/btree_gc.c
@@ -1690,6 +1690,7 @@ static int bch2_gc_write_stripes_key(struct btree_trans *trans,
 	struct printbuf buf = PRINTBUF;
 	const struct bch_stripe *s;
 	struct gc_stripe *m;
+	bool bad = false;
 	unsigned i;
 	int ret = 0;
 
@@ -1699,18 +1700,21 @@ static int bch2_gc_write_stripes_key(struct btree_trans *trans,
 	s = bkey_s_c_to_stripe(k).v;
 	m = genradix_ptr(&c->gc_stripes, k.k->p.offset);
 
-	for (i = 0; i < s->nr_blocks; i++)
-		if (stripe_blockcount_get(s, i) != (m ? m->block_sectors[i] : 0))
-			goto inconsistent;
-	return 0;
-inconsistent:
-	if (fsck_err_on(true, c,
-			"stripe has wrong block sector count %u:\n"
-			"  %s\n"
-			"  should be %u", i,
-			(printbuf_reset(&buf),
-			 bch2_bkey_val_to_text(&buf, c, k), buf.buf),
-			m ? m->block_sectors[i] : 0)) {
+	for (i = 0; i < s->nr_blocks; i++) {
+		u32 old = stripe_blockcount_get(s, i);
+		u32 new = (m ? m->block_sectors[i] : 0);
+
+		if (old != new) {
+			prt_printf(&buf, "stripe block %u has wrong sector count: got %u, should be %u\n",
+				   i, old, new);
+			bad = true;
+		}
+	}
+
+	if (bad)
+		bch2_bkey_val_to_text(&buf, c, k);
+
+	if (fsck_err_on(bad, c, "%s", buf.buf)) {
 		struct bkey_i_stripe *new;
 
 		new = bch2_trans_kmalloc(trans, bkey_bytes(k.k));
diff --git a/libbcachefs/buckets.h b/libbcachefs/buckets.h
index 22721bfe..d677b022 100644
--- a/libbcachefs/buckets.h
+++ b/libbcachefs/buckets.h
@@ -157,6 +157,9 @@ static inline u64 bch2_dev_buckets_reserved(struct bch_dev *ca, enum alloc_reser
 	switch (reserve) {
 	case RESERVE_NR:
 		unreachable();
+	case RESERVE_stripe:
+		reserved += ca->mi.nbuckets >> 6;
+		fallthrough;
 	case RESERVE_none:
 		reserved += ca->mi.nbuckets >> 6;
 		fallthrough;
diff --git a/libbcachefs/ec.c b/libbcachefs/ec.c
index a09b39c7..f7598883 100644
--- a/libbcachefs/ec.c
+++ b/libbcachefs/ec.c
@@ -866,8 +866,18 @@ static int ec_stripe_key_update(struct btree_trans *trans,
 			goto err;
 		}
 
-		for (i = 0; i < new->v.nr_blocks; i++)
-			stripe_blockcount_set(&new->v, i, stripe_blockcount_get(old, i));
+		for (i = 0; i < new->v.nr_blocks; i++) {
+			unsigned v = stripe_blockcount_get(old, i);
+
+			if (!v)
+				continue;
+
+			BUG_ON(old->ptrs[i].dev != new->v.ptrs[i].dev ||
+			       old->ptrs[i].gen != new->v.ptrs[i].gen ||
+			       old->ptrs[i].offset != new->v.ptrs[i].offset);
+
+			stripe_blockcount_set(&new->v, i, v);
+		}
 	}
 
 	ret = bch2_trans_update(trans, &iter, &new->k_i, 0);
@@ -1336,7 +1346,7 @@ static int ec_new_stripe_alloc(struct bch_fs *c, struct ec_stripe_head *h)
 static struct ec_stripe_head *
 ec_new_stripe_head_alloc(struct bch_fs *c, unsigned target,
 			 unsigned algo, unsigned redundancy,
-			 bool copygc)
+			 enum alloc_reserve reserve)
 {
 	struct ec_stripe_head *h;
 	struct bch_dev *ca;
@@ -1352,7 +1362,7 @@ ec_new_stripe_head_alloc(struct bch_fs *c, unsigned target,
 	h->target	= target;
 	h->algo		= algo;
 	h->redundancy	= redundancy;
-	h->copygc	= copygc;
+	h->reserve	= reserve;
 
 	rcu_read_lock();
 	h->devs = target_rw_devs(c, BCH_DATA_user, target);
@@ -1387,7 +1397,7 @@ struct ec_stripe_head *__bch2_ec_stripe_head_get(struct btree_trans *trans,
 						 unsigned target,
 						 unsigned algo,
 						 unsigned redundancy,
-						 bool copygc)
+						 enum alloc_reserve reserve)
 {
 	struct bch_fs *c = trans->c;
 	struct ec_stripe_head *h;
@@ -1404,21 +1414,21 @@ struct ec_stripe_head *__bch2_ec_stripe_head_get(struct btree_trans *trans,
 		if (h->target		== target &&
 		    h->algo		== algo &&
 		    h->redundancy	== redundancy &&
-		    h->copygc		== copygc) {
+		    h->reserve		== reserve) {
 			ret = bch2_trans_mutex_lock(trans, &h->lock);
 			if (ret)
 				h = ERR_PTR(ret);
 			goto found;
 		}
 
-	h = ec_new_stripe_head_alloc(c, target, algo, redundancy, copygc);
+	h = ec_new_stripe_head_alloc(c, target, algo, redundancy, reserve);
 found:
 	mutex_unlock(&c->ec_stripe_head_lock);
 	return h;
 }
 
 static int new_stripe_alloc_buckets(struct btree_trans *trans, struct ec_stripe_head *h,
-				    struct closure *cl)
+				    enum alloc_reserve reserve, struct closure *cl)
 {
 	struct bch_fs *c = trans->c;
 	struct bch_devs_mask devs = h->devs;
@@ -1428,14 +1438,12 @@ static int new_stripe_alloc_buckets(struct btree_trans *trans, struct ec_stripe_
 	bool have_cache = true;
 	int ret = 0;
 
-	for (i = 0; i < h->s->new_stripe.key.v.nr_blocks; i++) {
-		if (test_bit(i, h->s->blocks_gotten)) {
-			__clear_bit(h->s->new_stripe.key.v.ptrs[i].dev, devs.d);
-			if (i < h->s->nr_data)
-				nr_have_data++;
-			else
-				nr_have_parity++;
-		}
+	for_each_set_bit(i, h->s->blocks_gotten, h->s->new_stripe.key.v.nr_blocks) {
+		__clear_bit(h->s->new_stripe.key.v.ptrs[i].dev, devs.d);
+		if (i < h->s->nr_data)
+			nr_have_data++;
+		else
+			nr_have_parity++;
 	}
 
 	BUG_ON(nr_have_data	> h->s->nr_data);
@@ -1450,9 +1458,7 @@ static int new_stripe_alloc_buckets(struct btree_trans *trans, struct ec_stripe_
 					    &nr_have_parity,
 					    &have_cache,
 					    BCH_DATA_parity,
-					    h->copygc
-					    ? RESERVE_movinggc
-					    : RESERVE_none,
+					    reserve,
 					    cl);
 
 		open_bucket_for_each(c, &buckets, ob, i) {
@@ -1479,9 +1485,7 @@ static int new_stripe_alloc_buckets(struct btree_trans *trans, struct ec_stripe_
 					    &nr_have_data,
 					    &have_cache,
 					    BCH_DATA_user,
-					    h->copygc
-					    ? RESERVE_movinggc
-					    : RESERVE_none,
+					    reserve,
 					    cl);
 
 		open_bucket_for_each(c, &buckets, ob, i) {
@@ -1548,10 +1552,11 @@ static int __bch2_ec_stripe_head_reuse(struct btree_trans *trans, struct ec_stri
 	if (idx < 0)
 		return -BCH_ERR_ENOSPC_stripe_reuse;
 
-	h->s->have_existing_stripe = true;
 	ret = get_stripe_key_trans(trans, idx, &h->s->existing_stripe);
 	if (ret) {
-		bch2_fs_fatal_error(c, "error reading stripe key: %i", ret);
+		bch2_stripe_close(c, h->s);
+		if (!bch2_err_matches(ret, BCH_ERR_transaction_restart))
+			bch2_fs_fatal_error(c, "error reading stripe key: %s", bch2_err_str(ret));
 		return ret;
 	}
 
@@ -1566,6 +1571,17 @@ static int __bch2_ec_stripe_head_reuse(struct btree_trans *trans, struct ec_stri
 	BUG_ON(h->s->existing_stripe.size != h->blocksize);
 	BUG_ON(h->s->existing_stripe.size != h->s->existing_stripe.key.v.sectors);
 
+	/*
+	 * Free buckets we initially allocated - they might conflict with
+	 * blocks from the stripe we're reusing:
+	 */
+	for_each_set_bit(i, h->s->blocks_gotten, h->s->new_stripe.key.v.nr_blocks) {
+		bch2_open_bucket_put(c, c->open_buckets + h->s->blocks[i]);
+		h->s->blocks[i] = 0;
+	}
+	memset(h->s->blocks_gotten, 0, sizeof(h->s->blocks_gotten));
+	memset(h->s->blocks_allocated, 0, sizeof(h->s->blocks_allocated));
+
 	for (i = 0; i < h->s->existing_stripe.key.v.nr_blocks; i++) {
 		if (stripe_blockcount_get(&h->s->existing_stripe.key.v, i)) {
 			__set_bit(i, h->s->blocks_gotten);
@@ -1575,8 +1591,10 @@ static int __bch2_ec_stripe_head_reuse(struct btree_trans *trans, struct ec_stri
 		ec_block_io(c, &h->s->existing_stripe, READ, i, &h->s->iodone);
 	}
 
-	bkey_copy(&h->s->new_stripe.key.k_i,
-		  &h->s->existing_stripe.key.k_i);
+	bkey_copy(&h->s->new_stripe.key.k_i, &h->s->existing_stripe.key.k_i);
+	h->s->have_existing_stripe = true;
+
+	pr_info("reused %llu", h->s->idx);
 
 	return 0;
 }
@@ -1590,13 +1608,14 @@ static int __bch2_ec_stripe_head_reserve(struct btree_trans *trans, struct ec_st
 	struct bpos start_pos = bpos_max(min_pos, POS(0, c->ec_stripe_hint));
 	int ret;
 
-	BUG_ON(h->s->res.sectors);
-
-	ret = bch2_disk_reservation_get(c, &h->s->res,
+	if (!h->s->res.sectors) {
+		ret = bch2_disk_reservation_get(c, &h->s->res,
 					h->blocksize,
-					h->s->nr_parity, 0);
-	if (ret)
-		return ret;
+					h->s->nr_parity,
+					BCH_DISK_RESERVATION_NOFAIL);
+		if (ret)
+			return ret;
+	}
 
 	for_each_btree_key_norestart(trans, iter, BTREE_ID_stripes, start_pos,
 			   BTREE_ITER_SLOTS|BTREE_ITER_INTENT, k, ret) {
@@ -1640,22 +1659,21 @@ struct ec_stripe_head *bch2_ec_stripe_head_get(struct btree_trans *trans,
 					       unsigned target,
 					       unsigned algo,
 					       unsigned redundancy,
-					       bool copygc,
+					       enum alloc_reserve reserve,
 					       struct closure *cl)
 {
 	struct bch_fs *c = trans->c;
 	struct ec_stripe_head *h;
+	bool waiting = false;
 	int ret;
-	bool needs_stripe_new;
 
-	h = __bch2_ec_stripe_head_get(trans, target, algo, redundancy, copygc);
+	h = __bch2_ec_stripe_head_get(trans, target, algo, redundancy, reserve);
 	if (!h)
 		bch_err(c, "no stripe head");
 	if (IS_ERR_OR_NULL(h))
 		return h;
 
-	needs_stripe_new = !h->s;
-	if (needs_stripe_new) {
+	if (!h->s) {
 		if (ec_new_stripe_alloc(c, h)) {
 			ret = -ENOMEM;
 			bch_err(c, "failed to allocate new stripe");
@@ -1666,32 +1684,60 @@ struct ec_stripe_head *bch2_ec_stripe_head_get(struct btree_trans *trans,
 			BUG();
 	}
 
-	/*
-	 * Try reserve a new stripe before reusing an
-	 * existing stripe. This will prevent unnecessary
-	 * read amplification during write oriented workloads.
-	 */
-	ret = 0;
-	if (!h->s->allocated && !h->s->res.sectors && !h->s->have_existing_stripe)
-		ret = __bch2_ec_stripe_head_reserve(trans, h);
-	if (bch2_err_matches(ret, BCH_ERR_transaction_restart))
+	if (h->s->allocated)
+		goto allocated;
+
+	if (h->s->idx)
+		goto alloc_existing;
+#if 0
+	/* First, try to allocate a full stripe: */
+	ret =   new_stripe_alloc_buckets(trans, h, RESERVE_stripe, NULL) ?:
+		__bch2_ec_stripe_head_reserve(trans, h);
+	if (!ret)
+		goto allocated;
+	if (bch2_err_matches(ret, BCH_ERR_transaction_restart) ||
+	    bch2_err_matches(ret, ENOMEM))
 		goto err;
 
-	if (ret && needs_stripe_new)
-		ret = __bch2_ec_stripe_head_reuse(trans, h);
-	if (ret) {
-		bch_err_ratelimited(c, "failed to get stripe: %s", bch2_err_str(ret));
-		goto err;
+	if (ret == -BCH_ERR_open_buckets_empty) {
+		/* don't want to reuse in this case */
 	}
-
-	if (!h->s->allocated) {
-		ret = new_stripe_alloc_buckets(trans, h, cl);
+#endif
+	/*
+	 * Not enough buckets available for a full stripe: we must reuse an
+	 * existing stripe:
+	 */
+	while (1) {
+		ret = __bch2_ec_stripe_head_reuse(trans, h);
 		if (ret)
+			ret = __bch2_ec_stripe_head_reserve(trans, h);
+		if (!ret)
+			break;
+		pr_info("err %s", bch2_err_str(ret));
+		if (ret == -BCH_ERR_ENOSPC_stripe_reuse && cl)
+			ret = -BCH_ERR_stripe_alloc_blocked;
+		if (waiting || !cl)
 			goto err;
 
-		h->s->allocated = true;
+		/* XXX freelist_wait? */
+		closure_wait(&c->freelist_wait, cl);
+		waiting = true;
 	}
 
+	if (waiting)
+		closure_wake_up(&c->freelist_wait);
+alloc_existing:
+	/*
+	 * Retry allocating buckets, with the reserve watermark for this
+	 * particular write:
+	 */
+	ret = new_stripe_alloc_buckets(trans, h, reserve, cl);
+	if (ret)
+		goto err;
+allocated:
+	h->s->allocated = true;
+	BUG_ON(!h->s->idx);
+
 	BUG_ON(trans->restarted);
 	return h;
 err:
diff --git a/libbcachefs/ec.h b/libbcachefs/ec.h
index c9e4fb21..56d1b5e7 100644
--- a/libbcachefs/ec.h
+++ b/libbcachefs/ec.h
@@ -181,7 +181,7 @@ struct ec_stripe_head {
 	unsigned		target;
 	unsigned		algo;
 	unsigned		redundancy;
-	bool			copygc;
+	enum alloc_reserve	reserve;
 
 	struct bch_devs_mask	devs;
 	unsigned		nr_active_devs;
@@ -205,7 +205,8 @@ int bch2_ec_stripe_new_alloc(struct bch_fs *, struct ec_stripe_head *);
 
 void bch2_ec_stripe_head_put(struct bch_fs *, struct ec_stripe_head *);
 struct ec_stripe_head *bch2_ec_stripe_head_get(struct btree_trans *,
-			unsigned, unsigned, unsigned, bool, struct closure *);
+			unsigned, unsigned, unsigned,
+			enum alloc_reserve, struct closure *);
 
 void bch2_stripes_heap_update(struct bch_fs *, struct stripe *, size_t);
 void bch2_stripes_heap_del(struct bch_fs *, struct stripe *, size_t);
diff --git a/libbcachefs/errcode.h b/libbcachefs/errcode.h
index 6129af61..283303db 100644
--- a/libbcachefs/errcode.h
+++ b/libbcachefs/errcode.h
@@ -93,6 +93,7 @@
 	x(BCH_ERR_operation_blocked,	journal_res_get_blocked)		\
 	x(BCH_ERR_operation_blocked,	journal_preres_get_blocked)		\
 	x(BCH_ERR_operation_blocked,	bucket_alloc_blocked)			\
+	x(BCH_ERR_operation_blocked,	stripe_alloc_blocked)			\
 	x(BCH_ERR_invalid,		invalid_sb)				\
 	x(BCH_ERR_invalid_sb,		invalid_sb_magic)			\
 	x(BCH_ERR_invalid_sb,		invalid_sb_version)			\
diff --git a/rust-src/bch_bindgen/src/fs.rs b/rust-src/bch_bindgen/src/fs.rs
index 1176846d..b26c51b6 100644
--- a/rust-src/bch_bindgen/src/fs.rs
+++ b/rust-src/bch_bindgen/src/fs.rs
@@ -9,15 +9,12 @@ pub struct Fs {
 }
 
 impl Fs {
-    pub fn open(devices: &Vec<PathBuf>, opts: c::bch_opts) -> Result<Fs, bch_errcode> {
-        let devices: Vec<_> = devices.iter()
-            .map(|i| CString::new(i.as_os_str().as_bytes()).unwrap()).collect();
-        let dev_c_strs: Vec<_> = devices.iter()
-            .map(|i| { let p: *const i8 = i.as_ptr(); p })
+    pub fn open(devs: &Vec<PathBuf>, opts: c::bch_opts) -> Result<Fs, bch_errcode> {
+        let devs: Vec<_> = devs.iter()
+            .map(|i| CString::new(i.as_os_str().as_bytes()).unwrap().into_raw())
             .collect();
-        let dev_c_strarray: *const *mut i8 = dev_c_strs[..].as_ptr() as *const *mut i8;
 
-        let ret = unsafe { c::bch2_fs_open(dev_c_strarray, dev_c_strs.len() as u32, opts) };
+        let ret = unsafe { c::bch2_fs_open(devs[..].as_ptr(), devs.len() as u32, opts) };
 
         errptr_to_result(ret).map(|fs| Fs { raw: fs})
     }
diff --git a/rust-src/bch_bindgen/src/lib.rs b/rust-src/bch_bindgen/src/lib.rs
index 3bc21a18..d2b58511 100644
--- a/rust-src/bch_bindgen/src/lib.rs
+++ b/rust-src/bch_bindgen/src/lib.rs
@@ -91,7 +91,7 @@ impl FromStr for c::btree_id {
 
     fn from_str(s: &str) -> Result<Self, Self::Err> {
         let s = CString::new(s).unwrap();
-        let p: *const i8 = s.as_ptr();
+        let p = s.as_ptr();
 
         let v = unsafe {c::match_string(c::bch2_btree_ids[..].as_ptr(), (-(1 as isize)) as usize, p)};
         if v >= 0 {