2024-05-20 05:52:18

by harshad shirwadkar

[permalink] [raw]
Subject: [PATCH 00/10] Ext4 fast commit performance patch series

This is the V5 of the patch series. This patch series supersedes the patch
series "ext4: remove journal barrier during fast commit" sent in July 2022[1].
Except following three patches all the other patches are newly introduced in
this series.

* ext4: convert i_fc_lock to spinlock
* ext4: for committing inode, make ext4_fc_track_inode wait
* ext4: drop i_fc_updates from inode fc info

This patchset does not introduce any on-disk format and / or user visible API
change. This patchset reworks fast commit commit path improve overall
performance of the commit path. Following optimizations have been added in this
series:

* Avoid having to lock the journal throughout the fast commit.
* Remove tracking of open handles per inode.
* Avoid issuing cache flushes when fast commits are contained within a single
FUA writes and there is no data that needs flushing.
* Temporarily lift committing thread's priority to match that of the journal
thread to reduce scheduling delays.

With the changes introduced in this patch series, now the commit path for fast
commits is as follows:

1. Lock the journal by calling jbd2_journal_lock_updates_no_rsv(). This ensures
that all the exsiting handles finish and no new handles can start.
2. Mark all the fast commit eligible inodes as undergoing fast commit
by setting "EXT4_STATE_FC_COMMITTING" state.
3. Unlock the journal by calling jbd2_journal_unlock_updates. This allows
starting of new handles. If new handles try to start an update on any of the
inodes that are being committed, ext4_fc_track_inode() will block until
those inodes have finished the fast commit.
4. Submit data buffers of all the committing inodes.
5. Wait for [4] to complete.
6. Commit all the directory entry updates in the fast commit space.
7. Commit all the changed inodes in the fast commit space and clear
"EXT4_STATE_FC_COMMITTING" for all the inodes.
8. Write tail tag to ensure atomicity of commits.

(The above flow has been documented in the code as well)

I verified that the patch series introduces no regressions in "log" groups when
"fast_commit" feature is enabled.

Also, we have a paper on fast commits in USENIX ATC 2024 this year which should
become available on the website[2] in a few months.

[1] https://lwn.net/Articles/902022/
[2] https://www.usenix.org/conference/atc24

Harshad Shirwadkar (10):
ext4: convert i_fc_lock to spinlock
ext4: for committing inode, make ext4_fc_track_inode wait
ext4: mark inode dirty before grabbing i_data_sem in ext4_setattr
ext4: rework fast commit commit path
ext4: drop i_fc_updates from inode fc info
ext4: update code documentation
ext4: add nolock mode to ext4_map_blocks()
ext4: introduce selective flushing in fast commit
ext4: temporarily elevate commit thread priority
ext4: make fast commit ineligible on ext4_reserve_inode_write failure

fs/ext4/ext4.h | 29 ++--
fs/ext4/ext4_jbd2.h | 20 +--
fs/ext4/fast_commit.c | 279 ++++++++++++++++++++----------------
fs/ext4/fast_commit.h | 1 +
fs/ext4/inline.c | 3 +
fs/ext4/inode.c | 53 ++++---
fs/ext4/super.c | 7 +-
fs/jbd2/journal.c | 2 -
fs/jbd2/transaction.c | 41 ++++--
include/linux/jbd2.h | 1 +
include/trace/events/ext4.h | 7 +-
11 files changed, 265 insertions(+), 178 deletions(-)

--
2.45.0.rc1.225.g2a3ae87e7f-goog



2024-05-20 05:52:18

by harshad shirwadkar

[permalink] [raw]
Subject: [PATCH 01/10] ext4: convert i_fc_lock to spinlock

Convert ext4_inode_info->i_fc_lock to spinlock to avoid sleeping
in invalid contexts.

Signed-off-by: Harshad Shirwadkar <[email protected]>
---
fs/ext4/ext4.h | 7 +++++--
fs/ext4/fast_commit.c | 24 +++++++++++-------------
fs/ext4/super.c | 2 +-
3 files changed, 17 insertions(+), 16 deletions(-)

diff --git a/fs/ext4/ext4.h b/fs/ext4/ext4.h
index 983dad8c07ec..611b8c80d99c 100644
--- a/fs/ext4/ext4.h
+++ b/fs/ext4/ext4.h
@@ -1062,8 +1062,11 @@ struct ext4_inode_info {
/* Fast commit wait queue for this inode */
wait_queue_head_t i_fc_wait;

- /* Protect concurrent accesses on i_fc_lblk_start, i_fc_lblk_len */
- struct mutex i_fc_lock;
+ /*
+ * Protect concurrent accesses on i_fc_lblk_start, i_fc_lblk_len
+ * and inode's EXT4_FC_STATE_COMMITTING state bit.
+ */
+ spinlock_t i_fc_lock;

/*
* i_disksize keeps track of what the inode size is ON DISK, not
diff --git a/fs/ext4/fast_commit.c b/fs/ext4/fast_commit.c
index 87c009e0c59a..a1aadebfcd66 100644
--- a/fs/ext4/fast_commit.c
+++ b/fs/ext4/fast_commit.c
@@ -382,7 +382,7 @@ static int ext4_fc_track_template(
int ret;

tid = handle->h_transaction->t_tid;
- mutex_lock(&ei->i_fc_lock);
+ spin_lock(&ei->i_fc_lock);
if (tid == ei->i_sync_tid) {
update = true;
} else {
@@ -390,7 +390,7 @@ static int ext4_fc_track_template(
ei->i_sync_tid = tid;
}
ret = __fc_track_fn(inode, args, update);
- mutex_unlock(&ei->i_fc_lock);
+ spin_unlock(&ei->i_fc_lock);

if (!enqueue)
return ret;
@@ -424,19 +424,19 @@ static int __track_dentry_update(struct inode *inode, void *arg, bool update)
struct super_block *sb = inode->i_sb;
struct ext4_sb_info *sbi = EXT4_SB(sb);

- mutex_unlock(&ei->i_fc_lock);
+ spin_unlock(&ei->i_fc_lock);

if (IS_ENCRYPTED(dir)) {
ext4_fc_mark_ineligible(sb, EXT4_FC_REASON_ENCRYPTED_FILENAME,
NULL);
- mutex_lock(&ei->i_fc_lock);
+ spin_lock(&ei->i_fc_lock);
return -EOPNOTSUPP;
}

node = kmem_cache_alloc(ext4_fc_dentry_cachep, GFP_NOFS);
if (!node) {
ext4_fc_mark_ineligible(sb, EXT4_FC_REASON_NOMEM, NULL);
- mutex_lock(&ei->i_fc_lock);
+ spin_lock(&ei->i_fc_lock);
return -ENOMEM;
}

@@ -448,7 +448,7 @@ static int __track_dentry_update(struct inode *inode, void *arg, bool update)
if (!node->fcd_name.name) {
kmem_cache_free(ext4_fc_dentry_cachep, node);
ext4_fc_mark_ineligible(sb, EXT4_FC_REASON_NOMEM, NULL);
- mutex_lock(&ei->i_fc_lock);
+ spin_lock(&ei->i_fc_lock);
return -ENOMEM;
}
memcpy((u8 *)node->fcd_name.name, dentry->d_name.name,
@@ -482,7 +482,7 @@ static int __track_dentry_update(struct inode *inode, void *arg, bool update)
list_add_tail(&node->fcd_dilist, &ei->i_fc_dilist);
}
spin_unlock(&sbi->s_fc_lock);
- mutex_lock(&ei->i_fc_lock);
+ spin_lock(&ei->i_fc_lock);

return 0;
}
@@ -614,10 +614,8 @@ static int __track_range(struct inode *inode, void *arg, bool update)
struct __track_range_args *__arg =
(struct __track_range_args *)arg;

- if (inode->i_ino < EXT4_FIRST_INO(inode->i_sb)) {
- ext4_debug("Special inode %ld being modified\n", inode->i_ino);
+ if (inode->i_ino < EXT4_FIRST_INO(inode->i_sb))
return -ECANCELED;
- }

oldstart = ei->i_fc_lblk_start;

@@ -896,15 +894,15 @@ static int ext4_fc_write_inode_data(struct inode *inode, u32 *crc)
struct ext4_extent *ex;
int ret;

- mutex_lock(&ei->i_fc_lock);
+ spin_lock(&ei->i_fc_lock);
if (ei->i_fc_lblk_len == 0) {
- mutex_unlock(&ei->i_fc_lock);
+ spin_unlock(&ei->i_fc_lock);
return 0;
}
old_blk_size = ei->i_fc_lblk_start;
new_blk_size = ei->i_fc_lblk_start + ei->i_fc_lblk_len - 1;
ei->i_fc_lblk_len = 0;
- mutex_unlock(&ei->i_fc_lock);
+ spin_unlock(&ei->i_fc_lock);

cur_lblk_off = old_blk_size;
ext4_debug("will try writing %d to %d for inode %ld\n",
diff --git a/fs/ext4/super.c b/fs/ext4/super.c
index f9a4a4e89dac..77173ec91e49 100644
--- a/fs/ext4/super.c
+++ b/fs/ext4/super.c
@@ -1436,7 +1436,7 @@ static struct inode *ext4_alloc_inode(struct super_block *sb)
atomic_set(&ei->i_unwritten, 0);
INIT_WORK(&ei->i_rsv_conversion_work, ext4_end_io_rsv_work);
ext4_fc_init_inode(&ei->vfs_inode);
- mutex_init(&ei->i_fc_lock);
+ spin_lock_init(&ei->i_fc_lock);
return &ei->vfs_inode;
}

--
2.45.0.rc1.225.g2a3ae87e7f-goog


2024-05-20 05:52:22

by harshad shirwadkar

[permalink] [raw]
Subject: [PATCH 03/10] ext4: mark inode dirty before grabbing i_data_sem in ext4_setattr

Mark inode dirty first and then grab i_data_sem in ext4_setattr().

Signed-off-by: Harshad Shirwadkar <[email protected]>
---
fs/ext4/inode.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c
index aa6440992a55..26b9d3076536 100644
--- a/fs/ext4/inode.c
+++ b/fs/ext4/inode.c
@@ -5410,12 +5410,12 @@ int ext4_setattr(struct mnt_idmap *idmap, struct dentry *dentry,
(attr->ia_size > 0 ? attr->ia_size - 1 : 0) >>
inode->i_sb->s_blocksize_bits);

- down_write(&EXT4_I(inode)->i_data_sem);
- old_disksize = EXT4_I(inode)->i_disksize;
- EXT4_I(inode)->i_disksize = attr->ia_size;
rc = ext4_mark_inode_dirty(handle, inode);
if (!error)
error = rc;
+ down_write(&EXT4_I(inode)->i_data_sem);
+ EXT4_I(inode)->i_disksize = attr->ia_size;
+
/*
* We have to update i_size under i_data_sem together
* with i_disksize to avoid races with writeback code
--
2.45.0.rc1.225.g2a3ae87e7f-goog


2024-05-20 05:52:25

by harshad shirwadkar

[permalink] [raw]
Subject: [PATCH 04/10] ext4: rework fast commit commit path

This patch reworks fast commit's commit path to remove locking the
journal for the entire duration of a fast commit. Instead, we only lock
the journal while marking all the eligible inodes as "committing". This
allows handles to make progress in parallel with the fast commit.

Signed-off-by: Harshad Shirwadkar <[email protected]>
---
fs/ext4/fast_commit.c | 75 ++++++++++++++++++++++++++++---------------
fs/jbd2/journal.c | 2 --
fs/jbd2/transaction.c | 41 +++++++++++++++--------
include/linux/jbd2.h | 1 +
4 files changed, 79 insertions(+), 40 deletions(-)

diff --git a/fs/ext4/fast_commit.c b/fs/ext4/fast_commit.c
index fa93ce399440..3aca5b20aac5 100644
--- a/fs/ext4/fast_commit.c
+++ b/fs/ext4/fast_commit.c
@@ -290,20 +290,30 @@ void ext4_fc_del(struct inode *inode)
if (ext4_fc_disabled(inode->i_sb))
return;

-restart:
spin_lock(&EXT4_SB(inode->i_sb)->s_fc_lock);
if (list_empty(&ei->i_fc_list) && list_empty(&ei->i_fc_dilist)) {
spin_unlock(&EXT4_SB(inode->i_sb)->s_fc_lock);
return;
}

- if (ext4_test_inode_state(inode, EXT4_STATE_FC_COMMITTING)) {
- ext4_fc_wait_committing_inode(inode);
- goto restart;
- }
-
- if (!list_empty(&ei->i_fc_list))
- list_del_init(&ei->i_fc_list);
+ /*
+ * Since ext4_fc_del is called from ext4_evict_inode while having a
+ * handle open, there is no need for us to wait here even if a fast
+ * commit is going on. That is because, if this inode is being
+ * committed, ext4_mark_inode_dirty would have waited for inode commit
+ * operation to finish before we come here. So, by the time we come
+ * here, inode's EXT4_STATE_FC_COMMITTING would have been cleared. So,
+ * we shouldn't see EXT4_STATE_FC_COMMITTING to be set on this inode
+ * here.
+ *
+ * We may come here without any handles open in the "no_delete" case of
+ * ext4_evict_inode as well. However, if that happens, we first mark the
+ * file system as fast commit ineligible anyway. So, even in that case,
+ * it is okay to remove the inode from the fc list.
+ */
+ WARN_ON(ext4_test_inode_state(inode, EXT4_STATE_FC_COMMITTING)
+ && !ext4_test_mount_flag(inode->i_sb, EXT4_MF_FC_INELIGIBLE));
+ list_del_init(&ei->i_fc_list);

/*
* Since this inode is getting removed, let's also remove all FC
@@ -326,8 +336,6 @@ void ext4_fc_del(struct inode *inode)
fc_dentry->fcd_name.len > DNAME_INLINE_LEN)
kfree(fc_dentry->fcd_name.name);
kmem_cache_free(ext4_fc_dentry_cachep, fc_dentry);
-
- return;
}

/*
@@ -999,19 +1007,6 @@ static int ext4_fc_submit_inode_data_all(journal_t *journal)

spin_lock(&sbi->s_fc_lock);
list_for_each_entry(ei, &sbi->s_fc_q[FC_Q_MAIN], i_fc_list) {
- ext4_set_inode_state(&ei->vfs_inode, EXT4_STATE_FC_COMMITTING);
- while (atomic_read(&ei->i_fc_updates)) {
- DEFINE_WAIT(wait);
-
- prepare_to_wait(&ei->i_fc_wait, &wait,
- TASK_UNINTERRUPTIBLE);
- if (atomic_read(&ei->i_fc_updates)) {
- spin_unlock(&sbi->s_fc_lock);
- schedule();
- spin_lock(&sbi->s_fc_lock);
- }
- finish_wait(&ei->i_fc_wait, &wait);
- }
spin_unlock(&sbi->s_fc_lock);
ret = jbd2_submit_inode_data(journal, ei->jinode);
if (ret)
@@ -1124,6 +1119,20 @@ static int ext4_fc_perform_commit(journal_t *journal)
int ret = 0;
u32 crc = 0;

+ /*
+ * Wait for all the handles of the current transaction to complete
+ * and then lock the journal. Since this is essentially the commit
+ * path, we don't need to wait for reserved handles.
+ */
+ jbd2_journal_lock_updates_no_rsv(journal);
+ spin_lock(&sbi->s_fc_lock);
+ list_for_each_entry(iter, &sbi->s_fc_q[FC_Q_MAIN], i_fc_list) {
+ ext4_set_inode_state(&iter->vfs_inode,
+ EXT4_STATE_FC_COMMITTING);
+ }
+ spin_unlock(&sbi->s_fc_lock);
+ jbd2_journal_unlock_updates(journal);
+
ret = ext4_fc_submit_inode_data_all(journal);
if (ret)
return ret;
@@ -1174,6 +1183,18 @@ static int ext4_fc_perform_commit(journal_t *journal)
ret = ext4_fc_write_inode(inode, &crc);
if (ret)
goto out;
+ ext4_clear_inode_state(inode, EXT4_STATE_FC_COMMITTING);
+ /*
+ * Make sure clearing of EXT4_STATE_FC_COMMITTING is
+ * visible before we send the wakeup. Pairs with implicit
+ * barrier in prepare_to_wait() in ext4_fc_track_inode().
+ */
+ smp_mb();
+#if (BITS_PER_LONG < 64)
+ wake_up_bit(&iter->i_state_flags, EXT4_STATE_FC_COMMITTING);
+#else
+ wake_up_bit(&iter->i_flags, EXT4_STATE_FC_COMMITTING);
+#endif
spin_lock(&sbi->s_fc_lock);
}
spin_unlock(&sbi->s_fc_lock);
@@ -1311,13 +1332,17 @@ static void ext4_fc_cleanup(journal_t *journal, int full, tid_t tid)
spin_lock(&sbi->s_fc_lock);
list_for_each_entry_safe(iter, iter_n, &sbi->s_fc_q[FC_Q_MAIN],
i_fc_list) {
- list_del_init(&iter->i_fc_list);
ext4_clear_inode_state(&iter->vfs_inode,
EXT4_STATE_FC_COMMITTING);
if (iter->i_sync_tid <= tid)
ext4_fc_reset_inode(&iter->vfs_inode);
- /* Make sure EXT4_STATE_FC_COMMITTING bit is clear */
+ /*
+ * Make sure clearing of EXT4_STATE_FC_COMMITTING is
+ * visible before we send the wakeup. Pairs with implicit
+ * barrier in prepare_to_wait() in ext4_fc_track_inode().
+ */
smp_mb();
+ list_del_init(&iter->i_fc_list);
#if (BITS_PER_LONG < 64)
wake_up_bit(&iter->i_state_flags, EXT4_STATE_FC_COMMITTING);
#else
diff --git a/fs/jbd2/journal.c b/fs/jbd2/journal.c
index b6c114c11b97..f243552eadad 100644
--- a/fs/jbd2/journal.c
+++ b/fs/jbd2/journal.c
@@ -742,7 +742,6 @@ int jbd2_fc_begin_commit(journal_t *journal, tid_t tid)
}
journal->j_flags |= JBD2_FAST_COMMIT_ONGOING;
write_unlock(&journal->j_state_lock);
- jbd2_journal_lock_updates(journal);

return 0;
}
@@ -754,7 +753,6 @@ EXPORT_SYMBOL(jbd2_fc_begin_commit);
*/
static int __jbd2_fc_end_commit(journal_t *journal, tid_t tid, bool fallback)
{
- jbd2_journal_unlock_updates(journal);
if (journal->j_fc_cleanup_callback)
journal->j_fc_cleanup_callback(journal, 0, tid);
write_lock(&journal->j_state_lock);
diff --git a/fs/jbd2/transaction.c b/fs/jbd2/transaction.c
index cb0b8d6fc0c6..4361e5c56490 100644
--- a/fs/jbd2/transaction.c
+++ b/fs/jbd2/transaction.c
@@ -865,25 +865,15 @@ void jbd2_journal_wait_updates(journal_t *journal)
}
}

-/**
- * jbd2_journal_lock_updates () - establish a transaction barrier.
- * @journal: Journal to establish a barrier on.
- *
- * This locks out any further updates from being started, and blocks
- * until all existing updates have completed, returning only once the
- * journal is in a quiescent state with no updates running.
- *
- * The journal lock should not be held on entry.
- */
-void jbd2_journal_lock_updates(journal_t *journal)
+static void __jbd2_journal_lock_updates(journal_t *journal, bool wait_on_rsv)
{
jbd2_might_wait_for_commit(journal);

write_lock(&journal->j_state_lock);
++journal->j_barrier_count;

- /* Wait until there are no reserved handles */
- if (atomic_read(&journal->j_reserved_credits)) {
+ if (wait_on_rsv && atomic_read(&journal->j_reserved_credits)) {
+ /* Wait until there are no reserved handles */
write_unlock(&journal->j_state_lock);
wait_event(journal->j_wait_reserved,
atomic_read(&journal->j_reserved_credits) == 0);
@@ -904,6 +894,31 @@ void jbd2_journal_lock_updates(journal_t *journal)
mutex_lock(&journal->j_barrier);
}

+/**
+ * jbd2_journal_lock_updates () - establish a transaction barrier.
+ * @journal: Journal to establish a barrier on.
+ *
+ * This locks out any further updates from being started, and blocks
+ * until all existing updates have completed, returning only once the
+ * journal is in a quiescent state with no updates running.
+ *
+ * The journal lock should not be held on entry.
+ */
+void jbd2_journal_lock_updates(journal_t *journal)
+{
+ __jbd2_journal_lock_updates(journal, true);
+}
+
+/**
+ * jbd2_journal_lock_updates_no_rsv - same as above, but doesn't
+ * wait for reserved handles to finish.
+ * @journal: Journal to establish barrier on.
+ */
+void jbd2_journal_lock_updates_no_rsv(journal_t *journal)
+{
+ __jbd2_journal_lock_updates(journal, false);
+}
+
/**
* jbd2_journal_unlock_updates () - release barrier
* @journal: Journal to release the barrier on.
diff --git a/include/linux/jbd2.h b/include/linux/jbd2.h
index 7479f64c0939..cfac287c0ad4 100644
--- a/include/linux/jbd2.h
+++ b/include/linux/jbd2.h
@@ -1531,6 +1531,7 @@ bool jbd2_journal_try_to_free_buffers(journal_t *journal, struct folio *folio);
extern int jbd2_journal_stop(handle_t *);
extern int jbd2_journal_flush(journal_t *journal, unsigned int flags);
extern void jbd2_journal_lock_updates (journal_t *);
+extern void jbd2_journal_lock_updates_no_rsv(journal_t *journal);
extern void jbd2_journal_unlock_updates (journal_t *);

void jbd2_journal_wait_updates(journal_t *);
--
2.45.0.rc1.225.g2a3ae87e7f-goog


2024-05-20 05:52:27

by harshad shirwadkar

[permalink] [raw]
Subject: [PATCH 05/10] ext4: drop i_fc_updates from inode fc info

The new logic introduced in this series does not require tracking number
of active handles open on an inode. So, drop it.

Signed-off-by: Harshad Shirwadkar <[email protected]>
Reviewed-by: Jan Kara <[email protected]>
---
fs/ext4/ext4.h | 5 ----
fs/ext4/fast_commit.c | 68 -------------------------------------------
2 files changed, 73 deletions(-)

diff --git a/fs/ext4/ext4.h b/fs/ext4/ext4.h
index 611b8c80d99c..d802040e94df 100644
--- a/fs/ext4/ext4.h
+++ b/fs/ext4/ext4.h
@@ -1056,9 +1056,6 @@ struct ext4_inode_info {
/* End of lblk range that needs to be committed in this fast commit */
ext4_lblk_t i_fc_lblk_len;

- /* Number of ongoing updates on this inode */
- atomic_t i_fc_updates;
-
/* Fast commit wait queue for this inode */
wait_queue_head_t i_fc_wait;

@@ -2903,8 +2900,6 @@ void __ext4_fc_track_create(handle_t *handle, struct inode *inode,
void ext4_fc_track_create(handle_t *handle, struct dentry *dentry);
void ext4_fc_track_inode(handle_t *handle, struct inode *inode);
void ext4_fc_mark_ineligible(struct super_block *sb, int reason, handle_t *handle);
-void ext4_fc_start_update(struct inode *inode);
-void ext4_fc_stop_update(struct inode *inode);
void ext4_fc_del(struct inode *inode);
bool ext4_fc_replay_check_excluded(struct super_block *sb, ext4_fsblk_t block);
void ext4_fc_replay_cleanup(struct super_block *sb);
diff --git a/fs/ext4/fast_commit.c b/fs/ext4/fast_commit.c
index 3aca5b20aac5..ecbbcaf78598 100644
--- a/fs/ext4/fast_commit.c
+++ b/fs/ext4/fast_commit.c
@@ -201,32 +201,6 @@ void ext4_fc_init_inode(struct inode *inode)
INIT_LIST_HEAD(&ei->i_fc_list);
INIT_LIST_HEAD(&ei->i_fc_dilist);
init_waitqueue_head(&ei->i_fc_wait);
- atomic_set(&ei->i_fc_updates, 0);
-}
-
-/* This function must be called with sbi->s_fc_lock held. */
-static void ext4_fc_wait_committing_inode(struct inode *inode)
-__releases(&EXT4_SB(inode->i_sb)->s_fc_lock)
-{
- wait_queue_head_t *wq;
- struct ext4_inode_info *ei = EXT4_I(inode);
-
-#if (BITS_PER_LONG < 64)
- DEFINE_WAIT_BIT(wait, &ei->i_state_flags,
- EXT4_STATE_FC_COMMITTING);
- wq = bit_waitqueue(&ei->i_state_flags,
- EXT4_STATE_FC_COMMITTING);
-#else
- DEFINE_WAIT_BIT(wait, &ei->i_flags,
- EXT4_STATE_FC_COMMITTING);
- wq = bit_waitqueue(&ei->i_flags,
- EXT4_STATE_FC_COMMITTING);
-#endif
- lockdep_assert_held(&EXT4_SB(inode->i_sb)->s_fc_lock);
- prepare_to_wait(wq, &wait.wq_entry, TASK_UNINTERRUPTIBLE);
- spin_unlock(&EXT4_SB(inode->i_sb)->s_fc_lock);
- schedule();
- finish_wait(wq, &wait.wq_entry);
}

static bool ext4_fc_disabled(struct super_block *sb)
@@ -235,48 +209,6 @@ static bool ext4_fc_disabled(struct super_block *sb)
(EXT4_SB(sb)->s_mount_state & EXT4_FC_REPLAY));
}

-/*
- * Inform Ext4's fast about start of an inode update
- *
- * This function is called by the high level call VFS callbacks before
- * performing any inode update. This function blocks if there's an ongoing
- * fast commit on the inode in question.
- */
-void ext4_fc_start_update(struct inode *inode)
-{
- struct ext4_inode_info *ei = EXT4_I(inode);
-
- if (ext4_fc_disabled(inode->i_sb))
- return;
-
-restart:
- spin_lock(&EXT4_SB(inode->i_sb)->s_fc_lock);
- if (list_empty(&ei->i_fc_list))
- goto out;
-
- if (ext4_test_inode_state(inode, EXT4_STATE_FC_COMMITTING)) {
- ext4_fc_wait_committing_inode(inode);
- goto restart;
- }
-out:
- atomic_inc(&ei->i_fc_updates);
- spin_unlock(&EXT4_SB(inode->i_sb)->s_fc_lock);
-}
-
-/*
- * Stop inode update and wake up waiting fast commits if any.
- */
-void ext4_fc_stop_update(struct inode *inode)
-{
- struct ext4_inode_info *ei = EXT4_I(inode);
-
- if (ext4_fc_disabled(inode->i_sb))
- return;
-
- if (atomic_dec_and_test(&ei->i_fc_updates))
- wake_up_all(&ei->i_fc_wait);
-}
-
/*
* Remove inode from fast commit list. If the inode is being committed
* we wait until inode commit is done.
--
2.45.0.rc1.225.g2a3ae87e7f-goog


2024-05-20 05:52:29

by harshad shirwadkar

[permalink] [raw]
Subject: [PATCH 06/10] ext4: update code documentation

This patch updates code documentation to reflect the commit path changes
made in this series.

Signed-off-by: Harshad Shirwadkar <[email protected]>
Reviewed-by: Jan Kara <[email protected]>
---
fs/ext4/fast_commit.c | 39 ++++++++++++++++++++++++++-------------
1 file changed, 26 insertions(+), 13 deletions(-)

diff --git a/fs/ext4/fast_commit.c b/fs/ext4/fast_commit.c
index ecbbcaf78598..b81b0292aa59 100644
--- a/fs/ext4/fast_commit.c
+++ b/fs/ext4/fast_commit.c
@@ -49,14 +49,21 @@
* that need to be committed during a fast commit in another in memory queue of
* inodes. During the commit operation, we commit in the following order:
*
- * [1] Lock inodes for any further data updates by setting COMMITTING state
- * [2] Submit data buffers of all the inodes
- * [3] Wait for [2] to complete
- * [4] Commit all the directory entry updates in the fast commit space
- * [5] Commit all the changed inode structures
- * [6] Write tail tag (this tag ensures the atomicity, please read the following
+ * [1] Lock the journal by calling jbd2_journal_lock_updates. This ensures that
+ * all the exsiting handles finish and no new handles can start.
+ * [2] Mark all the fast commit eligible inodes as undergoing fast commit
+ * by setting "EXT4_STATE_FC_COMMITTING" state.
+ * [3] Unlock the journal by calling jbd2_journal_unlock_updates. This allows
+ * starting of new handles. If new handles try to start an update on
+ * any of the inodes that are being committed, ext4_fc_track_inode()
+ * will block until those inodes have finished the fast commit.
+ * [4] Submit data buffers of all the committing inodes.
+ * [5] Wait for [4] to complete.
+ * [6] Commit all the directory entry updates in the fast commit space.
+ * [7] Commit all the changed inodes in the fast commit space and clear
+ * "EXT4_STATE_FC_COMMITTING" for these inodes.
+ * [8] Write tail tag (this tag ensures the atomicity, please read the following
* section for more details).
- * [7] Wait for [4], [5] and [6] to complete.
*
* All the inode updates must call ext4_fc_start_update() before starting an
* update. If such an ongoing update is present, fast commit waits for it to
@@ -142,6 +149,13 @@
* similarly. Thus, by converting a non-idempotent procedure into a series of
* idempotent outcomes, fast commits ensured idempotence during the replay.
*
+ * Locking
+ * -------
+ * sbi->s_fc_lock protects the fast commit inodes queue and the fast commit
+ * dentry queue. ei->i_fc_lock protects the fast commit related info in a given
+ * inode. Most of the code avoids acquiring both the locks, but if one must do
+ * that then sbi->s_fc_lock must be acquired before ei->i_fc_lock.
+ *
* TODOs
* -----
*
@@ -156,13 +170,12 @@
* fast commit recovery even if that area is invalidated by later full
* commits.
*
- * 1) Fast commit's commit path locks the entire file system during fast
- * commit. This has significant performance penalty. Instead of that, we
- * should use ext4_fc_start/stop_update functions to start inode level
- * updates from ext4_journal_start/stop. Once we do that we can drop file
- * system locking during commit path.
+ * 1) Handle more ineligible cases.
*
- * 2) Handle more ineligible cases.
+ * 2) Change ext4_fc_commit() to lookup logical to physical mapping using extent
+ * status tree. This would get rid of the need to call ext4_fc_track_inode()
+ * before acquiring i_data_sem. To do that we would need to ensure that
+ * modified extents from the extent status tree are not evicted from memory.
*/

#include <trace/events/ext4.h>
--
2.45.0.rc1.225.g2a3ae87e7f-goog


2024-05-20 05:52:32

by harshad shirwadkar

[permalink] [raw]
Subject: [PATCH 07/10] ext4: add nolock mode to ext4_map_blocks()

Add nolock flag to ext4_map_blocks() which skips grabbing
i_data_sem in ext4_map_blocks. In FC commit path, we first
mark the inode as committing and thereby prevent any mutations
on it. Thus, it should be safe to call ext4_map_blocks()
without i_data_sem in this case. This is a workaround to
the problem mentioned in RFC V4 version cover letter[1] of this
patch series which pointed out that there is in incosistency between
ext4_map_blocks() behavior when EXT4_GET_BLOCKS_CACHED_NOWAIT is
passed. This patch gets rid of the need to call ext4_map_blocks()
with EXT4_GET_BLOCKS_CACHED_NOWAIT and instead call it with
EXT4_GET_BLOCKS_NOLOCK. I verified that generic/311 which failed
in cached_nowait mode passes with nolock mode.

[1] https://lwn.net/Articles/902022/

Signed-off-by: Harshad Shirwadkar <[email protected]>
---
fs/ext4/ext4.h | 1 +
fs/ext4/fast_commit.c | 16 ++++++++--------
fs/ext4/inode.c | 14 ++++++++++++--
3 files changed, 21 insertions(+), 10 deletions(-)

diff --git a/fs/ext4/ext4.h b/fs/ext4/ext4.h
index d802040e94df..196c513f82dd 100644
--- a/fs/ext4/ext4.h
+++ b/fs/ext4/ext4.h
@@ -720,6 +720,7 @@ enum {
#define EXT4_GET_BLOCKS_IO_SUBMIT 0x0400
/* Caller is in the atomic contex, find extent if it has been cached */
#define EXT4_GET_BLOCKS_CACHED_NOWAIT 0x0800
+#define EXT4_GET_BLOCKS_NOLOCK 0x1000

/*
* The bit position of these flags must not overlap with any of the
diff --git a/fs/ext4/fast_commit.c b/fs/ext4/fast_commit.c
index b81b0292aa59..0b7064f8dfa5 100644
--- a/fs/ext4/fast_commit.c
+++ b/fs/ext4/fast_commit.c
@@ -559,13 +559,6 @@ void ext4_fc_track_inode(handle_t *handle, struct inode *inode)
!list_empty(&ei->i_fc_list))
return;

- /*
- * If we come here, we may sleep while waiting for the inode to
- * commit. We shouldn't be holding i_data_sem in write mode when we go
- * to sleep since the commit path needs to grab the lock while
- * committing the inode.
- */
- WARN_ON(lockdep_is_held_type(&ei->i_data_sem, 1));

while (ext4_test_inode_state(inode, EXT4_STATE_FC_COMMITTING)) {
#if (BITS_PER_LONG < 64)
@@ -898,7 +891,14 @@ static int ext4_fc_write_inode_data(struct inode *inode, u32 *crc)
while (cur_lblk_off <= new_blk_size) {
map.m_lblk = cur_lblk_off;
map.m_len = new_blk_size - cur_lblk_off + 1;
- ret = ext4_map_blocks(NULL, inode, &map, 0);
+ /*
+ * Given that this inode is being committed,
+ * EXT4_STATE_FC_COMMITTING is already set on this inode.
+ * Which means all the mutations on the inode are paused
+ * until the commit operation is complete. Thus it is safe
+ * call ext4_map_blocks() in no lock mode.
+ */
+ ret = ext4_map_blocks(NULL, inode, &map, EXT4_GET_BLOCKS_NOLOCK);
if (ret < 0)
return -ECANCELED;

diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c
index 26b9d3076536..c6405c45970e 100644
--- a/fs/ext4/inode.c
+++ b/fs/ext4/inode.c
@@ -546,7 +546,8 @@ int ext4_map_blocks(handle_t *handle, struct inode *inode,
* Try to see if we can get the block without requesting a new
* file system block.
*/
- down_read(&EXT4_I(inode)->i_data_sem);
+ if (!(flags & EXT4_GET_BLOCKS_NOLOCK))
+ down_read(&EXT4_I(inode)->i_data_sem);
if (ext4_test_inode_flag(inode, EXT4_INODE_EXTENTS)) {
retval = ext4_ext_map_blocks(handle, inode, map, 0);
} else {
@@ -573,7 +574,15 @@ int ext4_map_blocks(handle_t *handle, struct inode *inode,
ext4_es_insert_extent(inode, map->m_lblk, map->m_len,
map->m_pblk, status);
}
- up_read((&EXT4_I(inode)->i_data_sem));
+ /*
+ * We should never call ext4_map_blocks() in nolock mode outside
+ * of fast commit path.
+ */
+ WARN_ON((flags & EXT4_GET_BLOCKS_NOLOCK) &&
+ !ext4_test_inode_state(inode,
+ EXT4_STATE_FC_COMMITTING));
+ if (!(flags & EXT4_GET_BLOCKS_NOLOCK))
+ up_read((&EXT4_I(inode)->i_data_sem));

found:
if (retval > 0 && map->m_flags & EXT4_MAP_MAPPED) {
@@ -614,6 +623,7 @@ int ext4_map_blocks(handle_t *handle, struct inode *inode,
* the write lock of i_data_sem, and call get_block()
* with create == 1 flag.
*/
+ WARN_ON((flags & EXT4_GET_BLOCKS_NOLOCK) != 0);
down_write(&EXT4_I(inode)->i_data_sem);

/*
--
2.45.0.rc1.225.g2a3ae87e7f-goog


2024-05-20 05:52:34

by harshad shirwadkar

[permalink] [raw]
Subject: [PATCH 08/10] ext4: introduce selective flushing in fast commit

With fast commits, if the entire commit is contained within a single
block and there isn't any data that needs a flush, we can avoid sending
expensive cache flush to disk. Single block metadata only fast commits
can be written using FUA to guarantee consistency.

Signed-off-by: Harshad Shirwadkar <[email protected]>
---
fs/ext4/ext4.h | 12 ++++++++++++
fs/ext4/ext4_jbd2.h | 20 ++++++++++++--------
fs/ext4/fast_commit.c | 23 ++++++++++++++++++-----
3 files changed, 42 insertions(+), 13 deletions(-)

diff --git a/fs/ext4/ext4.h b/fs/ext4/ext4.h
index 196c513f82dd..3721daea2890 100644
--- a/fs/ext4/ext4.h
+++ b/fs/ext4/ext4.h
@@ -1744,6 +1744,13 @@ struct ext4_sb_info {
*/
struct list_head s_fc_dentry_q[2]; /* directory entry updates */
unsigned int s_fc_bytes;
+
+ /*
+ * This flag indicates whether a full flush is needed on
+ * next fast commit.
+ */
+ int fc_flush_required;
+
/*
* Main fast commit lock. This lock protects accesses to the
* following fields:
@@ -2905,6 +2912,11 @@ void ext4_fc_del(struct inode *inode);
bool ext4_fc_replay_check_excluded(struct super_block *sb, ext4_fsblk_t block);
void ext4_fc_replay_cleanup(struct super_block *sb);
int ext4_fc_commit(journal_t *journal, tid_t commit_tid);
+static inline void ext4_fc_mark_needs_flush(struct super_block *sb)
+{
+ EXT4_SB(sb)->fc_flush_required = 1;
+}
+
int __init ext4_fc_init_dentry_cache(void);
void ext4_fc_destroy_dentry_cache(void);
int ext4_fc_record_regions(struct super_block *sb, int ino,
diff --git a/fs/ext4/ext4_jbd2.h b/fs/ext4/ext4_jbd2.h
index 0c77697d5e90..e3a4f5c49b6e 100644
--- a/fs/ext4/ext4_jbd2.h
+++ b/fs/ext4/ext4_jbd2.h
@@ -420,19 +420,23 @@ static inline int ext4_journal_force_commit(journal_t *journal)
static inline int ext4_jbd2_inode_add_write(handle_t *handle,
struct inode *inode, loff_t start_byte, loff_t length)
{
- if (ext4_handle_valid(handle))
- return jbd2_journal_inode_ranged_write(handle,
- EXT4_I(inode)->jinode, start_byte, length);
- return 0;
+ if (!ext4_handle_valid(handle))
+ return 0;
+
+ ext4_fc_mark_needs_flush(inode->i_sb);
+ return jbd2_journal_inode_ranged_write(handle,
+ EXT4_I(inode)->jinode, start_byte, length);
}

static inline int ext4_jbd2_inode_add_wait(handle_t *handle,
struct inode *inode, loff_t start_byte, loff_t length)
{
- if (ext4_handle_valid(handle))
- return jbd2_journal_inode_ranged_wait(handle,
- EXT4_I(inode)->jinode, start_byte, length);
- return 0;
+ if (!ext4_handle_valid(handle))
+ return 0;
+
+ ext4_fc_mark_needs_flush(inode->i_sb);
+ return jbd2_journal_inode_ranged_wait(handle,
+ EXT4_I(inode)->jinode, start_byte, length);
}

static inline void ext4_update_inode_fsync_trans(handle_t *handle,
diff --git a/fs/ext4/fast_commit.c b/fs/ext4/fast_commit.c
index 0b7064f8dfa5..35c89bee452c 100644
--- a/fs/ext4/fast_commit.c
+++ b/fs/ext4/fast_commit.c
@@ -638,11 +638,24 @@ void ext4_fc_track_range(handle_t *handle, struct inode *inode, ext4_lblk_t star
static void ext4_fc_submit_bh(struct super_block *sb, bool is_tail)
{
blk_opf_t write_flags = REQ_SYNC;
- struct buffer_head *bh = EXT4_SB(sb)->s_fc_bh;
+ struct ext4_sb_info *sbi = EXT4_SB(sb);
+ struct buffer_head *bh = sbi->s_fc_bh;
+ int old = 1, new = 0;
+
+ if (!is_tail) {
+ /*
+ * This commit has at least 1 non-tail block,
+ * thus FLUSH is required.
+ */
+ ext4_fc_mark_needs_flush(sb);
+ } else {
+ /* Use cmpxchg to ensure that no flush requrest is lost. */
+ if (cmpxchg(&sbi->fc_flush_required, old, new))
+ /* Old value was 1, so request a flush. */
+ write_flags |= REQ_PREFLUSH;
+ write_flags |= REQ_FUA;
+ }

- /* Add REQ_FUA | REQ_PREFLUSH only its tail */
- if (test_opt(sb, BARRIER) && is_tail)
- write_flags |= REQ_FUA | REQ_PREFLUSH;
lock_buffer(bh);
set_buffer_dirty(bh);
set_buffer_uptodate(bh);
@@ -1090,7 +1103,7 @@ static int ext4_fc_perform_commit(journal_t *journal)
* If file system device is different from journal device, issue a cache
* flush before we start writing fast commit blocks.
*/
- if (journal->j_fs_dev != journal->j_dev)
+ if (sbi->fc_flush_required && journal->j_fs_dev != journal->j_dev)
blkdev_issue_flush(journal->j_fs_dev);

blk_start_plug(&plug);
--
2.45.0.rc1.225.g2a3ae87e7f-goog


2024-05-20 05:52:36

by harshad shirwadkar

[permalink] [raw]
Subject: [PATCH 02/10] ext4: for committing inode, make ext4_fc_track_inode wait

If the inode that's being requested to track using ext4_fc_track_inode
is being committed, then wait until the inode finishes the
commit. Also, add calls to ext4_fc_track_inode at the right places.

With this patch, now calling ext4_reserve_inode_write() results in
inode being tracked for next fast commit. A subtle lock ordering
requirement with i_data_sem (which is documented in the code) requires
that ext4_fc_track_inode() be called before grabbing i_data_sem. So,
this patch also adds explicit ext4_fc_track_inode() calls in places
where i_data_sem grabbed.

Signed-off-by: Harshad Shirwadkar <[email protected]>
---
fs/ext4/fast_commit.c | 34 ++++++++++++++++++++++++++++++++++
fs/ext4/inline.c | 3 +++
fs/ext4/inode.c | 4 ++++
3 files changed, 41 insertions(+)

diff --git a/fs/ext4/fast_commit.c b/fs/ext4/fast_commit.c
index a1aadebfcd66..fa93ce399440 100644
--- a/fs/ext4/fast_commit.c
+++ b/fs/ext4/fast_commit.c
@@ -581,6 +581,8 @@ static int __track_inode(struct inode *inode, void *arg, bool update)

void ext4_fc_track_inode(handle_t *handle, struct inode *inode)
{
+ struct ext4_inode_info *ei = EXT4_I(inode);
+ wait_queue_head_t *wq;
int ret;

if (S_ISDIR(inode->i_mode))
@@ -598,6 +600,38 @@ void ext4_fc_track_inode(handle_t *handle, struct inode *inode)
if (ext4_test_mount_flag(inode->i_sb, EXT4_MF_FC_INELIGIBLE))
return;

+ if (!test_opt2(inode->i_sb, JOURNAL_FAST_COMMIT) ||
+ (EXT4_SB(inode->i_sb)->s_mount_state & EXT4_FC_REPLAY) ||
+ ext4_test_mount_flag(inode->i_sb, EXT4_MF_FC_INELIGIBLE) ||
+ !list_empty(&ei->i_fc_list))
+ return;
+
+ /*
+ * If we come here, we may sleep while waiting for the inode to
+ * commit. We shouldn't be holding i_data_sem in write mode when we go
+ * to sleep since the commit path needs to grab the lock while
+ * committing the inode.
+ */
+ WARN_ON(lockdep_is_held_type(&ei->i_data_sem, 1));
+
+ while (ext4_test_inode_state(inode, EXT4_STATE_FC_COMMITTING)) {
+#if (BITS_PER_LONG < 64)
+ DEFINE_WAIT_BIT(wait, &ei->i_state_flags,
+ EXT4_STATE_FC_COMMITTING);
+ wq = bit_waitqueue(&ei->i_state_flags,
+ EXT4_STATE_FC_COMMITTING);
+#else
+ DEFINE_WAIT_BIT(wait, &ei->i_flags,
+ EXT4_STATE_FC_COMMITTING);
+ wq = bit_waitqueue(&ei->i_flags,
+ EXT4_STATE_FC_COMMITTING);
+#endif
+ prepare_to_wait(wq, &wait.wq_entry, TASK_UNINTERRUPTIBLE);
+ if (ext4_test_inode_state(inode, EXT4_STATE_FC_COMMITTING))
+ schedule();
+ finish_wait(wq, &wait.wq_entry);
+ }
+
ret = ext4_fc_track_template(handle, inode, __track_inode, NULL, 1);
trace_ext4_fc_track_inode(handle, inode, ret);
}
diff --git a/fs/ext4/inline.c b/fs/ext4/inline.c
index d5bd1e3a5d36..bd5778e680c0 100644
--- a/fs/ext4/inline.c
+++ b/fs/ext4/inline.c
@@ -596,6 +596,7 @@ static int ext4_convert_inline_data_to_extent(struct address_space *mapping,
goto out;
}

+ ext4_fc_track_inode(handle, inode);
ret = ext4_destroy_inline_data_nolock(handle, inode);
if (ret)
goto out;
@@ -696,6 +697,7 @@ int ext4_try_to_write_inline_data(struct address_space *mapping,
goto convert;
}

+ ext4_fc_track_inode(handle, inode);
ret = ext4_journal_get_write_access(handle, inode->i_sb, iloc.bh,
EXT4_JTR_NONE);
if (ret)
@@ -948,6 +950,7 @@ int ext4_da_write_inline_data_begin(struct address_space *mapping,
if (ret < 0)
goto out_release_page;
}
+ ext4_fc_track_inode(handle, inode);
ret = ext4_journal_get_write_access(handle, inode->i_sb, iloc.bh,
EXT4_JTR_NONE);
if (ret)
diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c
index 4bae9ccf5fe0..aa6440992a55 100644
--- a/fs/ext4/inode.c
+++ b/fs/ext4/inode.c
@@ -607,6 +607,7 @@ int ext4_map_blocks(handle_t *handle, struct inode *inode,
*/
map->m_flags &= ~EXT4_MAP_FLAGS;

+ ext4_fc_track_inode(handle, inode);
/*
* New blocks allocate and/or writing to unwritten extent
* will possibly result in updating i_data, so we take
@@ -3978,6 +3979,7 @@ int ext4_punch_hole(struct file *file, loff_t offset, loff_t length)
if (stop_block > first_block) {
ext4_lblk_t hole_len = stop_block - first_block;

+ ext4_fc_track_inode(handle, inode);
down_write(&EXT4_I(inode)->i_data_sem);
ext4_discard_preallocations(inode);

@@ -4131,6 +4133,7 @@ int ext4_truncate(struct inode *inode)
if (err)
goto out_stop;

+ ext4_fc_track_inode(handle, inode);
down_write(&EXT4_I(inode)->i_data_sem);

ext4_discard_preallocations(inode);
@@ -5727,6 +5730,7 @@ ext4_reserve_inode_write(handle_t *handle, struct inode *inode,
brelse(iloc->bh);
iloc->bh = NULL;
}
+ ext4_fc_track_inode(handle, inode);
}
ext4_std_error(inode->i_sb, err);
return err;
--
2.45.0.rc1.225.g2a3ae87e7f-goog


2024-05-20 05:52:39

by harshad shirwadkar

[permalink] [raw]
Subject: [PATCH 09/10] ext4: temporarily elevate commit thread priority

Unlike JBD2 based full commits, there is no dedicated journal thread
for fast commits. Thus to reduce scheduling delays between IO
submission and completion, temporarily elevate the committer thread's
priority to match the configured priority of the JBD2 journal
thread.

Signed-off-by: Harshad Shirwadkar <[email protected]>
---
fs/ext4/ext4.h | 4 +++-
fs/ext4/fast_commit.c | 13 +++++++++++++
fs/ext4/super.c | 5 ++---
3 files changed, 18 insertions(+), 4 deletions(-)

diff --git a/fs/ext4/ext4.h b/fs/ext4/ext4.h
index 3721daea2890..d52df8a85271 100644
--- a/fs/ext4/ext4.h
+++ b/fs/ext4/ext4.h
@@ -2287,10 +2287,12 @@ static inline int ext4_forced_shutdown(struct super_block *sb)
#define EXT4_DEFM_NODELALLOC 0x0800

/*
- * Default journal batch times
+ * Default journal batch times and ioprio.
*/
#define EXT4_DEF_MIN_BATCH_TIME 0
#define EXT4_DEF_MAX_BATCH_TIME 15000 /* 15ms */
+#define EXT4_DEF_JOURNAL_IOPRIO (IOPRIO_PRIO_VALUE(IOPRIO_CLASS_BE, 3))
+

/*
* Minimum number of groups in a flexgroup before we separate out
diff --git a/fs/ext4/fast_commit.c b/fs/ext4/fast_commit.c
index 35c89bee452c..d354839dbf7e 100644
--- a/fs/ext4/fast_commit.c
+++ b/fs/ext4/fast_commit.c
@@ -1205,6 +1205,7 @@ int ext4_fc_commit(journal_t *journal, tid_t commit_tid)
int subtid = atomic_read(&sbi->s_fc_subtid);
int status = EXT4_FC_STATUS_OK, fc_bufs_before = 0;
ktime_t start_time, commit_time;
+ int old_ioprio, journal_ioprio;

if (!test_opt2(sb, JOURNAL_FAST_COMMIT))
return jbd2_complete_transaction(journal, commit_tid);
@@ -1242,6 +1243,16 @@ int ext4_fc_commit(journal_t *journal, tid_t commit_tid)
goto fallback;
}

+ /*
+ * Now that we know that this thread is going to do a fast commit,
+ * elevate the priority to match that of the journal thread.
+ */
+ if (journal->j_task->io_context)
+ journal_ioprio = sbi->s_journal->j_task->io_context->ioprio;
+ else
+ journal_ioprio = EXT4_DEF_JOURNAL_IOPRIO;
+ old_ioprio = get_current_ioprio();
+ set_task_ioprio(current, journal_ioprio);
fc_bufs_before = (sbi->s_fc_bytes + bsize - 1) / bsize;
ret = ext4_fc_perform_commit(journal);
if (ret < 0) {
@@ -1256,6 +1267,7 @@ int ext4_fc_commit(journal_t *journal, tid_t commit_tid)
}
atomic_inc(&sbi->s_fc_subtid);
ret = jbd2_fc_end_commit(journal);
+ set_task_ioprio(current, old_ioprio);
/*
* weight the commit time higher than the average time so we
* don't react too strongly to vast changes in the commit time
@@ -1265,6 +1277,7 @@ int ext4_fc_commit(journal_t *journal, tid_t commit_tid)
return ret;

fallback:
+ set_task_ioprio(current, old_ioprio);
ret = jbd2_fc_end_commit_fallback(journal);
ext4_fc_update_stats(sb, status, 0, 0, commit_tid);
return ret;
diff --git a/fs/ext4/super.c b/fs/ext4/super.c
index 77173ec91e49..18d9d2631559 100644
--- a/fs/ext4/super.c
+++ b/fs/ext4/super.c
@@ -1833,7 +1833,6 @@ static const struct fs_parameter_spec ext4_param_specs[] = {
{}
};

-#define DEFAULT_JOURNAL_IOPRIO (IOPRIO_PRIO_VALUE(IOPRIO_CLASS_BE, 3))

#define MOPT_SET 0x0001
#define MOPT_CLEAR 0x0002
@@ -5211,7 +5210,7 @@ static int __ext4_fill_super(struct fs_context *fc, struct super_block *sb)

/* Set defaults for the variables that will be set during parsing */
if (!(ctx->spec & EXT4_SPEC_JOURNAL_IOPRIO))
- ctx->journal_ioprio = DEFAULT_JOURNAL_IOPRIO;
+ ctx->journal_ioprio = EXT4_DEF_JOURNAL_IOPRIO;

sbi->s_inode_readahead_blks = EXT4_DEF_INODE_READAHEAD_BLKS;
sbi->s_sectors_written_start =
@@ -6471,7 +6470,7 @@ static int __ext4_remount(struct fs_context *fc, struct super_block *sb)
ctx->journal_ioprio =
sbi->s_journal->j_task->io_context->ioprio;
else
- ctx->journal_ioprio = DEFAULT_JOURNAL_IOPRIO;
+ ctx->journal_ioprio = EXT4_DEF_JOURNAL_IOPRIO;

}

--
2.45.0.rc1.225.g2a3ae87e7f-goog


2024-05-20 05:52:41

by harshad shirwadkar

[permalink] [raw]
Subject: [PATCH 10/10] ext4: make fast commit ineligible on ext4_reserve_inode_write failure

Fast commit by default makes every inode on which
ext4_reserve_inode_write() is called. Thus, if that function
fails for some reason, make the next fast commit ineligible.

Signed-off-by: Harshad Shirwadkar <[email protected]>
---
fs/ext4/fast_commit.c | 1 +
fs/ext4/fast_commit.h | 1 +
fs/ext4/inode.c | 31 +++++++++++++++++++------------
include/trace/events/ext4.h | 7 +++++--
4 files changed, 26 insertions(+), 14 deletions(-)

diff --git a/fs/ext4/fast_commit.c b/fs/ext4/fast_commit.c
index d354839dbf7e..5ab13cb017a1 100644
--- a/fs/ext4/fast_commit.c
+++ b/fs/ext4/fast_commit.c
@@ -2291,6 +2291,7 @@ static const char * const fc_ineligible_reasons[] = {
[EXT4_FC_REASON_FALLOC_RANGE] = "Falloc range op",
[EXT4_FC_REASON_INODE_JOURNAL_DATA] = "Data journalling",
[EXT4_FC_REASON_ENCRYPTED_FILENAME] = "Encrypted filename",
+ [EXT4_FC_REASON_INODE_RSV_WRITE_FAIL] = "Inode reserve write failure"
};

int ext4_fc_info_show(struct seq_file *seq, void *v)
diff --git a/fs/ext4/fast_commit.h b/fs/ext4/fast_commit.h
index 2fadb2c4780c..f7f85c3dd3af 100644
--- a/fs/ext4/fast_commit.h
+++ b/fs/ext4/fast_commit.h
@@ -97,6 +97,7 @@ enum {
EXT4_FC_REASON_FALLOC_RANGE,
EXT4_FC_REASON_INODE_JOURNAL_DATA,
EXT4_FC_REASON_ENCRYPTED_FILENAME,
+ EXT4_FC_REASON_INODE_RSV_WRITE_FAIL,
EXT4_FC_REASON_MAX
};

diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c
index c6405c45970e..18bab8023a16 100644
--- a/fs/ext4/inode.c
+++ b/fs/ext4/inode.c
@@ -5420,7 +5420,7 @@ int ext4_setattr(struct mnt_idmap *idmap, struct dentry *dentry,
(attr->ia_size > 0 ? attr->ia_size - 1 : 0) >>
inode->i_sb->s_blocksize_bits);

- rc = ext4_mark_inode_dirty(handle, inode);
+ rc = ext4_mark_inode_dirty(handle, inode);
if (!error)
error = rc;
down_write(&EXT4_I(inode)->i_data_sem);
@@ -5728,20 +5728,27 @@ ext4_reserve_inode_write(handle_t *handle, struct inode *inode,
{
int err;

- if (unlikely(ext4_forced_shutdown(inode->i_sb)))
- return -EIO;
+ if (unlikely(ext4_forced_shutdown(inode->i_sb))) {
+ err = -EIO;
+ goto out;
+ }

err = ext4_get_inode_loc(inode, iloc);
- if (!err) {
- BUFFER_TRACE(iloc->bh, "get_write_access");
- err = ext4_journal_get_write_access(handle, inode->i_sb,
- iloc->bh, EXT4_JTR_NONE);
- if (err) {
- brelse(iloc->bh);
- iloc->bh = NULL;
- }
- ext4_fc_track_inode(handle, inode);
+ if (err)
+ goto out;
+
+ BUFFER_TRACE(iloc->bh, "get_write_access");
+ err = ext4_journal_get_write_access(handle, inode->i_sb,
+ iloc->bh, EXT4_JTR_NONE);
+ if (err) {
+ brelse(iloc->bh);
+ iloc->bh = NULL;
}
+ ext4_fc_track_inode(handle, inode);
+out:
+ if (err)
+ ext4_fc_mark_ineligible(inode->i_sb,
+ EXT4_FC_REASON_INODE_RSV_WRITE_FAIL, handle);
ext4_std_error(inode->i_sb, err);
return err;
}
diff --git a/include/trace/events/ext4.h b/include/trace/events/ext4.h
index a697f4b77162..597845d5c1e8 100644
--- a/include/trace/events/ext4.h
+++ b/include/trace/events/ext4.h
@@ -105,6 +105,7 @@ TRACE_DEFINE_ENUM(EXT4_FC_REASON_RENAME_DIR);
TRACE_DEFINE_ENUM(EXT4_FC_REASON_FALLOC_RANGE);
TRACE_DEFINE_ENUM(EXT4_FC_REASON_INODE_JOURNAL_DATA);
TRACE_DEFINE_ENUM(EXT4_FC_REASON_ENCRYPTED_FILENAME);
+TRACE_DEFINE_ENUM(EXT4_FC_REASON_INODE_RSV_WRITE_FAIL);
TRACE_DEFINE_ENUM(EXT4_FC_REASON_MAX);

#define show_fc_reason(reason) \
@@ -118,7 +119,8 @@ TRACE_DEFINE_ENUM(EXT4_FC_REASON_MAX);
{ EXT4_FC_REASON_RENAME_DIR, "RENAME_DIR"}, \
{ EXT4_FC_REASON_FALLOC_RANGE, "FALLOC_RANGE"}, \
{ EXT4_FC_REASON_INODE_JOURNAL_DATA, "INODE_JOURNAL_DATA"}, \
- { EXT4_FC_REASON_ENCRYPTED_FILENAME, "ENCRYPTED_FILENAME"})
+ { EXT4_FC_REASON_ENCRYPTED_FILENAME, "ENCRYPTED_FILENAME"}, \
+ { EXT4_FC_REASON_INODE_RSV_WRITE_FAIL, "INODE_RSV_WRITE_FAIL"})

TRACE_DEFINE_ENUM(CR_POWER2_ALIGNED);
TRACE_DEFINE_ENUM(CR_GOAL_LEN_FAST);
@@ -2805,7 +2807,7 @@ TRACE_EVENT(ext4_fc_stats,
),

TP_printk("dev %d,%d fc ineligible reasons:\n"
- "%s:%u, %s:%u, %s:%u, %s:%u, %s:%u, %s:%u, %s:%u, %s:%u, %s:%u, %s:%u"
+ "%s:%u, %s:%u, %s:%u, %s:%u, %s:%u, %s:%u, %s:%u, %s:%u, %s:%u, %s:%u, %s:%u"
"num_commits:%lu, ineligible: %lu, numblks: %lu",
MAJOR(__entry->dev), MINOR(__entry->dev),
FC_REASON_NAME_STAT(EXT4_FC_REASON_XATTR),
@@ -2818,6 +2820,7 @@ TRACE_EVENT(ext4_fc_stats,
FC_REASON_NAME_STAT(EXT4_FC_REASON_FALLOC_RANGE),
FC_REASON_NAME_STAT(EXT4_FC_REASON_INODE_JOURNAL_DATA),
FC_REASON_NAME_STAT(EXT4_FC_REASON_ENCRYPTED_FILENAME),
+ FC_REASON_NAME_STAT(EXT4_FC_REASON_INODE_RSV_WRITE_FAIL),
__entry->fc_commits, __entry->fc_ineligible_commits,
__entry->fc_numblks)
);
--
2.45.0.rc1.225.g2a3ae87e7f-goog


2024-05-20 07:46:37

by kernel test robot

[permalink] [raw]
Subject: Re: [PATCH 03/10] ext4: mark inode dirty before grabbing i_data_sem in ext4_setattr

Hi Harshad,

kernel test robot noticed the following build warnings:

[auto build test WARNING on tytso-ext4/dev]
[also build test WARNING on linus/master v6.9 next-20240520]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url: https://github.com/intel-lab-lkp/linux/commits/Harshad-Shirwadkar/ext4-convert-i_fc_lock-to-spinlock/20240520-135501
base: https://git.kernel.org/pub/scm/linux/kernel/git/tytso/ext4.git dev
patch link: https://lore.kernel.org/r/20240520055153.136091-4-harshadshirwadkar%40gmail.com
patch subject: [PATCH 03/10] ext4: mark inode dirty before grabbing i_data_sem in ext4_setattr
config: arm-defconfig (https://download.01.org/0day-ci/archive/20240520/[email protected]/config)
compiler: clang version 14.0.6 (https://github.com/llvm/llvm-project f28c006a5895fc0e329fe15fead81e37457cb1d1)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240520/[email protected]/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <[email protected]>
| Closes: https://lore.kernel.org/oe-kbuild-all/[email protected]/

All warnings (new ones prefixed by >>):

>> fs/ext4/inode.c:5427:33: warning: variable 'old_disksize' is uninitialized when used here [-Wuninitialized]
EXT4_I(inode)->i_disksize = old_disksize;
^~~~~~~~~~~~
fs/ext4/inode.c:5343:22: note: initialize the variable 'old_disksize' to silence this warning
loff_t old_disksize;
^
= 0
1 warning generated.


vim +/old_disksize +5427 fs/ext4/inode.c

53e872681fed6a4 Jan Kara 2012-12-25 5241
ac27a0ec112a089 Dave Kleikamp 2006-10-11 5242 /*
617ba13b31fbf50 Mingming Cao 2006-10-11 5243 * ext4_setattr()
ac27a0ec112a089 Dave Kleikamp 2006-10-11 5244 *
ac27a0ec112a089 Dave Kleikamp 2006-10-11 5245 * Called from notify_change.
ac27a0ec112a089 Dave Kleikamp 2006-10-11 5246 *
ac27a0ec112a089 Dave Kleikamp 2006-10-11 5247 * We want to trap VFS attempts to truncate the file as soon as
ac27a0ec112a089 Dave Kleikamp 2006-10-11 5248 * possible. In particular, we want to make sure that when the VFS
ac27a0ec112a089 Dave Kleikamp 2006-10-11 5249 * shrinks i_size, we put the inode on the orphan list and modify
ac27a0ec112a089 Dave Kleikamp 2006-10-11 5250 * i_disksize immediately, so that during the subsequent flushing of
ac27a0ec112a089 Dave Kleikamp 2006-10-11 5251 * dirty pages and freeing of disk blocks, we can guarantee that any
ac27a0ec112a089 Dave Kleikamp 2006-10-11 5252 * commit will leave the blocks being flushed in an unused state on
ac27a0ec112a089 Dave Kleikamp 2006-10-11 5253 * disk. (On recovery, the inode will get truncated and the blocks will
ac27a0ec112a089 Dave Kleikamp 2006-10-11 5254 * be freed, so we have a strong guarantee that no future commit will
ac27a0ec112a089 Dave Kleikamp 2006-10-11 5255 * leave these blocks visible to the user.)
ac27a0ec112a089 Dave Kleikamp 2006-10-11 5256 *
678aaf481496b01 Jan Kara 2008-07-11 5257 * Another thing we have to assure is that if we are in ordered mode
678aaf481496b01 Jan Kara 2008-07-11 5258 * and inode is still attached to the committing transaction, we must
678aaf481496b01 Jan Kara 2008-07-11 5259 * we start writeout of all the dirty pages which are being truncated.
678aaf481496b01 Jan Kara 2008-07-11 5260 * This way we are sure that all the data written in the previous
678aaf481496b01 Jan Kara 2008-07-11 5261 * transaction are already on disk (truncate waits for pages under
678aaf481496b01 Jan Kara 2008-07-11 5262 * writeback).
678aaf481496b01 Jan Kara 2008-07-11 5263 *
f340b3d90274859 hongnanli 2022-01-21 5264 * Called with inode->i_rwsem down.
ac27a0ec112a089 Dave Kleikamp 2006-10-11 5265 */
c1632a0f1120933 Christian Brauner 2023-01-13 5266 int ext4_setattr(struct mnt_idmap *idmap, struct dentry *dentry,
549c7297717c32e Christian Brauner 2021-01-21 5267 struct iattr *attr)
ac27a0ec112a089 Dave Kleikamp 2006-10-11 5268 {
2b0143b5c986be1 David Howells 2015-03-17 5269 struct inode *inode = d_inode(dentry);
ac27a0ec112a089 Dave Kleikamp 2006-10-11 5270 int error, rc = 0;
3d287de3b828226 Dmitry Monakhov 2010-10-27 5271 int orphan = 0;
ac27a0ec112a089 Dave Kleikamp 2006-10-11 5272 const unsigned int ia_valid = attr->ia_valid;
a642c2c0827f560 Jeff Layton 2022-09-08 5273 bool inc_ivers = true;
ac27a0ec112a089 Dave Kleikamp 2006-10-11 5274
eb8ab4443aec5ff Jan Kara 2023-06-16 5275 if (unlikely(ext4_forced_shutdown(inode->i_sb)))
0db1ff222d40f16 Theodore Ts'o 2017-02-05 5276 return -EIO;
0db1ff222d40f16 Theodore Ts'o 2017-02-05 5277
02b016ca7f99229 Theodore Ts'o 2019-06-09 5278 if (unlikely(IS_IMMUTABLE(inode)))
02b016ca7f99229 Theodore Ts'o 2019-06-09 5279 return -EPERM;
02b016ca7f99229 Theodore Ts'o 2019-06-09 5280
02b016ca7f99229 Theodore Ts'o 2019-06-09 5281 if (unlikely(IS_APPEND(inode) &&
02b016ca7f99229 Theodore Ts'o 2019-06-09 5282 (ia_valid & (ATTR_MODE | ATTR_UID |
02b016ca7f99229 Theodore Ts'o 2019-06-09 5283 ATTR_GID | ATTR_TIMES_SET))))
02b016ca7f99229 Theodore Ts'o 2019-06-09 5284 return -EPERM;
02b016ca7f99229 Theodore Ts'o 2019-06-09 5285
c1632a0f1120933 Christian Brauner 2023-01-13 5286 error = setattr_prepare(idmap, dentry, attr);
ac27a0ec112a089 Dave Kleikamp 2006-10-11 5287 if (error)
ac27a0ec112a089 Dave Kleikamp 2006-10-11 5288 return error;
ac27a0ec112a089 Dave Kleikamp 2006-10-11 5289
3ce2b8ddd84d507 Eric Biggers 2017-10-18 5290 error = fscrypt_prepare_setattr(dentry, attr);
3ce2b8ddd84d507 Eric Biggers 2017-10-18 5291 if (error)
3ce2b8ddd84d507 Eric Biggers 2017-10-18 5292 return error;
3ce2b8ddd84d507 Eric Biggers 2017-10-18 5293
c93d8f88580921c Eric Biggers 2019-07-22 5294 error = fsverity_prepare_setattr(dentry, attr);
c93d8f88580921c Eric Biggers 2019-07-22 5295 if (error)
c93d8f88580921c Eric Biggers 2019-07-22 5296 return error;
c93d8f88580921c Eric Biggers 2019-07-22 5297
f861646a65623bc Christian Brauner 2023-01-13 5298 if (is_quota_modification(idmap, inode, attr)) {
a7cdadee0e89486 Jan Kara 2015-06-29 5299 error = dquot_initialize(inode);
a7cdadee0e89486 Jan Kara 2015-06-29 5300 if (error)
a7cdadee0e89486 Jan Kara 2015-06-29 5301 return error;
a7cdadee0e89486 Jan Kara 2015-06-29 5302 }
2729cfdcfa1cc49 Harshad Shirwadkar 2021-12-23 5303
0dbe12f2e49c046 Christian Brauner 2023-01-13 5304 if (i_uid_needs_update(idmap, attr, inode) ||
0dbe12f2e49c046 Christian Brauner 2023-01-13 5305 i_gid_needs_update(idmap, attr, inode)) {
ac27a0ec112a089 Dave Kleikamp 2006-10-11 5306 handle_t *handle;
ac27a0ec112a089 Dave Kleikamp 2006-10-11 5307
ac27a0ec112a089 Dave Kleikamp 2006-10-11 5308 /* (user+group)*(old+new) structure, inode write (sb,
ac27a0ec112a089 Dave Kleikamp 2006-10-11 5309 * inode block, ? - but truncate inode update has it) */
9924a92a8c21757 Theodore Ts'o 2013-02-08 5310 handle = ext4_journal_start(inode, EXT4_HT_QUOTA,
9924a92a8c21757 Theodore Ts'o 2013-02-08 5311 (EXT4_MAXQUOTAS_INIT_BLOCKS(inode->i_sb) +
194074acacebc16 Dmitry Monakhov 2009-12-08 5312 EXT4_MAXQUOTAS_DEL_BLOCKS(inode->i_sb)) + 3);
ac27a0ec112a089 Dave Kleikamp 2006-10-11 5313 if (IS_ERR(handle)) {
ac27a0ec112a089 Dave Kleikamp 2006-10-11 5314 error = PTR_ERR(handle);
ac27a0ec112a089 Dave Kleikamp 2006-10-11 5315 goto err_out;
ac27a0ec112a089 Dave Kleikamp 2006-10-11 5316 }
7a9ca53aea10ad4 Tahsin Erdogan 2017-06-22 5317
7a9ca53aea10ad4 Tahsin Erdogan 2017-06-22 5318 /* dquot_transfer() calls back ext4_get_inode_usage() which
7a9ca53aea10ad4 Tahsin Erdogan 2017-06-22 5319 * counts xattr inode references.
7a9ca53aea10ad4 Tahsin Erdogan 2017-06-22 5320 */
7a9ca53aea10ad4 Tahsin Erdogan 2017-06-22 5321 down_read(&EXT4_I(inode)->xattr_sem);
f861646a65623bc Christian Brauner 2023-01-13 5322 error = dquot_transfer(idmap, inode, attr);
7a9ca53aea10ad4 Tahsin Erdogan 2017-06-22 5323 up_read(&EXT4_I(inode)->xattr_sem);
7a9ca53aea10ad4 Tahsin Erdogan 2017-06-22 5324
ac27a0ec112a089 Dave Kleikamp 2006-10-11 5325 if (error) {
617ba13b31fbf50 Mingming Cao 2006-10-11 5326 ext4_journal_stop(handle);
ac27a0ec112a089 Dave Kleikamp 2006-10-11 5327 return error;
ac27a0ec112a089 Dave Kleikamp 2006-10-11 5328 }
ac27a0ec112a089 Dave Kleikamp 2006-10-11 5329 /* Update corresponding info in inode so that everything is in
ac27a0ec112a089 Dave Kleikamp 2006-10-11 5330 * one transaction */
0dbe12f2e49c046 Christian Brauner 2023-01-13 5331 i_uid_update(idmap, attr, inode);
0dbe12f2e49c046 Christian Brauner 2023-01-13 5332 i_gid_update(idmap, attr, inode);
617ba13b31fbf50 Mingming Cao 2006-10-11 5333 error = ext4_mark_inode_dirty(handle, inode);
617ba13b31fbf50 Mingming Cao 2006-10-11 5334 ext4_journal_stop(handle);
512c15ef05d73a0 Pan Bian 2021-01-17 5335 if (unlikely(error)) {
4209ae12b12265d Harshad Shirwadkar 2020-04-26 5336 return error;
ac27a0ec112a089 Dave Kleikamp 2006-10-11 5337 }
512c15ef05d73a0 Pan Bian 2021-01-17 5338 }
ac27a0ec112a089 Dave Kleikamp 2006-10-11 5339
3da40c7b089810a Josef Bacik 2015-06-22 5340 if (attr->ia_valid & ATTR_SIZE) {
5208386c501276d Jan Kara 2013-08-17 5341 handle_t *handle;
3da40c7b089810a Josef Bacik 2015-06-22 5342 loff_t oldsize = inode->i_size;
f4534c9fc94d223 Ye Bin 2022-03-26 5343 loff_t old_disksize;
b9c1c26739ec2d4 Jan Kara 2019-05-30 5344 int shrink = (attr->ia_size < inode->i_size);
562c72aa57c36b1 Christoph Hellwig 2011-06-24 5345
12e9b892002d9af Dmitry Monakhov 2010-05-16 5346 if (!(ext4_test_inode_flag(inode, EXT4_INODE_EXTENTS))) {
e2b4657453c0d55 Eric Sandeen 2008-01-28 5347 struct ext4_sb_info *sbi = EXT4_SB(inode->i_sb);
e2b4657453c0d55 Eric Sandeen 2008-01-28 5348
aa75f4d3daaeb13 Harshad Shirwadkar 2020-10-15 5349 if (attr->ia_size > sbi->s_bitmap_maxbytes) {
0c095c7f113e9fd Theodore Ts'o 2010-07-27 5350 return -EFBIG;
e2b4657453c0d55 Eric Sandeen 2008-01-28 5351 }
aa75f4d3daaeb13 Harshad Shirwadkar 2020-10-15 5352 }
aa75f4d3daaeb13 Harshad Shirwadkar 2020-10-15 5353 if (!S_ISREG(inode->i_mode)) {
3da40c7b089810a Josef Bacik 2015-06-22 5354 return -EINVAL;
aa75f4d3daaeb13 Harshad Shirwadkar 2020-10-15 5355 }
dff6efc326a4d5f Christoph Hellwig 2013-11-19 5356
a642c2c0827f560 Jeff Layton 2022-09-08 5357 if (attr->ia_size == inode->i_size)
a642c2c0827f560 Jeff Layton 2022-09-08 5358 inc_ivers = false;
dff6efc326a4d5f Christoph Hellwig 2013-11-19 5359
b9c1c26739ec2d4 Jan Kara 2019-05-30 5360 if (shrink) {
b9c1c26739ec2d4 Jan Kara 2019-05-30 5361 if (ext4_should_order_data(inode)) {
5208386c501276d Jan Kara 2013-08-17 5362 error = ext4_begin_ordered_truncate(inode,
5208386c501276d Jan Kara 2013-08-17 5363 attr->ia_size);
5208386c501276d Jan Kara 2013-08-17 5364 if (error)
5208386c501276d Jan Kara 2013-08-17 5365 goto err_out;
5208386c501276d Jan Kara 2013-08-17 5366 }
b9c1c26739ec2d4 Jan Kara 2019-05-30 5367 /*
b9c1c26739ec2d4 Jan Kara 2019-05-30 5368 * Blocks are going to be removed from the inode. Wait
b9c1c26739ec2d4 Jan Kara 2019-05-30 5369 * for dio in flight.
b9c1c26739ec2d4 Jan Kara 2019-05-30 5370 */
b9c1c26739ec2d4 Jan Kara 2019-05-30 5371 inode_dio_wait(inode);
b9c1c26739ec2d4 Jan Kara 2019-05-30 5372 }
b9c1c26739ec2d4 Jan Kara 2019-05-30 5373
d4f5258eae7b38c Jan Kara 2021-02-04 5374 filemap_invalidate_lock(inode->i_mapping);
b9c1c26739ec2d4 Jan Kara 2019-05-30 5375
b9c1c26739ec2d4 Jan Kara 2019-05-30 5376 rc = ext4_break_layouts(inode);
b9c1c26739ec2d4 Jan Kara 2019-05-30 5377 if (rc) {
d4f5258eae7b38c Jan Kara 2021-02-04 5378 filemap_invalidate_unlock(inode->i_mapping);
aa75f4d3daaeb13 Harshad Shirwadkar 2020-10-15 5379 goto err_out;
b9c1c26739ec2d4 Jan Kara 2019-05-30 5380 }
b9c1c26739ec2d4 Jan Kara 2019-05-30 5381
3da40c7b089810a Josef Bacik 2015-06-22 5382 if (attr->ia_size != inode->i_size) {
9924a92a8c21757 Theodore Ts'o 2013-02-08 5383 handle = ext4_journal_start(inode, EXT4_HT_INODE, 3);
ac27a0ec112a089 Dave Kleikamp 2006-10-11 5384 if (IS_ERR(handle)) {
ac27a0ec112a089 Dave Kleikamp 2006-10-11 5385 error = PTR_ERR(handle);
b9c1c26739ec2d4 Jan Kara 2019-05-30 5386 goto out_mmap_sem;
ac27a0ec112a089 Dave Kleikamp 2006-10-11 5387 }
3da40c7b089810a Josef Bacik 2015-06-22 5388 if (ext4_handle_valid(handle) && shrink) {
617ba13b31fbf50 Mingming Cao 2006-10-11 5389 error = ext4_orphan_add(handle, inode);
3d287de3b828226 Dmitry Monakhov 2010-10-27 5390 orphan = 1;
3d287de3b828226 Dmitry Monakhov 2010-10-27 5391 }
911af577de4e444 Eryu Guan 2015-07-28 5392 /*
911af577de4e444 Eryu Guan 2015-07-28 5393 * Update c/mtime on truncate up, ext4_truncate() will
911af577de4e444 Eryu Guan 2015-07-28 5394 * update c/mtime in shrink case below
911af577de4e444 Eryu Guan 2015-07-28 5395 */
1bc33893e79a79d Jeff Layton 2023-07-05 5396 if (!shrink)
b898ab233611f79 Jeff Layton 2023-10-04 5397 inode_set_mtime_to_ts(inode,
b898ab233611f79 Jeff Layton 2023-10-04 5398 inode_set_ctime_current(inode));
aa75f4d3daaeb13 Harshad Shirwadkar 2020-10-15 5399
aa75f4d3daaeb13 Harshad Shirwadkar 2020-10-15 5400 if (shrink)
a80f7fcf18672ae Harshad Shirwadkar 2020-11-05 5401 ext4_fc_track_range(handle, inode,
aa75f4d3daaeb13 Harshad Shirwadkar 2020-10-15 5402 (attr->ia_size > 0 ? attr->ia_size - 1 : 0) >>
aa75f4d3daaeb13 Harshad Shirwadkar 2020-10-15 5403 inode->i_sb->s_blocksize_bits,
9725958bb75cdfa Xin Yin 2021-12-23 5404 EXT_MAX_BLOCKS - 1);
aa75f4d3daaeb13 Harshad Shirwadkar 2020-10-15 5405 else
aa75f4d3daaeb13 Harshad Shirwadkar 2020-10-15 5406 ext4_fc_track_range(
a80f7fcf18672ae Harshad Shirwadkar 2020-11-05 5407 handle, inode,
aa75f4d3daaeb13 Harshad Shirwadkar 2020-10-15 5408 (oldsize > 0 ? oldsize - 1 : oldsize) >>
aa75f4d3daaeb13 Harshad Shirwadkar 2020-10-15 5409 inode->i_sb->s_blocksize_bits,
aa75f4d3daaeb13 Harshad Shirwadkar 2020-10-15 5410 (attr->ia_size > 0 ? attr->ia_size - 1 : 0) >>
aa75f4d3daaeb13 Harshad Shirwadkar 2020-10-15 5411 inode->i_sb->s_blocksize_bits);
aa75f4d3daaeb13 Harshad Shirwadkar 2020-10-15 5412
617ba13b31fbf50 Mingming Cao 2006-10-11 5413 rc = ext4_mark_inode_dirty(handle, inode);
ac27a0ec112a089 Dave Kleikamp 2006-10-11 5414 if (!error)
ac27a0ec112a089 Dave Kleikamp 2006-10-11 5415 error = rc;
a879a75fe2d0cec Harshad Shirwadkar 2024-05-20 5416 down_write(&EXT4_I(inode)->i_data_sem);
a879a75fe2d0cec Harshad Shirwadkar 2024-05-20 5417 EXT4_I(inode)->i_disksize = attr->ia_size;
a879a75fe2d0cec Harshad Shirwadkar 2024-05-20 5418
90e775b71ac4e68 Jan Kara 2013-08-17 5419 /*
90e775b71ac4e68 Jan Kara 2013-08-17 5420 * We have to update i_size under i_data_sem together
90e775b71ac4e68 Jan Kara 2013-08-17 5421 * with i_disksize to avoid races with writeback code
90e775b71ac4e68 Jan Kara 2013-08-17 5422 * running ext4_wb_update_i_disksize().
90e775b71ac4e68 Jan Kara 2013-08-17 5423 */
90e775b71ac4e68 Jan Kara 2013-08-17 5424 if (!error)
90e775b71ac4e68 Jan Kara 2013-08-17 5425 i_size_write(inode, attr->ia_size);
f4534c9fc94d223 Ye Bin 2022-03-26 5426 else
f4534c9fc94d223 Ye Bin 2022-03-26 @5427 EXT4_I(inode)->i_disksize = old_disksize;
90e775b71ac4e68 Jan Kara 2013-08-17 5428 up_write(&EXT4_I(inode)->i_data_sem);
617ba13b31fbf50 Mingming Cao 2006-10-11 5429 ext4_journal_stop(handle);
b9c1c26739ec2d4 Jan Kara 2019-05-30 5430 if (error)
b9c1c26739ec2d4 Jan Kara 2019-05-30 5431 goto out_mmap_sem;
82a25b027ca48d7 Jan Kara 2019-05-23 5432 if (!shrink) {
b9c1c26739ec2d4 Jan Kara 2019-05-30 5433 pagecache_isize_extended(inode, oldsize,
b9c1c26739ec2d4 Jan Kara 2019-05-30 5434 inode->i_size);
b9c1c26739ec2d4 Jan Kara 2019-05-30 5435 } else if (ext4_should_journal_data(inode)) {
82a25b027ca48d7 Jan Kara 2019-05-23 5436 ext4_wait_for_tail_page_commit(inode);
b9c1c26739ec2d4 Jan Kara 2019-05-30 5437 }
430657b6be896db Ross Zwisler 2018-07-29 5438 }
430657b6be896db Ross Zwisler 2018-07-29 5439
53e872681fed6a4 Jan Kara 2012-12-25 5440 /*
53e872681fed6a4 Jan Kara 2012-12-25 5441 * Truncate pagecache after we've waited for commit
53e872681fed6a4 Jan Kara 2012-12-25 5442 * in data=journal mode to make pages freeable.
53e872681fed6a4 Jan Kara 2012-12-25 5443 */
7caef26767c1727 Kirill A. Shutemov 2013-09-12 5444 truncate_pagecache(inode, inode->i_size);
b9c1c26739ec2d4 Jan Kara 2019-05-30 5445 /*
b9c1c26739ec2d4 Jan Kara 2019-05-30 5446 * Call ext4_truncate() even if i_size didn't change to
b9c1c26739ec2d4 Jan Kara 2019-05-30 5447 * truncate possible preallocated blocks.
b9c1c26739ec2d4 Jan Kara 2019-05-30 5448 */
b9c1c26739ec2d4 Jan Kara 2019-05-30 5449 if (attr->ia_size <= oldsize) {
2c98eb5ea249767 Theodore Ts'o 2016-11-13 5450 rc = ext4_truncate(inode);
2c98eb5ea249767 Theodore Ts'o 2016-11-13 5451 if (rc)
2c98eb5ea249767 Theodore Ts'o 2016-11-13 5452 error = rc;
2c98eb5ea249767 Theodore Ts'o 2016-11-13 5453 }
b9c1c26739ec2d4 Jan Kara 2019-05-30 5454 out_mmap_sem:
d4f5258eae7b38c Jan Kara 2021-02-04 5455 filemap_invalidate_unlock(inode->i_mapping);
3da40c7b089810a Josef Bacik 2015-06-22 5456 }
ac27a0ec112a089 Dave Kleikamp 2006-10-11 5457
2c98eb5ea249767 Theodore Ts'o 2016-11-13 5458 if (!error) {
a642c2c0827f560 Jeff Layton 2022-09-08 5459 if (inc_ivers)
a642c2c0827f560 Jeff Layton 2022-09-08 5460 inode_inc_iversion(inode);
c1632a0f1120933 Christian Brauner 2023-01-13 5461 setattr_copy(idmap, inode, attr);
1025774ce411f2b Christoph Hellwig 2010-06-04 5462 mark_inode_dirty(inode);
1025774ce411f2b Christoph Hellwig 2010-06-04 5463 }
1025774ce411f2b Christoph Hellwig 2010-06-04 5464
1025774ce411f2b Christoph Hellwig 2010-06-04 5465 /*
1025774ce411f2b Christoph Hellwig 2010-06-04 5466 * If the call to ext4_truncate failed to get a transaction handle at
1025774ce411f2b Christoph Hellwig 2010-06-04 5467 * all, we need to clean up the in-core orphan list manually.
1025774ce411f2b Christoph Hellwig 2010-06-04 5468 */
3d287de3b828226 Dmitry Monakhov 2010-10-27 5469 if (orphan && inode->i_nlink)
617ba13b31fbf50 Mingming Cao 2006-10-11 5470 ext4_orphan_del(NULL, inode);
ac27a0ec112a089 Dave Kleikamp 2006-10-11 5471
2c98eb5ea249767 Theodore Ts'o 2016-11-13 5472 if (!error && (ia_valid & ATTR_MODE))
13e83a4923bea7c Christian Brauner 2023-01-13 5473 rc = posix_acl_chmod(idmap, dentry, inode->i_mode);
ac27a0ec112a089 Dave Kleikamp 2006-10-11 5474
ac27a0ec112a089 Dave Kleikamp 2006-10-11 5475 err_out:
aa75f4d3daaeb13 Harshad Shirwadkar 2020-10-15 5476 if (error)
617ba13b31fbf50 Mingming Cao 2006-10-11 5477 ext4_std_error(inode->i_sb, error);
ac27a0ec112a089 Dave Kleikamp 2006-10-11 5478 if (!error)
ac27a0ec112a089 Dave Kleikamp 2006-10-11 5479 error = rc;
ac27a0ec112a089 Dave Kleikamp 2006-10-11 5480 return error;
ac27a0ec112a089 Dave Kleikamp 2006-10-11 5481 }
ac27a0ec112a089 Dave Kleikamp 2006-10-11 5482

--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

2024-05-20 18:59:21

by kernel test robot

[permalink] [raw]
Subject: Re: [PATCH 10/10] ext4: make fast commit ineligible on ext4_reserve_inode_write failure

Hi Harshad,

kernel test robot noticed the following build warnings:

[auto build test WARNING on tytso-ext4/dev]
[also build test WARNING on linus/master v6.9 next-20240520]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url: https://github.com/intel-lab-lkp/linux/commits/Harshad-Shirwadkar/ext4-convert-i_fc_lock-to-spinlock/20240520-135501
base: https://git.kernel.org/pub/scm/linux/kernel/git/tytso/ext4.git dev
patch link: https://lore.kernel.org/r/20240520055153.136091-11-harshadshirwadkar%40gmail.com
patch subject: [PATCH 10/10] ext4: make fast commit ineligible on ext4_reserve_inode_write failure
config: i386-randconfig-141-20240520 (https://download.01.org/0day-ci/archive/20240521/[email protected]/config)
compiler: clang version 18.1.5 (https://github.com/llvm/llvm-project 617a15a9eac96088ae5e9134248d8236e34b91b1)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <[email protected]>
| Closes: https://lore.kernel.org/oe-kbuild-all/[email protected]/

New smatch warnings:
fs/ext4/inode.c:5423 ext4_setattr() warn: inconsistent indenting

Old smatch warnings:
fs/ext4/inode.c:2422 mpage_prepare_extent_to_map() warn: missing error code 'err'
fs/ext4/inode.c:5437 ext4_setattr() error: uninitialized symbol 'old_disksize'.
fs/ext4/inode.c:6150 ext4_page_mkwrite() error: uninitialized symbol 'get_block'.

vim +5423 fs/ext4/inode.c

53e872681fed6a Jan Kara 2012-12-25 5251
ac27a0ec112a08 Dave Kleikamp 2006-10-11 5252 /*
617ba13b31fbf5 Mingming Cao 2006-10-11 5253 * ext4_setattr()
ac27a0ec112a08 Dave Kleikamp 2006-10-11 5254 *
ac27a0ec112a08 Dave Kleikamp 2006-10-11 5255 * Called from notify_change.
ac27a0ec112a08 Dave Kleikamp 2006-10-11 5256 *
ac27a0ec112a08 Dave Kleikamp 2006-10-11 5257 * We want to trap VFS attempts to truncate the file as soon as
ac27a0ec112a08 Dave Kleikamp 2006-10-11 5258 * possible. In particular, we want to make sure that when the VFS
ac27a0ec112a08 Dave Kleikamp 2006-10-11 5259 * shrinks i_size, we put the inode on the orphan list and modify
ac27a0ec112a08 Dave Kleikamp 2006-10-11 5260 * i_disksize immediately, so that during the subsequent flushing of
ac27a0ec112a08 Dave Kleikamp 2006-10-11 5261 * dirty pages and freeing of disk blocks, we can guarantee that any
ac27a0ec112a08 Dave Kleikamp 2006-10-11 5262 * commit will leave the blocks being flushed in an unused state on
ac27a0ec112a08 Dave Kleikamp 2006-10-11 5263 * disk. (On recovery, the inode will get truncated and the blocks will
ac27a0ec112a08 Dave Kleikamp 2006-10-11 5264 * be freed, so we have a strong guarantee that no future commit will
ac27a0ec112a08 Dave Kleikamp 2006-10-11 5265 * leave these blocks visible to the user.)
ac27a0ec112a08 Dave Kleikamp 2006-10-11 5266 *
678aaf481496b0 Jan Kara 2008-07-11 5267 * Another thing we have to assure is that if we are in ordered mode
678aaf481496b0 Jan Kara 2008-07-11 5268 * and inode is still attached to the committing transaction, we must
678aaf481496b0 Jan Kara 2008-07-11 5269 * we start writeout of all the dirty pages which are being truncated.
678aaf481496b0 Jan Kara 2008-07-11 5270 * This way we are sure that all the data written in the previous
678aaf481496b0 Jan Kara 2008-07-11 5271 * transaction are already on disk (truncate waits for pages under
678aaf481496b0 Jan Kara 2008-07-11 5272 * writeback).
678aaf481496b0 Jan Kara 2008-07-11 5273 *
f340b3d9027485 hongnanli 2022-01-21 5274 * Called with inode->i_rwsem down.
ac27a0ec112a08 Dave Kleikamp 2006-10-11 5275 */
c1632a0f112093 Christian Brauner 2023-01-13 5276 int ext4_setattr(struct mnt_idmap *idmap, struct dentry *dentry,
549c7297717c32 Christian Brauner 2021-01-21 5277 struct iattr *attr)
ac27a0ec112a08 Dave Kleikamp 2006-10-11 5278 {
2b0143b5c986be David Howells 2015-03-17 5279 struct inode *inode = d_inode(dentry);
ac27a0ec112a08 Dave Kleikamp 2006-10-11 5280 int error, rc = 0;
3d287de3b82822 Dmitry Monakhov 2010-10-27 5281 int orphan = 0;
ac27a0ec112a08 Dave Kleikamp 2006-10-11 5282 const unsigned int ia_valid = attr->ia_valid;
a642c2c0827f56 Jeff Layton 2022-09-08 5283 bool inc_ivers = true;
ac27a0ec112a08 Dave Kleikamp 2006-10-11 5284
eb8ab4443aec5f Jan Kara 2023-06-16 5285 if (unlikely(ext4_forced_shutdown(inode->i_sb)))
0db1ff222d40f1 Theodore Ts'o 2017-02-05 5286 return -EIO;
0db1ff222d40f1 Theodore Ts'o 2017-02-05 5287
02b016ca7f9922 Theodore Ts'o 2019-06-09 5288 if (unlikely(IS_IMMUTABLE(inode)))
02b016ca7f9922 Theodore Ts'o 2019-06-09 5289 return -EPERM;
02b016ca7f9922 Theodore Ts'o 2019-06-09 5290
02b016ca7f9922 Theodore Ts'o 2019-06-09 5291 if (unlikely(IS_APPEND(inode) &&
02b016ca7f9922 Theodore Ts'o 2019-06-09 5292 (ia_valid & (ATTR_MODE | ATTR_UID |
02b016ca7f9922 Theodore Ts'o 2019-06-09 5293 ATTR_GID | ATTR_TIMES_SET))))
02b016ca7f9922 Theodore Ts'o 2019-06-09 5294 return -EPERM;
02b016ca7f9922 Theodore Ts'o 2019-06-09 5295
c1632a0f112093 Christian Brauner 2023-01-13 5296 error = setattr_prepare(idmap, dentry, attr);
ac27a0ec112a08 Dave Kleikamp 2006-10-11 5297 if (error)
ac27a0ec112a08 Dave Kleikamp 2006-10-11 5298 return error;
ac27a0ec112a08 Dave Kleikamp 2006-10-11 5299
3ce2b8ddd84d50 Eric Biggers 2017-10-18 5300 error = fscrypt_prepare_setattr(dentry, attr);
3ce2b8ddd84d50 Eric Biggers 2017-10-18 5301 if (error)
3ce2b8ddd84d50 Eric Biggers 2017-10-18 5302 return error;
3ce2b8ddd84d50 Eric Biggers 2017-10-18 5303
c93d8f88580921 Eric Biggers 2019-07-22 5304 error = fsverity_prepare_setattr(dentry, attr);
c93d8f88580921 Eric Biggers 2019-07-22 5305 if (error)
c93d8f88580921 Eric Biggers 2019-07-22 5306 return error;
c93d8f88580921 Eric Biggers 2019-07-22 5307
f861646a65623b Christian Brauner 2023-01-13 5308 if (is_quota_modification(idmap, inode, attr)) {
a7cdadee0e8948 Jan Kara 2015-06-29 5309 error = dquot_initialize(inode);
a7cdadee0e8948 Jan Kara 2015-06-29 5310 if (error)
a7cdadee0e8948 Jan Kara 2015-06-29 5311 return error;
a7cdadee0e8948 Jan Kara 2015-06-29 5312 }
2729cfdcfa1cc4 Harshad Shirwadkar 2021-12-23 5313
0dbe12f2e49c04 Christian Brauner 2023-01-13 5314 if (i_uid_needs_update(idmap, attr, inode) ||
0dbe12f2e49c04 Christian Brauner 2023-01-13 5315 i_gid_needs_update(idmap, attr, inode)) {
ac27a0ec112a08 Dave Kleikamp 2006-10-11 5316 handle_t *handle;
ac27a0ec112a08 Dave Kleikamp 2006-10-11 5317
ac27a0ec112a08 Dave Kleikamp 2006-10-11 5318 /* (user+group)*(old+new) structure, inode write (sb,
ac27a0ec112a08 Dave Kleikamp 2006-10-11 5319 * inode block, ? - but truncate inode update has it) */
9924a92a8c2175 Theodore Ts'o 2013-02-08 5320 handle = ext4_journal_start(inode, EXT4_HT_QUOTA,
9924a92a8c2175 Theodore Ts'o 2013-02-08 5321 (EXT4_MAXQUOTAS_INIT_BLOCKS(inode->i_sb) +
194074acacebc1 Dmitry Monakhov 2009-12-08 5322 EXT4_MAXQUOTAS_DEL_BLOCKS(inode->i_sb)) + 3);
ac27a0ec112a08 Dave Kleikamp 2006-10-11 5323 if (IS_ERR(handle)) {
ac27a0ec112a08 Dave Kleikamp 2006-10-11 5324 error = PTR_ERR(handle);
ac27a0ec112a08 Dave Kleikamp 2006-10-11 5325 goto err_out;
ac27a0ec112a08 Dave Kleikamp 2006-10-11 5326 }
7a9ca53aea10ad Tahsin Erdogan 2017-06-22 5327
7a9ca53aea10ad Tahsin Erdogan 2017-06-22 5328 /* dquot_transfer() calls back ext4_get_inode_usage() which
7a9ca53aea10ad Tahsin Erdogan 2017-06-22 5329 * counts xattr inode references.
7a9ca53aea10ad Tahsin Erdogan 2017-06-22 5330 */
7a9ca53aea10ad Tahsin Erdogan 2017-06-22 5331 down_read(&EXT4_I(inode)->xattr_sem);
f861646a65623b Christian Brauner 2023-01-13 5332 error = dquot_transfer(idmap, inode, attr);
7a9ca53aea10ad Tahsin Erdogan 2017-06-22 5333 up_read(&EXT4_I(inode)->xattr_sem);
7a9ca53aea10ad Tahsin Erdogan 2017-06-22 5334
ac27a0ec112a08 Dave Kleikamp 2006-10-11 5335 if (error) {
617ba13b31fbf5 Mingming Cao 2006-10-11 5336 ext4_journal_stop(handle);
ac27a0ec112a08 Dave Kleikamp 2006-10-11 5337 return error;
ac27a0ec112a08 Dave Kleikamp 2006-10-11 5338 }
ac27a0ec112a08 Dave Kleikamp 2006-10-11 5339 /* Update corresponding info in inode so that everything is in
ac27a0ec112a08 Dave Kleikamp 2006-10-11 5340 * one transaction */
0dbe12f2e49c04 Christian Brauner 2023-01-13 5341 i_uid_update(idmap, attr, inode);
0dbe12f2e49c04 Christian Brauner 2023-01-13 5342 i_gid_update(idmap, attr, inode);
617ba13b31fbf5 Mingming Cao 2006-10-11 5343 error = ext4_mark_inode_dirty(handle, inode);
617ba13b31fbf5 Mingming Cao 2006-10-11 5344 ext4_journal_stop(handle);
512c15ef05d73a Pan Bian 2021-01-17 5345 if (unlikely(error)) {
4209ae12b12265 Harshad Shirwadkar 2020-04-26 5346 return error;
ac27a0ec112a08 Dave Kleikamp 2006-10-11 5347 }
512c15ef05d73a Pan Bian 2021-01-17 5348 }
ac27a0ec112a08 Dave Kleikamp 2006-10-11 5349
3da40c7b089810 Josef Bacik 2015-06-22 5350 if (attr->ia_valid & ATTR_SIZE) {
5208386c501276 Jan Kara 2013-08-17 5351 handle_t *handle;
3da40c7b089810 Josef Bacik 2015-06-22 5352 loff_t oldsize = inode->i_size;
f4534c9fc94d22 Ye Bin 2022-03-26 5353 loff_t old_disksize;
b9c1c26739ec2d Jan Kara 2019-05-30 5354 int shrink = (attr->ia_size < inode->i_size);
562c72aa57c36b Christoph Hellwig 2011-06-24 5355
12e9b892002d9a Dmitry Monakhov 2010-05-16 5356 if (!(ext4_test_inode_flag(inode, EXT4_INODE_EXTENTS))) {
e2b4657453c0d5 Eric Sandeen 2008-01-28 5357 struct ext4_sb_info *sbi = EXT4_SB(inode->i_sb);
e2b4657453c0d5 Eric Sandeen 2008-01-28 5358
aa75f4d3daaeb1 Harshad Shirwadkar 2020-10-15 5359 if (attr->ia_size > sbi->s_bitmap_maxbytes) {
0c095c7f113e9f Theodore Ts'o 2010-07-27 5360 return -EFBIG;
e2b4657453c0d5 Eric Sandeen 2008-01-28 5361 }
aa75f4d3daaeb1 Harshad Shirwadkar 2020-10-15 5362 }
aa75f4d3daaeb1 Harshad Shirwadkar 2020-10-15 5363 if (!S_ISREG(inode->i_mode)) {
3da40c7b089810 Josef Bacik 2015-06-22 5364 return -EINVAL;
aa75f4d3daaeb1 Harshad Shirwadkar 2020-10-15 5365 }
dff6efc326a4d5 Christoph Hellwig 2013-11-19 5366
a642c2c0827f56 Jeff Layton 2022-09-08 5367 if (attr->ia_size == inode->i_size)
a642c2c0827f56 Jeff Layton 2022-09-08 5368 inc_ivers = false;
dff6efc326a4d5 Christoph Hellwig 2013-11-19 5369
b9c1c26739ec2d Jan Kara 2019-05-30 5370 if (shrink) {
b9c1c26739ec2d Jan Kara 2019-05-30 5371 if (ext4_should_order_data(inode)) {
5208386c501276 Jan Kara 2013-08-17 5372 error = ext4_begin_ordered_truncate(inode,
5208386c501276 Jan Kara 2013-08-17 5373 attr->ia_size);
5208386c501276 Jan Kara 2013-08-17 5374 if (error)
5208386c501276 Jan Kara 2013-08-17 5375 goto err_out;
5208386c501276 Jan Kara 2013-08-17 5376 }
b9c1c26739ec2d Jan Kara 2019-05-30 5377 /*
b9c1c26739ec2d Jan Kara 2019-05-30 5378 * Blocks are going to be removed from the inode. Wait
b9c1c26739ec2d Jan Kara 2019-05-30 5379 * for dio in flight.
b9c1c26739ec2d Jan Kara 2019-05-30 5380 */
b9c1c26739ec2d Jan Kara 2019-05-30 5381 inode_dio_wait(inode);
b9c1c26739ec2d Jan Kara 2019-05-30 5382 }
b9c1c26739ec2d Jan Kara 2019-05-30 5383
d4f5258eae7b38 Jan Kara 2021-02-04 5384 filemap_invalidate_lock(inode->i_mapping);
b9c1c26739ec2d Jan Kara 2019-05-30 5385
b9c1c26739ec2d Jan Kara 2019-05-30 5386 rc = ext4_break_layouts(inode);
b9c1c26739ec2d Jan Kara 2019-05-30 5387 if (rc) {
d4f5258eae7b38 Jan Kara 2021-02-04 5388 filemap_invalidate_unlock(inode->i_mapping);
aa75f4d3daaeb1 Harshad Shirwadkar 2020-10-15 5389 goto err_out;
b9c1c26739ec2d Jan Kara 2019-05-30 5390 }
b9c1c26739ec2d Jan Kara 2019-05-30 5391
3da40c7b089810 Josef Bacik 2015-06-22 5392 if (attr->ia_size != inode->i_size) {
9924a92a8c2175 Theodore Ts'o 2013-02-08 5393 handle = ext4_journal_start(inode, EXT4_HT_INODE, 3);
ac27a0ec112a08 Dave Kleikamp 2006-10-11 5394 if (IS_ERR(handle)) {
ac27a0ec112a08 Dave Kleikamp 2006-10-11 5395 error = PTR_ERR(handle);
b9c1c26739ec2d Jan Kara 2019-05-30 5396 goto out_mmap_sem;
ac27a0ec112a08 Dave Kleikamp 2006-10-11 5397 }
3da40c7b089810 Josef Bacik 2015-06-22 5398 if (ext4_handle_valid(handle) && shrink) {
617ba13b31fbf5 Mingming Cao 2006-10-11 5399 error = ext4_orphan_add(handle, inode);
3d287de3b82822 Dmitry Monakhov 2010-10-27 5400 orphan = 1;
3d287de3b82822 Dmitry Monakhov 2010-10-27 5401 }
911af577de4e44 Eryu Guan 2015-07-28 5402 /*
911af577de4e44 Eryu Guan 2015-07-28 5403 * Update c/mtime on truncate up, ext4_truncate() will
911af577de4e44 Eryu Guan 2015-07-28 5404 * update c/mtime in shrink case below
911af577de4e44 Eryu Guan 2015-07-28 5405 */
1bc33893e79a79 Jeff Layton 2023-07-05 5406 if (!shrink)
b898ab233611f7 Jeff Layton 2023-10-04 5407 inode_set_mtime_to_ts(inode,
b898ab233611f7 Jeff Layton 2023-10-04 5408 inode_set_ctime_current(inode));
aa75f4d3daaeb1 Harshad Shirwadkar 2020-10-15 5409
aa75f4d3daaeb1 Harshad Shirwadkar 2020-10-15 5410 if (shrink)
a80f7fcf18672a Harshad Shirwadkar 2020-11-05 5411 ext4_fc_track_range(handle, inode,
aa75f4d3daaeb1 Harshad Shirwadkar 2020-10-15 5412 (attr->ia_size > 0 ? attr->ia_size - 1 : 0) >>
aa75f4d3daaeb1 Harshad Shirwadkar 2020-10-15 5413 inode->i_sb->s_blocksize_bits,
9725958bb75cdf Xin Yin 2021-12-23 5414 EXT_MAX_BLOCKS - 1);
aa75f4d3daaeb1 Harshad Shirwadkar 2020-10-15 5415 else
aa75f4d3daaeb1 Harshad Shirwadkar 2020-10-15 5416 ext4_fc_track_range(
a80f7fcf18672a Harshad Shirwadkar 2020-11-05 5417 handle, inode,
aa75f4d3daaeb1 Harshad Shirwadkar 2020-10-15 5418 (oldsize > 0 ? oldsize - 1 : oldsize) >>
aa75f4d3daaeb1 Harshad Shirwadkar 2020-10-15 5419 inode->i_sb->s_blocksize_bits,
aa75f4d3daaeb1 Harshad Shirwadkar 2020-10-15 5420 (attr->ia_size > 0 ? attr->ia_size - 1 : 0) >>
aa75f4d3daaeb1 Harshad Shirwadkar 2020-10-15 5421 inode->i_sb->s_blocksize_bits);
aa75f4d3daaeb1 Harshad Shirwadkar 2020-10-15 5422
617ba13b31fbf5 Mingming Cao 2006-10-11 @5423 rc = ext4_mark_inode_dirty(handle, inode);
ac27a0ec112a08 Dave Kleikamp 2006-10-11 5424 if (!error)
ac27a0ec112a08 Dave Kleikamp 2006-10-11 5425 error = rc;
a879a75fe2d0ce Harshad Shirwadkar 2024-05-20 5426 down_write(&EXT4_I(inode)->i_data_sem);
a879a75fe2d0ce Harshad Shirwadkar 2024-05-20 5427 EXT4_I(inode)->i_disksize = attr->ia_size;
a879a75fe2d0ce Harshad Shirwadkar 2024-05-20 5428
90e775b71ac4e6 Jan Kara 2013-08-17 5429 /*
90e775b71ac4e6 Jan Kara 2013-08-17 5430 * We have to update i_size under i_data_sem together
90e775b71ac4e6 Jan Kara 2013-08-17 5431 * with i_disksize to avoid races with writeback code
90e775b71ac4e6 Jan Kara 2013-08-17 5432 * running ext4_wb_update_i_disksize().
90e775b71ac4e6 Jan Kara 2013-08-17 5433 */
90e775b71ac4e6 Jan Kara 2013-08-17 5434 if (!error)
90e775b71ac4e6 Jan Kara 2013-08-17 5435 i_size_write(inode, attr->ia_size);
f4534c9fc94d22 Ye Bin 2022-03-26 5436 else
f4534c9fc94d22 Ye Bin 2022-03-26 5437 EXT4_I(inode)->i_disksize = old_disksize;
90e775b71ac4e6 Jan Kara 2013-08-17 5438 up_write(&EXT4_I(inode)->i_data_sem);
617ba13b31fbf5 Mingming Cao 2006-10-11 5439 ext4_journal_stop(handle);
b9c1c26739ec2d Jan Kara 2019-05-30 5440 if (error)
b9c1c26739ec2d Jan Kara 2019-05-30 5441 goto out_mmap_sem;
82a25b027ca48d Jan Kara 2019-05-23 5442 if (!shrink) {
b9c1c26739ec2d Jan Kara 2019-05-30 5443 pagecache_isize_extended(inode, oldsize,
b9c1c26739ec2d Jan Kara 2019-05-30 5444 inode->i_size);
b9c1c26739ec2d Jan Kara 2019-05-30 5445 } else if (ext4_should_journal_data(inode)) {
82a25b027ca48d Jan Kara 2019-05-23 5446 ext4_wait_for_tail_page_commit(inode);
b9c1c26739ec2d Jan Kara 2019-05-30 5447 }
430657b6be896d Ross Zwisler 2018-07-29 5448 }
430657b6be896d Ross Zwisler 2018-07-29 5449
53e872681fed6a Jan Kara 2012-12-25 5450 /*
53e872681fed6a Jan Kara 2012-12-25 5451 * Truncate pagecache after we've waited for commit
53e872681fed6a Jan Kara 2012-12-25 5452 * in data=journal mode to make pages freeable.
53e872681fed6a Jan Kara 2012-12-25 5453 */
7caef26767c172 Kirill A. Shutemov 2013-09-12 5454 truncate_pagecache(inode, inode->i_size);
b9c1c26739ec2d Jan Kara 2019-05-30 5455 /*
b9c1c26739ec2d Jan Kara 2019-05-30 5456 * Call ext4_truncate() even if i_size didn't change to
b9c1c26739ec2d Jan Kara 2019-05-30 5457 * truncate possible preallocated blocks.
b9c1c26739ec2d Jan Kara 2019-05-30 5458 */
b9c1c26739ec2d Jan Kara 2019-05-30 5459 if (attr->ia_size <= oldsize) {
2c98eb5ea24976 Theodore Ts'o 2016-11-13 5460 rc = ext4_truncate(inode);
2c98eb5ea24976 Theodore Ts'o 2016-11-13 5461 if (rc)
2c98eb5ea24976 Theodore Ts'o 2016-11-13 5462 error = rc;
2c98eb5ea24976 Theodore Ts'o 2016-11-13 5463 }
b9c1c26739ec2d Jan Kara 2019-05-30 5464 out_mmap_sem:
d4f5258eae7b38 Jan Kara 2021-02-04 5465 filemap_invalidate_unlock(inode->i_mapping);
3da40c7b089810 Josef Bacik 2015-06-22 5466 }
ac27a0ec112a08 Dave Kleikamp 2006-10-11 5467
2c98eb5ea24976 Theodore Ts'o 2016-11-13 5468 if (!error) {
a642c2c0827f56 Jeff Layton 2022-09-08 5469 if (inc_ivers)
a642c2c0827f56 Jeff Layton 2022-09-08 5470 inode_inc_iversion(inode);
c1632a0f112093 Christian Brauner 2023-01-13 5471 setattr_copy(idmap, inode, attr);
1025774ce411f2 Christoph Hellwig 2010-06-04 5472 mark_inode_dirty(inode);
1025774ce411f2 Christoph Hellwig 2010-06-04 5473 }
1025774ce411f2 Christoph Hellwig 2010-06-04 5474
1025774ce411f2 Christoph Hellwig 2010-06-04 5475 /*
1025774ce411f2 Christoph Hellwig 2010-06-04 5476 * If the call to ext4_truncate failed to get a transaction handle at
1025774ce411f2 Christoph Hellwig 2010-06-04 5477 * all, we need to clean up the in-core orphan list manually.
1025774ce411f2 Christoph Hellwig 2010-06-04 5478 */
3d287de3b82822 Dmitry Monakhov 2010-10-27 5479 if (orphan && inode->i_nlink)
617ba13b31fbf5 Mingming Cao 2006-10-11 5480 ext4_orphan_del(NULL, inode);
ac27a0ec112a08 Dave Kleikamp 2006-10-11 5481
2c98eb5ea24976 Theodore Ts'o 2016-11-13 5482 if (!error && (ia_valid & ATTR_MODE))
13e83a4923bea7 Christian Brauner 2023-01-13 5483 rc = posix_acl_chmod(idmap, dentry, inode->i_mode);
ac27a0ec112a08 Dave Kleikamp 2006-10-11 5484
ac27a0ec112a08 Dave Kleikamp 2006-10-11 5485 err_out:
aa75f4d3daaeb1 Harshad Shirwadkar 2020-10-15 5486 if (error)
617ba13b31fbf5 Mingming Cao 2006-10-11 5487 ext4_std_error(inode->i_sb, error);
ac27a0ec112a08 Dave Kleikamp 2006-10-11 5488 if (!error)
ac27a0ec112a08 Dave Kleikamp 2006-10-11 5489 error = rc;
ac27a0ec112a08 Dave Kleikamp 2006-10-11 5490 return error;
ac27a0ec112a08 Dave Kleikamp 2006-10-11 5491 }
ac27a0ec112a08 Dave Kleikamp 2006-10-11 5492

--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

2024-05-20 22:08:25

by kernel test robot

[permalink] [raw]
Subject: Re: [PATCH 04/10] ext4: rework fast commit commit path

Hi Harshad,

kernel test robot noticed the following build errors:

[auto build test ERROR on tytso-ext4/dev]
[also build test ERROR on linus/master v6.9 next-20240520]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url: https://github.com/intel-lab-lkp/linux/commits/Harshad-Shirwadkar/ext4-convert-i_fc_lock-to-spinlock/20240520-135501
base: https://git.kernel.org/pub/scm/linux/kernel/git/tytso/ext4.git dev
patch link: https://lore.kernel.org/r/20240520055153.136091-5-harshadshirwadkar%40gmail.com
patch subject: [PATCH 04/10] ext4: rework fast commit commit path
config: hexagon-allmodconfig (https://download.01.org/0day-ci/archive/20240521/[email protected]/config)
compiler: clang version 19.0.0git (https://github.com/llvm/llvm-project fa9b1be45088dce1e4b602d451f118128b94237b)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240521/[email protected]/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <[email protected]>
| Closes: https://lore.kernel.org/oe-kbuild-all/[email protected]/

All errors (new ones prefixed by >>, old ones prefixed by <<):

WARNING: modpost: missing MODULE_DESCRIPTION() in drivers/staging/greybus/gb-audio-manager.o
WARNING: modpost: missing MODULE_DESCRIPTION() in drivers/staging/greybus/gb-gbphy.o
WARNING: modpost: missing MODULE_DESCRIPTION() in drivers/staging/greybus/gb-gpio.o
WARNING: modpost: missing MODULE_DESCRIPTION() in drivers/staging/greybus/gb-i2c.o
WARNING: modpost: missing MODULE_DESCRIPTION() in drivers/staging/greybus/gb-pwm.o
WARNING: modpost: missing MODULE_DESCRIPTION() in drivers/staging/greybus/gb-sdio.o
WARNING: modpost: missing MODULE_DESCRIPTION() in drivers/staging/greybus/gb-spi.o
WARNING: modpost: missing MODULE_DESCRIPTION() in drivers/staging/greybus/gb-uart.o
WARNING: modpost: missing MODULE_DESCRIPTION() in drivers/staging/greybus/gb-usb.o
WARNING: modpost: missing MODULE_DESCRIPTION() in drivers/platform/goldfish/goldfish_pipe.o
WARNING: modpost: missing MODULE_DESCRIPTION() in drivers/platform/chrome/cros_kunit_proto_test.o
WARNING: modpost: missing MODULE_DESCRIPTION() in drivers/mailbox/mtk-cmdq-mailbox.o
WARNING: modpost: missing MODULE_DESCRIPTION() in drivers/devfreq/governor_simpleondemand.o
WARNING: modpost: missing MODULE_DESCRIPTION() in drivers/devfreq/governor_performance.o
WARNING: modpost: missing MODULE_DESCRIPTION() in drivers/devfreq/governor_powersave.o
WARNING: modpost: missing MODULE_DESCRIPTION() in drivers/devfreq/governor_userspace.o
WARNING: modpost: missing MODULE_DESCRIPTION() in drivers/perf/arm-ccn.o
WARNING: modpost: missing MODULE_DESCRIPTION() in drivers/perf/fsl_imx8_ddr_perf.o
WARNING: modpost: missing MODULE_DESCRIPTION() in drivers/perf/arm_cspmu/arm_cspmu_module.o
WARNING: modpost: missing MODULE_DESCRIPTION() in drivers/perf/arm_cspmu/nvidia_cspmu.o
WARNING: modpost: missing MODULE_DESCRIPTION() in drivers/perf/arm_cspmu/ampere_cspmu.o
WARNING: modpost: missing MODULE_DESCRIPTION() in drivers/hwtracing/intel_th/intel_th_msu_sink.o
WARNING: modpost: missing MODULE_DESCRIPTION() in drivers/nvmem/nvmem-apple-efuses.o
WARNING: modpost: missing MODULE_DESCRIPTION() in drivers/nvmem/nvmem_brcm_nvram.o
WARNING: modpost: missing MODULE_DESCRIPTION() in drivers/nvmem/nvmem_u-boot-env.o
WARNING: modpost: missing MODULE_DESCRIPTION() in drivers/interconnect/imx/imx-interconnect.o
WARNING: modpost: missing MODULE_DESCRIPTION() in drivers/interconnect/imx/imx8mm-interconnect.o
WARNING: modpost: missing MODULE_DESCRIPTION() in drivers/interconnect/imx/imx8mq-interconnect.o
WARNING: modpost: missing MODULE_DESCRIPTION() in drivers/interconnect/imx/imx8mn-interconnect.o
WARNING: modpost: missing MODULE_DESCRIPTION() in drivers/interconnect/imx/imx8mp-interconnect.o
WARNING: modpost: missing MODULE_DESCRIPTION() in drivers/hte/hte-tegra194-test.o
WARNING: modpost: missing MODULE_DESCRIPTION() in drivers/vdpa/vdpa.o
WARNING: modpost: missing MODULE_DESCRIPTION() in drivers/parport/parport.o
WARNING: modpost: missing MODULE_DESCRIPTION() in drivers/mtd/parsers/brcm_u-boot.o
WARNING: modpost: missing MODULE_DESCRIPTION() in drivers/mtd/parsers/tplink_safeloader.o
WARNING: modpost: missing MODULE_DESCRIPTION() in drivers/mtd/chips/cfi_util.o
WARNING: modpost: missing MODULE_DESCRIPTION() in drivers/mtd/chips/cfi_cmdset_0020.o
WARNING: modpost: missing MODULE_DESCRIPTION() in drivers/mtd/maps/map_funcs.o
WARNING: modpost: missing MODULE_DESCRIPTION() in drivers/spmi/hisi-spmi-controller.o
WARNING: modpost: missing MODULE_DESCRIPTION() in drivers/spmi/spmi-pmic-arb.o
WARNING: modpost: missing MODULE_DESCRIPTION() in drivers/uio/uio.o
WARNING: modpost: missing MODULE_DESCRIPTION() in drivers/uio/uio_pruss.o
WARNING: modpost: missing MODULE_DESCRIPTION() in drivers/pcmcia/pcmcia_rsrc.o
WARNING: modpost: missing MODULE_DESCRIPTION() in drivers/hwmon/corsair-cpro.o
WARNING: modpost: missing MODULE_DESCRIPTION() in drivers/hwmon/mr75203.o
WARNING: modpost: missing MODULE_DESCRIPTION() in drivers/vhost/vringh.o
WARNING: modpost: missing MODULE_DESCRIPTION() in drivers/greybus/greybus.o
WARNING: modpost: missing MODULE_DESCRIPTION() in drivers/greybus/gb-es2.o
WARNING: modpost: missing MODULE_DESCRIPTION() in drivers/rpmsg/rpmsg_char.o
WARNING: modpost: missing MODULE_DESCRIPTION() in drivers/iio/adc/ingenic-adc.o
WARNING: modpost: missing MODULE_DESCRIPTION() in drivers/iio/adc/xilinx-ams.o
WARNING: modpost: missing MODULE_DESCRIPTION() in drivers/iio/buffer/kfifo_buf.o
WARNING: modpost: missing MODULE_DESCRIPTION() in drivers/fsi/fsi-core.o
WARNING: modpost: missing MODULE_DESCRIPTION() in drivers/fsi/fsi-master-hub.o
WARNING: modpost: missing MODULE_DESCRIPTION() in drivers/fsi/fsi-master-aspeed.o
WARNING: modpost: missing MODULE_DESCRIPTION() in drivers/fsi/fsi-master-gpio.o
WARNING: modpost: missing MODULE_DESCRIPTION() in drivers/fsi/fsi-master-ast-cf.o
WARNING: modpost: missing MODULE_DESCRIPTION() in drivers/fsi/fsi-scom.o
WARNING: modpost: missing MODULE_DESCRIPTION() in drivers/siox/siox-bus-gpio.o
WARNING: modpost: missing MODULE_DESCRIPTION() in drivers/counter/ftm-quaddec.o
WARNING: modpost: missing MODULE_DESCRIPTION() in sound/core/snd-pcm-dmaengine.o
WARNING: modpost: missing MODULE_DESCRIPTION() in sound/core/sound_kunit.o
WARNING: modpost: missing MODULE_DESCRIPTION() in sound/drivers/snd-pcmtest.o
WARNING: modpost: missing MODULE_DESCRIPTION() in sound/pci/hda/snd-hda-cirrus-scodec-test.o
WARNING: modpost: missing MODULE_DESCRIPTION() in sound/soc/soc-topology-test.o
WARNING: modpost: missing MODULE_DESCRIPTION() in sound/soc/codecs/snd-soc-ab8500-codec.o
WARNING: modpost: missing MODULE_DESCRIPTION() in sound/soc/codecs/snd-soc-sigmadsp.o
WARNING: modpost: missing MODULE_DESCRIPTION() in sound/soc/codecs/snd-soc-wm-adsp.o
WARNING: modpost: missing MODULE_DESCRIPTION() in sound/soc/fsl/imx-pcm-dma.o
WARNING: modpost: missing MODULE_DESCRIPTION() in sound/soc/mxs/snd-soc-mxs-pcm.o
WARNING: modpost: missing MODULE_DESCRIPTION() in sound/soc/qcom/snd-soc-qcom-common.o
WARNING: modpost: missing MODULE_DESCRIPTION() in sound/soc/qcom/snd-soc-qcom-sdw.o
WARNING: modpost: missing MODULE_DESCRIPTION() in sound/soc/qcom/qdsp6/snd-q6dsp-common.o
WARNING: modpost: missing MODULE_DESCRIPTION() in sound/soc/sof/intel/snd-sof-intel-atom.o
WARNING: modpost: missing MODULE_DESCRIPTION() in sound/soc/sof/intel/snd-sof-acpi-intel-byt.o
WARNING: modpost: missing MODULE_DESCRIPTION() in sound/soc/sof/intel/snd-sof-acpi-intel-bdw.o
WARNING: modpost: missing MODULE_DESCRIPTION() in sound/soc/sof/imx/snd-sof-imx8.o
WARNING: modpost: missing MODULE_DESCRIPTION() in sound/soc/sof/imx/snd-sof-imx8m.o
WARNING: modpost: missing MODULE_DESCRIPTION() in sound/soc/sof/imx/snd-sof-imx8ulp.o
WARNING: modpost: missing MODULE_DESCRIPTION() in sound/soc/sof/imx/imx-common.o
WARNING: modpost: missing MODULE_DESCRIPTION() in sound/soc/sof/mediatek/mtk-adsp-common.o
WARNING: modpost: missing MODULE_DESCRIPTION() in sound/soc/sof/mediatek/mt8195/snd-sof-mt8195.o
WARNING: modpost: missing MODULE_DESCRIPTION() in sound/soc/sof/mediatek/mt8186/snd-sof-mt8186.o
WARNING: modpost: missing MODULE_DESCRIPTION() in sound/soc/sof/snd-sof-utils.o
WARNING: modpost: missing MODULE_DESCRIPTION() in sound/soc/sof/snd-sof-acpi.o
WARNING: modpost: missing MODULE_DESCRIPTION() in sound/soc/sof/snd-sof-of.o
WARNING: modpost: missing MODULE_DESCRIPTION() in sound/soc/xilinx/snd-soc-xlnx-i2s.o
WARNING: modpost: missing MODULE_DESCRIPTION() in sound/soc/xilinx/snd-soc-xlnx-formatter-pcm.o
WARNING: modpost: missing MODULE_DESCRIPTION() in sound/ac97_bus.o
WARNING: modpost: missing MODULE_DESCRIPTION() in samples/vfio-mdev/mtty.o
WARNING: modpost: missing MODULE_DESCRIPTION() in samples/vfio-mdev/mdpy.o
WARNING: modpost: missing MODULE_DESCRIPTION() in samples/vfio-mdev/mdpy-fb.o
WARNING: modpost: missing MODULE_DESCRIPTION() in samples/vfio-mdev/mbochs.o
WARNING: modpost: missing MODULE_DESCRIPTION() in samples/configfs/configfs_sample.o
WARNING: modpost: missing MODULE_DESCRIPTION() in samples/kfifo/bytestream-example.o
WARNING: modpost: missing MODULE_DESCRIPTION() in samples/kfifo/dma-example.o
WARNING: modpost: missing MODULE_DESCRIPTION() in samples/kfifo/inttype-example.o
WARNING: modpost: missing MODULE_DESCRIPTION() in samples/kfifo/record-example.o
WARNING: modpost: missing MODULE_DESCRIPTION() in samples/kobject/kobject-example.o
WARNING: modpost: missing MODULE_DESCRIPTION() in samples/kobject/kset-example.o
>> ERROR: modpost: "jbd2_journal_lock_updates_no_rsv" [fs/ext4/ext4.ko] undefined!

--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

2024-05-23 14:40:29

by Dan Carpenter

[permalink] [raw]
Subject: Re: [PATCH 09/10] ext4: temporarily elevate commit thread priority

Hi Harshad,

kernel test robot noticed the following build warnings:

https://git-scm.com/docs/git-format-patch#_base_tree_information]

url: https://github.com/intel-lab-lkp/linux/commits/Harshad-Shirwadkar/ext4-convert-i_fc_lock-to-spinlock/20240520-135501
base: https://git.kernel.org/pub/scm/linux/kernel/git/tytso/ext4.git dev
patch link: https://lore.kernel.org/r/20240520055153.136091-10-harshadshirwadkar%40gmail.com
patch subject: [PATCH 09/10] ext4: temporarily elevate commit thread priority
config: i386-randconfig-141-20240520 (https://download.01.org/0day-ci/archive/20240521/[email protected]/config)
compiler: clang version 18.1.5 (https://github.com/llvm/llvm-project 617a15a9eac96088ae5e9134248d8236e34b91b1)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <[email protected]>
| Reported-by: Dan Carpenter <[email protected]>
| Closes: https://lore.kernel.org/r/[email protected]/

smatch warnings:
fs/ext4/fast_commit.c:1280 ext4_fc_commit() error: uninitialized symbol 'old_ioprio'.

vim +/old_ioprio +1280 fs/ext4/fast_commit.c

aa75f4d3daaeb1 Harshad Shirwadkar 2020-10-15 1200 int ext4_fc_commit(journal_t *journal, tid_t commit_tid)
aa75f4d3daaeb1 Harshad Shirwadkar 2020-10-15 1201 {
c30365b90ab26f Yu Zhe 2022-04-01 1202 struct super_block *sb = journal->j_private;
aa75f4d3daaeb1 Harshad Shirwadkar 2020-10-15 1203 struct ext4_sb_info *sbi = EXT4_SB(sb);
aa75f4d3daaeb1 Harshad Shirwadkar 2020-10-15 1204 int nblks = 0, ret, bsize = journal->j_blocksize;
aa75f4d3daaeb1 Harshad Shirwadkar 2020-10-15 1205 int subtid = atomic_read(&sbi->s_fc_subtid);
0915e464cb2746 Harshad Shirwadkar 2021-12-23 1206 int status = EXT4_FC_STATUS_OK, fc_bufs_before = 0;
aa75f4d3daaeb1 Harshad Shirwadkar 2020-10-15 1207 ktime_t start_time, commit_time;
c3b2c196d67585 Harshad Shirwadkar 2024-05-20 1208 int old_ioprio, journal_ioprio;
aa75f4d3daaeb1 Harshad Shirwadkar 2020-10-15 1209
7f142440847480 Ritesh Harjani 2022-03-12 1210 if (!test_opt2(sb, JOURNAL_FAST_COMMIT))
7f142440847480 Ritesh Harjani 2022-03-12 1211 return jbd2_complete_transaction(journal, commit_tid);
7f142440847480 Ritesh Harjani 2022-03-12 1212
5641ace54471cb Ritesh Harjani 2022-03-12 1213 trace_ext4_fc_commit_start(sb, commit_tid);
aa75f4d3daaeb1 Harshad Shirwadkar 2020-10-15 1214
aa75f4d3daaeb1 Harshad Shirwadkar 2020-10-15 1215 start_time = ktime_get();
aa75f4d3daaeb1 Harshad Shirwadkar 2020-10-15 1216
aa75f4d3daaeb1 Harshad Shirwadkar 2020-10-15 1217 restart_fc:
aa75f4d3daaeb1 Harshad Shirwadkar 2020-10-15 1218 ret = jbd2_fc_begin_commit(journal, commit_tid);
aa75f4d3daaeb1 Harshad Shirwadkar 2020-10-15 1219 if (ret == -EALREADY) {
aa75f4d3daaeb1 Harshad Shirwadkar 2020-10-15 1220 /* There was an ongoing commit, check if we need to restart */
aa75f4d3daaeb1 Harshad Shirwadkar 2020-10-15 1221 if (atomic_read(&sbi->s_fc_subtid) <= subtid &&
aa75f4d3daaeb1 Harshad Shirwadkar 2020-10-15 1222 commit_tid > journal->j_commit_sequence)
aa75f4d3daaeb1 Harshad Shirwadkar 2020-10-15 1223 goto restart_fc;
d9bf099cb980d6 Ritesh Harjani 2022-03-12 1224 ext4_fc_update_stats(sb, EXT4_FC_STATUS_SKIPPED, 0, 0,
d9bf099cb980d6 Ritesh Harjani 2022-03-12 1225 commit_tid);
0915e464cb2746 Harshad Shirwadkar 2021-12-23 1226 return 0;
aa75f4d3daaeb1 Harshad Shirwadkar 2020-10-15 1227 } else if (ret) {
0915e464cb2746 Harshad Shirwadkar 2021-12-23 1228 /*
0915e464cb2746 Harshad Shirwadkar 2021-12-23 1229 * Commit couldn't start. Just update stats and perform a
0915e464cb2746 Harshad Shirwadkar 2021-12-23 1230 * full commit.
0915e464cb2746 Harshad Shirwadkar 2021-12-23 1231 */
d9bf099cb980d6 Ritesh Harjani 2022-03-12 1232 ext4_fc_update_stats(sb, EXT4_FC_STATUS_FAILED, 0, 0,
d9bf099cb980d6 Ritesh Harjani 2022-03-12 1233 commit_tid);
0915e464cb2746 Harshad Shirwadkar 2021-12-23 1234 return jbd2_complete_transaction(journal, commit_tid);
aa75f4d3daaeb1 Harshad Shirwadkar 2020-10-15 1235 }
0915e464cb2746 Harshad Shirwadkar 2021-12-23 1236
7bbbe241ec7ce0 Harshad Shirwadkar 2021-12-23 1237 /*
7bbbe241ec7ce0 Harshad Shirwadkar 2021-12-23 1238 * After establishing journal barrier via jbd2_fc_begin_commit(), check
7bbbe241ec7ce0 Harshad Shirwadkar 2021-12-23 1239 * if we are fast commit ineligible.
7bbbe241ec7ce0 Harshad Shirwadkar 2021-12-23 1240 */
7bbbe241ec7ce0 Harshad Shirwadkar 2021-12-23 1241 if (ext4_test_mount_flag(sb, EXT4_MF_FC_INELIGIBLE)) {
0915e464cb2746 Harshad Shirwadkar 2021-12-23 1242 status = EXT4_FC_STATUS_INELIGIBLE;
0915e464cb2746 Harshad Shirwadkar 2021-12-23 1243 goto fallback;

old_ioprio not initialized.

7bbbe241ec7ce0 Harshad Shirwadkar 2021-12-23 1244 }
aa75f4d3daaeb1 Harshad Shirwadkar 2020-10-15 1245
c3b2c196d67585 Harshad Shirwadkar 2024-05-20 1246 /*
c3b2c196d67585 Harshad Shirwadkar 2024-05-20 1247 * Now that we know that this thread is going to do a fast commit,
c3b2c196d67585 Harshad Shirwadkar 2024-05-20 1248 * elevate the priority to match that of the journal thread.
c3b2c196d67585 Harshad Shirwadkar 2024-05-20 1249 */
c3b2c196d67585 Harshad Shirwadkar 2024-05-20 1250 if (journal->j_task->io_context)
c3b2c196d67585 Harshad Shirwadkar 2024-05-20 1251 journal_ioprio = sbi->s_journal->j_task->io_context->ioprio;
c3b2c196d67585 Harshad Shirwadkar 2024-05-20 1252 else
c3b2c196d67585 Harshad Shirwadkar 2024-05-20 1253 journal_ioprio = EXT4_DEF_JOURNAL_IOPRIO;
c3b2c196d67585 Harshad Shirwadkar 2024-05-20 1254 old_ioprio = get_current_ioprio();
c3b2c196d67585 Harshad Shirwadkar 2024-05-20 1255 set_task_ioprio(current, journal_ioprio);
aa75f4d3daaeb1 Harshad Shirwadkar 2020-10-15 1256 fc_bufs_before = (sbi->s_fc_bytes + bsize - 1) / bsize;
aa75f4d3daaeb1 Harshad Shirwadkar 2020-10-15 1257 ret = ext4_fc_perform_commit(journal);
aa75f4d3daaeb1 Harshad Shirwadkar 2020-10-15 1258 if (ret < 0) {
0915e464cb2746 Harshad Shirwadkar 2021-12-23 1259 status = EXT4_FC_STATUS_FAILED;
0915e464cb2746 Harshad Shirwadkar 2021-12-23 1260 goto fallback;
aa75f4d3daaeb1 Harshad Shirwadkar 2020-10-15 1261 }
aa75f4d3daaeb1 Harshad Shirwadkar 2020-10-15 1262 nblks = (sbi->s_fc_bytes + bsize - 1) / bsize - fc_bufs_before;
aa75f4d3daaeb1 Harshad Shirwadkar 2020-10-15 1263 ret = jbd2_fc_wait_bufs(journal, nblks);
aa75f4d3daaeb1 Harshad Shirwadkar 2020-10-15 1264 if (ret < 0) {
0915e464cb2746 Harshad Shirwadkar 2021-12-23 1265 status = EXT4_FC_STATUS_FAILED;
0915e464cb2746 Harshad Shirwadkar 2021-12-23 1266 goto fallback;
aa75f4d3daaeb1 Harshad Shirwadkar 2020-10-15 1267 }
aa75f4d3daaeb1 Harshad Shirwadkar 2020-10-15 1268 atomic_inc(&sbi->s_fc_subtid);
0915e464cb2746 Harshad Shirwadkar 2021-12-23 1269 ret = jbd2_fc_end_commit(journal);
c3b2c196d67585 Harshad Shirwadkar 2024-05-20 1270 set_task_ioprio(current, old_ioprio);
aa75f4d3daaeb1 Harshad Shirwadkar 2020-10-15 1271 /*
0915e464cb2746 Harshad Shirwadkar 2021-12-23 1272 * weight the commit time higher than the average time so we
0915e464cb2746 Harshad Shirwadkar 2021-12-23 1273 * don't react too strongly to vast changes in the commit time
aa75f4d3daaeb1 Harshad Shirwadkar 2020-10-15 1274 */
0915e464cb2746 Harshad Shirwadkar 2021-12-23 1275 commit_time = ktime_to_ns(ktime_sub(ktime_get(), start_time));
d9bf099cb980d6 Ritesh Harjani 2022-03-12 1276 ext4_fc_update_stats(sb, status, commit_time, nblks, commit_tid);
0915e464cb2746 Harshad Shirwadkar 2021-12-23 1277 return ret;
0915e464cb2746 Harshad Shirwadkar 2021-12-23 1278
0915e464cb2746 Harshad Shirwadkar 2021-12-23 1279 fallback:
c3b2c196d67585 Harshad Shirwadkar 2024-05-20 @1280 set_task_ioprio(current, old_ioprio);
^^^^^^^^^^
Uninitialized

0915e464cb2746 Harshad Shirwadkar 2021-12-23 1281 ret = jbd2_fc_end_commit_fallback(journal);
d9bf099cb980d6 Ritesh Harjani 2022-03-12 1282 ext4_fc_update_stats(sb, status, 0, 0, commit_tid);
0915e464cb2746 Harshad Shirwadkar 2021-12-23 1283 return ret;
aa75f4d3daaeb1 Harshad Shirwadkar 2020-10-15 1284 }

--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki