lkml.org 
[lkml]   [2022]   [Jul]   [19]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    From
    Subject[PATCH 5.18 027/231] btrfs: return -EAGAIN for NOWAIT dio reads/writes on compressed and inline extents
    Date
    From: Filipe Manana <fdmanana@suse.com>

    commit a4527e1853f8ff6e0b7c2dadad6268bd38427a31 upstream.

    When doing a direct IO read or write, we always return -ENOTBLK when we
    find a compressed extent (or an inline extent) so that we fallback to
    buffered IO. This however is not ideal in case we are in a NOWAIT context
    (io_uring for example), because buffered IO can block and we currently
    have no support for NOWAIT semantics for buffered IO, so if we need to
    fallback to buffered IO we should first signal the caller that we may
    need to block by returning -EAGAIN instead.

    This behaviour can also result in short reads being returned to user
    space, which although it's not incorrect and user space should be able
    to deal with partial reads, it's somewhat surprising and even some popular
    applications like QEMU (Link tag #1) and MariaDB (Link tag #2) don't
    deal with short reads properly (or at all).

    The short read case happens when we try to read from a range that has a
    non-compressed and non-inline extent followed by a compressed extent.
    After having read the first extent, when we find the compressed extent we
    return -ENOTBLK from btrfs_dio_iomap_begin(), which results in iomap to
    treat the request as a short read, returning 0 (success) and waiting for
    previously submitted bios to complete (this happens at
    fs/iomap/direct-io.c:__iomap_dio_rw()). After that, and while at
    btrfs_file_read_iter(), we call filemap_read() to use buffered IO to
    read the remaining data, and pass it the number of bytes we were able to
    read with direct IO. Than at filemap_read() if we get a page fault error
    when accessing the read buffer, we return a partial read instead of an
    -EFAULT error, because the number of bytes previously read is greater
    than zero.

    So fix this by returning -EAGAIN for NOWAIT direct IO when we find a
    compressed or an inline extent.

    Reported-by: Dominique MARTINET <dominique.martinet@atmark-techno.com>
    Link: https://lore.kernel.org/linux-btrfs/YrrFGO4A1jS0GI0G@atmark-techno.com/
    Link: https://jira.mariadb.org/browse/MDEV-27900?focusedCommentId=216582&page=com.atlassian.jira.plugin.system.issuetabpanels%3Acomment-tabpanel#comment-216582
    Tested-by: Dominique MARTINET <dominique.martinet@atmark-techno.com>
    CC: stable@vger.kernel.org # 5.10+
    Reviewed-by: Christoph Hellwig <hch@lst.de>
    Signed-off-by: Filipe Manana <fdmanana@suse.com>
    Signed-off-by: David Sterba <dsterba@suse.com>
    Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
    ---
    fs/btrfs/inode.c | 14 +++++++++++++-
    1 file changed, 13 insertions(+), 1 deletion(-)

    --- a/fs/btrfs/inode.c
    +++ b/fs/btrfs/inode.c
    @@ -7639,7 +7639,19 @@ static int btrfs_dio_iomap_begin(struct
    if (test_bit(EXTENT_FLAG_COMPRESSED, &em->flags) ||
    em->block_start == EXTENT_MAP_INLINE) {
    free_extent_map(em);
    - ret = -ENOTBLK;
    + /*
    + * If we are in a NOWAIT context, return -EAGAIN in order to
    + * fallback to buffered IO. This is not only because we can
    + * block with buffered IO (no support for NOWAIT semantics at
    + * the moment) but also to avoid returning short reads to user
    + * space - this happens if we were able to read some data from
    + * previous non-compressed extents and then when we fallback to
    + * buffered IO, at btrfs_file_read_iter() by calling
    + * filemap_read(), we fail to fault in pages for the read buffer,
    + * in which case filemap_read() returns a short read (the number
    + * of bytes previously read is > 0, so it does not return -EFAULT).
    + */
    + ret = (flags & IOMAP_NOWAIT) ? -EAGAIN : -ENOTBLK;
    goto unlock_err;
    }


    \
     
     \ /
      Last update: 2022-07-19 14:55    [W:2.512 / U:0.076 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site