From 0521bd0e6bd9e77296e407d04a66a0f0f347dd77 Mon Sep 17 00:00:00 2001 From: Zach Brown Date: Wed, 26 Jun 2024 13:36:22 -0700 Subject: [PATCH] Make offline extent creation use one transaction Initially setattr_more followed the general pattern where extent manipulation might require multiple transactions if there are lots of extent items to work with. The scoutfs_data_init_offline_extent() function that creates an offline extent handled transactions itself. But in this case the call only supports adding a single offline extent. It will always use a small fixed amount of metadata and could be combined with other metadata changes in one atomic transaction. This changes scoutfs_data_init_offline_extent() to have the caller handle transactions, inode updates, etc. This lets the caller perform all the restore changes in one transaction. This interface change will then be used as we add another caller that adds a single offline extent in the same way. Signed-off-by: Zach Brown --- kmod/src/data.c | 25 +++---------------------- kmod/src/ioctl.c | 16 ++++++++-------- 2 files changed, 11 insertions(+), 30 deletions(-) diff --git a/kmod/src/data.c b/kmod/src/data.c index 54a1857b..1e48b6c3 100644 --- a/kmod/src/data.c +++ b/kmod/src/data.c @@ -1155,9 +1155,9 @@ out: * on regular files with no data extents. It's used to restore a file * with an offline extent which can then trigger staging. * - * The caller has taken care of locking the inode. We're updating the - * inode offline count as we create the offline extent so we take care - * of the index locking, updating, and transaction. + * The caller must take care of cluster locking, transactions, inode + * updates, and index updates (so that they can atomically make this + * change along with other metadata changes). */ int scoutfs_data_init_offline_extent(struct inode *inode, u64 size, struct scoutfs_lock *lock) @@ -1171,7 +1171,6 @@ int scoutfs_data_init_offline_extent(struct inode *inode, u64 size, .lock = lock, }; const u64 count = DIV_ROUND_UP(size, SCOUTFS_BLOCK_SM_SIZE); - LIST_HEAD(ind_locks); u64 on; u64 off; int ret; @@ -1184,28 +1183,10 @@ int scoutfs_data_init_offline_extent(struct inode *inode, u64 size, goto out; } - /* we're updating meta_seq with offline block count */ - ret = scoutfs_inode_index_lock_hold(inode, &ind_locks, false, true); - if (ret < 0) - goto out; - - ret = scoutfs_dirty_inode_item(inode, lock); - if (ret < 0) - goto unlock; - down_write(&si->extent_sem); ret = scoutfs_ext_insert(sb, &data_ext_ops, &args, 0, count, 0, SEF_OFFLINE); up_write(&si->extent_sem); - if (ret < 0) - goto unlock; - - scoutfs_update_inode_item(inode, lock, &ind_locks); - -unlock: - scoutfs_release_trans(sb); - scoutfs_inode_index_unlock(sb, &ind_locks); - ret = 0; out: return ret; } diff --git a/kmod/src/ioctl.c b/kmod/src/ioctl.c index 6bf70403..885ad330 100644 --- a/kmod/src/ioctl.c +++ b/kmod/src/ioctl.c @@ -669,19 +669,18 @@ static long scoutfs_ioc_setattr_more(struct file *file, unsigned long arg) goto unlock; } - /* create offline extents in potentially many transactions */ + /* setting only so we don't see 0 data seq with nonzero data_version */ + set_data_seq = sm.data_version != 0 ? true : false; + ret = scoutfs_inode_index_lock_hold(inode, &ind_locks, set_data_seq, true); + if (ret) + goto unlock; + if (sm.flags & SCOUTFS_IOC_SETATTR_MORE_OFFLINE) { ret = scoutfs_data_init_offline_extent(inode, sm.i_size, lock); if (ret) - goto unlock; + goto release; } - /* setting only so we don't see 0 data seq with nonzero data_version */ - set_data_seq = sm.data_version != 0 ? true : false; - ret = scoutfs_inode_index_lock_hold(inode, &ind_locks, set_data_seq, false); - if (ret) - goto unlock; - if (sm.data_version) scoutfs_inode_set_data_version(inode, sm.data_version); if (sm.i_size) @@ -694,6 +693,7 @@ static long scoutfs_ioc_setattr_more(struct file *file, unsigned long arg) scoutfs_update_inode_item(inode, lock, &ind_locks); ret = 0; +release: scoutfs_release_trans(sb); unlock: scoutfs_inode_index_unlock(sb, &ind_locks);