0.3.10

Transactions are now committed regularly, so it should no longer be
strictly necessary to unmount the filesystem cleanly for writes to take
effect. It's still recommended, of course.

The release also fixes two bugs. One was a big performance regression
from 0.3.9, seen when deleting large numbers of files. The other was
that mounts would go read-only if two snapshots were attempted with the
same name.

Support is added for the new 6.10 kernel version, with a patch by Woody
Suwalski.

Signed-off-by: Ernesto A. Fernández <ernesto@corellium.com>

apfs.h

@@ -150,9 +150,9 @@ struct apfs_spaceman {
 	struct buffer_head *sm_ip_bmaps[];
 };
 
-#define TRANSACTION_MAIN_QUEUE_MAX	4096
-#define TRANSACTION_BUFFERS_MAX		65536
-#define TRANSACTION_STARTS_MAX		65536
+#define APFS_TRANS_MAIN_QUEUE_MAX	10000
+#define APFS_TRANS_BUFFERS_MAX		65536
+#define APFS_TRANS_STARTS_MAX		65536
 
 /* Possible states for the container transaction structure */
 #define APFS_NX_TRANS_FORCE_COMMIT	1	/* Commit guaranteed */
@@ -166,22 +166,15 @@ struct apfs_spaceman {
 struct apfs_nx_transaction {
 	unsigned int t_state;
 
+	struct delayed_work t_work;	/* Work task for transaction commits */
+	struct super_block *t_work_sb;	/* sb that last queued a commit */
+
 	struct list_head t_inodes;	/* List of inodes in the transaction */
 	struct list_head t_buffers;	/* List of buffers in the transaction */
 	size_t t_buffers_count;		/* Count of items on the list */
 	int t_starts_count;		/* Count of starts for transaction */
 };
 
-/*
- * Structure that keeps track of a volume transaction.
- */
-struct apfs_vol_transaction {
-	struct buffer_head *t_old_vsb;  /* Volume superblock being replaced */
-
-	struct apfs_node t_old_omap_root; /* Omap root node being replaced */
-	struct apfs_node t_old_cat_root;  /* Catalog root node being replaced */
-};
-
 /* State bits for buffer heads in a transaction */
 #define BH_TRANS	BH_PrivateStart		/* Attached to a transaction */
 #define BH_CSUM		(BH_PrivateStart + 1)	/* Requires checksum update */
@@ -254,6 +247,7 @@ struct apfs_nxsb_info {
 
 	struct apfs_spaceman *nx_spaceman;
 	struct apfs_nx_transaction nx_transaction;
+	int nx_trans_buffers_max;
 
 	/* For now, a single semaphore for every operation */
 	struct rw_semaphore nx_big_sem;
@@ -333,9 +327,6 @@ struct apfs_sb_info {
 
 	struct apfs_crypto_state_val *s_dflt_pfk; /* default per-file key */
 
-	struct apfs_vol_transaction s_transaction;
-	int s_trans_buffers_max;
-
 	struct inode *s_private_dir;	/* Inode for the private directory */
 	struct work_struct s_orphan_cleanup_work;
 };
@@ -1081,6 +1072,7 @@ extern int apfs_sync_fs(struct super_block *sb, int wait);
 
 /* transaction.c */
 extern int apfs_cpoint_data_free(struct super_block *sb, u64 bno);
+extern void apfs_transaction_init(struct apfs_nx_transaction *trans);
 extern int apfs_transaction_start(struct super_block *sb, struct apfs_max_ops maxops);
 extern int apfs_transaction_commit(struct super_block *sb);
 extern void apfs_inode_join_transaction(struct super_block *sb, struct inode *inode);

dkms.conf

@@ -1,5 +1,5 @@
 PACKAGE_NAME="linux-apfs-rw"
-PACKAGE_VERSION="0.3.9"
+PACKAGE_VERSION="0.3.10"
 
 BUILT_MODULE_NAME[0]="apfs"
 DEST_MODULE_LOCATION[0]="/extra"

extents.c

@@ -1651,7 +1651,7 @@ static int apfs_dstream_delete_front(struct super_block *sb, u64 ds_id)
 	int ret;
 
 	fq = &sm_raw->sm_fq[APFS_SFQ_MAIN];
-	if (le64_to_cpu(fq->sfq_count) > TRANSACTION_MAIN_QUEUE_MAX)
+	if (le64_to_cpu(fq->sfq_count) > APFS_TRANS_MAIN_QUEUE_MAX)
 		return -EAGAIN;
 
 	query = apfs_alloc_query(sbi->s_cat_root, NULL /* parent */);
@@ -1710,7 +1710,7 @@ next_extent:
 		}
 	}
 
-	if (le64_to_cpu(fq->sfq_count) <= TRANSACTION_MAIN_QUEUE_MAX)
+	if (le64_to_cpu(fq->sfq_count) <= APFS_TRANS_MAIN_QUEUE_MAX)
 		goto next_extent;
 	ret = -EAGAIN;
 out:

snapshot.c

@@ -149,9 +149,9 @@ static int apfs_create_snap_name_rec(struct apfs_node *snap_root, const char *na
 
 	err = apfs_btree_query(sb, &query);
 	if (err == 0) {
-		/* TODO: avoid transaction abort here */
-		apfs_info(sb, "a snapshot with that name already exists");
-		err = -EEXIST;
+		/* This should never happen here, the caller has checked */
+		apfs_alert(sb, "snapshot name record already exists");
+		err = -EFSCORRUPTED;
 		goto fail;
 	}
 	if (err != -ENODATA) {
@@ -332,6 +332,56 @@ static int apfs_update_omap_snapshots(struct super_block *sb)
 	return err;
 }
 
+/**
+ * apfs_snapshot_name_check - Check if a snapshot with the given name exists
+ * @sb:		filesystem superblock
+ * @name:	name to check
+ * @name_len:	length of @name
+ * @eexist:	on return, whether the name exists or not
+ *
+ * Returns 0 on success, or a negative error code in case of failure.
+ */
+static int apfs_snapshot_name_check(struct super_block *sb, const char *name, int name_len, bool *eexist)
+{
+	struct apfs_sb_info *sbi = APFS_SB(sb);
+	struct apfs_superblock *vsb_raw = sbi->s_vsb_raw;
+	struct apfs_node *snap_root = NULL;
+	struct apfs_query *query = NULL;
+	int err;
+
+	snap_root = apfs_read_node(sb, le64_to_cpu(vsb_raw->apfs_snap_meta_tree_oid), APFS_OBJ_PHYSICAL, false /* write */);
+	if (IS_ERR(snap_root)) {
+		apfs_err(sb, "failed to read snap meta root 0x%llx", le64_to_cpu(vsb_raw->apfs_snap_meta_tree_oid));
+		return PTR_ERR(snap_root);
+	}
+	vsb_raw = NULL;
+
+	query = apfs_alloc_query(snap_root, NULL /* parent */);
+	if (!query) {
+		err = -ENOMEM;
+		goto out;
+	}
+	apfs_init_snap_name_key(name, &query->key);
+	query->flags |= APFS_QUERY_SNAP_META | APFS_QUERY_EXACT;
+
+	err = apfs_btree_query(sb, &query);
+	if (err == 0) {
+		*eexist = true;
+	} else if (err == -ENODATA) {
+		*eexist = false;
+		err = 0;
+	} else {
+		apfs_err(sb, "query failed (%s)", name);
+	}
+
+out:
+	apfs_free_query(query);
+	query = NULL;
+	apfs_node_free(snap_root);
+	snap_root = NULL;
+	return err;
+}
+
 /**
  * apfs_do_ioc_takesnapshot - Actual work for apfs_ioc_take_snapshot()
  * @mntpoint:	inode of the mount point to snapshot
@@ -348,12 +398,31 @@ static int apfs_do_ioc_take_snapshot(struct inode *mntpoint, const char *name, i
 	/* TODO: remember to update the maxops in the future */
 	struct apfs_max_ops maxops = {0};
 	u64 sblock_oid;
+	bool eexist = false;
 	int err;
 
 	err = apfs_transaction_start(sb, maxops);
 	if (err)
 		return err;
 
+	/*
+	 * This check can fail in normal operation, so run it before making any
+	 * changes and exit the transaction cleanly if necessary. The name
+	 * lookup will have to be repeated later, but it's ok because I don't
+	 * expect snapshot creation to be a bottleneck for anyone.
+	 */
+	err = apfs_snapshot_name_check(sb, name, name_len, &eexist);
+	if (err) {
+		apfs_err(sb, "snapshot name existence check failed");
+		goto fail;
+	}
+	if (eexist) {
+		err = apfs_transaction_commit(sb);
+		if (err)
+			goto fail;
+		return -EEXIST;
+	}
+
 	/*
 	 * Flush the extent caches to the extenref tree before it gets moved to
 	 * the snapshot. It seems safer in general to avoid big unpredictable

spaceman.c

@@ -548,7 +548,7 @@ static int apfs_flush_free_queue(struct super_block *sb, unsigned int qid, bool
 		sfq_count = le64_to_cpu(fq->sfq_count);
 		if (qid == APFS_SFQ_IP && sfq_count * 6 <= le64_to_cpu(sm_raw->sm_ip_block_count))
 			break;
-		if (qid == APFS_SFQ_MAIN && sfq_count <= TRANSACTION_MAIN_QUEUE_MAX - 200)
+		if (qid == APFS_SFQ_MAIN && sfq_count <= APFS_TRANS_MAIN_QUEUE_MAX - 200)
 			break;
 	}
 

super.c

@@ -57,7 +57,11 @@ static struct apfs_nxsb_info *apfs_nx_find_by_dev(dev_t dev)
  */
 static int apfs_sb_set_blocksize(struct super_block *sb, int size)
 {
+#if LINUX_VERSION_CODE >= KERNEL_VERSION(6, 10, 0)
+	if (set_blocksize(APFS_NXI(sb)->nx_bdev_file, size))
+#else
 	if (set_blocksize(APFS_NXI(sb)->nx_bdev, size))
+#endif
 		return 0;
 	sb->s_blocksize = size;
 	sb->s_blocksize_bits = blksize_bits(size);
@@ -402,14 +406,11 @@ int apfs_map_volume_super(struct super_block *sb, bool write)
 	struct apfs_omap *omap = NULL;
 	struct apfs_node *vnode;
 	struct buffer_head *bh;
-	struct apfs_vol_transaction *trans = &sbi->s_transaction;
 	u64 vol_id;
 	u64 vsb;
 	int err;
 
 	ASSERT(msb_raw);
-	ASSERT(trans->t_old_vsb == sbi->s_vobject.o_bh);
-	(void)trans;
 
 	/* Get the id for the requested volume number */
 	if (sbi->s_vol_nr >= APFS_NX_MAX_FILE_SYSTEMS) {
@@ -676,10 +677,15 @@ int apfs_read_catalog(struct super_block *sb, bool write)
 static void apfs_put_super(struct super_block *sb)
 {
 	struct apfs_sb_info *sbi = APFS_SB(sb);
+	struct apfs_nx_transaction *trans = NULL;
 
 	/* Cleanups won't reschedule themselves during unmount */
 	flush_work(&sbi->s_orphan_cleanup_work);
 
+	/* We are about to commit anyway */
+	trans = &APFS_NXI(sb)->nx_transaction;
+	cancel_delayed_work_sync(&trans->t_work);
+
 	/* Stop flushing orphans and update the volume as needed */
 	if (!(sb->s_flags & SB_RDONLY)) {
 		struct apfs_superblock *vsb_raw;
@@ -1387,7 +1393,7 @@ static int apfs_setup_bdi(struct super_block *sb)
 
 static void apfs_set_trans_buffer_limit(struct super_block *sb)
 {
-	struct apfs_sb_info *sbi = APFS_SB(sb);
+	struct apfs_nxsb_info *nxi = APFS_NXI(sb);
 	unsigned long memsize_in_blocks;
 	struct sysinfo info = {0};
 
@@ -1400,10 +1406,10 @@ static void apfs_set_trans_buffer_limit(struct super_block *sb)
 	 * commit often enough. This hack should make that happen in general,
 	 * but I still need to get the reclaim to work eventually (TODO).
 	 */
-	if (memsize_in_blocks >= 16 * TRANSACTION_BUFFERS_MAX)
-		sbi->s_trans_buffers_max = TRANSACTION_BUFFERS_MAX;
+	if (memsize_in_blocks >= 16 * APFS_TRANS_BUFFERS_MAX)
+		nxi->nx_trans_buffers_max = APFS_TRANS_BUFFERS_MAX;
 	else
-		sbi->s_trans_buffers_max = memsize_in_blocks / 16;
+		nxi->nx_trans_buffers_max = memsize_in_blocks / 16;
 }
 
 static int apfs_fill_super(struct super_block *sb, void *data, int silent)
@@ -1651,6 +1657,7 @@ static int apfs_attach_nxi(struct apfs_sb_info *sbi, const char *dev_name, fmode
 		init_rwsem(&nxi->nx_big_sem);
 		list_add(&nxi->nx_list, &nxs);
 		INIT_LIST_HEAD(&nxi->vol_list);
+		apfs_transaction_init(&nxi->nx_transaction);
 	}
 
 	list_add(&sbi->list, &nxi->vol_list);

transaction.c

@@ -20,8 +20,12 @@ static int apfs_checkpoint_end(struct super_block *sb)
 	struct apfs_nxsb_info *nxi = APFS_NXI(sb);
 	struct apfs_obj_phys *obj = &nxi->nx_raw->nx_o;
 	struct buffer_head *bh = NULL;
+#if LINUX_VERSION_CODE >= KERNEL_VERSION(6, 10, 0)
+	struct address_space *bdev_map = nxi->nx_bdev->bd_mapping;
+#else
 	struct inode *bdev_inode = nxi->nx_bdev->bd_inode;
 	struct address_space *bdev_map = bdev_inode->i_mapping;
+#endif
 	int err;
 
 	ASSERT(!(sb->s_flags & SB_RDONLY));
@@ -262,6 +266,55 @@ static int apfs_read_ephemeral_objects(struct super_block *sb)
 	return 0;
 }
 
+static void apfs_trans_commit_work(struct work_struct *work)
+{
+	struct super_block *sb = NULL;
+	struct apfs_nxsb_info *nxi = NULL;
+	struct apfs_nx_transaction *trans = NULL;
+	int err;
+
+	trans = container_of(to_delayed_work(work), struct apfs_nx_transaction, t_work);
+	nxi = container_of(trans, struct apfs_nxsb_info, nx_transaction);
+	sb = trans->t_work_sb;
+
+	/*
+	 * If sb is set then the transaction already started, there is no need
+	 * for apfs_transaction_start() here. It would be cleaner to call it
+	 * anyway (and check in there if sb is set), but maxops is a problem
+	 * because we don't need any space. I really need to rethink that stuff
+	 * (TODO).
+	 */
+	down_write(&nxi->nx_big_sem);
+	mutex_lock(&nxs_mutex);
+	if (!sb || sb->s_flags & SB_RDONLY) {
+		/* The commit already took place, or there was an abort */
+		mutex_unlock(&nxs_mutex);
+		up_write(&nxi->nx_big_sem);
+		return;
+	}
+
+	trans->t_state |= APFS_NX_TRANS_FORCE_COMMIT;
+	err = apfs_transaction_commit(sb);
+	if (err) {
+		apfs_err(sb, "queued commit failed (err:%d)", err);
+		apfs_transaction_abort(sb);
+	}
+}
+
+/**
+ * apfs_transaction_init - Initialize the transaction struct for the container
+ * @trans: the transaction structure
+ */
+void apfs_transaction_init(struct apfs_nx_transaction *trans)
+{
+	trans->t_state = 0;
+	INIT_DELAYED_WORK(&trans->t_work, apfs_trans_commit_work);
+	INIT_LIST_HEAD(&trans->t_inodes);
+	INIT_LIST_HEAD(&trans->t_buffers);
+	trans->t_buffers_count = 0;
+	trans->t_starts_count = 0;
+}
+
 /**
  * apfs_transaction_start - Begin a new transaction
  * @sb:		superblock structure
@@ -272,9 +325,7 @@ static int apfs_read_ephemeral_objects(struct super_block *sb)
  */
 int apfs_transaction_start(struct super_block *sb, struct apfs_max_ops maxops)
 {
-	struct apfs_sb_info *sbi = APFS_SB(sb);
 	struct apfs_nxsb_info *nxi = APFS_NXI(sb);
-	struct apfs_vol_transaction *vol_trans = &sbi->s_transaction;
 	struct apfs_nx_transaction *nx_trans = &nxi->nx_transaction;
 	int err;
 
@@ -306,9 +357,6 @@ int apfs_transaction_start(struct super_block *sb, struct apfs_max_ops maxops)
 		++nxi->nx_xid;
 		nxi->nx_raw->nx_next_xid = cpu_to_le64(nxi->nx_xid + 1);
 
-		INIT_LIST_HEAD(&nx_trans->t_inodes);
-		INIT_LIST_HEAD(&nx_trans->t_buffers);
-
 		err = apfs_read_spaceman(sb);
 		if (err) {
 			apfs_err(sb, "failed to read the spaceman");
@@ -328,16 +376,7 @@ int apfs_transaction_start(struct super_block *sb, struct apfs_max_ops maxops)
 		return -ENOSPC;
 	}
 
-	if (!vol_trans->t_old_vsb) {
-		vol_trans->t_old_vsb = sbi->s_vobject.o_bh;
-		get_bh(vol_trans->t_old_vsb);
-
-		/* Backup the old tree roots; the node struct issues make this ugly */
-		vol_trans->t_old_cat_root = *sbi->s_cat_root;
-		get_bh(vol_trans->t_old_cat_root.object.o_bh);
-		vol_trans->t_old_omap_root = *sbi->s_omap->omap_root;
-		get_bh(vol_trans->t_old_omap_root.object.o_bh);
-
+	if (nx_trans->t_starts_count == 0) {
 		err = apfs_map_volume_super(sb, true /* write */);
 		if (err) {
 			apfs_err(sb, "CoW failed for volume super");
@@ -596,7 +635,6 @@ static int apfs_transaction_commit_nx(struct super_block *sb)
 {
 	struct apfs_spaceman *sm = APFS_SM(sb);
 	struct apfs_nxsb_info *nxi = APFS_NXI(sb);
-	struct apfs_sb_info *sbi;
 	struct apfs_nx_transaction *nx_trans = &nxi->nx_transaction;
 	struct apfs_bh_info *bhi, *tmp;
 	int err = 0;
@@ -692,24 +730,6 @@ static int apfs_transaction_commit_nx(struct super_block *sb)
 		return err;
 	}
 
-	list_for_each_entry(sbi, &nxi->vol_list, list) {
-		struct apfs_vol_transaction *vol_trans = &sbi->s_transaction;
-
-		if (!vol_trans->t_old_vsb)
-			continue;
-
-		brelse(vol_trans->t_old_vsb);
-		vol_trans->t_old_vsb = NULL;
-
-		/* XXX: forget the buffers for the b-tree roots */
-		vol_trans->t_old_omap_root.object.data = NULL;
-		brelse(vol_trans->t_old_omap_root.object.o_bh);
-		vol_trans->t_old_omap_root.object.o_bh = NULL;
-		vol_trans->t_old_cat_root.object.data = NULL;
-		brelse(vol_trans->t_old_cat_root.object.o_bh);
-		vol_trans->t_old_cat_root.object.o_bh = NULL;
-	}
-
 	for (bmap_idx = 0; bmap_idx < APFS_SM(sb)->sm_ip_bmaps_count; ++bmap_idx) {
 		brelse(APFS_SM(sb)->sm_ip_bmaps[bmap_idx]);
 		APFS_SM(sb)->sm_ip_bmaps[bmap_idx] = NULL;
@@ -749,9 +769,9 @@ static bool apfs_transaction_need_commit(struct super_block *sb)
 		struct apfs_spaceman_phys *sm_raw = sm->sm_raw;
 		struct apfs_spaceman_free_queue *fq_ip = &sm_raw->sm_fq[APFS_SFQ_IP];
 		struct apfs_spaceman_free_queue *fq_main = &sm_raw->sm_fq[APFS_SFQ_MAIN];
-		int buffers_max = APFS_SB(sb)->s_trans_buffers_max;
-		int starts_max = TRANSACTION_STARTS_MAX;
-		int mq_max = TRANSACTION_MAIN_QUEUE_MAX;
+		int buffers_max = nxi->nx_trans_buffers_max;
+		int starts_max = APFS_TRANS_STARTS_MAX;
+		int mq_max = APFS_TRANS_MAIN_QUEUE_MAX;
 
 		/*
 		 * Try to avoid committing halfway through a data block write,
@@ -796,14 +816,22 @@ static bool apfs_transaction_need_commit(struct super_block *sb)
 int apfs_transaction_commit(struct super_block *sb)
 {
 	struct apfs_nxsb_info *nxi = APFS_NXI(sb);
+	struct apfs_nx_transaction *trans = NULL;
 	int err = 0;
 
+	trans = &nxi->nx_transaction;
+
 	if (apfs_transaction_need_commit(sb)) {
 		err = apfs_transaction_commit_nx(sb);
 		if (err) {
 			apfs_err(sb, "transaction commit failed");
 			return err;
 		}
+		trans->t_work_sb = NULL;
+		cancel_delayed_work(&trans->t_work);
+	} else {
+		trans->t_work_sb = sb;
+		mod_delayed_work(system_wq, &trans->t_work, msecs_to_jiffies(100));
 	}
 
 	mutex_unlock(&nxs_mutex);
@@ -889,7 +917,6 @@ static void apfs_force_readonly(struct apfs_nxsb_info *nxi)
  */
 void apfs_transaction_abort(struct super_block *sb)
 {
-	struct apfs_sb_info *sbi;
 	struct apfs_nxsb_info *nxi = APFS_NXI(sb);
 	struct apfs_nx_transaction *nx_trans = &nxi->nx_transaction;
 	struct apfs_bh_info *bhi, *tmp;
@@ -924,35 +951,6 @@ void apfs_transaction_abort(struct super_block *sb)
 		kfree(bhi);
 	}
 
-	/*
-	 * TODO: get rid of all this stuff, it makes little sense. Maybe do an
-	 * actual read-only remount?
-	 */
-	list_for_each_entry(sbi, &nxi->vol_list, list) {
-		struct apfs_vol_transaction *vol_trans = &sbi->s_transaction;
-
-		if (!vol_trans->t_old_vsb)
-			continue;
-
-		/* Restore volume state for all volumes */
-		brelse(sbi->s_vobject.o_bh);
-		sbi->s_vobject.o_bh = vol_trans->t_old_vsb;
-		sbi->s_vobject.data = sbi->s_vobject.o_bh->b_data;
-		sbi->s_vobject.block_nr = vol_trans->t_old_vsb->b_blocknr;
-		sbi->s_vsb_raw = (void *)vol_trans->t_old_vsb->b_data;
-		vol_trans->t_old_vsb = NULL;
-
-		/* XXX: restore the old b-tree root nodes */
-		brelse(sbi->s_omap->omap_root->object.o_bh);
-		*(sbi->s_omap->omap_root) = vol_trans->t_old_omap_root;
-		vol_trans->t_old_omap_root.object.o_bh = NULL;
-		vol_trans->t_old_omap_root.object.data = NULL;
-		brelse(sbi->s_cat_root->object.o_bh);
-		*(sbi->s_cat_root) = vol_trans->t_old_cat_root;
-		vol_trans->t_old_cat_root.object.o_bh = NULL;
-		vol_trans->t_old_cat_root.object.data = NULL;
-	}
-
 	sm = APFS_SM(sb);
 	if (sm) {
 		for (bmap_idx = 0; bmap_idx < sm->sm_ip_bmaps_count; ++bmap_idx) {