From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-16.4 required=3.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,INCLUDES_CR_TRAILER,INCLUDES_PATCH, MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS autolearn=unavailable autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id A836BC433EF for ; Fri, 3 Sep 2021 18:53:59 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 890B261056 for ; Fri, 3 Sep 2021 18:53:59 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1351309AbhICSy6 (ORCPT ); Fri, 3 Sep 2021 14:54:58 -0400 Received: from mail.kernel.org ([198.145.29.99]:44568 "EHLO mail.kernel.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1351269AbhICSyw (ORCPT ); Fri, 3 Sep 2021 14:54:52 -0400 Received: by mail.kernel.org (Postfix) with ESMTPSA id 1408461051; Fri, 3 Sep 2021 18:53:52 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1630695232; bh=nmWhUNMQR3hZroaQr4jgy+GDIFc+zeOl4mdCOJr1lbQ=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=UX4IrgsY3Rjnb/gjA+Erw9nXWJ1hFrekn3inpWCxSdIpKDlGOC32dDW9h0HKOMFVX yUm7vMNz2JIE7YqOemA12d4ds8SPFWhrU+zXk+JcHQXUN98B5V9Hb25hk7Y6CpuBje XXj+xWD89+/JEbqBdt14/DXiww578nstfY2t4cn7GiQLpdgB/de0QWE1xU8roGx7mV 8FwUZNqZNJQ5U4N6HmCtdgeqMpyjsAQh0usMunyVAU2LNbCUJk5I3anqGoQZpCag4e uE3aslQN31+867om4vwVPA5+/TBiMFpGXLb+KtvoBkP7tkG6M72ZeDV+OE7MAm6CFh O5Uw5/znR6ZUw== Date: Fri, 3 Sep 2021 11:53:51 -0700 From: "Darrick J. Wong" To: Andreas Gruenbacher Cc: Linus Torvalds , Alexander Viro , Christoph Hellwig , Jan Kara , Matthew Wilcox , cluster-devel@redhat.com, linux-fsdevel@vger.kernel.org, linux-kernel@vger.kernel.org, ocfs2-devel@oss.oracle.com Subject: Re: [PATCH v7 16/19] iomap: Add done_before argument to iomap_dio_rw Message-ID: <20210903185351.GD9892@magnolia> References: <20210827164926.1726765-1-agruenba@redhat.com> <20210827164926.1726765-17-agruenba@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20210827164926.1726765-17-agruenba@redhat.com> Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Fri, Aug 27, 2021 at 06:49:23PM +0200, Andreas Gruenbacher wrote: > Add a done_before argument to iomap_dio_rw that indicates how much of > the request has already been transferred. When the request succeeds, we > report that done_before additional bytes were tranferred. This is > useful for finishing a request asynchronously when part of the request > has already been completed synchronously. > > We'll use that to allow iomap_dio_rw to be used with page faults > disabled: when a page fault occurs while submitting a request, we > synchronously complete the part of the request that has already been > submitted. The caller can then take care of the page fault and call > iomap_dio_rw again for the rest of the request, passing in the number of > bytes already tranferred. > > Signed-off-by: Andreas Gruenbacher > --- > fs/btrfs/file.c | 5 +++-- > fs/ext4/file.c | 5 +++-- > fs/gfs2/file.c | 4 ++-- > fs/iomap/direct-io.c | 11 ++++++++--- > fs/xfs/xfs_file.c | 6 +++--- > fs/zonefs/super.c | 4 ++-- > include/linux/iomap.h | 4 ++-- > 7 files changed, 23 insertions(+), 16 deletions(-) > > diff --git a/fs/btrfs/file.c b/fs/btrfs/file.c > index 281c77cfe91a..8817fe6b5fc0 100644 > --- a/fs/btrfs/file.c > +++ b/fs/btrfs/file.c > @@ -1945,7 +1945,7 @@ static ssize_t btrfs_direct_write(struct kiocb *iocb, struct iov_iter *from) > } > > dio = __iomap_dio_rw(iocb, from, &btrfs_dio_iomap_ops, &btrfs_dio_ops, > - 0); > + 0, 0); > > btrfs_inode_unlock(inode, ilock_flags); > > @@ -3637,7 +3637,8 @@ static ssize_t btrfs_direct_read(struct kiocb *iocb, struct iov_iter *to) > return 0; > > btrfs_inode_lock(inode, BTRFS_ILOCK_SHARED); > - ret = iomap_dio_rw(iocb, to, &btrfs_dio_iomap_ops, &btrfs_dio_ops, 0); > + ret = iomap_dio_rw(iocb, to, &btrfs_dio_iomap_ops, &btrfs_dio_ops, > + 0, 0); > btrfs_inode_unlock(inode, BTRFS_ILOCK_SHARED); > return ret; > } > diff --git a/fs/ext4/file.c b/fs/ext4/file.c > index 816dedcbd541..4a5e7fd31fb5 100644 > --- a/fs/ext4/file.c > +++ b/fs/ext4/file.c > @@ -74,7 +74,7 @@ static ssize_t ext4_dio_read_iter(struct kiocb *iocb, struct iov_iter *to) > return generic_file_read_iter(iocb, to); > } > > - ret = iomap_dio_rw(iocb, to, &ext4_iomap_ops, NULL, 0); > + ret = iomap_dio_rw(iocb, to, &ext4_iomap_ops, NULL, 0, 0); > inode_unlock_shared(inode); > > file_accessed(iocb->ki_filp); > @@ -566,7 +566,8 @@ static ssize_t ext4_dio_write_iter(struct kiocb *iocb, struct iov_iter *from) > if (ilock_shared) > iomap_ops = &ext4_iomap_overwrite_ops; > ret = iomap_dio_rw(iocb, from, iomap_ops, &ext4_dio_write_ops, > - (unaligned_io || extend) ? IOMAP_DIO_FORCE_WAIT : 0); > + (unaligned_io || extend) ? IOMAP_DIO_FORCE_WAIT : 0, > + 0); > if (ret == -ENOTBLK) > ret = 0; > > diff --git a/fs/gfs2/file.c b/fs/gfs2/file.c > index fce3a5249e19..64bf2f68e6d6 100644 > --- a/fs/gfs2/file.c > +++ b/fs/gfs2/file.c > @@ -822,7 +822,7 @@ static ssize_t gfs2_file_direct_read(struct kiocb *iocb, struct iov_iter *to, > if (ret) > goto out_uninit; > > - ret = iomap_dio_rw(iocb, to, &gfs2_iomap_ops, NULL, 0); > + ret = iomap_dio_rw(iocb, to, &gfs2_iomap_ops, NULL, 0, 0); > gfs2_glock_dq(gh); > out_uninit: > gfs2_holder_uninit(gh); > @@ -856,7 +856,7 @@ static ssize_t gfs2_file_direct_write(struct kiocb *iocb, struct iov_iter *from, > if (offset + len > i_size_read(&ip->i_inode)) > goto out; > > - ret = iomap_dio_rw(iocb, from, &gfs2_iomap_ops, NULL, 0); > + ret = iomap_dio_rw(iocb, from, &gfs2_iomap_ops, NULL, 0, 0); > if (ret == -ENOTBLK) > ret = 0; > out: > diff --git a/fs/iomap/direct-io.c b/fs/iomap/direct-io.c > index ba88fe51b77a..dcf9a2b4381f 100644 > --- a/fs/iomap/direct-io.c > +++ b/fs/iomap/direct-io.c > @@ -31,6 +31,7 @@ struct iomap_dio { > atomic_t ref; > unsigned flags; > int error; > + size_t done_before; So, now that I actually understand the reason why the count of previously transferred bytes has to be passed into the iomap_dio, I would like this field to have a comment so that stupid maintainers like me don't forget the subtleties again: /* * For asynchronous IO, we have one chance to call the iocb * completion method with the results of the directio operation. * If this operation is a resubmission after a previous partial * completion (e.g. page fault), we need to know about that * progress so that we can report that and the result of the * resubmission to the iocb completion. */ size_t done_before; With that added, I think I can live with this enough to: Reviewed-by: Darrick J. Wong --D > bool wait_for_completion; > > union { > @@ -126,6 +127,9 @@ ssize_t iomap_dio_complete(struct iomap_dio *dio) > if (ret > 0 && (dio->flags & IOMAP_DIO_NEED_SYNC)) > ret = generic_write_sync(iocb, ret); > > + if (ret > 0) > + ret += dio->done_before; > + > kfree(dio); > > return ret; > @@ -450,7 +454,7 @@ iomap_dio_actor(struct inode *inode, loff_t pos, loff_t length, > struct iomap_dio * > __iomap_dio_rw(struct kiocb *iocb, struct iov_iter *iter, > const struct iomap_ops *ops, const struct iomap_dio_ops *dops, > - unsigned int dio_flags) > + unsigned int dio_flags, size_t done_before) > { > struct address_space *mapping = iocb->ki_filp->f_mapping; > struct inode *inode = file_inode(iocb->ki_filp); > @@ -477,6 +481,7 @@ __iomap_dio_rw(struct kiocb *iocb, struct iov_iter *iter, > dio->dops = dops; > dio->error = 0; > dio->flags = 0; > + dio->done_before = done_before; > > dio->submit.iter = iter; > dio->submit.waiter = current; > @@ -648,11 +653,11 @@ EXPORT_SYMBOL_GPL(__iomap_dio_rw); > ssize_t > iomap_dio_rw(struct kiocb *iocb, struct iov_iter *iter, > const struct iomap_ops *ops, const struct iomap_dio_ops *dops, > - unsigned int dio_flags) > + unsigned int dio_flags, size_t done_before) > { > struct iomap_dio *dio; > > - dio = __iomap_dio_rw(iocb, iter, ops, dops, dio_flags); > + dio = __iomap_dio_rw(iocb, iter, ops, dops, dio_flags, done_before); > if (IS_ERR_OR_NULL(dio)) > return PTR_ERR_OR_ZERO(dio); > return iomap_dio_complete(dio); > diff --git a/fs/xfs/xfs_file.c b/fs/xfs/xfs_file.c > index cc3cfb12df53..3103d9bda466 100644 > --- a/fs/xfs/xfs_file.c > +++ b/fs/xfs/xfs_file.c > @@ -259,7 +259,7 @@ xfs_file_dio_read( > ret = xfs_ilock_iocb(iocb, XFS_IOLOCK_SHARED); > if (ret) > return ret; > - ret = iomap_dio_rw(iocb, to, &xfs_read_iomap_ops, NULL, 0); > + ret = iomap_dio_rw(iocb, to, &xfs_read_iomap_ops, NULL, 0, 0); > xfs_iunlock(ip, XFS_IOLOCK_SHARED); > > return ret; > @@ -569,7 +569,7 @@ xfs_file_dio_write_aligned( > } > trace_xfs_file_direct_write(iocb, from); > ret = iomap_dio_rw(iocb, from, &xfs_direct_write_iomap_ops, > - &xfs_dio_write_ops, 0); > + &xfs_dio_write_ops, 0, 0); > out_unlock: > if (iolock) > xfs_iunlock(ip, iolock); > @@ -647,7 +647,7 @@ xfs_file_dio_write_unaligned( > > trace_xfs_file_direct_write(iocb, from); > ret = iomap_dio_rw(iocb, from, &xfs_direct_write_iomap_ops, > - &xfs_dio_write_ops, flags); > + &xfs_dio_write_ops, flags, 0); > > /* > * Retry unaligned I/O with exclusive blocking semantics if the DIO > diff --git a/fs/zonefs/super.c b/fs/zonefs/super.c > index 70055d486bf7..85ca2f5fe06e 100644 > --- a/fs/zonefs/super.c > +++ b/fs/zonefs/super.c > @@ -864,7 +864,7 @@ static ssize_t zonefs_file_dio_write(struct kiocb *iocb, struct iov_iter *from) > ret = zonefs_file_dio_append(iocb, from); > else > ret = iomap_dio_rw(iocb, from, &zonefs_iomap_ops, > - &zonefs_write_dio_ops, 0); > + &zonefs_write_dio_ops, 0, 0); > if (zi->i_ztype == ZONEFS_ZTYPE_SEQ && > (ret > 0 || ret == -EIOCBQUEUED)) { > if (ret > 0) > @@ -999,7 +999,7 @@ static ssize_t zonefs_file_read_iter(struct kiocb *iocb, struct iov_iter *to) > } > file_accessed(iocb->ki_filp); > ret = iomap_dio_rw(iocb, to, &zonefs_iomap_ops, > - &zonefs_read_dio_ops, 0); > + &zonefs_read_dio_ops, 0, 0); > } else { > ret = generic_file_read_iter(iocb, to); > if (ret == -EIO) > diff --git a/include/linux/iomap.h b/include/linux/iomap.h > index bcae4814b8e3..908bda10024c 100644 > --- a/include/linux/iomap.h > +++ b/include/linux/iomap.h > @@ -276,10 +276,10 @@ struct iomap_dio_ops { > > ssize_t iomap_dio_rw(struct kiocb *iocb, struct iov_iter *iter, > const struct iomap_ops *ops, const struct iomap_dio_ops *dops, > - unsigned int dio_flags); > + unsigned int dio_flags, size_t done_before); > struct iomap_dio *__iomap_dio_rw(struct kiocb *iocb, struct iov_iter *iter, > const struct iomap_ops *ops, const struct iomap_dio_ops *dops, > - unsigned int dio_flags); > + unsigned int dio_flags, size_t done_before); > ssize_t iomap_dio_complete(struct iomap_dio *dio); > int iomap_dio_iopoll(struct kiocb *kiocb, bool spin); > > -- > 2.26.3 > From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-14.0 required=3.0 tests=BAYES_00,INCLUDES_CR_TRAILER, INCLUDES_PATCH,MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS autolearn=unavailable autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 700FFC433FE for ; Fri, 3 Sep 2021 18:54:00 +0000 (UTC) Received: from mx0a-00069f02.pphosted.com (mx0a-00069f02.pphosted.com [205.220.165.32]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id 2DEA661051 for ; Fri, 3 Sep 2021 18:54:00 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.4.1 mail.kernel.org 2DEA661051 Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=kernel.org Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=oss.oracle.com Received: from pps.filterd (m0246629.ppops.net [127.0.0.1]) by mx0b-00069f02.pphosted.com (8.16.1.2/8.16.1.2) with SMTP id 183ITpqW017618; Fri, 3 Sep 2021 18:53:59 GMT Received: from userp3020.oracle.com (userp3020.oracle.com [156.151.31.79]) by mx0b-00069f02.pphosted.com with ESMTP id 3auh1r1d79-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=OK); Fri, 03 Sep 2021 18:53:59 +0000 Received: from pps.filterd (userp3020.oracle.com [127.0.0.1]) by userp3020.oracle.com (8.16.0.42/8.16.0.42) with SMTP id 183IU4Li044301; Fri, 3 Sep 2021 18:53:58 GMT Received: from oss.oracle.com (oss-old-reserved.oracle.com [137.254.22.2]) by userp3020.oracle.com with ESMTP id 3ate01xegx-1 (version=TLSv1 cipher=AES256-SHA bits=256 verify=NO); Fri, 03 Sep 2021 18:53:57 +0000 Received: from localhost ([127.0.0.1] helo=lb-oss.oracle.com) by oss.oracle.com with esmtp (Exim 4.63) (envelope-from ) id 1mMEK4-000754-Lv; Fri, 03 Sep 2021 11:53:56 -0700 Received: from aserp3030.oracle.com ([141.146.126.71]) by oss.oracle.com with esmtp (Exim 4.63) (envelope-from ) id 1mMEK2-00074o-SE for ocfs2-devel@oss.oracle.com; Fri, 03 Sep 2021 11:53:54 -0700 Received: from pps.filterd (aserp3030.oracle.com [127.0.0.1]) by aserp3030.oracle.com (8.16.0.42/8.16.0.42) with SMTP id 183ITuu9181147 for ; Fri, 3 Sep 2021 18:53:54 GMT Received: from mx0b-00069f01.pphosted.com (mx0b-00069f01.pphosted.com [205.220.177.26]) by aserp3030.oracle.com with ESMTP id 3atdyyunms-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=OK) for ; Fri, 03 Sep 2021 18:53:54 +0000 Received: from pps.filterd (m0246577.ppops.net [127.0.0.1]) by mx0b-00069f01.pphosted.com (8.16.1.2/8.16.0.43) with SMTP id 183GRnB5030904 for ; Fri, 3 Sep 2021 18:53:54 GMT Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by mx0b-00069f01.pphosted.com with ESMTP id 3au7wcbwg7-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NO) for ; Fri, 03 Sep 2021 18:53:53 +0000 Received: by mail.kernel.org (Postfix) with ESMTPSA id 1408461051; Fri, 3 Sep 2021 18:53:52 +0000 (UTC) Date: Fri, 3 Sep 2021 11:53:51 -0700 From: "Darrick J. Wong" To: Andreas Gruenbacher Message-ID: <20210903185351.GD9892@magnolia> References: <20210827164926.1726765-1-agruenba@redhat.com> <20210827164926.1726765-17-agruenba@redhat.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <20210827164926.1726765-17-agruenba@redhat.com> X-Source-IP: 198.145.29.99 X-ServerName: mail.kernel.org X-Proofpoint-SPF-Result: pass X-Proofpoint-SPF-Record: v=spf1 mx include:_spf.kernel.org ~all X-Proofpoint-Virus-Version: vendor=nai engine=6300 definitions=10096 signatures=668682 X-Proofpoint-Spam-Reason: safe X-Spam: OrgSafeList X-SpamRule: orgsafelist Cc: cluster-devel@redhat.com, Jan Kara , linux-kernel@vger.kernel.org, Christoph Hellwig , Alexander Viro , linux-fsdevel@vger.kernel.org, Linus Torvalds , ocfs2-devel@oss.oracle.com Subject: Re: [Ocfs2-devel] [PATCH v7 16/19] iomap: Add done_before argument to iomap_dio_rw X-BeenThere: ocfs2-devel@oss.oracle.com X-Mailman-Version: 2.1.9 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Sender: ocfs2-devel-bounces@oss.oracle.com Errors-To: ocfs2-devel-bounces@oss.oracle.com X-Proofpoint-Virus-Version: vendor=nai engine=6300 definitions=10096 signatures=668682 X-Proofpoint-Spam-Details: rule=notspam policy=default score=0 adultscore=0 bulkscore=0 mlxlogscore=999 mlxscore=0 suspectscore=0 spamscore=0 phishscore=0 malwarescore=0 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.12.0-2108310000 definitions=main-2109030110 X-Proofpoint-GUID: 6j2wVerZY9lSDbQsMsvbwicTOGKG8xn4 X-Proofpoint-ORIG-GUID: 6j2wVerZY9lSDbQsMsvbwicTOGKG8xn4 On Fri, Aug 27, 2021 at 06:49:23PM +0200, Andreas Gruenbacher wrote: > Add a done_before argument to iomap_dio_rw that indicates how much of > the request has already been transferred. When the request succeeds, we > report that done_before additional bytes were tranferred. This is > useful for finishing a request asynchronously when part of the request > has already been completed synchronously. > > We'll use that to allow iomap_dio_rw to be used with page faults > disabled: when a page fault occurs while submitting a request, we > synchronously complete the part of the request that has already been > submitted. The caller can then take care of the page fault and call > iomap_dio_rw again for the rest of the request, passing in the number of > bytes already tranferred. > > Signed-off-by: Andreas Gruenbacher > --- > fs/btrfs/file.c | 5 +++-- > fs/ext4/file.c | 5 +++-- > fs/gfs2/file.c | 4 ++-- > fs/iomap/direct-io.c | 11 ++++++++--- > fs/xfs/xfs_file.c | 6 +++--- > fs/zonefs/super.c | 4 ++-- > include/linux/iomap.h | 4 ++-- > 7 files changed, 23 insertions(+), 16 deletions(-) > > diff --git a/fs/btrfs/file.c b/fs/btrfs/file.c > index 281c77cfe91a..8817fe6b5fc0 100644 > --- a/fs/btrfs/file.c > +++ b/fs/btrfs/file.c > @@ -1945,7 +1945,7 @@ static ssize_t btrfs_direct_write(struct kiocb *iocb, struct iov_iter *from) > } > > dio = __iomap_dio_rw(iocb, from, &btrfs_dio_iomap_ops, &btrfs_dio_ops, > - 0); > + 0, 0); > > btrfs_inode_unlock(inode, ilock_flags); > > @@ -3637,7 +3637,8 @@ static ssize_t btrfs_direct_read(struct kiocb *iocb, struct iov_iter *to) > return 0; > > btrfs_inode_lock(inode, BTRFS_ILOCK_SHARED); > - ret = iomap_dio_rw(iocb, to, &btrfs_dio_iomap_ops, &btrfs_dio_ops, 0); > + ret = iomap_dio_rw(iocb, to, &btrfs_dio_iomap_ops, &btrfs_dio_ops, > + 0, 0); > btrfs_inode_unlock(inode, BTRFS_ILOCK_SHARED); > return ret; > } > diff --git a/fs/ext4/file.c b/fs/ext4/file.c > index 816dedcbd541..4a5e7fd31fb5 100644 > --- a/fs/ext4/file.c > +++ b/fs/ext4/file.c > @@ -74,7 +74,7 @@ static ssize_t ext4_dio_read_iter(struct kiocb *iocb, struct iov_iter *to) > return generic_file_read_iter(iocb, to); > } > > - ret = iomap_dio_rw(iocb, to, &ext4_iomap_ops, NULL, 0); > + ret = iomap_dio_rw(iocb, to, &ext4_iomap_ops, NULL, 0, 0); > inode_unlock_shared(inode); > > file_accessed(iocb->ki_filp); > @@ -566,7 +566,8 @@ static ssize_t ext4_dio_write_iter(struct kiocb *iocb, struct iov_iter *from) > if (ilock_shared) > iomap_ops = &ext4_iomap_overwrite_ops; > ret = iomap_dio_rw(iocb, from, iomap_ops, &ext4_dio_write_ops, > - (unaligned_io || extend) ? IOMAP_DIO_FORCE_WAIT : 0); > + (unaligned_io || extend) ? IOMAP_DIO_FORCE_WAIT : 0, > + 0); > if (ret == -ENOTBLK) > ret = 0; > > diff --git a/fs/gfs2/file.c b/fs/gfs2/file.c > index fce3a5249e19..64bf2f68e6d6 100644 > --- a/fs/gfs2/file.c > +++ b/fs/gfs2/file.c > @@ -822,7 +822,7 @@ static ssize_t gfs2_file_direct_read(struct kiocb *iocb, struct iov_iter *to, > if (ret) > goto out_uninit; > > - ret = iomap_dio_rw(iocb, to, &gfs2_iomap_ops, NULL, 0); > + ret = iomap_dio_rw(iocb, to, &gfs2_iomap_ops, NULL, 0, 0); > gfs2_glock_dq(gh); > out_uninit: > gfs2_holder_uninit(gh); > @@ -856,7 +856,7 @@ static ssize_t gfs2_file_direct_write(struct kiocb *iocb, struct iov_iter *from, > if (offset + len > i_size_read(&ip->i_inode)) > goto out; > > - ret = iomap_dio_rw(iocb, from, &gfs2_iomap_ops, NULL, 0); > + ret = iomap_dio_rw(iocb, from, &gfs2_iomap_ops, NULL, 0, 0); > if (ret == -ENOTBLK) > ret = 0; > out: > diff --git a/fs/iomap/direct-io.c b/fs/iomap/direct-io.c > index ba88fe51b77a..dcf9a2b4381f 100644 > --- a/fs/iomap/direct-io.c > +++ b/fs/iomap/direct-io.c > @@ -31,6 +31,7 @@ struct iomap_dio { > atomic_t ref; > unsigned flags; > int error; > + size_t done_before; So, now that I actually understand the reason why the count of previously transferred bytes has to be passed into the iomap_dio, I would like this field to have a comment so that stupid maintainers like me don't forget the subtleties again: /* * For asynchronous IO, we have one chance to call the iocb * completion method with the results of the directio operation. * If this operation is a resubmission after a previous partial * completion (e.g. page fault), we need to know about that * progress so that we can report that and the result of the * resubmission to the iocb completion. */ size_t done_before; With that added, I think I can live with this enough to: Reviewed-by: Darrick J. Wong --D > bool wait_for_completion; > > union { > @@ -126,6 +127,9 @@ ssize_t iomap_dio_complete(struct iomap_dio *dio) > if (ret > 0 && (dio->flags & IOMAP_DIO_NEED_SYNC)) > ret = generic_write_sync(iocb, ret); > > + if (ret > 0) > + ret += dio->done_before; > + > kfree(dio); > > return ret; > @@ -450,7 +454,7 @@ iomap_dio_actor(struct inode *inode, loff_t pos, loff_t length, > struct iomap_dio * > __iomap_dio_rw(struct kiocb *iocb, struct iov_iter *iter, > const struct iomap_ops *ops, const struct iomap_dio_ops *dops, > - unsigned int dio_flags) > + unsigned int dio_flags, size_t done_before) > { > struct address_space *mapping = iocb->ki_filp->f_mapping; > struct inode *inode = file_inode(iocb->ki_filp); > @@ -477,6 +481,7 @@ __iomap_dio_rw(struct kiocb *iocb, struct iov_iter *iter, > dio->dops = dops; > dio->error = 0; > dio->flags = 0; > + dio->done_before = done_before; > > dio->submit.iter = iter; > dio->submit.waiter = current; > @@ -648,11 +653,11 @@ EXPORT_SYMBOL_GPL(__iomap_dio_rw); > ssize_t > iomap_dio_rw(struct kiocb *iocb, struct iov_iter *iter, > const struct iomap_ops *ops, const struct iomap_dio_ops *dops, > - unsigned int dio_flags) > + unsigned int dio_flags, size_t done_before) > { > struct iomap_dio *dio; > > - dio = __iomap_dio_rw(iocb, iter, ops, dops, dio_flags); > + dio = __iomap_dio_rw(iocb, iter, ops, dops, dio_flags, done_before); > if (IS_ERR_OR_NULL(dio)) > return PTR_ERR_OR_ZERO(dio); > return iomap_dio_complete(dio); > diff --git a/fs/xfs/xfs_file.c b/fs/xfs/xfs_file.c > index cc3cfb12df53..3103d9bda466 100644 > --- a/fs/xfs/xfs_file.c > +++ b/fs/xfs/xfs_file.c > @@ -259,7 +259,7 @@ xfs_file_dio_read( > ret = xfs_ilock_iocb(iocb, XFS_IOLOCK_SHARED); > if (ret) > return ret; > - ret = iomap_dio_rw(iocb, to, &xfs_read_iomap_ops, NULL, 0); > + ret = iomap_dio_rw(iocb, to, &xfs_read_iomap_ops, NULL, 0, 0); > xfs_iunlock(ip, XFS_IOLOCK_SHARED); > > return ret; > @@ -569,7 +569,7 @@ xfs_file_dio_write_aligned( > } > trace_xfs_file_direct_write(iocb, from); > ret = iomap_dio_rw(iocb, from, &xfs_direct_write_iomap_ops, > - &xfs_dio_write_ops, 0); > + &xfs_dio_write_ops, 0, 0); > out_unlock: > if (iolock) > xfs_iunlock(ip, iolock); > @@ -647,7 +647,7 @@ xfs_file_dio_write_unaligned( > > trace_xfs_file_direct_write(iocb, from); > ret = iomap_dio_rw(iocb, from, &xfs_direct_write_iomap_ops, > - &xfs_dio_write_ops, flags); > + &xfs_dio_write_ops, flags, 0); > > /* > * Retry unaligned I/O with exclusive blocking semantics if the DIO > diff --git a/fs/zonefs/super.c b/fs/zonefs/super.c > index 70055d486bf7..85ca2f5fe06e 100644 > --- a/fs/zonefs/super.c > +++ b/fs/zonefs/super.c > @@ -864,7 +864,7 @@ static ssize_t zonefs_file_dio_write(struct kiocb *iocb, struct iov_iter *from) > ret = zonefs_file_dio_append(iocb, from); > else > ret = iomap_dio_rw(iocb, from, &zonefs_iomap_ops, > - &zonefs_write_dio_ops, 0); > + &zonefs_write_dio_ops, 0, 0); > if (zi->i_ztype == ZONEFS_ZTYPE_SEQ && > (ret > 0 || ret == -EIOCBQUEUED)) { > if (ret > 0) > @@ -999,7 +999,7 @@ static ssize_t zonefs_file_read_iter(struct kiocb *iocb, struct iov_iter *to) > } > file_accessed(iocb->ki_filp); > ret = iomap_dio_rw(iocb, to, &zonefs_iomap_ops, > - &zonefs_read_dio_ops, 0); > + &zonefs_read_dio_ops, 0, 0); > } else { > ret = generic_file_read_iter(iocb, to); > if (ret == -EIO) > diff --git a/include/linux/iomap.h b/include/linux/iomap.h > index bcae4814b8e3..908bda10024c 100644 > --- a/include/linux/iomap.h > +++ b/include/linux/iomap.h > @@ -276,10 +276,10 @@ struct iomap_dio_ops { > > ssize_t iomap_dio_rw(struct kiocb *iocb, struct iov_iter *iter, > const struct iomap_ops *ops, const struct iomap_dio_ops *dops, > - unsigned int dio_flags); > + unsigned int dio_flags, size_t done_before); > struct iomap_dio *__iomap_dio_rw(struct kiocb *iocb, struct iov_iter *iter, > const struct iomap_ops *ops, const struct iomap_dio_ops *dops, > - unsigned int dio_flags); > + unsigned int dio_flags, size_t done_before); > ssize_t iomap_dio_complete(struct iomap_dio *dio); > int iomap_dio_iopoll(struct kiocb *kiocb, bool spin); > > -- > 2.26.3 > _______________________________________________ Ocfs2-devel mailing list Ocfs2-devel@oss.oracle.com https://oss.oracle.com/mailman/listinfo/ocfs2-devel From mboxrd@z Thu Jan 1 00:00:00 1970 From: Darrick J. Wong Date: Fri, 3 Sep 2021 11:53:51 -0700 Subject: [Cluster-devel] [PATCH v7 16/19] iomap: Add done_before argument to iomap_dio_rw In-Reply-To: <20210827164926.1726765-17-agruenba@redhat.com> References: <20210827164926.1726765-1-agruenba@redhat.com> <20210827164926.1726765-17-agruenba@redhat.com> Message-ID: <20210903185351.GD9892@magnolia> List-Id: To: cluster-devel.redhat.com MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit On Fri, Aug 27, 2021 at 06:49:23PM +0200, Andreas Gruenbacher wrote: > Add a done_before argument to iomap_dio_rw that indicates how much of > the request has already been transferred. When the request succeeds, we > report that done_before additional bytes were tranferred. This is > useful for finishing a request asynchronously when part of the request > has already been completed synchronously. > > We'll use that to allow iomap_dio_rw to be used with page faults > disabled: when a page fault occurs while submitting a request, we > synchronously complete the part of the request that has already been > submitted. The caller can then take care of the page fault and call > iomap_dio_rw again for the rest of the request, passing in the number of > bytes already tranferred. > > Signed-off-by: Andreas Gruenbacher > --- > fs/btrfs/file.c | 5 +++-- > fs/ext4/file.c | 5 +++-- > fs/gfs2/file.c | 4 ++-- > fs/iomap/direct-io.c | 11 ++++++++--- > fs/xfs/xfs_file.c | 6 +++--- > fs/zonefs/super.c | 4 ++-- > include/linux/iomap.h | 4 ++-- > 7 files changed, 23 insertions(+), 16 deletions(-) > > diff --git a/fs/btrfs/file.c b/fs/btrfs/file.c > index 281c77cfe91a..8817fe6b5fc0 100644 > --- a/fs/btrfs/file.c > +++ b/fs/btrfs/file.c > @@ -1945,7 +1945,7 @@ static ssize_t btrfs_direct_write(struct kiocb *iocb, struct iov_iter *from) > } > > dio = __iomap_dio_rw(iocb, from, &btrfs_dio_iomap_ops, &btrfs_dio_ops, > - 0); > + 0, 0); > > btrfs_inode_unlock(inode, ilock_flags); > > @@ -3637,7 +3637,8 @@ static ssize_t btrfs_direct_read(struct kiocb *iocb, struct iov_iter *to) > return 0; > > btrfs_inode_lock(inode, BTRFS_ILOCK_SHARED); > - ret = iomap_dio_rw(iocb, to, &btrfs_dio_iomap_ops, &btrfs_dio_ops, 0); > + ret = iomap_dio_rw(iocb, to, &btrfs_dio_iomap_ops, &btrfs_dio_ops, > + 0, 0); > btrfs_inode_unlock(inode, BTRFS_ILOCK_SHARED); > return ret; > } > diff --git a/fs/ext4/file.c b/fs/ext4/file.c > index 816dedcbd541..4a5e7fd31fb5 100644 > --- a/fs/ext4/file.c > +++ b/fs/ext4/file.c > @@ -74,7 +74,7 @@ static ssize_t ext4_dio_read_iter(struct kiocb *iocb, struct iov_iter *to) > return generic_file_read_iter(iocb, to); > } > > - ret = iomap_dio_rw(iocb, to, &ext4_iomap_ops, NULL, 0); > + ret = iomap_dio_rw(iocb, to, &ext4_iomap_ops, NULL, 0, 0); > inode_unlock_shared(inode); > > file_accessed(iocb->ki_filp); > @@ -566,7 +566,8 @@ static ssize_t ext4_dio_write_iter(struct kiocb *iocb, struct iov_iter *from) > if (ilock_shared) > iomap_ops = &ext4_iomap_overwrite_ops; > ret = iomap_dio_rw(iocb, from, iomap_ops, &ext4_dio_write_ops, > - (unaligned_io || extend) ? IOMAP_DIO_FORCE_WAIT : 0); > + (unaligned_io || extend) ? IOMAP_DIO_FORCE_WAIT : 0, > + 0); > if (ret == -ENOTBLK) > ret = 0; > > diff --git a/fs/gfs2/file.c b/fs/gfs2/file.c > index fce3a5249e19..64bf2f68e6d6 100644 > --- a/fs/gfs2/file.c > +++ b/fs/gfs2/file.c > @@ -822,7 +822,7 @@ static ssize_t gfs2_file_direct_read(struct kiocb *iocb, struct iov_iter *to, > if (ret) > goto out_uninit; > > - ret = iomap_dio_rw(iocb, to, &gfs2_iomap_ops, NULL, 0); > + ret = iomap_dio_rw(iocb, to, &gfs2_iomap_ops, NULL, 0, 0); > gfs2_glock_dq(gh); > out_uninit: > gfs2_holder_uninit(gh); > @@ -856,7 +856,7 @@ static ssize_t gfs2_file_direct_write(struct kiocb *iocb, struct iov_iter *from, > if (offset + len > i_size_read(&ip->i_inode)) > goto out; > > - ret = iomap_dio_rw(iocb, from, &gfs2_iomap_ops, NULL, 0); > + ret = iomap_dio_rw(iocb, from, &gfs2_iomap_ops, NULL, 0, 0); > if (ret == -ENOTBLK) > ret = 0; > out: > diff --git a/fs/iomap/direct-io.c b/fs/iomap/direct-io.c > index ba88fe51b77a..dcf9a2b4381f 100644 > --- a/fs/iomap/direct-io.c > +++ b/fs/iomap/direct-io.c > @@ -31,6 +31,7 @@ struct iomap_dio { > atomic_t ref; > unsigned flags; > int error; > + size_t done_before; So, now that I actually understand the reason why the count of previously transferred bytes has to be passed into the iomap_dio, I would like this field to have a comment so that stupid maintainers like me don't forget the subtleties again: /* * For asynchronous IO, we have one chance to call the iocb * completion method with the results of the directio operation. * If this operation is a resubmission after a previous partial * completion (e.g. page fault), we need to know about that * progress so that we can report that and the result of the * resubmission to the iocb completion. */ size_t done_before; With that added, I think I can live with this enough to: Reviewed-by: Darrick J. Wong --D > bool wait_for_completion; > > union { > @@ -126,6 +127,9 @@ ssize_t iomap_dio_complete(struct iomap_dio *dio) > if (ret > 0 && (dio->flags & IOMAP_DIO_NEED_SYNC)) > ret = generic_write_sync(iocb, ret); > > + if (ret > 0) > + ret += dio->done_before; > + > kfree(dio); > > return ret; > @@ -450,7 +454,7 @@ iomap_dio_actor(struct inode *inode, loff_t pos, loff_t length, > struct iomap_dio * > __iomap_dio_rw(struct kiocb *iocb, struct iov_iter *iter, > const struct iomap_ops *ops, const struct iomap_dio_ops *dops, > - unsigned int dio_flags) > + unsigned int dio_flags, size_t done_before) > { > struct address_space *mapping = iocb->ki_filp->f_mapping; > struct inode *inode = file_inode(iocb->ki_filp); > @@ -477,6 +481,7 @@ __iomap_dio_rw(struct kiocb *iocb, struct iov_iter *iter, > dio->dops = dops; > dio->error = 0; > dio->flags = 0; > + dio->done_before = done_before; > > dio->submit.iter = iter; > dio->submit.waiter = current; > @@ -648,11 +653,11 @@ EXPORT_SYMBOL_GPL(__iomap_dio_rw); > ssize_t > iomap_dio_rw(struct kiocb *iocb, struct iov_iter *iter, > const struct iomap_ops *ops, const struct iomap_dio_ops *dops, > - unsigned int dio_flags) > + unsigned int dio_flags, size_t done_before) > { > struct iomap_dio *dio; > > - dio = __iomap_dio_rw(iocb, iter, ops, dops, dio_flags); > + dio = __iomap_dio_rw(iocb, iter, ops, dops, dio_flags, done_before); > if (IS_ERR_OR_NULL(dio)) > return PTR_ERR_OR_ZERO(dio); > return iomap_dio_complete(dio); > diff --git a/fs/xfs/xfs_file.c b/fs/xfs/xfs_file.c > index cc3cfb12df53..3103d9bda466 100644 > --- a/fs/xfs/xfs_file.c > +++ b/fs/xfs/xfs_file.c > @@ -259,7 +259,7 @@ xfs_file_dio_read( > ret = xfs_ilock_iocb(iocb, XFS_IOLOCK_SHARED); > if (ret) > return ret; > - ret = iomap_dio_rw(iocb, to, &xfs_read_iomap_ops, NULL, 0); > + ret = iomap_dio_rw(iocb, to, &xfs_read_iomap_ops, NULL, 0, 0); > xfs_iunlock(ip, XFS_IOLOCK_SHARED); > > return ret; > @@ -569,7 +569,7 @@ xfs_file_dio_write_aligned( > } > trace_xfs_file_direct_write(iocb, from); > ret = iomap_dio_rw(iocb, from, &xfs_direct_write_iomap_ops, > - &xfs_dio_write_ops, 0); > + &xfs_dio_write_ops, 0, 0); > out_unlock: > if (iolock) > xfs_iunlock(ip, iolock); > @@ -647,7 +647,7 @@ xfs_file_dio_write_unaligned( > > trace_xfs_file_direct_write(iocb, from); > ret = iomap_dio_rw(iocb, from, &xfs_direct_write_iomap_ops, > - &xfs_dio_write_ops, flags); > + &xfs_dio_write_ops, flags, 0); > > /* > * Retry unaligned I/O with exclusive blocking semantics if the DIO > diff --git a/fs/zonefs/super.c b/fs/zonefs/super.c > index 70055d486bf7..85ca2f5fe06e 100644 > --- a/fs/zonefs/super.c > +++ b/fs/zonefs/super.c > @@ -864,7 +864,7 @@ static ssize_t zonefs_file_dio_write(struct kiocb *iocb, struct iov_iter *from) > ret = zonefs_file_dio_append(iocb, from); > else > ret = iomap_dio_rw(iocb, from, &zonefs_iomap_ops, > - &zonefs_write_dio_ops, 0); > + &zonefs_write_dio_ops, 0, 0); > if (zi->i_ztype == ZONEFS_ZTYPE_SEQ && > (ret > 0 || ret == -EIOCBQUEUED)) { > if (ret > 0) > @@ -999,7 +999,7 @@ static ssize_t zonefs_file_read_iter(struct kiocb *iocb, struct iov_iter *to) > } > file_accessed(iocb->ki_filp); > ret = iomap_dio_rw(iocb, to, &zonefs_iomap_ops, > - &zonefs_read_dio_ops, 0); > + &zonefs_read_dio_ops, 0, 0); > } else { > ret = generic_file_read_iter(iocb, to); > if (ret == -EIO) > diff --git a/include/linux/iomap.h b/include/linux/iomap.h > index bcae4814b8e3..908bda10024c 100644 > --- a/include/linux/iomap.h > +++ b/include/linux/iomap.h > @@ -276,10 +276,10 @@ struct iomap_dio_ops { > > ssize_t iomap_dio_rw(struct kiocb *iocb, struct iov_iter *iter, > const struct iomap_ops *ops, const struct iomap_dio_ops *dops, > - unsigned int dio_flags); > + unsigned int dio_flags, size_t done_before); > struct iomap_dio *__iomap_dio_rw(struct kiocb *iocb, struct iov_iter *iter, > const struct iomap_ops *ops, const struct iomap_dio_ops *dops, > - unsigned int dio_flags); > + unsigned int dio_flags, size_t done_before); > ssize_t iomap_dio_complete(struct iomap_dio *dio); > int iomap_dio_iopoll(struct kiocb *kiocb, bool spin); > > -- > 2.26.3 >