Re: [PATCH] btrfs: Prevent nowait or async read from doing sync IO

[Pavel Begunkov <asml.silence@gmail.com>]

On 12/01/2021 15:36, David Sterba wrote:
> On Fri, Jan 08, 2021 at 12:02:48AM +0000, Martin Raiber wrote:
>> When reading from btrfs file via io_uring I get following
>> call traces:
> 
> Is there a way to reproduce by common tools (fio) or is a specialized
> one needed?

I'm not familiar with this particular issue, but:
should _probably_ be reproducible with fio with io_uring engine
or fio/t/io_uring tool.

> 
>> [<0>] wait_on_page_bit+0x12b/0x270
>> [<0>] read_extent_buffer_pages+0x2ad/0x360
>> [<0>] btree_read_extent_buffer_pages+0x97/0x110
>> [<0>] read_tree_block+0x36/0x60
>> [<0>] read_block_for_search.isra.0+0x1a9/0x360
>> [<0>] btrfs_search_slot+0x23d/0x9f0
>> [<0>] btrfs_lookup_csum+0x75/0x170
>> [<0>] btrfs_lookup_bio_sums+0x23d/0x630
>> [<0>] btrfs_submit_data_bio+0x109/0x180
>> [<0>] submit_one_bio+0x44/0x70
>> [<0>] extent_readahead+0x37a/0x3a0
>> [<0>] read_pages+0x8e/0x1f0
>> [<0>] page_cache_ra_unbounded+0x1aa/0x1f0
>> [<0>] generic_file_buffered_read+0x3eb/0x830
>> [<0>] io_iter_do_read+0x1a/0x40
>> [<0>] io_read+0xde/0x350
>> [<0>] io_issue_sqe+0x5cd/0xed0
>> [<0>] __io_queue_sqe+0xf9/0x370
>> [<0>] io_submit_sqes+0x637/0x910
>> [<0>] __x64_sys_io_uring_enter+0x22e/0x390
>> [<0>] do_syscall_64+0x33/0x80
>> [<0>] entry_SYSCALL_64_after_hwframe+0x44/0xa9
>>
>> Prevent those by setting IOCB_NOIO before calling
>> generic_file_buffered_read.
>>
>> Async read has the same problem. So disable that by removing
>> FMODE_BUF_RASYNC. This was added with commit
>> 8730f12b7962b21ea9ad2756abce1e205d22db84 ("btrfs: flag files as
> 
> Oh yeah that's the commit that went to btrfs code out-of-band. I am not
> familiar with the io_uring support and have no good idea what the new
> flag was supposed to do.

iirc, Jens did make buffered IO asynchronous by waiting on a page
with wait_page_queue, but don't remember well enough.

> 
>> supporting buffered async reads") with 5.9. Io_uring will read
>> the data via worker threads if it can't be read without sync IO
>> this way.
> 
> What are the implications of that? Like more context switching (due to
> the worker threads) or other potential performance related problems?

io_uring splits submission and completion steps and usually expect
submissions to happen quick and not block (at least for long),
otherwise it can't submit other requests, that reduces QD and so
forth. In the worst case it can serialise it to QD1. I guess the
same can be applied to AIO.

-- 
Pavel Begunkov