From patchwork Sat Oct 17 20:10:54 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Kent Overstreet X-Patchwork-Id: 11842975 Return-Path: Received: from mail.kernel.org (pdx-korg-mail-1.web.codeaurora.org [172.30.200.123]) by pdx-korg-patchwork-2.web.codeaurora.org (Postfix) with ESMTP id 1D72817C9 for ; Sat, 17 Oct 2020 20:11:22 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id EE8B32076A for ; Sat, 17 Oct 2020 20:11:21 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b="sHcQXNn5" Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S2439705AbgJQULG (ORCPT ); Sat, 17 Oct 2020 16:11:06 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:46112 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S2439678AbgJQULG (ORCPT ); Sat, 17 Oct 2020 16:11:06 -0400 Received: from mail-il1-x141.google.com (mail-il1-x141.google.com [IPv6:2607:f8b0:4864:20::141]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id E5709C061755; Sat, 17 Oct 2020 13:11:05 -0700 (PDT) Received: by mail-il1-x141.google.com with SMTP id i7so4412754ils.7; Sat, 17 Oct 2020 13:11:05 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=from:to:cc:subject:date:message-id:in-reply-to:references :mime-version:content-transfer-encoding; bh=p9F1LK3+Lz+UVgIB4qvSHJTUsvrmhbMwqJqjXnJwHos=; b=sHcQXNn5xF/A6sGX1LBmwxA3byeUIlxEVucjHVnbsDDJnGnbj4DpNpYYauHSI9+dp9 OvvhHthQdOnOvwI8t8NVEFaDKzvZ/zWDYoR0xx8C+ANJSUjzMyxOh24VJ8nKhm9BINh4 8ZjhAXNzFDr11wStyBhC81W5TIjcZvNn8rte9kVAy6fyUx9quW6oFIit6lNMucEX8nl2 gPOFLwJAWACgSi8bbeMNzsOTboq2dT9UNyFg44x0naLukWlxuS59oXfdlnrQrfms0db3 uAx61Whk6EtvwFpl2104FjupiyA205ofH+Utcfw8CzsYurJZTfBGEhemO6zcpjh0CJqq sRbw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:cc:subject:date:message-id:in-reply-to :references:mime-version:content-transfer-encoding; bh=p9F1LK3+Lz+UVgIB4qvSHJTUsvrmhbMwqJqjXnJwHos=; b=I4CtWNvsHxE7u0b8dlEa3TL1U93zV/Nq227ld4O7EI+6WthxnIfTq61pGG/KxozWVo u6fbpPG0/BPTWYFZEoHODj2b+NZc3mjHUd4+T+/sX/VgjPjh40mP8a4eDoKOGk+GXeU1 lBqOo42UxYoYXJcKxOk0Np3GvHUyQQAVljBcz4A+63/tqhsqfAltwfyuRo6QKOOzfTAk Gh9It33rujIv7/VyVLk6C9xUvxg4YS4j0B8TmJlYa6kfiBMQHAOxyarAFkH8uQcSuiPU wnaGZt50Af9gbxXsLfNMnmzMty42306SmHm8qQnMwO34To46+lLbwFDj5gGVkamPoOw5 /fAQ== X-Gm-Message-State: AOAM532ujXdDhH21aLCKXYUOpYrt3K4C3QuG2X//LSv77C12nd9caj32 +cMWzWO+wfg1gev6lNa5Ol3E23vqAQ== X-Google-Smtp-Source: ABdhPJzNVycPuz/SmeG/mGfd+maO7tdKOAyIJzhiVsVN2pWezTRJgFfMOG89hiX33VQ7k78305HPFw== X-Received: by 2002:a05:6e02:14d1:: with SMTP id o17mr801546ilk.119.1602965464473; Sat, 17 Oct 2020 13:11:04 -0700 (PDT) Received: from zaphod.evilpiepirate.org ([2601:19b:c500:a1:d1eb:cfce:9b0b:f9e0]) by smtp.gmail.com with ESMTPSA id e204sm5583483iof.16.2020.10.17.13.11.03 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Sat, 17 Oct 2020 13:11:03 -0700 (PDT) From: Kent Overstreet To: linux-kernel@vger.kernel.org, linux-fsdevel@vger.kernel.org, akpm@linux-foundation.org Cc: Kent Overstreet , willy@infradead.org, Jens Axboe Subject: [PATCH 1/2] fs: Break generic_file_buffered_read up into multiple functions Date: Sat, 17 Oct 2020 16:10:54 -0400 Message-Id: <20201017201055.2216969-2-kent.overstreet@gmail.com> X-Mailer: git-send-email 2.28.0 In-Reply-To: <20201017201055.2216969-1-kent.overstreet@gmail.com> References: <20201017201055.2216969-1-kent.overstreet@gmail.com> MIME-Version: 1.0 Precedence: bulk List-ID: X-Mailing-List: linux-fsdevel@vger.kernel.org This is prep work for changing generic_file_buffered_read() to use find_get_pages_contig() to batch up all the pagecache lookups. This patch should be functionally identical to the existing code and changes as little as of the flow control as possible. More refactoring could be done, this patch is intended to be relatively minimal. Signed-off-by: Kent Overstreet Cc: Matthew Wilcox (Oracle) Cc: Jens Axboe Reviewed-by: Jens Axboe --- mm/filemap.c | 473 ++++++++++++++++++++++++++++----------------------- 1 file changed, 261 insertions(+), 212 deletions(-) diff --git a/mm/filemap.c b/mm/filemap.c index 99c49eeae7..482fd75d66 100644 --- a/mm/filemap.c +++ b/mm/filemap.c @@ -2138,6 +2138,234 @@ static void shrink_readahead_size_eio(struct file_ra_state *ra) ra->ra_pages /= 4; } +static int lock_page_for_iocb(struct kiocb *iocb, struct page *page) +{ + if (iocb->ki_flags & IOCB_WAITQ) + return lock_page_async(page, iocb->ki_waitq); + else if (iocb->ki_flags & IOCB_NOWAIT) + return trylock_page(page) ? 0 : -EAGAIN; + else + return lock_page_killable(page); +} + +static int generic_file_buffered_read_page_ok(struct kiocb *iocb, + struct iov_iter *iter, + struct page *page) +{ + struct address_space *mapping = iocb->ki_filp->f_mapping; + struct inode *inode = mapping->host; + struct file_ra_state *ra = &iocb->ki_filp->f_ra; + unsigned int offset = iocb->ki_pos & ~PAGE_MASK; + unsigned int bytes, copied; + loff_t isize, end_offset; + + BUG_ON(iocb->ki_pos >> PAGE_SHIFT != page->index); + + /* + * i_size must be checked after we know the page is Uptodate. + * + * Checking i_size after the check allows us to calculate + * the correct value for "bytes", which means the zero-filled + * part of the page is not copied back to userspace (unless + * another truncate extends the file - this is desired though). + */ + + isize = i_size_read(inode); + if (unlikely(iocb->ki_pos >= isize)) + return 1; + + end_offset = min_t(loff_t, isize, iocb->ki_pos + iter->count); + + bytes = min_t(loff_t, end_offset - iocb->ki_pos, PAGE_SIZE - offset); + + /* If users can be writing to this page using arbitrary + * virtual addresses, take care about potential aliasing + * before reading the page on the kernel side. + */ + if (mapping_writably_mapped(mapping)) + flush_dcache_page(page); + + /* + * Ok, we have the page, and it's up-to-date, so + * now we can copy it to user space... + */ + + copied = copy_page_to_iter(page, offset, bytes, iter); + + iocb->ki_pos += copied; + + /* + * When a sequential read accesses a page several times, + * only mark it as accessed the first time. + */ + if (iocb->ki_pos >> PAGE_SHIFT != ra->prev_pos >> PAGE_SHIFT) + mark_page_accessed(page); + + ra->prev_pos = iocb->ki_pos; + + if (copied < bytes) + return -EFAULT; + + return !iov_iter_count(iter) || iocb->ki_pos == isize; +} + +static struct page * +generic_file_buffered_read_readpage(struct kiocb *iocb, + struct file *filp, + struct address_space *mapping, + struct page *page) +{ + struct file_ra_state *ra = &filp->f_ra; + int error; + + if (iocb->ki_flags & (IOCB_NOIO | IOCB_NOWAIT)) { + unlock_page(page); + put_page(page); + return ERR_PTR(-EAGAIN); + } + + /* + * A previous I/O error may have been due to temporary + * failures, eg. multipath errors. + * PG_error will be set again if readpage fails. + */ + ClearPageError(page); + /* Start the actual read. The read will unlock the page. */ + error = mapping->a_ops->readpage(filp, page); + + if (unlikely(error)) { + put_page(page); + return error != AOP_TRUNCATED_PAGE ? ERR_PTR(error) : NULL; + } + + if (!PageUptodate(page)) { + error = lock_page_for_iocb(iocb, page); + if (unlikely(error)) { + put_page(page); + return ERR_PTR(error); + } + if (!PageUptodate(page)) { + if (page->mapping == NULL) { + /* + * invalidate_mapping_pages got it + */ + unlock_page(page); + put_page(page); + return NULL; + } + unlock_page(page); + shrink_readahead_size_eio(ra); + put_page(page); + return ERR_PTR(-EIO); + } + unlock_page(page); + } + + return page; +} + +static struct page * +generic_file_buffered_read_pagenotuptodate(struct kiocb *iocb, + struct file *filp, + struct iov_iter *iter, + struct page *page, + loff_t pos, loff_t count) +{ + struct address_space *mapping = filp->f_mapping; + struct inode *inode = mapping->host; + int error; + + /* + * See comment in do_read_cache_page on why + * wait_on_page_locked is used to avoid unnecessarily + * serialisations and why it's safe. + */ + if (iocb->ki_flags & IOCB_WAITQ) { + error = wait_on_page_locked_async(page, + iocb->ki_waitq); + } else { + error = wait_on_page_locked_killable(page); + } + if (unlikely(error)) { + put_page(page); + return ERR_PTR(error); + } + if (PageUptodate(page)) + return page; + + if (inode->i_blkbits == PAGE_SHIFT || + !mapping->a_ops->is_partially_uptodate) + goto page_not_up_to_date; + /* pipes can't handle partially uptodate pages */ + if (unlikely(iov_iter_is_pipe(iter))) + goto page_not_up_to_date; + if (!trylock_page(page)) + goto page_not_up_to_date; + /* Did it get truncated before we got the lock? */ + if (!page->mapping) + goto page_not_up_to_date_locked; + if (!mapping->a_ops->is_partially_uptodate(page, + pos & ~PAGE_MASK, count)) + goto page_not_up_to_date_locked; + unlock_page(page); + return page; + +page_not_up_to_date: + /* Get exclusive access to the page ... */ + error = lock_page_for_iocb(iocb, page); + if (unlikely(error)) { + put_page(page); + return ERR_PTR(error); + } + +page_not_up_to_date_locked: + /* Did it get truncated before we got the lock? */ + if (!page->mapping) { + unlock_page(page); + put_page(page); + return NULL; + } + + /* Did somebody else fill it already? */ + if (PageUptodate(page)) { + unlock_page(page); + return page; + } + + return generic_file_buffered_read_readpage(iocb, filp, mapping, page); +} + +static struct page * +generic_file_buffered_read_no_cached_page(struct kiocb *iocb, + struct iov_iter *iter) +{ + struct file *filp = iocb->ki_filp; + struct address_space *mapping = filp->f_mapping; + pgoff_t index = iocb->ki_pos >> PAGE_SHIFT; + struct page *page; + int error; + + if (iocb->ki_flags & IOCB_NOIO) + return ERR_PTR(-EAGAIN); + + /* + * Ok, it wasn't cached, so we need to create a new + * page.. + */ + page = page_cache_alloc(mapping); + if (!page) + return ERR_PTR(-ENOMEM); + + error = add_to_page_cache_lru(page, mapping, index, + mapping_gfp_constraint(mapping, GFP_KERNEL)); + if (error) { + put_page(page); + return error != -EEXIST ? ERR_PTR(error) : NULL; + } + + return generic_file_buffered_read_readpage(iocb, filp, mapping, page); +} + /** * generic_file_buffered_read - generic file read routine * @iocb: the iocb to read @@ -2161,29 +2389,19 @@ ssize_t generic_file_buffered_read(struct kiocb *iocb, struct address_space *mapping = filp->f_mapping; struct inode *inode = mapping->host; struct file_ra_state *ra = &filp->f_ra; - loff_t *ppos = &iocb->ki_pos; - pgoff_t index; + size_t orig_count = iov_iter_count(iter); pgoff_t last_index; - pgoff_t prev_index; - unsigned long offset; /* offset into pagecache page */ - unsigned int prev_offset; int error = 0; - if (unlikely(*ppos >= inode->i_sb->s_maxbytes)) + if (unlikely(iocb->ki_pos >= inode->i_sb->s_maxbytes)) return 0; iov_iter_truncate(iter, inode->i_sb->s_maxbytes); - index = *ppos >> PAGE_SHIFT; - prev_index = ra->prev_pos >> PAGE_SHIFT; - prev_offset = ra->prev_pos & (PAGE_SIZE-1); - last_index = (*ppos + iter->count + PAGE_SIZE-1) >> PAGE_SHIFT; - offset = *ppos & ~PAGE_MASK; + last_index = (iocb->ki_pos + iter->count + PAGE_SIZE-1) >> PAGE_SHIFT; for (;;) { + pgoff_t index = iocb->ki_pos >> PAGE_SHIFT; struct page *page; - pgoff_t end_index; - loff_t isize; - unsigned long nr, ret; cond_resched(); find_page: @@ -2192,6 +2410,14 @@ ssize_t generic_file_buffered_read(struct kiocb *iocb, goto out; } + /* + * We can't return -EIOCBQUEUED once we've done some work, so + * ensure we don't block: + */ + if ((iocb->ki_flags & IOCB_WAITQ) && + (written + orig_count - iov_iter_count(iter))) + iocb->ki_flags |= IOCB_NOWAIT; + page = find_get_page(mapping, index); if (!page) { if (iocb->ki_flags & IOCB_NOIO) @@ -2200,8 +2426,15 @@ ssize_t generic_file_buffered_read(struct kiocb *iocb, ra, filp, index, last_index - index); page = find_get_page(mapping, index); - if (unlikely(page == NULL)) - goto no_cached_page; + if (unlikely(page == NULL)) { + page = generic_file_buffered_read_no_cached_page(iocb, iter); + if (!page) + goto find_page; + if (IS_ERR(page)) { + error = PTR_ERR(page); + goto out; + } + } } if (PageReadahead(page)) { if (iocb->ki_flags & IOCB_NOIO) { @@ -2213,221 +2446,37 @@ ssize_t generic_file_buffered_read(struct kiocb *iocb, index, last_index - index); } if (!PageUptodate(page)) { - /* - * See comment in do_read_cache_page on why - * wait_on_page_locked is used to avoid unnecessarily - * serialisations and why it's safe. - */ - if (iocb->ki_flags & IOCB_WAITQ) { - if (written) { - put_page(page); - goto out; - } - error = wait_on_page_locked_async(page, - iocb->ki_waitq); - } else { - if (iocb->ki_flags & IOCB_NOWAIT) { - put_page(page); - goto would_block; - } - error = wait_on_page_locked_killable(page); - } - if (unlikely(error)) - goto readpage_error; - if (PageUptodate(page)) - goto page_ok; - - if (inode->i_blkbits == PAGE_SHIFT || - !mapping->a_ops->is_partially_uptodate) - goto page_not_up_to_date; - /* pipes can't handle partially uptodate pages */ - if (unlikely(iov_iter_is_pipe(iter))) - goto page_not_up_to_date; - if (!trylock_page(page)) - goto page_not_up_to_date; - /* Did it get truncated before we got the lock? */ - if (!page->mapping) - goto page_not_up_to_date_locked; - if (!mapping->a_ops->is_partially_uptodate(page, - offset, iter->count)) - goto page_not_up_to_date_locked; - unlock_page(page); - } -page_ok: - /* - * i_size must be checked after we know the page is Uptodate. - * - * Checking i_size after the check allows us to calculate - * the correct value for "nr", which means the zero-filled - * part of the page is not copied back to userspace (unless - * another truncate extends the file - this is desired though). - */ - - isize = i_size_read(inode); - end_index = (isize - 1) >> PAGE_SHIFT; - if (unlikely(!isize || index > end_index)) { - put_page(page); - goto out; - } - - /* nr is the maximum number of bytes to copy from this page */ - nr = PAGE_SIZE; - if (index == end_index) { - nr = ((isize - 1) & ~PAGE_MASK) + 1; - if (nr <= offset) { + if (iocb->ki_flags & IOCB_NOWAIT) { put_page(page); + error = -EAGAIN; goto out; } - } - nr = nr - offset; - - /* If users can be writing to this page using arbitrary - * virtual addresses, take care about potential aliasing - * before reading the page on the kernel side. - */ - if (mapping_writably_mapped(mapping)) - flush_dcache_page(page); - - /* - * When a sequential read accesses a page several times, - * only mark it as accessed the first time. - */ - if (prev_index != index || offset != prev_offset) - mark_page_accessed(page); - prev_index = index; - - /* - * Ok, we have the page, and it's up-to-date, so - * now we can copy it to user space... - */ - - ret = copy_page_to_iter(page, offset, nr, iter); - offset += ret; - index += offset >> PAGE_SHIFT; - offset &= ~PAGE_MASK; - prev_offset = offset; - - put_page(page); - written += ret; - if (!iov_iter_count(iter)) - goto out; - if (ret < nr) { - error = -EFAULT; - goto out; - } - continue; - -page_not_up_to_date: - /* Get exclusive access to the page ... */ - if (iocb->ki_flags & IOCB_WAITQ) - error = lock_page_async(page, iocb->ki_waitq); - else - error = lock_page_killable(page); - if (unlikely(error)) - goto readpage_error; - -page_not_up_to_date_locked: - /* Did it get truncated before we got the lock? */ - if (!page->mapping) { - unlock_page(page); - put_page(page); - continue; - } - - /* Did somebody else fill it already? */ - if (PageUptodate(page)) { - unlock_page(page); - goto page_ok; - } - -readpage: - if (iocb->ki_flags & (IOCB_NOIO | IOCB_NOWAIT)) { - unlock_page(page); - put_page(page); - goto would_block; - } - /* - * A previous I/O error may have been due to temporary - * failures, eg. multipath errors. - * PG_error will be set again if readpage fails. - */ - ClearPageError(page); - /* Start the actual read. The read will unlock the page. */ - error = mapping->a_ops->readpage(filp, page); - - if (unlikely(error)) { - if (error == AOP_TRUNCATED_PAGE) { - put_page(page); - error = 0; + page = generic_file_buffered_read_pagenotuptodate(iocb, + filp, iter, page, iocb->ki_pos, iter->count); + if (!page) goto find_page; + if (IS_ERR(page)) { + error = PTR_ERR(page); + goto out; } - goto readpage_error; - } - - if (!PageUptodate(page)) { - if (iocb->ki_flags & IOCB_WAITQ) - error = lock_page_async(page, iocb->ki_waitq); - else - error = lock_page_killable(page); - - if (unlikely(error)) - goto readpage_error; - if (!PageUptodate(page)) { - if (page->mapping == NULL) { - /* - * invalidate_mapping_pages got it - */ - unlock_page(page); - put_page(page); - goto find_page; - } - unlock_page(page); - shrink_readahead_size_eio(ra); - error = -EIO; - goto readpage_error; - } - unlock_page(page); } - goto page_ok; - -readpage_error: - /* UHHUH! A synchronous read error occurred. Report it */ + error = generic_file_buffered_read_page_ok(iocb, iter, page); put_page(page); - goto out; -no_cached_page: - /* - * Ok, it wasn't cached, so we need to create a new - * page.. - */ - page = page_cache_alloc(mapping); - if (!page) { - error = -ENOMEM; - goto out; - } - error = add_to_page_cache_lru(page, mapping, index, - mapping_gfp_constraint(mapping, GFP_KERNEL)); if (error) { - put_page(page); - if (error == -EEXIST) { + if (error > 0) error = 0; - goto find_page; - } goto out; } - goto readpage; } would_block: error = -EAGAIN; out: - ra->prev_pos = prev_index; - ra->prev_pos <<= PAGE_SHIFT; - ra->prev_pos |= prev_offset; - - *ppos = ((loff_t)index << PAGE_SHIFT) + offset; file_accessed(filp); + written += orig_count - iov_iter_count(iter); + return written ? written : error; } EXPORT_SYMBOL_GPL(generic_file_buffered_read); From patchwork Sat Oct 17 20:10:55 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Kent Overstreet X-Patchwork-Id: 11842977 Return-Path: Received: from mail.kernel.org (pdx-korg-mail-1.web.codeaurora.org [172.30.200.123]) by pdx-korg-patchwork-2.web.codeaurora.org (Postfix) with ESMTP id 42D191580 for ; Sat, 17 Oct 2020 20:11:22 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 280A72075E for ; Sat, 17 Oct 2020 20:11:22 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b="stHdPGw+" Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S2439713AbgJQULH (ORCPT ); Sat, 17 Oct 2020 16:11:07 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:46118 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S2439678AbgJQULG (ORCPT ); Sat, 17 Oct 2020 16:11:06 -0400 Received: from mail-il1-x144.google.com (mail-il1-x144.google.com [IPv6:2607:f8b0:4864:20::144]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id C53F6C061755; Sat, 17 Oct 2020 13:11:06 -0700 (PDT) Received: by mail-il1-x144.google.com with SMTP id l16so6398837ilj.9; Sat, 17 Oct 2020 13:11:06 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=from:to:cc:subject:date:message-id:in-reply-to:references :mime-version:content-transfer-encoding; bh=I2HTdHUjGksNurF6c8jj/TqT2hOjm1bZIM6AKpACqag=; b=stHdPGw+XninTpDUS8RQvmt0MYfa6Ym0tLVnH4c2fhJeaVyVJxEUlS9zV/pBZHjSxJ K+oGqDy63+FBJnNM3Ep13Nxpx/N+9wX8D/uyrBPlsivyrTcYmLUXIUn4iEO++0cgvByY x3r2HVs84LKePtyeOmr0VLNlFdpPF1XXPUOo2yaAfo8FW7xZe8v7rvNa2w2j4WpySsfB fEdikhCDTwCNEnnNygNDlaX5+GY3knYnF9ArcjhnfxchWmDPR31LUBqSDc/Va4aZVX8i 88Aqcx/LvZyh7JbjwwR1lcauAPD47StMqFQDK6evAzeoAPtRsJupgoSLZjWzODEj3UT5 jY1Q== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:cc:subject:date:message-id:in-reply-to :references:mime-version:content-transfer-encoding; bh=I2HTdHUjGksNurF6c8jj/TqT2hOjm1bZIM6AKpACqag=; b=XYYYjDuUgyawrMjjxZHpdthniIFV5faJmR3V4aq/So8+rJZXImmQgJIR03dX0Kd42t PnYjm3zL9vP4DCTxGvluYnr9ZQZzOp1Og1kqexdPASlHy1ISMuGx6i9+RmcB9D0CBPTG bS1li55jvliz+/dNi552Sbd/cpxs5eCEHqdvaxw/+7z5obLzd4aL1WjiwXNnt0kNp2DQ HtgkWD18hC/avZ5aifbhF2xFEzvftBdJb0umwXZHCd5g0jeFybqaWLJygz8WPhbW1Smg TWicBIvMd4yhgLr/W4fR8Mb5pGL93uWOfWtMCYXiEvyxMSPIhxwNRw4cwP0/sfN0SLDe lGGw== X-Gm-Message-State: AOAM530OOuHeflc/9B8dyA6ZGzkbgNRHiGW2i8081bxlemGm6jjBI8zr zJWo7peP/HNGHXWlqByDRO3C7Re3bg== X-Google-Smtp-Source: ABdhPJyTFdG8q/4Y4gVVl0yCLfq97cm8Y3feGzrEIYfVCNhpOsUc2hUOBjsyemOKiSsJ/gXXF/bxoA== X-Received: by 2002:a92:98c5:: with SMTP id a66mr6834896ill.50.1602965465742; Sat, 17 Oct 2020 13:11:05 -0700 (PDT) Received: from zaphod.evilpiepirate.org ([2601:19b:c500:a1:d1eb:cfce:9b0b:f9e0]) by smtp.gmail.com with ESMTPSA id e204sm5583483iof.16.2020.10.17.13.11.04 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Sat, 17 Oct 2020 13:11:05 -0700 (PDT) From: Kent Overstreet To: linux-kernel@vger.kernel.org, linux-fsdevel@vger.kernel.org, akpm@linux-foundation.org Cc: Kent Overstreet , willy@infradead.org Subject: [PATCH 2/2] fs: generic_file_buffered_read() now uses find_get_pages_contig Date: Sat, 17 Oct 2020 16:10:55 -0400 Message-Id: <20201017201055.2216969-3-kent.overstreet@gmail.com> X-Mailer: git-send-email 2.28.0 In-Reply-To: <20201017201055.2216969-1-kent.overstreet@gmail.com> References: <20201017201055.2216969-1-kent.overstreet@gmail.com> MIME-Version: 1.0 Precedence: bulk List-ID: X-Mailing-List: linux-fsdevel@vger.kernel.org Convert generic_file_buffered_read() to get pages to read from in batches, and then copy data to userspace from many pages at once - in particular, we now don't touch any cachelines that might be contended while we're in the loop to copy data to userspace. This is is a performance improvement on workloads that do buffered reads with large blocksizes, and a very large performance improvement if that file is also being accessed concurrently by different threads. On smaller reads (512 bytes), there's a very small performance improvement (1%, within the margin of error). Signed-off-by: Kent Overstreet Reviewed-by: Jens Axboe --- mm/filemap.c | 300 +++++++++++++++++++++++++++++---------------------- 1 file changed, 172 insertions(+), 128 deletions(-) diff --git a/mm/filemap.c b/mm/filemap.c index 482fd75d66..8cf5b5d9e6 100644 --- a/mm/filemap.c +++ b/mm/filemap.c @@ -2148,67 +2148,6 @@ static int lock_page_for_iocb(struct kiocb *iocb, struct page *page) return lock_page_killable(page); } -static int generic_file_buffered_read_page_ok(struct kiocb *iocb, - struct iov_iter *iter, - struct page *page) -{ - struct address_space *mapping = iocb->ki_filp->f_mapping; - struct inode *inode = mapping->host; - struct file_ra_state *ra = &iocb->ki_filp->f_ra; - unsigned int offset = iocb->ki_pos & ~PAGE_MASK; - unsigned int bytes, copied; - loff_t isize, end_offset; - - BUG_ON(iocb->ki_pos >> PAGE_SHIFT != page->index); - - /* - * i_size must be checked after we know the page is Uptodate. - * - * Checking i_size after the check allows us to calculate - * the correct value for "bytes", which means the zero-filled - * part of the page is not copied back to userspace (unless - * another truncate extends the file - this is desired though). - */ - - isize = i_size_read(inode); - if (unlikely(iocb->ki_pos >= isize)) - return 1; - - end_offset = min_t(loff_t, isize, iocb->ki_pos + iter->count); - - bytes = min_t(loff_t, end_offset - iocb->ki_pos, PAGE_SIZE - offset); - - /* If users can be writing to this page using arbitrary - * virtual addresses, take care about potential aliasing - * before reading the page on the kernel side. - */ - if (mapping_writably_mapped(mapping)) - flush_dcache_page(page); - - /* - * Ok, we have the page, and it's up-to-date, so - * now we can copy it to user space... - */ - - copied = copy_page_to_iter(page, offset, bytes, iter); - - iocb->ki_pos += copied; - - /* - * When a sequential read accesses a page several times, - * only mark it as accessed the first time. - */ - if (iocb->ki_pos >> PAGE_SHIFT != ra->prev_pos >> PAGE_SHIFT) - mark_page_accessed(page); - - ra->prev_pos = iocb->ki_pos; - - if (copied < bytes) - return -EFAULT; - - return !iov_iter_count(iter) || iocb->ki_pos == isize; -} - static struct page * generic_file_buffered_read_readpage(struct kiocb *iocb, struct file *filp, @@ -2366,6 +2305,92 @@ generic_file_buffered_read_no_cached_page(struct kiocb *iocb, return generic_file_buffered_read_readpage(iocb, filp, mapping, page); } +static int generic_file_buffered_read_get_pages(struct kiocb *iocb, + struct iov_iter *iter, + struct page **pages, + unsigned int nr) +{ + struct file *filp = iocb->ki_filp; + struct address_space *mapping = filp->f_mapping; + struct file_ra_state *ra = &filp->f_ra; + pgoff_t index = iocb->ki_pos >> PAGE_SHIFT; + pgoff_t last_index = (iocb->ki_pos + iter->count + PAGE_SIZE-1) >> PAGE_SHIFT; + int i, j, nr_got, err = 0; + + nr = min_t(unsigned long, last_index - index, nr); +find_page: + if (fatal_signal_pending(current)) + return -EINTR; + + nr_got = find_get_pages_contig(mapping, index, nr, pages); + if (nr_got) + goto got_pages; + + if (iocb->ki_flags & IOCB_NOIO) + return -EAGAIN; + + page_cache_sync_readahead(mapping, ra, filp, index, last_index - index); + + nr_got = find_get_pages_contig(mapping, index, nr, pages); + if (nr_got) + goto got_pages; + + pages[0] = generic_file_buffered_read_no_cached_page(iocb, iter); + err = PTR_ERR_OR_ZERO(pages[0]); + if (!IS_ERR_OR_NULL(pages[0])) + nr_got = 1; +got_pages: + for (i = 0; i < nr_got; i++) { + struct page *page = pages[i]; + pgoff_t pg_index = index + i; + loff_t pg_pos = max(iocb->ki_pos, + (loff_t) pg_index << PAGE_SHIFT); + loff_t pg_count = iocb->ki_pos + iter->count - pg_pos; + + if (PageReadahead(page)) { + if (iocb->ki_flags & IOCB_NOIO) { + for (j = i; j < nr_got; j++) + put_page(pages[j]); + nr_got = i; + err = -EAGAIN; + break; + } + page_cache_async_readahead(mapping, ra, filp, page, + pg_index, last_index - pg_index); + } + + if (!PageUptodate(page)) { + if ((iocb->ki_flags & IOCB_NOWAIT) || + ((iocb->ki_flags & IOCB_WAITQ) && i)) { + for (j = i; j < nr_got; j++) + put_page(pages[j]); + nr_got = i; + err = -EAGAIN; + break; + } + + page = generic_file_buffered_read_pagenotuptodate(iocb, + filp, iter, page, pg_pos, pg_count); + if (IS_ERR_OR_NULL(page)) { + for (j = i + 1; j < nr_got; j++) + put_page(pages[j]); + nr_got = i; + err = PTR_ERR_OR_ZERO(page); + break; + } + } + } + + if (likely(nr_got)) + return nr_got; + if (err) + return err; + /* + * No pages and no error means we raced and should retry: + */ + goto find_page; +} + /** * generic_file_buffered_read - generic file read routine * @iocb: the iocb to read @@ -2386,96 +2411,115 @@ ssize_t generic_file_buffered_read(struct kiocb *iocb, struct iov_iter *iter, ssize_t written) { struct file *filp = iocb->ki_filp; + struct file_ra_state *ra = &filp->f_ra; struct address_space *mapping = filp->f_mapping; struct inode *inode = mapping->host; - struct file_ra_state *ra = &filp->f_ra; - size_t orig_count = iov_iter_count(iter); - pgoff_t last_index; - int error = 0; + struct page *pages_onstack[PAGEVEC_SIZE], **pages = NULL; + unsigned int nr_pages = min_t(unsigned int, 512, + ((iocb->ki_pos + iter->count + PAGE_SIZE - 1) >> PAGE_SHIFT) - + (iocb->ki_pos >> PAGE_SHIFT)); + int i, pg_nr, error = 0; + bool writably_mapped; + loff_t isize, end_offset; if (unlikely(iocb->ki_pos >= inode->i_sb->s_maxbytes)) return 0; iov_iter_truncate(iter, inode->i_sb->s_maxbytes); - last_index = (iocb->ki_pos + iter->count + PAGE_SIZE-1) >> PAGE_SHIFT; + if (nr_pages > ARRAY_SIZE(pages_onstack)) + pages = kmalloc_array(nr_pages, sizeof(void *), GFP_KERNEL); - for (;;) { - pgoff_t index = iocb->ki_pos >> PAGE_SHIFT; - struct page *page; + if (!pages) { + pages = pages_onstack; + nr_pages = min_t(unsigned int, nr_pages, ARRAY_SIZE(pages_onstack)); + } + do { cond_resched(); -find_page: - if (fatal_signal_pending(current)) { - error = -EINTR; - goto out; - } /* * We can't return -EIOCBQUEUED once we've done some work, so * ensure we don't block: */ - if ((iocb->ki_flags & IOCB_WAITQ) && - (written + orig_count - iov_iter_count(iter))) + if ((iocb->ki_flags & IOCB_WAITQ) && written) iocb->ki_flags |= IOCB_NOWAIT; - page = find_get_page(mapping, index); - if (!page) { - if (iocb->ki_flags & IOCB_NOIO) - goto would_block; - page_cache_sync_readahead(mapping, - ra, filp, - index, last_index - index); - page = find_get_page(mapping, index); - if (unlikely(page == NULL)) { - page = generic_file_buffered_read_no_cached_page(iocb, iter); - if (!page) - goto find_page; - if (IS_ERR(page)) { - error = PTR_ERR(page); - goto out; - } - } - } - if (PageReadahead(page)) { - if (iocb->ki_flags & IOCB_NOIO) { - put_page(page); - goto out; - } - page_cache_async_readahead(mapping, - ra, filp, page, - index, last_index - index); - } - if (!PageUptodate(page)) { - if (iocb->ki_flags & IOCB_NOWAIT) { - put_page(page); - error = -EAGAIN; - goto out; - } - page = generic_file_buffered_read_pagenotuptodate(iocb, - filp, iter, page, iocb->ki_pos, iter->count); - if (!page) - goto find_page; - if (IS_ERR(page)) { - error = PTR_ERR(page); - goto out; - } + i = 0; + pg_nr = generic_file_buffered_read_get_pages(iocb, iter, + pages, nr_pages); + if (pg_nr < 0) { + error = pg_nr; + break; } - error = generic_file_buffered_read_page_ok(iocb, iter, page); - put_page(page); + /* + * i_size must be checked after we know the pages are Uptodate. + * + * Checking i_size after the check allows us to calculate + * the correct value for "nr", which means the zero-filled + * part of the page is not copied back to userspace (unless + * another truncate extends the file - this is desired though). + */ + isize = i_size_read(inode); + if (unlikely(iocb->ki_pos >= isize)) + goto put_pages; - if (error) { - if (error > 0) - error = 0; - goto out; + end_offset = min_t(loff_t, isize, iocb->ki_pos + iter->count); + + while ((iocb->ki_pos >> PAGE_SHIFT) + pg_nr > + (end_offset + PAGE_SIZE - 1) >> PAGE_SHIFT) + put_page(pages[--pg_nr]); + + /* + * Once we start copying data, we don't want to be touching any + * cachelines that might be contended: + */ + writably_mapped = mapping_writably_mapped(mapping); + + /* + * When a sequential read accesses a page several times, only + * mark it as accessed the first time. + */ + if (iocb->ki_pos >> PAGE_SHIFT != + ra->prev_pos >> PAGE_SHIFT) + mark_page_accessed(pages[0]); + for (i = 1; i < pg_nr; i++) + mark_page_accessed(pages[i]); + + for (i = 0; i < pg_nr; i++) { + unsigned int offset = iocb->ki_pos & ~PAGE_MASK; + unsigned int bytes = min_t(loff_t, end_offset - iocb->ki_pos, + PAGE_SIZE - offset); + unsigned int copied; + + /* + * If users can be writing to this page using arbitrary + * virtual addresses, take care about potential aliasing + * before reading the page on the kernel side. + */ + if (writably_mapped) + flush_dcache_page(pages[i]); + + copied = copy_page_to_iter(pages[i], offset, bytes, iter); + + written += copied; + iocb->ki_pos += copied; + ra->prev_pos = iocb->ki_pos; + + if (copied < bytes) { + error = -EFAULT; + break; + } } - } +put_pages: + for (i = 0; i < pg_nr; i++) + put_page(pages[i]); + } while (iov_iter_count(iter) && iocb->ki_pos < isize && !error); -would_block: - error = -EAGAIN; -out: file_accessed(filp); - written += orig_count - iov_iter_count(iter); + + if (pages != pages_onstack) + kfree(pages); return written ? written : error; }