On 01/27, NeilBrown wrote:
> f2fs_write_single_data_page() can return -EAGAIN if it cannot get
> the cp_rwsem lock - it holds a page lock and so cannot wait for it.
>
> Some code which calls f2fs_write_single_data_page() use
> congestion_wait() and then tries again. congestion_wait() doesn't do
> anything useful as congestion is no longer tracked. So this is just a
> simple sleep.
>
> A better approach is it wait until the cp_rwsem lock can be taken - then
> try again. There is certainly no point trying again *before* the lock
> can be taken.
>
> Signed-off-by: NeilBrown <neilb@xxxxxxx>
> ---
> fs/f2fs/compress.c | 6 +++---
> fs/f2fs/data.c | 9 ++++++---
> 2 files changed, 9 insertions(+), 6 deletions(-)
>
> diff --git a/fs/f2fs/compress.c b/fs/f2fs/compress.c
> index d0c3aeba5945..58ff7f4b296c 100644
> --- a/fs/f2fs/compress.c
> +++ b/fs/f2fs/compress.c
> @@ -1505,9 +1505,9 @@ static int f2fs_write_raw_pages(struct compress_ctx *cc,
> if (IS_NOQUOTA(cc->inode))
> return 0;
> ret = 0;
> - cond_resched();
> - congestion_wait(BLK_RW_ASYNC,
> - DEFAULT_IO_TIMEOUT);
> + /* Wait until we can get the lock, then try again. */
> + f2fs_lock_op(F2FS_I_SB(cc->inode));
> + f2fs_unlock_op(F2FS_I_SB(cc->inode));
Since checkpoint uses down_write(cp_rwsem), I'm not sure the write path is safe
and needs to wait for checkpoint. Can we just do io_schedule_timeout()?
> goto retry_write;
> }
> return ret;
> diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
> index 8c417864c66a..1d2341163e2c 100644
> --- a/fs/f2fs/data.c
> +++ b/fs/f2fs/data.c
> @@ -3047,9 +3047,12 @@ static int f2fs_write_cache_pages(struct address_space *mapping,
> } else if (ret == -EAGAIN) {
> ret = 0;
> if (wbc->sync_mode == WB_SYNC_ALL) {
> - cond_resched();
> - congestion_wait(BLK_RW_ASYNC,
> - DEFAULT_IO_TIMEOUT);
> + /* Wait until we can get the
> + * lock, then try again.
> + */
> + f2fs_lock_op(F2FS_I_SB(mapping->host));
> + f2fs_unlock_op(F2FS_I_SB(mapping->host));
> +
> goto retry_write;
> }
> goto next;
>
