From patchwork Wed May 30 09:48:42 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Andreas Gruenbacher X-Patchwork-Id: 10438081 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork.web.codeaurora.org (Postfix) with ESMTP id 4A879602CC for ; Wed, 30 May 2018 09:49:13 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 393EF2887F for ; Wed, 30 May 2018 09:49:13 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 2DDED288B7; Wed, 30 May 2018 09:49:13 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-7.9 required=2.0 tests=BAYES_00, MAILING_LIST_MULTI, RCVD_IN_DNSWL_HI autolearn=ham version=3.3.1 Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 9DDEC2887F for ; Wed, 30 May 2018 09:49:12 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S968990AbeE3JtK (ORCPT ); Wed, 30 May 2018 05:49:10 -0400 Received: from mx3-rdu2.redhat.com ([66.187.233.73]:55760 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S968899AbeE3JtC (ORCPT ); Wed, 30 May 2018 05:49:02 -0400 Received: from smtp.corp.redhat.com (int-mx06.intmail.prod.int.rdu2.redhat.com [10.11.54.6]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 5B4C181663C9; Wed, 30 May 2018 09:49:02 +0000 (UTC) Received: from max.home.com (unknown [10.36.118.91]) by smtp.corp.redhat.com (Postfix) with ESMTP id 744BA2166BB2; Wed, 30 May 2018 09:49:01 +0000 (UTC) From: Andreas Gruenbacher To: cluster-devel@redhat.com, Christoph Hellwig Cc: linux-fsdevel@vger.kernel.org, Andreas Gruenbacher Subject: [PATCH v5 14/14] gfs2: Handle stuffed files in iomap_{begin,end} Date: Wed, 30 May 2018 11:48:42 +0200 Message-Id: <20180530094842.13559-15-agruenba@redhat.com> In-Reply-To: <20180530094842.13559-1-agruenba@redhat.com> References: <20180530094842.13559-1-agruenba@redhat.com> X-Scanned-By: MIMEDefang 2.78 on 10.11.54.6 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.11.55.8]); Wed, 30 May 2018 09:49:02 +0000 (UTC) X-Greylist: inspected by milter-greylist-4.5.16 (mx1.redhat.com [10.11.55.8]); Wed, 30 May 2018 09:49:02 +0000 (UTC) for IP:'10.11.54.6' DOMAIN:'int-mx06.intmail.prod.int.rdu2.redhat.com' HELO:'smtp.corp.redhat.com' FROM:'agruenba@redhat.com' RCPT:'' Sender: linux-fsdevel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-fsdevel@vger.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP Proof of concept: move the gfs2 stuffed file handling from the iomap_write_{begin,end} handlers to the iomap_{begin,end} handlers. With this, the page written to is locked before faulting in the page to read from, so when both refer to the same page, we end up with a deadlock as demonstrated by xfstest generic/248. Signed-off-by: Andreas Gruenbacher --- fs/gfs2/bmap.c | 91 ++++++++++++++++++++----------------------- fs/iomap.c | 21 ++++++---- include/linux/iomap.h | 1 + 3 files changed, 57 insertions(+), 56 deletions(-) diff --git a/fs/gfs2/bmap.c b/fs/gfs2/bmap.c index c157af31dc56..05b599a70de6 100644 --- a/fs/gfs2/bmap.c +++ b/fs/gfs2/bmap.c @@ -966,15 +966,30 @@ static int gfs2_iomap_begin_write(struct inode *inode, loff_t pos, loff_t length struct gfs2_inode *ip = GFS2_I(inode); struct gfs2_sbd *sdp = GFS2_SB(inode); unsigned int data_blocks = 0, ind_blocks = 0, rblocks; - bool unstuff, alloc_required; + bool unstuff = false, alloc_required; int ret; ret = gfs2_write_lock(inode); if (ret) return ret; - unstuff = gfs2_is_stuffed(ip) && - pos + length > gfs2_max_stuffed_size(ip); + if (gfs2_is_stuffed(ip)) { + unstuff = pos + length > gfs2_max_stuffed_size(ip); + + if (!unstuff) { + iomap->page = grab_cache_page_write_begin( + inode->i_mapping, 0, flags); + ret = -ENOMEM; + if (!iomap->page) + goto out; + + if (!PageUptodate(iomap->page)) { + ret = stuffed_readpage(ip, iomap->page); + if (ret) + goto out; + } + } + } ret = gfs2_iomap_get(inode, pos, length, flags, iomap, &mp); if (ret) @@ -1044,6 +1059,12 @@ static int gfs2_iomap_begin_write(struct inode *inode, loff_t pos, loff_t length gfs2_quota_unlock(ip); out_release: release_metapath(&mp); +out: + if (iomap->page) { + unlock_page(iomap->page); + put_page(iomap->page); + iomap->page = NULL; + } gfs2_write_unlock(inode); return ret; } @@ -1073,54 +1094,10 @@ static int gfs2_iomap_begin(struct inode *inode, loff_t pos, loff_t length, return ret; } -static int -gfs2_iomap_write_begin(struct inode *inode, loff_t pos, unsigned len, - unsigned flags, struct page **pagep, struct iomap *iomap) -{ - struct gfs2_inode *ip = GFS2_I(inode); - struct page *page; - int ret; - - if (gfs2_is_stuffed(ip)) { - BUG_ON(pos + len > gfs2_max_stuffed_size(ip)); - - page = grab_cache_page_write_begin(inode->i_mapping, 0, flags); - if (!page) - return -ENOMEM; - - if (!PageUptodate(page)) { - ret = stuffed_readpage(ip, page); - if (ret) { - unlock_page(page); - put_page(page); - return ret; - } - } - *pagep = page; - return 0; - } - - return iomap_write_begin(inode, pos, len, flags, pagep, iomap); -} - static int gfs2_iomap_write_end(struct inode *inode, loff_t pos, unsigned len, unsigned copied, struct page *page) { struct gfs2_inode *ip = GFS2_I(inode); - struct buffer_head *dibh; - int ret; - - if (gfs2_is_stuffed(ip)) { - ret = gfs2_meta_inode_buffer(ip, &dibh); - if (ret) { - unlock_page(page); - put_page(page); - return ret; - } - ret = gfs2_stuffed_write_end(inode, dibh, pos, copied, page); - brelse(dibh); - return ret; - } if (gfs2_is_jdata(ip)) gfs2_page_add_databufs(ip, page, offset_in_page(pos), len); @@ -1139,6 +1116,23 @@ static int gfs2_iomap_end(struct inode *inode, loff_t pos, loff_t length, if ((flags & (IOMAP_WRITE | IOMAP_DIRECT)) != IOMAP_WRITE) return 0; + if (iomap->page) { + struct buffer_head *dibh; + + ret = gfs2_meta_inode_buffer(ip, &dibh); + if (ret) { + unlock_page(iomap->page); + put_page(iomap->page); + iomap->page = NULL; + goto out; + } + ret = gfs2_stuffed_write_end(inode, dibh, pos, written, + iomap->page); + iomap->page = NULL; + brelse(dibh); + goto out; + } + gfs2_ordered_add_inode(ip); if (tr->tr_num_buf_new) @@ -1153,6 +1147,7 @@ static int gfs2_iomap_end(struct inode *inode, loff_t pos, loff_t length, } } +out: if (inode == sdp->sd_rindex) { adjust_fs_space(inode); sdp->sd_rindex_uptodate = 0; @@ -1180,7 +1175,7 @@ static int gfs2_iomap_end(struct inode *inode, loff_t pos, loff_t length, const struct iomap_ops gfs2_iomap_ops = { .iomap_begin = gfs2_iomap_begin, - .write_begin = gfs2_iomap_write_begin, + .write_begin = iomap_write_begin, .write_end = gfs2_iomap_write_end, .iomap_end = gfs2_iomap_end, }; diff --git a/fs/iomap.c b/fs/iomap.c index 819b6c56ecfe..2a1d78976775 100644 --- a/fs/iomap.c +++ b/fs/iomap.c @@ -196,10 +196,13 @@ iomap_write_actor(struct inode *inode, loff_t pos, loff_t length, void *data, break; } - status = args->ops->write_begin(inode, pos, bytes, flags, &page, - iomap); - if (unlikely(status)) - break; + page = iomap->page; + if (!iomap->page) { + status = args->ops->write_begin(inode, pos, bytes, flags, &page, + iomap); + if (unlikely(status)) + break; + } if (mapping_writably_mapped(inode->i_mapping)) flush_dcache_page(page); @@ -208,10 +211,12 @@ iomap_write_actor(struct inode *inode, loff_t pos, loff_t length, void *data, flush_dcache_page(page); - status = args->ops->write_end(inode, pos, bytes, copied, page); - if (unlikely(status < 0)) - break; - copied = status; + if (!iomap->page) { + status = args->ops->write_end(inode, pos, bytes, copied, page); + if (unlikely(status < 0)) + break; + copied = status; + } cond_resched(); diff --git a/include/linux/iomap.h b/include/linux/iomap.h index 0a018a58d84e..db807157c154 100644 --- a/include/linux/iomap.h +++ b/include/linux/iomap.h @@ -47,6 +47,7 @@ struct iomap { u64 length; /* length of mapping, bytes */ u16 type; /* type of mapping */ u16 flags; /* flags for mapping */ + struct page *page; struct block_device *bdev; /* block device for I/O */ struct dax_device *dax_dev; /* dax_dev for dax operations */ };