From patchwork Thu Jul 2 16:51:17 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Andreas Gruenbacher X-Patchwork-Id: 11639817 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 C81F613B6 for ; Thu, 2 Jul 2020 16:51:39 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 9F205208D5 for ; Thu, 2 Jul 2020 16:51:39 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (1024-bit key) header.d=redhat.com header.i=@redhat.com header.b="PYPm+A7u" Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1726992AbgGBQvi (ORCPT ); Thu, 2 Jul 2020 12:51:38 -0400 Received: from us-smtp-1.mimecast.com ([207.211.31.81]:49137 "EHLO us-smtp-delivery-1.mimecast.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1726941AbgGBQvg (ORCPT ); Thu, 2 Jul 2020 12:51:36 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1593708693; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=FGS7zmuOCpOiRp7/K1lHThQp3FMTr/Ps3bbcjzSZ3xc=; b=PYPm+A7ulCIxP+PqFGrmqvTbaj9PORSNq1XKrqCs0f0BXATNP3+8tfvTbvu5EHURyfoeW7 LwsBhjRPt7UYNHFrWvkFY7WlHi9hrMajxPlqnCaZoWhgSCCJTVeP6hqq/biZpGZrCzx/iO kAPgiWUOfrYyfzmNrm3/mS2R0onlNdg= Received: from mimecast-mx01.redhat.com (mimecast-mx01.redhat.com [209.132.183.4]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-503-kTjKHIsVP9KWCXN-TygWfA-1; Thu, 02 Jul 2020 12:51:28 -0400 X-MC-Unique: kTjKHIsVP9KWCXN-TygWfA-1 Received: from smtp.corp.redhat.com (int-mx03.intmail.prod.int.phx2.redhat.com [10.5.11.13]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id B61EB8018AB; Thu, 2 Jul 2020 16:51:26 +0000 (UTC) Received: from max.home.com (unknown [10.40.192.26]) by smtp.corp.redhat.com (Postfix) with ESMTP id F02EB79231; Thu, 2 Jul 2020 16:51:24 +0000 (UTC) From: Andreas Gruenbacher To: Matthew Wilcox Cc: Dave Chinner , Linus Torvalds , linux-fsdevel@vger.kernel.org, linux-mm@kvack.org, linux-kernel@vger.kernel.org, Andreas Gruenbacher Subject: [RFC 1/4] gfs2: Revert readahead conversion Date: Thu, 2 Jul 2020 18:51:17 +0200 Message-Id: <20200702165120.1469875-2-agruenba@redhat.com> In-Reply-To: <20200702165120.1469875-1-agruenba@redhat.com> References: <20200702165120.1469875-1-agruenba@redhat.com> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 2.79 on 10.5.11.13 Sender: linux-fsdevel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-fsdevel@vger.kernel.org Commit d4388340ae0b ("fs: convert mpage_readpages to mpage_readahead") converted gfs2 and other filesystems from the ->readpages to the ->readahead address space operation. Other than ->readpages, ->readahead is passed the pages to readahead locked. Due to problems in the current page locking strategy, this is now causing deadlocks in gfs2. Fix this by reinstating mpage_readpages from before commit d4388340ae0b and by converting gfs2 back to ->readpages. Signed-off-by: Andreas Gruenbacher --- fs/gfs2/aops.c | 23 ++++++++----- fs/mpage.c | 75 +++++++++++++++++++++++++++++++++++++++++++ include/linux/mpage.h | 2 ++ 3 files changed, 92 insertions(+), 8 deletions(-) diff --git a/fs/gfs2/aops.c b/fs/gfs2/aops.c index 72c9560f4467..786c1ce8f030 100644 --- a/fs/gfs2/aops.c +++ b/fs/gfs2/aops.c @@ -577,7 +577,7 @@ int gfs2_internal_read(struct gfs2_inode *ip, char *buf, loff_t *pos, } /** - * gfs2_readahead - Read a bunch of pages at once + * gfs2_readpages - Read a bunch of pages at once * @file: The file to read from * @mapping: Address space info * @pages: List of pages to read @@ -590,24 +590,31 @@ int gfs2_internal_read(struct gfs2_inode *ip, char *buf, loff_t *pos, * obviously not something we'd want to do on too regular a basis. * Any I/O we ignore at this time will be done via readpage later. * 2. We don't handle stuffed files here we let readpage do the honours. - * 3. mpage_readahead() does most of the heavy lifting in the common case. + * 3. mpage_readpages() does most of the heavy lifting in the common case. * 4. gfs2_block_map() is relied upon to set BH_Boundary in the right places. */ -static void gfs2_readahead(struct readahead_control *rac) +static int gfs2_readpages(struct file *file, struct address_space *mapping, + struct list_head *pages, unsigned nr_pages) { - struct inode *inode = rac->mapping->host; + struct inode *inode = mapping->host; struct gfs2_inode *ip = GFS2_I(inode); + struct gfs2_sbd *sdp = GFS2_SB(inode); struct gfs2_holder gh; + int ret; gfs2_holder_init(ip->i_gl, LM_ST_SHARED, 0, &gh); - if (gfs2_glock_nq(&gh)) + ret = gfs2_glock_nq(&gh); + if (unlikely(ret)) goto out_uninit; if (!gfs2_is_stuffed(ip)) - mpage_readahead(rac, gfs2_block_map); + ret = mpage_readpages(mapping, pages, nr_pages, gfs2_block_map); gfs2_glock_dq(&gh); out_uninit: gfs2_holder_uninit(&gh); + if (unlikely(gfs2_withdrawn(sdp))) + ret = -EIO; + return ret; } /** @@ -826,7 +833,7 @@ static const struct address_space_operations gfs2_aops = { .writepage = gfs2_writepage, .writepages = gfs2_writepages, .readpage = gfs2_readpage, - .readahead = gfs2_readahead, + .readpages = gfs2_readpages, .bmap = gfs2_bmap, .invalidatepage = gfs2_invalidatepage, .releasepage = gfs2_releasepage, @@ -840,7 +847,7 @@ static const struct address_space_operations gfs2_jdata_aops = { .writepage = gfs2_jdata_writepage, .writepages = gfs2_jdata_writepages, .readpage = gfs2_readpage, - .readahead = gfs2_readahead, + .readpages = gfs2_readpages, .set_page_dirty = jdata_set_page_dirty, .bmap = gfs2_bmap, .invalidatepage = gfs2_invalidatepage, diff --git a/fs/mpage.c b/fs/mpage.c index 830e6cc2a9e7..5243a065a062 100644 --- a/fs/mpage.c +++ b/fs/mpage.c @@ -396,6 +396,81 @@ void mpage_readahead(struct readahead_control *rac, get_block_t get_block) } EXPORT_SYMBOL(mpage_readahead); +/** + * mpage_readpages - populate an address space with some pages & start reads against them + * @mapping: the address_space + * @pages: The address of a list_head which contains the target pages. These + * pages have their ->index populated and are otherwise uninitialised. + * The page at @pages->prev has the lowest file offset, and reads should be + * issued in @pages->prev to @pages->next order. + * @nr_pages: The number of pages at *@pages + * @get_block: The filesystem's block mapper function. + * + * This function walks the pages and the blocks within each page, building and + * emitting large BIOs. + * + * If anything unusual happens, such as: + * + * - encountering a page which has buffers + * - encountering a page which has a non-hole after a hole + * - encountering a page with non-contiguous blocks + * + * then this code just gives up and calls the buffer_head-based read function. + * It does handle a page which has holes at the end - that is a common case: + * the end-of-file on blocksize < PAGE_SIZE setups. + * + * BH_Boundary explanation: + * + * There is a problem. The mpage read code assembles several pages, gets all + * their disk mappings, and then submits them all. That's fine, but obtaining + * the disk mappings may require I/O. Reads of indirect blocks, for example. + * + * So an mpage read of the first 16 blocks of an ext2 file will cause I/O to be + * submitted in the following order: + * + * 12 0 1 2 3 4 5 6 7 8 9 10 11 13 14 15 16 + * + * because the indirect block has to be read to get the mappings of blocks + * 13,14,15,16. Obviously, this impacts performance. + * + * So what we do it to allow the filesystem's get_block() function to set + * BH_Boundary when it maps block 11. BH_Boundary says: mapping of the block + * after this one will require I/O against a block which is probably close to + * this one. So you should push what I/O you have currently accumulated. + * + * This all causes the disk requests to be issued in the correct order. + */ +int +mpage_readpages(struct address_space *mapping, struct list_head *pages, + unsigned nr_pages, get_block_t get_block) +{ + struct mpage_readpage_args args = { + .get_block = get_block, + .is_readahead = true, + }; + unsigned page_idx; + + for (page_idx = 0; page_idx < nr_pages; page_idx++) { + struct page *page = lru_to_page(pages); + + prefetchw(&page->flags); + list_del(&page->lru); + if (!add_to_page_cache_lru(page, mapping, + page->index, + readahead_gfp_mask(mapping))) { + args.page = page; + args.nr_pages = nr_pages - page_idx; + args.bio = do_mpage_readpage(&args); + } + put_page(page); + } + BUG_ON(!list_empty(pages)); + if (args.bio) + mpage_bio_submit(REQ_OP_READ, REQ_RAHEAD, args.bio); + return 0; +} +EXPORT_SYMBOL(mpage_readpages); + /* * This isn't called much at all */ diff --git a/include/linux/mpage.h b/include/linux/mpage.h index f4f5e90a6844..181f1b0fbd83 100644 --- a/include/linux/mpage.h +++ b/include/linux/mpage.h @@ -16,6 +16,8 @@ struct writeback_control; struct readahead_control; void mpage_readahead(struct readahead_control *, get_block_t get_block); +int mpage_readpages(struct address_space *, struct list_head *, unsigned, + get_block_t); int mpage_readpage(struct page *page, get_block_t get_block); int mpage_writepages(struct address_space *mapping, struct writeback_control *wbc, get_block_t get_block); From patchwork Thu Jul 2 16:51:18 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Andreas Gruenbacher X-Patchwork-Id: 11639815 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 833286C1 for ; Thu, 2 Jul 2020 16:51:38 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 6BAEB2088E for ; Thu, 2 Jul 2020 16:51:38 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (1024-bit key) header.d=redhat.com header.i=@redhat.com header.b="fadzxGD/" Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1726929AbgGBQve (ORCPT ); Thu, 2 Jul 2020 12:51:34 -0400 Received: from us-smtp-delivery-1.mimecast.com ([207.211.31.120]:38817 "EHLO us-smtp-1.mimecast.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1726300AbgGBQvd (ORCPT ); Thu, 2 Jul 2020 12:51:33 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1593708691; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=lasbI7WSZBNJiaWUYinp95/I+mBtTJm3Z60zJ8X/hrU=; b=fadzxGD/XkDTPnAnphYgEF7C53oeR3Fgq3s/e+SVtRk+NtHxJuUGiAbZWBESYNtoFBVy+2 zr0cZIUlGabLrkyWgnYeRqCBV1FHo6U34HHxD54rpuZpX/SUy+XXgMjqCVgLVBKEUBK/Zo 4/Vr49tnK5P2OqTe36gcw+jAoZjJUTM= Received: from mimecast-mx01.redhat.com (mimecast-mx01.redhat.com [209.132.183.4]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-79-sU6N5AAGOCmW1YN2G2tkNA-1; Thu, 02 Jul 2020 12:51:30 -0400 X-MC-Unique: sU6N5AAGOCmW1YN2G2tkNA-1 Received: from smtp.corp.redhat.com (int-mx03.intmail.prod.int.phx2.redhat.com [10.5.11.13]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id DA29513E9C1; Thu, 2 Jul 2020 16:51:28 +0000 (UTC) Received: from max.home.com (unknown [10.40.192.26]) by smtp.corp.redhat.com (Postfix) with ESMTP id 1F5E979231; Thu, 2 Jul 2020 16:51:26 +0000 (UTC) From: Andreas Gruenbacher To: Matthew Wilcox Cc: Dave Chinner , Linus Torvalds , linux-fsdevel@vger.kernel.org, linux-mm@kvack.org, linux-kernel@vger.kernel.org, Andreas Gruenbacher Subject: [RFC 2/4] fs: Add IOCB_NOIO flag for generic_file_read_iter Date: Thu, 2 Jul 2020 18:51:18 +0200 Message-Id: <20200702165120.1469875-3-agruenba@redhat.com> In-Reply-To: <20200702165120.1469875-1-agruenba@redhat.com> References: <20200702165120.1469875-1-agruenba@redhat.com> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 2.79 on 10.5.11.13 Sender: linux-fsdevel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-fsdevel@vger.kernel.org Add an IOCB_NOIO flag that indicates to generic_file_read_iter that it shouldn't trigger any filesystem I/O for the actual request or for readahead. This allows to do tentative reads out of the page cache as some filesystems allow, and to take the appropriate locks and retry the reads only if the requested pages are not cached. Signed-off-by: Andreas Gruenbacher --- include/linux/fs.h | 1 + mm/filemap.c | 16 ++++++++++++++-- 2 files changed, 15 insertions(+), 2 deletions(-) diff --git a/include/linux/fs.h b/include/linux/fs.h index 3f881a892ea7..1ab2ea19e883 100644 --- a/include/linux/fs.h +++ b/include/linux/fs.h @@ -315,6 +315,7 @@ enum rw_hint { #define IOCB_SYNC (1 << 5) #define IOCB_WRITE (1 << 6) #define IOCB_NOWAIT (1 << 7) +#define IOCB_NOIO (1 << 8) struct kiocb { struct file *ki_filp; diff --git a/mm/filemap.c b/mm/filemap.c index f0ae9a6308cb..e8318f99f468 100644 --- a/mm/filemap.c +++ b/mm/filemap.c @@ -2028,7 +2028,7 @@ ssize_t generic_file_buffered_read(struct kiocb *iocb, page = find_get_page(mapping, index); if (!page) { - if (iocb->ki_flags & IOCB_NOWAIT) + if (iocb->ki_flags & (IOCB_NOWAIT | IOCB_NOIO)) goto would_block; page_cache_sync_readahead(mapping, ra, filp, @@ -2038,12 +2038,16 @@ ssize_t generic_file_buffered_read(struct kiocb *iocb, goto no_cached_page; } 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) { + if (iocb->ki_flags & (IOCB_NOWAIT | IOCB_NOIO)) { put_page(page); goto would_block; } @@ -2249,6 +2253,14 @@ EXPORT_SYMBOL_GPL(generic_file_buffered_read); * * This is the "read_iter()" routine for all filesystems * that can use the page cache directly. + * + * The IOCB_NOWAIT flag in iocb->ki_flags indicates that -EAGAIN shall + * be returned when no data can be read without issuing I/O requests; + * this doesn't prevent readahead. The IOCB_NOIO flag indicates that + * -EAGAIN shall be returned when no data can be read without issuing + * I/O requests, and 0 shall be returned when readhead would be + * triggered. + * * Return: * * number of bytes copied, even for partial reads * * negative error code if nothing was read From patchwork Thu Jul 2 16:51:19 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Andreas Gruenbacher X-Patchwork-Id: 11639831 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 5DA4A13B6 for ; Thu, 2 Jul 2020 16:51:51 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 3D15F214DB for ; Thu, 2 Jul 2020 16:51:51 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (1024-bit key) header.d=redhat.com header.i=@redhat.com header.b="DoeOkZs8" Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1726996AbgGBQvj (ORCPT ); Thu, 2 Jul 2020 12:51:39 -0400 Received: from us-smtp-delivery-1.mimecast.com ([207.211.31.120]:38906 "EHLO us-smtp-1.mimecast.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1726796AbgGBQvh (ORCPT ); Thu, 2 Jul 2020 12:51:37 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1593708695; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=7+ndlDtkJHwk4RF17SCQ46WL0iE0gMN06cDaLV9IAwo=; b=DoeOkZs8q1RLP5oS3Q7jDgH7O7g/uTDq2DZrvyNqz6acsUGPuKWpTRz7gnMkUSNdBWwRpw 3VcPpHKzx1kOE5gJpihNnPtp9EJZA2+8DYrvuiVcn5qXymXE0XKIYctlkLPePsAVSemE8f B7rTO5HW5aSurl6vGJpU4Gl4GYeQRJ4= Received: from mimecast-mx01.redhat.com (mimecast-mx01.redhat.com [209.132.183.4]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-65-uHprCz_-N--qYsiVpAEwVg-1; Thu, 02 Jul 2020 12:51:32 -0400 X-MC-Unique: uHprCz_-N--qYsiVpAEwVg-1 Received: from smtp.corp.redhat.com (int-mx03.intmail.prod.int.phx2.redhat.com [10.5.11.13]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id 0DD66107BEF6; Thu, 2 Jul 2020 16:51:31 +0000 (UTC) Received: from max.home.com (unknown [10.40.192.26]) by smtp.corp.redhat.com (Postfix) with ESMTP id 430C279231; Thu, 2 Jul 2020 16:51:29 +0000 (UTC) From: Andreas Gruenbacher To: Matthew Wilcox Cc: Dave Chinner , Linus Torvalds , linux-fsdevel@vger.kernel.org, linux-mm@kvack.org, linux-kernel@vger.kernel.org, Andreas Gruenbacher Subject: [RFC 3/4] gfs2: Rework read and page fault locking Date: Thu, 2 Jul 2020 18:51:19 +0200 Message-Id: <20200702165120.1469875-4-agruenba@redhat.com> In-Reply-To: <20200702165120.1469875-1-agruenba@redhat.com> References: <20200702165120.1469875-1-agruenba@redhat.com> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 2.79 on 10.5.11.13 Sender: linux-fsdevel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-fsdevel@vger.kernel.org So far, gfs2 has taken the filesystem locks inside the ->readpage and ->readahead address space operations. These operations are too low-level, causing lock ordering problems and workarounds. To get rid of those, move the locking to the ->read_iter file and ->fault vm operations. The cache consistency model of filesystems like gfs2 is such that if data is found in the page cache, the data is up to date and can be used without taking any filesystem locks. If a page is not cached, filesystem locks must be taken before populating the page cache. To avoid taking the inode glock when the data is already cached, the ->read_iter file operation first tries to read the data with the IOCB_NOIO flag set. If that fails, the inode glock is taken and the operation is repeated with the IOCB_NOIO flag cleared. Signed-off-by: Andreas Gruenbacher --- fs/gfs2/aops.c | 59 ++++---------------------------------------------- fs/gfs2/file.c | 55 ++++++++++++++++++++++++++++++++++++++++++++-- 2 files changed, 57 insertions(+), 57 deletions(-) diff --git a/fs/gfs2/aops.c b/fs/gfs2/aops.c index 786c1ce8f030..28f097636e78 100644 --- a/fs/gfs2/aops.c +++ b/fs/gfs2/aops.c @@ -468,21 +468,10 @@ static int stuffed_readpage(struct gfs2_inode *ip, struct page *page) } -/** - * __gfs2_readpage - readpage - * @file: The file to read a page for - * @page: The page to read - * - * This is the core of gfs2's readpage. It's used by the internal file - * reading code as in that case we already hold the glock. Also it's - * called by gfs2_readpage() once the required lock has been granted. - */ - static int __gfs2_readpage(void *file, struct page *page) { struct gfs2_inode *ip = GFS2_I(page->mapping->host); struct gfs2_sbd *sdp = GFS2_SB(page->mapping->host); - int error; if (i_blocksize(page->mapping->host) == PAGE_SIZE && @@ -505,36 +494,11 @@ static int __gfs2_readpage(void *file, struct page *page) * gfs2_readpage - read a page of a file * @file: The file to read * @page: The page of the file - * - * This deals with the locking required. We have to unlock and - * relock the page in order to get the locking in the right - * order. */ static int gfs2_readpage(struct file *file, struct page *page) { - struct address_space *mapping = page->mapping; - struct gfs2_inode *ip = GFS2_I(mapping->host); - struct gfs2_holder gh; - int error; - - unlock_page(page); - gfs2_holder_init(ip->i_gl, LM_ST_SHARED, 0, &gh); - error = gfs2_glock_nq(&gh); - if (unlikely(error)) - goto out; - error = AOP_TRUNCATED_PAGE; - lock_page(page); - if (page->mapping == mapping && !PageUptodate(page)) - error = __gfs2_readpage(file, page); - else - unlock_page(page); - gfs2_glock_dq(&gh); -out: - gfs2_holder_uninit(&gh); - if (error && error != AOP_TRUNCATED_PAGE) - lock_page(page); - return error; + return __gfs2_readpage(file, page); } /** @@ -597,24 +561,9 @@ int gfs2_internal_read(struct gfs2_inode *ip, char *buf, loff_t *pos, static int gfs2_readpages(struct file *file, struct address_space *mapping, struct list_head *pages, unsigned nr_pages) { - struct inode *inode = mapping->host; - struct gfs2_inode *ip = GFS2_I(inode); - struct gfs2_sbd *sdp = GFS2_SB(inode); - struct gfs2_holder gh; - int ret; - - gfs2_holder_init(ip->i_gl, LM_ST_SHARED, 0, &gh); - ret = gfs2_glock_nq(&gh); - if (unlikely(ret)) - goto out_uninit; - if (!gfs2_is_stuffed(ip)) - ret = mpage_readpages(mapping, pages, nr_pages, gfs2_block_map); - gfs2_glock_dq(&gh); -out_uninit: - gfs2_holder_uninit(&gh); - if (unlikely(gfs2_withdrawn(sdp))) - ret = -EIO; - return ret; + if (!gfs2_is_stuffed(GFS2_I(mapping->host))) + return mpage_readpages(mapping, pages, nr_pages, gfs2_block_map); + return 0; } /** diff --git a/fs/gfs2/file.c b/fs/gfs2/file.c index fe305e4bfd37..607bbf4dfadb 100644 --- a/fs/gfs2/file.c +++ b/fs/gfs2/file.c @@ -558,8 +558,29 @@ static vm_fault_t gfs2_page_mkwrite(struct vm_fault *vmf) return block_page_mkwrite_return(ret); } +static vm_fault_t gfs2_fault(struct vm_fault *vmf) +{ + struct inode *inode = file_inode(vmf->vma->vm_file); + struct gfs2_inode *ip = GFS2_I(inode); + struct gfs2_holder gh; + vm_fault_t ret; + int err; + + gfs2_holder_init(ip->i_gl, LM_ST_SHARED, 0, &gh); + err = gfs2_glock_nq(&gh); + if (err) { + ret = block_page_mkwrite_return(err); + goto out_uninit; + } + ret = filemap_fault(vmf); + gfs2_glock_dq(&gh); +out_uninit: + gfs2_holder_uninit(&gh); + return ret; +} + static const struct vm_operations_struct gfs2_vm_ops = { - .fault = filemap_fault, + .fault = gfs2_fault, .map_pages = filemap_map_pages, .page_mkwrite = gfs2_page_mkwrite, }; @@ -824,15 +845,45 @@ static ssize_t gfs2_file_direct_write(struct kiocb *iocb, struct iov_iter *from) static ssize_t gfs2_file_read_iter(struct kiocb *iocb, struct iov_iter *to) { + struct gfs2_inode *ip; + struct gfs2_holder gh; + size_t written = 0; ssize_t ret; + gfs2_holder_mark_uninitialized(&gh); if (iocb->ki_flags & IOCB_DIRECT) { ret = gfs2_file_direct_read(iocb, to); if (likely(ret != -ENOTBLK)) return ret; iocb->ki_flags &= ~IOCB_DIRECT; } - return generic_file_read_iter(iocb, to); + iocb->ki_flags |= IOCB_NOIO; + ret = generic_file_read_iter(iocb, to); + iocb->ki_flags &= ~IOCB_NOIO; + if (ret >= 0) { + if (!iov_iter_count(to)) + return ret; + written = ret; + } else { + if (ret != -EAGAIN) + return ret; + if (iocb->ki_flags & IOCB_NOWAIT) + return ret; + } + ip = GFS2_I(iocb->ki_filp->f_mapping->host); + gfs2_holder_init(ip->i_gl, LM_ST_SHARED, 0, &gh); + ret = gfs2_glock_nq(&gh); + if (ret) + goto out_uninit; + ret = generic_file_read_iter(iocb, to); + if (ret > 0) + written += ret; + if (gfs2_holder_initialized(&gh)) + gfs2_glock_dq(&gh); +out_uninit: + if (gfs2_holder_initialized(&gh)) + gfs2_holder_uninit(&gh); + return written ? written : ret; } /** From patchwork Thu Jul 2 16:51:20 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Andreas Gruenbacher X-Patchwork-Id: 11639821 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 AA8D26C1 for ; Thu, 2 Jul 2020 16:51:41 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 899832088E for ; Thu, 2 Jul 2020 16:51:41 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (1024-bit key) header.d=redhat.com header.i=@redhat.com header.b="ff3t1jgr" Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1727018AbgGBQvk (ORCPT ); Thu, 2 Jul 2020 12:51:40 -0400 Received: from us-smtp-1.mimecast.com ([205.139.110.61]:28581 "EHLO us-smtp-delivery-1.mimecast.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1726964AbgGBQvi (ORCPT ); Thu, 2 Jul 2020 12:51:38 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1593708695; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=sRvSqiA63kUIbg8TR2a10APphiMEzUJywUYKdk62Smo=; b=ff3t1jgrlMLSeoH2sjtO6nlxwpqqbxjli+eEbQ72eRalJEDEq+WBPl38VDc1G2cTdnuyZ6 ws0pmk8G4TGmpPiG7J1BqcCTFGwVYkPWjtA3/xOCp3/cwEw1nLp0jqAOcVgvWMeY6dGcgz UB/HFA/pEuTsVTmYAugKi+SL2blTgjE= Received: from mimecast-mx01.redhat.com (mimecast-mx01.redhat.com [209.132.183.4]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-156-SKvVqmjYNAKsXvMS_sg_TQ-1; Thu, 02 Jul 2020 12:51:34 -0400 X-MC-Unique: SKvVqmjYNAKsXvMS_sg_TQ-1 Received: from smtp.corp.redhat.com (int-mx03.intmail.prod.int.phx2.redhat.com [10.5.11.13]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id 326B813E9C2; Thu, 2 Jul 2020 16:51:33 +0000 (UTC) Received: from max.home.com (unknown [10.40.192.26]) by smtp.corp.redhat.com (Postfix) with ESMTP id 697C379231; Thu, 2 Jul 2020 16:51:31 +0000 (UTC) From: Andreas Gruenbacher To: Matthew Wilcox Cc: Dave Chinner , Linus Torvalds , linux-fsdevel@vger.kernel.org, linux-mm@kvack.org, linux-kernel@vger.kernel.org, Andreas Gruenbacher Subject: [RFC 4/4] gfs2: Reinstate readahead conversion Date: Thu, 2 Jul 2020 18:51:20 +0200 Message-Id: <20200702165120.1469875-5-agruenba@redhat.com> In-Reply-To: <20200702165120.1469875-1-agruenba@redhat.com> References: <20200702165120.1469875-1-agruenba@redhat.com> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 2.79 on 10.5.11.13 Sender: linux-fsdevel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-fsdevel@vger.kernel.org Now that the locking order in gfs2 is fixed, switch back to using the ->readahead address space operation. With that, mpage_readpages is unused and can be removed. Signed-off-by: Andreas Gruenbacher --- fs/gfs2/aops.c | 19 +++++------ fs/mpage.c | 75 ------------------------------------------- include/linux/mpage.h | 2 -- 3 files changed, 10 insertions(+), 86 deletions(-) diff --git a/fs/gfs2/aops.c b/fs/gfs2/aops.c index 28f097636e78..68cd700a2719 100644 --- a/fs/gfs2/aops.c +++ b/fs/gfs2/aops.c @@ -541,7 +541,7 @@ int gfs2_internal_read(struct gfs2_inode *ip, char *buf, loff_t *pos, } /** - * gfs2_readpages - Read a bunch of pages at once + * gfs2_readahead - Read a bunch of pages at once * @file: The file to read from * @mapping: Address space info * @pages: List of pages to read @@ -554,16 +554,17 @@ int gfs2_internal_read(struct gfs2_inode *ip, char *buf, loff_t *pos, * obviously not something we'd want to do on too regular a basis. * Any I/O we ignore at this time will be done via readpage later. * 2. We don't handle stuffed files here we let readpage do the honours. - * 3. mpage_readpages() does most of the heavy lifting in the common case. + * 3. mpage_readahead() does most of the heavy lifting in the common case. * 4. gfs2_block_map() is relied upon to set BH_Boundary in the right places. */ -static int gfs2_readpages(struct file *file, struct address_space *mapping, - struct list_head *pages, unsigned nr_pages) +static void gfs2_readahead(struct readahead_control *rac) { - if (!gfs2_is_stuffed(GFS2_I(mapping->host))) - return mpage_readpages(mapping, pages, nr_pages, gfs2_block_map); - return 0; + struct inode *inode = rac->mapping->host; + struct gfs2_inode *ip = GFS2_I(inode); + + if (!gfs2_is_stuffed(ip)) + mpage_readahead(rac, gfs2_block_map); } /** @@ -782,7 +783,7 @@ static const struct address_space_operations gfs2_aops = { .writepage = gfs2_writepage, .writepages = gfs2_writepages, .readpage = gfs2_readpage, - .readpages = gfs2_readpages, + .readahead = gfs2_readahead, .bmap = gfs2_bmap, .invalidatepage = gfs2_invalidatepage, .releasepage = gfs2_releasepage, @@ -796,7 +797,7 @@ static const struct address_space_operations gfs2_jdata_aops = { .writepage = gfs2_jdata_writepage, .writepages = gfs2_jdata_writepages, .readpage = gfs2_readpage, - .readpages = gfs2_readpages, + .readahead = gfs2_readahead, .set_page_dirty = jdata_set_page_dirty, .bmap = gfs2_bmap, .invalidatepage = gfs2_invalidatepage, diff --git a/fs/mpage.c b/fs/mpage.c index 5243a065a062..830e6cc2a9e7 100644 --- a/fs/mpage.c +++ b/fs/mpage.c @@ -396,81 +396,6 @@ void mpage_readahead(struct readahead_control *rac, get_block_t get_block) } EXPORT_SYMBOL(mpage_readahead); -/** - * mpage_readpages - populate an address space with some pages & start reads against them - * @mapping: the address_space - * @pages: The address of a list_head which contains the target pages. These - * pages have their ->index populated and are otherwise uninitialised. - * The page at @pages->prev has the lowest file offset, and reads should be - * issued in @pages->prev to @pages->next order. - * @nr_pages: The number of pages at *@pages - * @get_block: The filesystem's block mapper function. - * - * This function walks the pages and the blocks within each page, building and - * emitting large BIOs. - * - * If anything unusual happens, such as: - * - * - encountering a page which has buffers - * - encountering a page which has a non-hole after a hole - * - encountering a page with non-contiguous blocks - * - * then this code just gives up and calls the buffer_head-based read function. - * It does handle a page which has holes at the end - that is a common case: - * the end-of-file on blocksize < PAGE_SIZE setups. - * - * BH_Boundary explanation: - * - * There is a problem. The mpage read code assembles several pages, gets all - * their disk mappings, and then submits them all. That's fine, but obtaining - * the disk mappings may require I/O. Reads of indirect blocks, for example. - * - * So an mpage read of the first 16 blocks of an ext2 file will cause I/O to be - * submitted in the following order: - * - * 12 0 1 2 3 4 5 6 7 8 9 10 11 13 14 15 16 - * - * because the indirect block has to be read to get the mappings of blocks - * 13,14,15,16. Obviously, this impacts performance. - * - * So what we do it to allow the filesystem's get_block() function to set - * BH_Boundary when it maps block 11. BH_Boundary says: mapping of the block - * after this one will require I/O against a block which is probably close to - * this one. So you should push what I/O you have currently accumulated. - * - * This all causes the disk requests to be issued in the correct order. - */ -int -mpage_readpages(struct address_space *mapping, struct list_head *pages, - unsigned nr_pages, get_block_t get_block) -{ - struct mpage_readpage_args args = { - .get_block = get_block, - .is_readahead = true, - }; - unsigned page_idx; - - for (page_idx = 0; page_idx < nr_pages; page_idx++) { - struct page *page = lru_to_page(pages); - - prefetchw(&page->flags); - list_del(&page->lru); - if (!add_to_page_cache_lru(page, mapping, - page->index, - readahead_gfp_mask(mapping))) { - args.page = page; - args.nr_pages = nr_pages - page_idx; - args.bio = do_mpage_readpage(&args); - } - put_page(page); - } - BUG_ON(!list_empty(pages)); - if (args.bio) - mpage_bio_submit(REQ_OP_READ, REQ_RAHEAD, args.bio); - return 0; -} -EXPORT_SYMBOL(mpage_readpages); - /* * This isn't called much at all */ diff --git a/include/linux/mpage.h b/include/linux/mpage.h index 181f1b0fbd83..f4f5e90a6844 100644 --- a/include/linux/mpage.h +++ b/include/linux/mpage.h @@ -16,8 +16,6 @@ struct writeback_control; struct readahead_control; void mpage_readahead(struct readahead_control *, get_block_t get_block); -int mpage_readpages(struct address_space *, struct list_head *, unsigned, - get_block_t); int mpage_readpage(struct page *page, get_block_t get_block); int mpage_writepages(struct address_space *mapping, struct writeback_control *wbc, get_block_t get_block);