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 Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by smtp.lore.kernel.org (Postfix) with ESMTP id B08FBC48BC3 for ; Sat, 17 Feb 2024 08:56:04 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 192656B009F; Sat, 17 Feb 2024 03:56:04 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id 11B956B00A0; Sat, 17 Feb 2024 03:56:04 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id ED6F66B00A1; Sat, 17 Feb 2024 03:56:03 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0015.hostedemail.com [216.40.44.15]) by kanga.kvack.org (Postfix) with ESMTP id DB9336B009F for ; Sat, 17 Feb 2024 03:56:03 -0500 (EST) Received: from smtpin26.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay01.hostedemail.com (Postfix) with ESMTP id 856CF1C0076 for ; Sat, 17 Feb 2024 08:56:03 +0000 (UTC) X-FDA: 81800688606.26.445CAF7 Received: from dggsgout11.his.huawei.com (unknown [45.249.212.51]) by imf17.hostedemail.com (Postfix) with ESMTP id 125494000B for ; Sat, 17 Feb 2024 08:55:58 +0000 (UTC) Authentication-Results: imf17.hostedemail.com; dkim=none; spf=pass (imf17.hostedemail.com: domain of yi.zhang@huaweicloud.com designates 45.249.212.51 as permitted sender) smtp.mailfrom=yi.zhang@huaweicloud.com; dmarc=none ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1708160161; h=from:from:sender:reply-to:subject:subject:date:date: message-id:message-id:to:to:cc:cc:mime-version:mime-version: content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=qP8mrY1WDlJzoePrliAqrGz6MEIZoCD3URuUQaCLp5k=; b=O2xc5xCHrEWUuW5O0jqs61H2Bz3w0ecBezTITqkufgAA0W+37YWcrj/Uho2Odi4xMs7IW1 lDxjCvetv9zZxd/VSLjYAla5Bxn5rxzb8LuWIPboncBrydkQbky6dzn/Iy5vIgE71AwZEm 31F/RP7+OC2ayH/0jJ1ZAGihzitHgwA= ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1708160161; a=rsa-sha256; cv=none; b=pgFWkg6Pfm5YyX8uF97W5zKu9U1Lq4bWrz+Ru7z9UVd+zcrsSwQhltzMw778lhihRK+IT/ BwCyAGAz7WodRHaxVdtHwza3JJPJK/DId780rkqH7hSrh/QZvvbwJNXbsxZ2TOYh8+CSh9 718iVeKdtmB4ucLjVm55h7F9a3nbLiA= ARC-Authentication-Results: i=1; imf17.hostedemail.com; dkim=none; spf=pass (imf17.hostedemail.com: domain of yi.zhang@huaweicloud.com designates 45.249.212.51 as permitted sender) smtp.mailfrom=yi.zhang@huaweicloud.com; dmarc=none Received: from mail.maildlp.com (unknown [172.19.93.142]) by dggsgout11.his.huawei.com (SkyGuard) with ESMTP id 4TcN1f31Y8z4f3kkM for ; Sat, 17 Feb 2024 16:55:50 +0800 (CST) Received: from mail02.huawei.com (unknown [10.116.40.112]) by mail.maildlp.com (Postfix) with ESMTP id 75CDF1A016E for ; Sat, 17 Feb 2024 16:55:53 +0800 (CST) Received: from [10.174.176.34] (unknown [10.174.176.34]) by APP1 (Coremail) with SMTP id cCh0CgBHZQ6XdNBl_ygdEQ--.60286S3; Sat, 17 Feb 2024 16:55:53 +0800 (CST) Subject: Re: [RFC PATCH v3 07/26] iomap: don't increase i_size if it's not a write operation To: Christoph Hellwig Cc: linux-ext4@vger.kernel.org, linux-fsdevel@vger.kernel.org, linux-mm@kvack.org, linux-kernel@vger.kernel.org, tytso@mit.edu, adilger.kernel@dilger.ca, jack@suse.cz, ritesh.list@gmail.com, djwong@kernel.org, willy@infradead.org, zokeefe@google.com, yi.zhang@huawei.com, chengzhihao1@huawei.com, yukuai3@huawei.com, wangkefeng.wang@huawei.com References: <20240127015825.1608160-1-yi.zhang@huaweicloud.com> <20240127015825.1608160-8-yi.zhang@huaweicloud.com> From: Zhang Yi Message-ID: <74ab3c3e-3daf-5374-75e5-bcb25ffdb527@huaweicloud.com> Date: Sat, 17 Feb 2024 16:55:51 +0800 User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:78.0) Gecko/20100101 Thunderbird/78.12.0 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit X-CM-TRANSID:cCh0CgBHZQ6XdNBl_ygdEQ--.60286S3 X-Coremail-Antispam: 1UD129KBjvJXoWxur4UGw45Cw47Gr4xtr48tFb_yoWruw1rpr yq93yDCFs3tF17Wr1kJFZ8XryYka4rKrW2kryxGw43ZFnIyr9rKF1rGa4Yv3Z8J3sxAr4f JF4kAa4kWF1UAr7anT9S1TB71UUUUUUqnTZGkaVYY2UrUUUUjbIjqfuFe4nvWSU5nxnvy2 9KBjDU0xBIdaVrnRJUUUv2b4IE77IF4wAFF20E14v26ryj6rWUM7CY07I20VC2zVCF04k2 6cxKx2IYs7xG6rWj6s0DM7CIcVAFz4kK6r1j6r18M28lY4IEw2IIxxk0rwA2F7IY1VAKz4 vEj48ve4kI8wA2z4x0Y4vE2Ix0cI8IcVAFwI0_Ar0_tr1l84ACjcxK6xIIjxv20xvEc7Cj xVAFwI0_Cr0_Gr1UM28EF7xvwVC2z280aVAFwI0_GcCE3s1l84ACjcxK6I8E87Iv6xkF7I 0E14v26rxl6s0DM2AIxVAIcxkEcVAq07x20xvEncxIr21l5I8CrVACY4xI64kE6c02F40E x7xfMcIj6xIIjxv20xvE14v26r1j6r18McIj6I8E87Iv67AKxVWUJVW8JwAm72CE4IkC6x 0Yz7v_Jr0_Gr1lF7xvr2IY64vIr41lFIxGxcIEc7CjxVA2Y2ka0xkIwI1lc7I2V7IY0VAS 07AlzVAYIcxG8wCF04k20xvY0x0EwIxGrwCFx2IqxVCFs4IE7xkEbVWUJVW8JwC20s026c 02F40E14v26r1j6r18MI8I3I0E7480Y4vE14v26r106r1rMI8E67AF67kF1VAFwI0_GFv_ WrylIxkGc2Ij64vIr41lIxAIcVC0I7IYx2IY67AKxVWUJVWUCwCI42IY6xIIjxv20xvEc7 CjxVAFwI0_Gr0_Cr1lIxAIcVCF04k26cxKx2IYs7xG6rW3Jr0E3s1lIxAIcVC2z280aVAF wI0_Jr0_Gr1lIxAIcVC2z280aVCY1x0267AKxVW8JVW8JrUvcSsGvfC2KfnxnUUI43ZEXa 7IU13rcDUUUUU== X-CM-SenderInfo: d1lo6xhdqjqx5xdzvxpfor3voofrz/ X-Rspamd-Queue-Id: 125494000B X-Rspam-User: X-Stat-Signature: wr4fk84bs1mecgj81764bmojk757g7bw X-Rspamd-Server: rspam03 X-HE-Tag: 1708160158-311263 X-HE-Meta: U2FsdGVkX1/esw1Gn6pbRCSNSTd5i5esYl3hpZN7gX5MQTTtzVZhWWLTRPqWU61ma/QmeI9T7+MtPR1+vlbOzFhP8DYUlDlEIkCSiJlFtYpVMjwdxV1HbQVa/zlmaFeleyXulqCZlFU8UjiOz8Lxm1GwqSRAJUp+Vt2owFFsosYCnO0q3P71DsN380kNrl22LAsZzGPQzTv25nmx/xGXc0NzRtm1LYS+5QVFEcF1xcNXNkU/YjVKptJAEVRvt0kYmFF/3s/ouuLNCSRQ0xb9vI4E8hC0KlJqLSWmlh6lcJJeBksmhPP34BL/Pt92iRkTo6TrppJ2HMgPyMcHFKuC4gD4jmVlotzNozVsaY7j9Rfvw6ifmCjfpRR4/gtdW3FUi0GoqhUdQhrfa7+dGxsC4CWNqDJrSw5NpZCNjPRjb/YZJbF1pKEUzzcDsoeFjvtN7nrzKEZu52uRGr8/mk6IDrzpjrT5+4eqYAioXAhUizHlogcuq+WhCsrYLtD3X2oTm4BN8sS/XF8ZirZqtKaK8q4tULUNOEe3xw6tRtJto2Xn+tSUzxV6OWxqXHU0B1LkXxPopJDrzjLt7bh2/wYjpX3WATywJhqlmhKLi3SVnT0/oKT5OY+OgkywmxjYuZbSRP4g9WYvExLqPdwWnkRreUTLfuhI0xVTPS04jeeju0Z7xAAWZ84SwAEvG6FwSx/sLpKxMucTq41ZPSrJPiMxJ1bfgkaRA9xmhcvh7jRH7oTEQDZvyLgl+lWiRbSZYQx45HufHu3zeHWl+7gAk61rNC3g38loY8b3RKH2Or2Ema6uLYcGWN1Ejtgsfdx7SewaaDTtwX6UKm+xQt58WkEd2m/O9v9VnoMgo7JnmvIa0xwrepB/o6DAW4o768BFVjaOQBpBdMrzDdOte6kfuKuhQ68eY4MavCQvJC0o1/vm5xQKhlepLriAMW0G7O83JlVSfQXOMHVgWmP+NgZs0Bw y4GkDO8I A8+8MsiF4XGViKDU= X-Bogosity: Ham, tests=bogofilter, spamicity=0.000000, version=1.2.4 Sender: owner-linux-mm@kvack.org Precedence: bulk X-Loop: owner-majordomo@kvack.org List-ID: List-Subscribe: List-Unsubscribe: On 2024/2/13 13:46, Christoph Hellwig wrote: > Wouldn't it make more sense to just move the size manipulation to the > write-only code? An untested version of that is below. With this Sorry for the late reply and thanks for your suggestion, The reason why I introduced this new helper iomap_write_end_simple() is I don't want to open code __iomap_put_folio() in each caller since corresponding to iomap_write_begin(), it's the responsibility for iomap_write_end_*() to put and unlock folio, so I'd like to keep it in iomap_write_end_*(). But I don't feel strongly about it, it's also fine by me to just move the size manipulation to the write-only code if you think it's better. > the naming of the status variable becomes even more confusing than > it already is, maybe we need to do a cleanup of the *_write_end > calling conventions as it always returns the passed in copied value > or 0. Indeed, it becomes more confused and deserve a cleanup. Thanks, Yi. > > diff --git a/fs/iomap/buffered-io.c b/fs/iomap/buffered-io.c > index 3dab060aed6d7b..8401a9ca702fc0 100644 > --- a/fs/iomap/buffered-io.c > +++ b/fs/iomap/buffered-io.c > @@ -876,34 +876,13 @@ static size_t iomap_write_end(struct iomap_iter *iter, loff_t pos, size_t len, > size_t copied, struct folio *folio) > { > const struct iomap *srcmap = iomap_iter_srcmap(iter); > - loff_t old_size = iter->inode->i_size; > - size_t ret; > - > - if (srcmap->type == IOMAP_INLINE) { > - ret = iomap_write_end_inline(iter, folio, pos, copied); > - } else if (srcmap->flags & IOMAP_F_BUFFER_HEAD) { > - ret = block_write_end(NULL, iter->inode->i_mapping, pos, len, > - copied, &folio->page, NULL); > - } else { > - ret = __iomap_write_end(iter->inode, pos, len, copied, folio); > - } > - > - /* > - * Update the in-memory inode size after copying the data into the page > - * cache. It's up to the file system to write the updated size to disk, > - * preferably after I/O completion so that no stale data is exposed. > - */ > - if (pos + ret > old_size) { > - i_size_write(iter->inode, pos + ret); > - iter->iomap.flags |= IOMAP_F_SIZE_CHANGED; > - } > - __iomap_put_folio(iter, pos, ret, folio); > > - if (old_size < pos) > - pagecache_isize_extended(iter->inode, old_size, pos); > - if (ret < len) > - iomap_write_failed(iter->inode, pos + ret, len - ret); > - return ret; > + if (srcmap->type == IOMAP_INLINE) > + return iomap_write_end_inline(iter, folio, pos, copied); > + if (srcmap->flags & IOMAP_F_BUFFER_HEAD) > + return block_write_end(NULL, iter->inode->i_mapping, pos, len, > + copied, &folio->page, NULL); > + return __iomap_write_end(iter->inode, pos, len, copied, folio); > } > > static loff_t iomap_write_iter(struct iomap_iter *iter, struct iov_iter *i) > @@ -918,6 +897,7 @@ static loff_t iomap_write_iter(struct iomap_iter *iter, struct iov_iter *i) > > do { > struct folio *folio; > + loff_t old_size; > size_t offset; /* Offset into folio */ > size_t bytes; /* Bytes to write to folio */ > size_t copied; /* Bytes copied from user */ > @@ -964,7 +944,24 @@ static loff_t iomap_write_iter(struct iomap_iter *iter, struct iov_iter *i) > > copied = copy_folio_from_iter_atomic(folio, offset, bytes, i); > status = iomap_write_end(iter, pos, bytes, copied, folio); > + /* > + * Update the in-memory inode size after copying the data into > + * the page cache. It's up to the file system to write the > + * updated size to disk, preferably after I/O completion so that > + * no stale data is exposed. > + */ > + old_size = iter->inode->i_size; > + if (pos + status > old_size) { > + i_size_write(iter->inode, pos + status); > + iter->iomap.flags |= IOMAP_F_SIZE_CHANGED; > + } > + __iomap_put_folio(iter, pos, status, folio); > > + if (old_size < pos) > + pagecache_isize_extended(iter->inode, old_size, pos); > + if (status < bytes) > + iomap_write_failed(iter->inode, pos + status, > + bytes - status); > if (unlikely(copied != status)) > iov_iter_revert(i, copied - status); > > @@ -1334,6 +1331,7 @@ static loff_t iomap_unshare_iter(struct iomap_iter *iter) > bytes = folio_size(folio) - offset; > > bytes = iomap_write_end(iter, pos, bytes, bytes, folio); > + __iomap_put_folio(iter, pos, bytes, folio); > if (WARN_ON_ONCE(bytes == 0)) > return -EIO; > > @@ -1398,6 +1396,7 @@ static loff_t iomap_zero_iter(struct iomap_iter *iter, bool *did_zero) > folio_mark_accessed(folio); > > bytes = iomap_write_end(iter, pos, bytes, bytes, folio); > + __iomap_put_folio(iter, pos, bytes, folio); > if (WARN_ON_ONCE(bytes == 0)) > return -EIO; > >