Received: by 2002:a05:6a10:1d13:0:0:0:0 with SMTP id pp19csp1486929pxb; Fri, 27 Aug 2021 09:54:24 -0700 (PDT) X-Google-Smtp-Source: ABdhPJxp6n6cz/nsD8VitzAlICEarrLo4lpv1mg77QE1ifG05bAOuzAjK6agRK69++j3FaaxI6Eh X-Received: by 2002:a17:906:5384:: with SMTP id g4mr11032996ejo.27.1630083264371; Fri, 27 Aug 2021 09:54:24 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1630083264; cv=none; d=google.com; s=arc-20160816; b=xKGFWKKUOHcl5SvdiUhCbp88ZMBIy9wX8IkhkHds9EjLJ5+d474MVhkgUUYpz67MrL fJmg6Hf0fYu+vMXHLWMN3bT7tHjRDj0o4g10p/4DKXPqu1tXVod48UsIVuyAinzPyA58 xTVXOQrZTmEV96+WD6iYq4xn9bLKXKpFQuiGiRErA7OfL+xr8VgdoeLavOJp3XfBX+ZK /cJhcmu/wfo9vaegcVnIBIBfPHVvX+4dQq5jFzpNfLOmhS16pxqivU1G64JsNYdqXXoh tAAd9Qy604hgqo9jzslD9z8JYZFE8wEIHzqtVIEA4pmX6RQC4+YJdU0EkcZLAhB8VByW idBQ== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:content-transfer-encoding:mime-version :references:in-reply-to:message-id:date:subject:cc:to:from :dkim-signature; bh=Cl0IwqXw99jVQk0cBYLPw1qKg8vsuQrBIrGkwiPpzVs=; b=rVxytDa6rVDRujoy3JM//6dCXTA+iH+ehTW+7pW1w5O6SDtZPdtw9MCd5mYelJm+ce XNJ855X43FmgVFuFPMz6k4MFX3abZtCR3D8wNsQ6Sggk/k7P4ViNSQIl1kaN2nCag2Z7 a9qx3LkgzKou0eRvQwlUCCAa4cs48kUUCBRAORQ/B0I/aFVlnoOeGd8rHYko83EpJbCC wIFnWuX7rVoboFfUnHcozRaXHRqX1frmUZqlcBs4LULCXAooDdV7Nqupk4ENKemk2jJE e5wD7aLaR+XqVm4vVlJx9PMoC7NzMrbXoYU1B4SkY9Wt6wKIbRaB4j0DS19nkYJVZmij 6JTA== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@redhat.com header.s=mimecast20190719 header.b=dOut81r6; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=redhat.com Return-Path: Received: from vger.kernel.org (vger.kernel.org. [23.128.96.18]) by mx.google.com with ESMTP id l11si7467317ejo.259.2021.08.27.09.54.00; Fri, 27 Aug 2021 09:54:24 -0700 (PDT) Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18; Authentication-Results: mx.google.com; dkim=pass header.i=@redhat.com header.s=mimecast20190719 header.b=dOut81r6; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=redhat.com Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S239095AbhH0Qwr (ORCPT + 99 others); Fri, 27 Aug 2021 12:52:47 -0400 Received: from us-smtp-delivery-124.mimecast.com ([170.10.133.124]:35358 "EHLO us-smtp-delivery-124.mimecast.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S233014AbhH0Qwf (ORCPT ); Fri, 27 Aug 2021 12:52:35 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1630083106; 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=Cl0IwqXw99jVQk0cBYLPw1qKg8vsuQrBIrGkwiPpzVs=; b=dOut81r6/2zU77ZaDbfccelR0xjm8l3dkn1a3kKaX449Mivj/m826tnEF+YDQGgX8CWAZ1 yuPSayU1nmBUn/x1e1e42aFlzmnJJ/Q/7sARq3i26KKYoZ1DBL8D8YsjQ1Xx4nyvbgvpl1 pNljbzqZuUFS0UdPHJgiSRL3HLeLIM0= 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-122-XdEF1v7wOEquM27j5olWTQ-1; Fri, 27 Aug 2021 12:51:43 -0400 X-MC-Unique: XdEF1v7wOEquM27j5olWTQ-1 Received: from smtp.corp.redhat.com (int-mx02.intmail.prod.int.phx2.redhat.com [10.5.11.12]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id C32AB190A7A5; Fri, 27 Aug 2021 16:51:41 +0000 (UTC) Received: from max.com (unknown [10.40.194.206]) by smtp.corp.redhat.com (Postfix) with ESMTP id 50FBA60C81; Fri, 27 Aug 2021 16:51:35 +0000 (UTC) From: Andreas Gruenbacher To: Linus Torvalds , Alexander Viro , Christoph Hellwig , "Darrick J. Wong" Cc: Jan Kara , Matthew Wilcox , cluster-devel@redhat.com, linux-fsdevel@vger.kernel.org, linux-kernel@vger.kernel.org, ocfs2-devel@oss.oracle.com, Andreas Gruenbacher Subject: [PATCH v7 19/19] gfs2: Fix mmap + page fault deadlocks for direct I/O Date: Fri, 27 Aug 2021 18:49:26 +0200 Message-Id: <20210827164926.1726765-20-agruenba@redhat.com> In-Reply-To: <20210827164926.1726765-1-agruenba@redhat.com> References: <20210827164926.1726765-1-agruenba@redhat.com> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Scanned-By: MIMEDefang 2.79 on 10.5.11.12 Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Also disable page faults during direct I/O requests and implement a similar kind of retry logic as in the buffered I/O case. The retry logic in the direct I/O case differs from the buffered I/O case in the following way: direct I/O doesn't provide the kinds of consistency guarantees between concurrent reads and writes that buffered I/O provides, so when we lose the inode glock while faulting in user pages, we always resume the operation. We never need to return a partial read or write. This locking problem was originally reported by Jan Kara. Linus came up with the proposal to disable page faults. Many thanks to Al Viro and Matthew Wilcox for their feedback. Signed-off-by: Andreas Gruenbacher --- fs/gfs2/file.c | 99 ++++++++++++++++++++++++++++++++++++++++++++------ 1 file changed, 87 insertions(+), 12 deletions(-) diff --git a/fs/gfs2/file.c b/fs/gfs2/file.c index 64bf2f68e6d6..6603d9cd8739 100644 --- a/fs/gfs2/file.c +++ b/fs/gfs2/file.c @@ -811,22 +811,64 @@ static ssize_t gfs2_file_direct_read(struct kiocb *iocb, struct iov_iter *to, { struct file *file = iocb->ki_filp; struct gfs2_inode *ip = GFS2_I(file->f_mapping->host); - size_t count = iov_iter_count(to); + size_t prev_count = 0, window_size = 0; + size_t written = 0; ssize_t ret; - if (!count) + /* + * In this function, we disable page faults when we're holding the + * inode glock while doing I/O. If a page fault occurs, we drop the + * inode glock, fault in the pages manually, and retry. + * + * Unlike generic_file_read_iter, for reads, iomap_dio_rw can trigger + * physical as well as manual page faults, and we need to disable both + * kinds. + * + * For direct I/O, gfs2 takes the inode glock in deferred mode. This + * locking mode is compatible with other deferred holders, so multiple + * processes and nodes can do direct I/O to a file at the same time. + * There's no guarantee that reads or writes will be atomic. Any + * coordination among readers and writers needs to happen externally. + */ + + if (!iov_iter_count(to)) return 0; /* skip atime */ gfs2_holder_init(ip->i_gl, LM_ST_DEFERRED, 0, gh); +retry: ret = gfs2_glock_nq(gh); if (ret) goto out_uninit; +retry_under_glock: + pagefault_disable(); + to->nofault = true; + ret = iomap_dio_rw(iocb, to, &gfs2_iomap_ops, NULL, + IOMAP_DIO_PARTIAL, written); + to->nofault = false; + pagefault_enable(); + if (ret > 0) + written = ret; - ret = iomap_dio_rw(iocb, to, &gfs2_iomap_ops, NULL, 0, 0); - gfs2_glock_dq(gh); + if (unlikely(iov_iter_count(to) && (ret > 0 || ret == -EFAULT)) && + should_fault_in_pages(to, &prev_count, &window_size)) { + size_t leftover; + + gfs2_holder_allow_demote(gh); + leftover = fault_in_iov_iter_writeable(to, window_size); + gfs2_holder_disallow_demote(gh); + if (leftover != window_size) { + if (!gfs2_holder_queued(gh)) + goto retry; + goto retry_under_glock; + } + } + if (gfs2_holder_queued(gh)) + gfs2_glock_dq(gh); out_uninit: gfs2_holder_uninit(gh); - return ret; + if (ret < 0) + return ret; + return written; } static ssize_t gfs2_file_direct_write(struct kiocb *iocb, struct iov_iter *from, @@ -835,10 +877,19 @@ static ssize_t gfs2_file_direct_write(struct kiocb *iocb, struct iov_iter *from, struct file *file = iocb->ki_filp; struct inode *inode = file->f_mapping->host; struct gfs2_inode *ip = GFS2_I(inode); - size_t len = iov_iter_count(from); - loff_t offset = iocb->ki_pos; + size_t prev_count = 0, window_size = 0; + size_t read = 0; ssize_t ret; + /* + * In this function, we disable page faults when we're holding the + * inode glock while doing I/O. If a page fault occurs, we drop the + * inode glock, fault in the pages manually, and retry. + * + * For writes, iomap_dio_rw only triggers manual page faults, so we + * don't need to disable physical ones. + */ + /* * Deferred lock, even if its a write, since we do no allocation on * this path. All we need to change is the atime, and this lock mode @@ -848,22 +899,46 @@ static ssize_t gfs2_file_direct_write(struct kiocb *iocb, struct iov_iter *from, * VFS does. */ gfs2_holder_init(ip->i_gl, LM_ST_DEFERRED, 0, gh); +retry: ret = gfs2_glock_nq(gh); if (ret) goto out_uninit; - +retry_under_glock: /* Silently fall back to buffered I/O when writing beyond EOF */ - if (offset + len > i_size_read(&ip->i_inode)) + if (iocb->ki_pos + iov_iter_count(from) > i_size_read(&ip->i_inode)) goto out; - ret = iomap_dio_rw(iocb, from, &gfs2_iomap_ops, NULL, 0, 0); + from->nofault = true; + ret = iomap_dio_rw(iocb, from, &gfs2_iomap_ops, NULL, + IOMAP_DIO_PARTIAL, read); + from->nofault = false; + if (ret == -ENOTBLK) ret = 0; + if (ret > 0) + read = ret; + + if (unlikely(iov_iter_count(from) && (ret > 0 || ret == -EFAULT)) && + should_fault_in_pages(from, &prev_count, &window_size)) { + size_t leftover; + + gfs2_holder_allow_demote(gh); + leftover = fault_in_iov_iter_readable(from, window_size); + gfs2_holder_disallow_demote(gh); + if (leftover != window_size) { + if (!gfs2_holder_queued(gh)) + goto retry; + goto retry_under_glock; + } + } out: - gfs2_glock_dq(gh); + if (gfs2_holder_queued(gh)) + gfs2_glock_dq(gh); out_uninit: gfs2_holder_uninit(gh); - return ret; + if (ret < 0) + return ret; + return read; } static ssize_t gfs2_file_read_iter(struct kiocb *iocb, struct iov_iter *to) -- 2.26.3