From patchwork Mon Dec 18 21:53:58 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Andrew Morton X-Patchwork-Id: 10121931 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 1C64B60327 for ; Mon, 18 Dec 2017 21:54:29 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 0DA6D28D6E for ; Mon, 18 Dec 2017 21:54:29 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 01D8728D73; Mon, 18 Dec 2017 21:54:28 +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=-1.9 required=2.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID autolearn=ham version=3.3.1 Received: from userp2120.oracle.com (userp2120.oracle.com [156.151.31.85]) (using TLSv1.2 with cipher AES256-SHA (256/256 bits)) (No client certificate requested) by mail.wl.linuxfoundation.org (Postfix) with ESMTPS id E4D9328D6E for ; Mon, 18 Dec 2017 21:54:27 +0000 (UTC) Received: from pps.filterd (userp2120.oracle.com [127.0.0.1]) by userp2120.oracle.com (8.16.0.21/8.16.0.21) with SMTP id vBILpRql055896; Mon, 18 Dec 2017 21:54:11 GMT DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=oracle.com; h=date : from : to : message-id : in-reply-to : references : mime-version : cc : subject : list-id : list-unsubscribe : list-archive : list-post : list-help : list-subscribe : content-type : content-transfer-encoding : sender; s=corp-2017-10-26; bh=L/HbgaFt0XEyuWHAaNVNJPNRb3/EHER2NlGIHh232zY=; b=p/15jh6EDFz/g6kPFdIRXpWIKmqLTZQS+omx9SnjXumUy4C2AJOe8pH4JNL8pPbGlCTL yHhoYuPwQ8j42Fe2kWI6YaaDJ++RHx8/7kAd7vR8rxPwHQQJycZjG16in+cniHhQzv+N y5Va8Rxm4h4mxJjKroIC1fIHDLcBx0fp/AT2fms/+p3zgNsmrU2xysGmdkX9lpbw8WU2 UaPdQV/O4sw+kxj10WmMpifQvhTTByV2kBRb8IKbjl/IeWTtm6lNWn9y5T+2lgc05bua /BXxgKF81jpG5Wo0JyOckHQGKmiPpxHIIuTWYqBLjsRc404IEtGR1rLR6x6JEBLh4434 9A== Received: from userv0022.oracle.com (userv0022.oracle.com [156.151.31.74]) by userp2120.oracle.com with ESMTP id 2exn5s0jnx-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=OK); Mon, 18 Dec 2017 21:54:10 +0000 Received: from oss.oracle.com (oss-old-reserved.oracle.com [137.254.22.2]) by userv0022.oracle.com (8.14.4/8.14.4) with ESMTP id vBILs6tJ006944 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=NO); Mon, 18 Dec 2017 21:54:07 GMT Received: from localhost ([127.0.0.1] helo=lb-oss.oracle.com) by oss.oracle.com with esmtp (Exim 4.63) (envelope-from ) id 1eR3MM-0006I7-Jg; Mon, 18 Dec 2017 13:54:06 -0800 Received: from aserv0022.oracle.com ([141.146.126.234]) by oss.oracle.com with esmtp (Exim 4.63) (envelope-from ) id 1eR3MK-0006Gx-B4 for ocfs2-devel@oss.oracle.com; Mon, 18 Dec 2017 13:54:05 -0800 Received: from userp2040.oracle.com (userp2040.oracle.com [156.151.31.90]) by aserv0022.oracle.com (8.14.4/8.14.4) with ESMTP id vBILs3PV002549 (version=TLSv1/SSLv3 cipher=AES256-SHA bits=256 verify=FAIL) for ; Mon, 18 Dec 2017 21:54:04 GMT Received: from pps.filterd (userp2040.oracle.com [127.0.0.1]) by userp2040.oracle.com (8.16.0.21/8.16.0.21) with SMTP id vBILpnZq021443 for ; Mon, 18 Dec 2017 21:54:03 GMT Received: from mail.linuxfoundation.org (mail.linuxfoundation.org [140.211.169.12]) by userp2040.oracle.com with ESMTP id 2exnbuuhf0-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NO) for ; Mon, 18 Dec 2017 21:54:03 +0000 Received: from akpm3.svl.corp.google.com (unknown [104.133.9.92]) by mail.linuxfoundation.org (Postfix) with ESMTPSA id B1148BBC; Mon, 18 Dec 2017 21:53:59 +0000 (UTC) Date: Mon, 18 Dec 2017 13:53:58 -0800 From: Andrew Morton To: Changwei Ge Message-Id: <20171218135358.cee63b6019681cd7cfc569ee@linux-foundation.org> In-Reply-To: <63ADC13FD55D6546B7DECE290D39E373F1F5C2CE@H3CMLB14-EX.srv.huawei-3com.com> References: <63ADC13FD55D6546B7DECE290D39E373F1F5C2CE@H3CMLB14-EX.srv.huawei-3com.com> X-Mailer: Sylpheed 3.4.1 (GTK+ 2.24.23; x86_64-pc-linux-gnu) Mime-Version: 1.0 X-CLX-Shades: MLX X-CLX-Response: 1TFkXGBwdEQpMehcaEQpZTRdnZnIRCllJFxpxGhAadwYYHh1xHxITEBp3Bhg aBhoRClleF2hjeREKSUYXRVhLSUZPdVpYRU5fSV5DRUQZdU9LEQpDThdAHksdaEYbGxxrG15NGn lHe25EaWkTblh5fGJDHEhGfxEKWFwXHwQaBBsYGAccS0hLTx4cGgUbGgQbGhoEHhIEHxAbHhofG hEKXlkXeEhOGH0RCk1cFx8SHhEKTFoXaGlNTV0RCkxGF2xraxEKQ1oXGx4aBBgbGwQbHBMEGxgR CkJeFxsRCkRJFxwRCkJGF2cTbWAbW2VCH359EQpCXBcaEQpCRRdteVpAfx5Tbx9hYhEKQk4Xb29 lYhxyBWZmXmERCkJMF2t8f2BFGGxzZRpkEQpCbBduElh9ek9Pb11MHxEKQkAXYHxOREdHSGx8QB 8RCkJYF2J9b3kBTxgZcHB7EQpaWBcbEQpwZxdvYRxZZ1IfehJsGBAeEhEKcGgXYUUcWlBCc0FSH ngQGRoRCnBoF25JY3hERhNnR3ASEBkaEQpwaBdpYhtIfRl4U0Z9WxAZGhEKcGgXYQEBeEJFRHIS ZFgQHhIRCnBoF2NDQEhzY2JJGV5yEBkaEQpwbBdmTmRdRUJsWmVkRhATGhEKcEMXbF1gQn1Dc15 uZx4QGRoRCm1+FxoRClhNF0sRIA== X-PDR: PASS X-Source-IP: 140.211.169.12 X-ServerName: mail.linuxfoundation.org X-Proofpoint-SPF-Result: pass X-Proofpoint-SPF-Record: v=spf1 ip4:140.211.169.12/30 include:_spf.google.com ~all X-Proofpoint-Virus-Version: vendor=nai engine=5900 definitions=8749 signatures=668649 X-Proofpoint-Spam-Details: rule=notspam policy=default score=0 priorityscore=0 malwarescore=0 suspectscore=0 phishscore=0 bulkscore=0 spamscore=0 clxscore=267 lowpriorityscore=0 mlxscore=0 impostorscore=0 mlxlogscore=999 adultscore=0 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.0.1-1711220000 definitions=main-1712180290 X-Spam: Clean Cc: "ocfs2-devel@oss.oracle.com" Subject: Re: [Ocfs2-devel] [PATCH] ocfs2: fall back to buffer IO when append dio is disabled with file hole existing 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: , Sender: ocfs2-devel-bounces@oss.oracle.com Errors-To: ocfs2-devel-bounces@oss.oracle.com X-Proofpoint-Virus-Version: vendor=nai engine=5900 definitions=8749 signatures=668649 X-Proofpoint-Spam-Details: rule=notspam policy=default score=0 suspectscore=0 malwarescore=0 phishscore=0 bulkscore=0 spamscore=0 mlxscore=0 mlxlogscore=999 adultscore=0 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.0.1-1711220000 definitions=main-1712180290 X-Virus-Scanned: ClamAV using ClamSMTP On Mon, 18 Dec 2017 12:06:21 +0000 Changwei Ge wrote: > Before ocfs2 supporting allocating clusters while doing append-dio, all append > dio will fall back to buffer io to allocate clusters firstly. Also, when it > steps on a file hole, it will fall back to buffer io, too. But for current > code, writing to file hole will leverage dio to allocate clusters. This is not > right, since whether append-io is enabled tells the capability whether ocfs2 can > allocate space while doing dio. > So introduce file hole check function back into ocfs2. > Once ocfs2 is doing dio upon a file hole with append-dio disabled, it will fall > back to buffer IO to allocate clusters. > hm, that's a bit hard to understand. Hopefully reviewers will know what's going on ;) > --- a/fs/ocfs2/aops.c > +++ b/fs/ocfs2/aops.c > @@ -2414,6 +2414,44 @@ static int ocfs2_dio_end_io(struct kiocb *iocb, > return ret; > } > > +/* > + * Will look for holes and unwritten extents in the range starting at > + * pos for count bytes (inclusive). > + */ > +static int ocfs2_check_range_for_holes(struct inode *inode, loff_t pos, > + size_t count) > +{ > + int ret = 0; > + unsigned int extent_flags; > + u32 cpos, clusters, extent_len, phys_cpos; > + struct super_block *sb = inode->i_sb; > + > + cpos = pos >> OCFS2_SB(sb)->s_clustersize_bits; > + clusters = ocfs2_clusters_for_bytes(sb, pos + count) - cpos; > + > + while (clusters) { > + ret = ocfs2_get_clusters(inode, cpos, &phys_cpos, &extent_len, > + &extent_flags); > + if (ret < 0) { > + mlog_errno(ret); > + goto out; > + } > + > + if (phys_cpos == 0 || (extent_flags & OCFS2_EXT_UNWRITTEN)) { > + ret = 1; > + break; > + } > + > + if (extent_len > clusters) > + extent_len = clusters; > + > + clusters -= extent_len; > + cpos += extent_len; > + } > +out: > + return ret; > +} A few thoughts: - a function which does "check_foo" isn't well named. Because the reader cannot determine whether the return value means "foo is true" or "foo is false". So a better name for this function is ocfs2_range_has_holes(), so the reader immediately understand what its return value *means". Also a bool return value is more logical. - Mixing "goto out" with "break" as above is a bit odd. So... --- a/fs/ocfs2/aops.c~ocfs2-fall-back-to-buffer-io-when-append-dio-is-disabled-with-file-hole-existing-fix +++ a/fs/ocfs2/aops.c @@ -2469,10 +2469,9 @@ static int ocfs2_dio_end_io(struct kiocb * Will look for holes and unwritten extents in the range starting at * pos for count bytes (inclusive). */ -static int ocfs2_check_range_for_holes(struct inode *inode, loff_t pos, - size_t count) +static bool ocfs2_range_has_holes(struct inode *inode, loff_t pos, size_t count) { - int ret = 0; + bool ret = false; unsigned int extent_flags; u32 cpos, clusters, extent_len, phys_cpos; struct super_block *sb = inode->i_sb; @@ -2489,8 +2488,8 @@ static int ocfs2_check_range_for_holes(s } if (phys_cpos == 0 || (extent_flags & OCFS2_EXT_UNWRITTEN)) { - ret = 1; - break; + ret = true; + goto out; } if (extent_len > clusters)