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=-17.6 required=3.0 tests=BAYES_00, HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_CR_TRAILER,INCLUDES_PATCH, MAILING_LIST_MULTI,NICE_REPLY_A,SPF_HELO_NONE,SPF_PASS,UNPARSEABLE_RELAY, USER_AGENT_SANE_1 autolearn=ham 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 BFDE3C433EF for ; Mon, 6 Sep 2021 06:46:20 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 9204A60187 for ; Mon, 6 Sep 2021 06:46:20 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S239543AbhIFGrX (ORCPT ); Mon, 6 Sep 2021 02:47:23 -0400 Received: from out30-131.freemail.mail.aliyun.com ([115.124.30.131]:39848 "EHLO out30-131.freemail.mail.aliyun.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S239244AbhIFGrX (ORCPT ); Mon, 6 Sep 2021 02:47:23 -0400 X-Alimail-AntiSpam: AC=PASS;BC=-1|-1;BR=01201311R101e4;CH=green;DM=||false|;DS=||;FP=0|-1|-1|-1|0|-1|-1|-1;HT=e01e04423;MF=jefflexu@linux.alibaba.com;NM=1;PH=DS;RN=6;SR=0;TI=SMTPD_---0UnMKCx2_1630910776; Received: from admindeMacBook-Pro-2.local(mailfrom:jefflexu@linux.alibaba.com fp:SMTPD_---0UnMKCx2_1630910776) by smtp.aliyun-inc.com(127.0.0.1); Mon, 06 Sep 2021 14:46:17 +0800 Subject: Re: [PATCH v2] ext4: fix reserved space counter leakage From: JeffleXu To: Eric Whitney Cc: tytso@mit.edu, adilger.kernel@dilger.ca, linux-ext4@vger.kernel.org, joseph.qi@linux.alibaba.com, hsiangkao@linux.alibaba.com References: <20210823061358.84473-1-jefflexu@linux.alibaba.com> <20210823203009.GA10429@localhost.localdomain> <77ac5ffe-9769-bcb4-0600-f72ddf0aa59a@linux.alibaba.com> Message-ID: Date: Mon, 6 Sep 2021 14:46:16 +0800 User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.15; rv:78.0) Gecko/20100101 Thunderbird/78.13.0 MIME-Version: 1.0 In-Reply-To: <77ac5ffe-9769-bcb4-0600-f72ddf0aa59a@linux.alibaba.com> Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit Precedence: bulk List-ID: X-Mailing-List: linux-ext4@vger.kernel.org On 8/25/21 9:38 AM, JeffleXu wrote: > > > On 8/24/21 4:30 AM, Eric Whitney wrote: >> * Jeffle Xu : >>> When ext4_insert_delayed block receives and recovers from an error from >>> ext4_es_insert_delayed_block(), e.g., ENOMEM, it does not release the >>> space it has reserved for that block insertion as it should. One effect >>> of this bug is that s_dirtyclusters_counter is not decremented and >>> remains incorrectly elevated until the file system has been unmounted. >>> This can result in premature ENOSPC returns and apparent loss of free >>> space. >>> >>> Another effect of this bug is that >>> /sys/fs/ext4//delayed_allocation_blocks can remain non-zero even >>> after syncfs has been executed on the filesystem. >>> >>> Besides, add check for s_dirtyclusters_counter when inode is going to be >>> evicted and freed. s_dirtyclusters_counter can still keep non-zero until >>> inode is written back in .evict_inode(), and thus the check is delayed >>> to .destroy_inode(). >>> >>> Fixes: 51865fda28e5 ("ext4: let ext4 maintain extent status tree") >>> Cc: >>> Suggested-by: Gao Xiang >>> Signed-off-by: Jeffle Xu >>> --- >>> changes since v1: >>> - improve commit log suggested by Eric Whitney >>> - update "Suggested-by" title for Gao Xian, who actually found this bug >>> code >>> - add check for s_dirtyclusters_counter in .destroy_inode() >>> --- >>> fs/ext4/inode.c | 5 +++++ >>> fs/ext4/super.c | 6 ++++++ >>> 2 files changed, 11 insertions(+) >>> >>> diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c >>> index d8de607849df..73daf9443e5e 100644 >>> --- a/fs/ext4/inode.c >>> +++ b/fs/ext4/inode.c >>> @@ -1640,6 +1640,7 @@ static int ext4_insert_delayed_block(struct inode *inode, ext4_lblk_t lblk) >>> struct ext4_sb_info *sbi = EXT4_SB(inode->i_sb); >>> int ret; >>> bool allocated = false; >>> + bool reserved = false; >>> >>> /* >>> * If the cluster containing lblk is shared with a delayed, >>> @@ -1656,6 +1657,7 @@ static int ext4_insert_delayed_block(struct inode *inode, ext4_lblk_t lblk) >>> ret = ext4_da_reserve_space(inode); >>> if (ret != 0) /* ENOSPC */ >>> goto errout; >>> + reserved = true; >>> } else { /* bigalloc */ >>> if (!ext4_es_scan_clu(inode, &ext4_es_is_delonly, lblk)) { >>> if (!ext4_es_scan_clu(inode, >>> @@ -1668,6 +1670,7 @@ static int ext4_insert_delayed_block(struct inode *inode, ext4_lblk_t lblk) >>> ret = ext4_da_reserve_space(inode); >>> if (ret != 0) /* ENOSPC */ >>> goto errout; >>> + reserved = true; >>> } else { >>> allocated = true; >>> } >>> @@ -1678,6 +1681,8 @@ static int ext4_insert_delayed_block(struct inode *inode, ext4_lblk_t lblk) >>> } >>> >>> ret = ext4_es_insert_delayed_block(inode, lblk, allocated); >>> + if (ret && reserved) >>> + ext4_da_release_space(inode, 1); >>> >>> errout: >>> return ret; >>> diff --git a/fs/ext4/super.c b/fs/ext4/super.c >>> index dfa09a277b56..61bf52b58fca 100644 >>> --- a/fs/ext4/super.c >>> +++ b/fs/ext4/super.c >>> @@ -1351,6 +1351,12 @@ static void ext4_destroy_inode(struct inode *inode) >>> true); >>> dump_stack(); >>> } >>> + >>> + if (EXT4_I(inode)->i_reserved_data_blocks) >>> + ext4_msg(inode->i_sb, KERN_ERR, >>> + "Inode %lu (%p): i_reserved_data_blocks (%u) not cleared!", >>> + inode->i_ino, EXT4_I(inode), >>> + EXT4_I(inode)->i_reserved_data_blocks); >>> } >>> >>> static void init_once(void *foo) >>> -- >>> 2.27.0 >>> >> >> Looks good, passed 4k xfstests-bld regression. Feel free to add: >> >> Reviewed-by: Eric Whitney > > > Hi tytso, it's a bug fix and it would be great if it could be merged to > 5.15. > ping ... -- Thanks, Jeffle