Re: [RFC PATCH] ext4: don't trap kswapd and allocating tasks on ext4 inode IO

From: Andreas Dilger
Date: Mon May 15 2017 - 21:34:36 EST


On May 15, 2017, at 5:46 PM, Johannes Weiner <hannes@xxxxxxxxxxx> wrote:
>
> We have observed across several workloads situations where kswapd and
> direct reclaimers get stuck in the inode shrinker of the ext4 / mount,
> causing allocation latencies across tasks in the system, while there
> are dozens of gigabytes of clean page cache covering multiple disks.
>
> The inode shrinker has provisions to skip any inodes that require
> writeback, to avoid tarpitting the entire system behind a single
> object when there are many other pools to recycle memory from. But
> that logic doesn't cover the situation where an ext4 inode is clean
> but journaled and tied to a commit that yet needs to hit the platter.
>
> Add a superblock operation that lets the generic inode shrinker query
> the filesystem whether evicting a given inode will require any IO; add
> an ext4 implementation that checks whether the journal is caught up to
> the commit id associated with the inode.
[snip]

> diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c
> index 5834c4d76be8..4cb6cf932d9a 100644
> --- a/fs/ext4/inode.c
> +++ b/fs/ext4/inode.c
> @@ -182,6 +182,23 @@ int ext4_truncate_restart_trans(handle_t *handle, struct inode *inode,
> return ret;
> }
>
> +int ext4_evict_needs_io(struct inode *inode)
> +{
> + if (inode->i_nlink &&
> + inode->i_ino != EXT4_JOURNAL_INO &&
> + ext4_should_journal_data(inode) &&
> + (S_ISLNK(inode->i_mode) || S_ISREG(inode->i_mode))) {
> + journal_t *journal = EXT4_SB(inode->i_sb)->s_journal;
> + tid_t commit_tid = EXT4_I(inode)->i_datasync_tid;
> + int err;
> +
> + err = __jbd2_complete_transaction(journal, commit_tid, false);

From a code style point of view, having a function exported with a name like
"__jbd2..." is probably bad. An argument of "false" is meaningless at the
caller here. It would be better to have a function name something like
jbd2_complete_transaction_nowait() exported, so that it is more clear what
this function does.

It would be even better to add a comment block for this function that
explains that the caller should use "jbd2_log_wait_commit(journal, tid)"
if it wants to wait for this transaction to complete commit.

Cheers, Andreas





Attachment: signature.asc
Description: Message signed with OpenPGP