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 75910C25B10 for ; Fri, 10 May 2024 03:40:00 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id D22CF6B0088; Thu, 9 May 2024 23:39:59 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id CD30B6B0089; Thu, 9 May 2024 23:39:59 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id B73666B008C; Thu, 9 May 2024 23:39:59 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0017.hostedemail.com [216.40.44.17]) by kanga.kvack.org (Postfix) with ESMTP id 90CE16B0088 for ; Thu, 9 May 2024 23:39:59 -0400 (EDT) Received: from smtpin24.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay06.hostedemail.com (Postfix) with ESMTP id 3A48CA2732 for ; Fri, 10 May 2024 03:39:59 +0000 (UTC) X-FDA: 82101082518.24.31CEC93 Received: from dggsgout11.his.huawei.com (unknown [45.249.212.51]) by imf12.hostedemail.com (Postfix) with ESMTP id 076F540012 for ; Fri, 10 May 2024 03:39:55 +0000 (UTC) Authentication-Results: imf12.hostedemail.com; dkim=none; dmarc=none; spf=none (imf12.hostedemail.com: domain of yi.zhang@huaweicloud.com has no SPF policy when checking 45.249.212.51) smtp.mailfrom=yi.zhang@huaweicloud.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1715312397; 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=ZTDKCxWnIqjU6NntWqniyz7S4EDqs9/1YbYmySIRDxM=; b=enNMqj+2if7ovQYx0G3PTzsYRxhCAFFGBrXv5BUaAzTjgvGh98TjsqUXvVdus7zRFu/Q1g BF/zUj3YpN7pnOjT2evHfCjekwqwYi/ZF2DiNfCoaSlSRYOmmapWxL7UyTFakWJgJAEstm NrKFAdJp4pKLrmrrewtbAkar3hb3mnQ= ARC-Authentication-Results: i=1; imf12.hostedemail.com; dkim=none; dmarc=none; spf=none (imf12.hostedemail.com: domain of yi.zhang@huaweicloud.com has no SPF policy when checking 45.249.212.51) smtp.mailfrom=yi.zhang@huaweicloud.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1715312397; a=rsa-sha256; cv=none; b=RR9P6tqSegWOYMg+Z45zvjY/eyBBSYxjNXZW6VpLWUQv2k6YdLmA6TxEgumUMKQT7zxaQx /nZ8RyI29btQZ4swaICYlXj88gZ1rCaT6ptqkvq//qVurO5uL3XZ+PoTwjQj71QMUVkQei cBlyuyfARCuByKNb0NjrVWpuR30BwAQ= Received: from mail.maildlp.com (unknown [172.19.93.142]) by dggsgout11.his.huawei.com (SkyGuard) with ESMTP id 4VbF4d2FkCz4f3m81 for ; Fri, 10 May 2024 11:39:45 +0800 (CST) Received: from mail02.huawei.com (unknown [10.116.40.75]) by mail.maildlp.com (Postfix) with ESMTP id AA1A71A017F for ; Fri, 10 May 2024 11:39:50 +0800 (CST) Received: from [10.174.179.80] (unknown [10.174.179.80]) by APP2 (Coremail) with SMTP id Syh0CgAnmAsElz1m8DcpMg--.16051S3; Fri, 10 May 2024 11:39:50 +0800 (CST) Subject: Re: [PATCH v3 03/26] ext4: correct the hole length returned by ext4_map_blocks() To: Luis Henriques , Theodore Ts'o Cc: linux-ext4@vger.kernel.org, linux-fsdevel@vger.kernel.org, linux-mm@kvack.org, linux-kernel@vger.kernel.org, adilger.kernel@dilger.ca, jack@suse.cz, ritesh.list@gmail.com, hch@infradead.org, 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-4-yi.zhang@huaweicloud.com> <87zfszuib1.fsf@brahms.olymp> <20240509163953.GI3620298@mit.edu> <87h6f6vqzj.fsf@brahms.olymp> From: Zhang Yi Message-ID: Date: Fri, 10 May 2024 11:39:48 +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: <87h6f6vqzj.fsf@brahms.olymp> Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit X-CM-TRANSID:Syh0CgAnmAsElz1m8DcpMg--.16051S3 X-Coremail-Antispam: 1UD129KBjvJXoWxJFyxGw15KFWfuF4DAF4Dtwb_yoWrCw48pF WfAa1Utr1kG340krZ7Aw1rX3WS9w45C3y3ArWfWryfAas8ur1kGFyxKFWY9F97ur48u3ya qayjqFy7KF1qvFDanT9S1TB71UUUUUUqnTZGkaVYY2UrUUUUjbIjqfuFe4nvWSU5nxnvy2 9KBjDU0xBIdaVrnRJUUUvIb4IE77IF4wAFF20E14v26ryj6rWUM7CY07I20VC2zVCF04k2 6cxKx2IYs7xG6rWj6s0DM7CIcVAFz4kK6r1j6r18M28lY4IEw2IIxxk0rwA2F7IY1VAKz4 vEj48ve4kI8wA2z4x0Y4vE2Ix0cI8IcVAFwI0_tr0E3s1l84ACjcxK6xIIjxv20xvEc7Cj xVAFwI0_Gr1j6F4UJwA2z4x0Y4vEx4A2jsIE14v26rxl6s0DM28EF7xvwVC2z280aVCY1x 0267AKxVW0oVCq3wAS0I0E0xvYzxvE52x082IY62kv0487Mc02F40EFcxC0VAKzVAqx4xG 6I80ewAv7VC0I7IYx2IY67AKxVWUJVWUGwAv7VC2z280aVAFwI0_Jr0_Gr1lOx8S6xCaFV Cjc4AY6r1j6r4UM4x0Y48IcVAKI48JM4IIrI8v6xkF7I0E8cxan2IY04v7Mxk0xIA0c2IE e2xFo4CEbIxvr21l42xK82IYc2Ij64vIr41l4I8I3I0E4IkC6x0Yz7v_Jr0_Gr1lx2IqxV Aqx4xG67AKxVWUJVWUGwC20s026x8GjcxK67AKxVWUGVWUWwC2zVAF1VAY17CE14v26r4a 6rW5MIIYrxkI7VAKI48JMIIF0xvE2Ix0cI8IcVAFwI0_Jr0_JF4lIxAIcVC0I7IYx2IY6x kF7I0E14v26r4j6F4UMIIF0xvE42xK8VAvwI8IcIk0rVWrZr1j6s0DMIIF0xvEx4A2jsIE 14v26r1j6r4UMIIF0xvEx4A2jsIEc7CjxVAFwI0_Gr0_Gr1UYxBIdaVFxhVjvjDU0xZFpf 9x07UZ18PUUUUU= X-CM-SenderInfo: d1lo6xhdqjqx5xdzvxpfor3voofrz/ X-Rspamd-Server: rspam01 X-Stat-Signature: wd9s4n5yf8g98hot9w5pjxdgydm48sr3 X-Rspam-User: X-Rspamd-Queue-Id: 076F540012 X-HE-Tag: 1715312395-244136 X-HE-Meta: U2FsdGVkX1+ol/iVantupSgQB51/heReIAlQZs19aT4UCNnSzNeAFCHCeF2PkuM5kGS2i+Tyxmj2QFn6/QhwmcEYL8zr/Fu4D3aGrS9vUtOGGe6jR4vS/YBqjIfFP5LR4E1mGX4jm2ioitf1N+QcbpLpjELpzjNt5h0mDm1/WADEkNGKpQP6qf6kmpbCRnVeJCrO8BjWVgEDHTDnIUqmQepPJU2SypW9Cqn79yHKb3G7uUWMG7wQCJLdRkkwzcC1V7VqDOV/VHoc6WwH2mFXRtZD+MQVmjQ160RE4iPc+Z7D12dJeMD+mKZgxTXsVq9L42YfIQenmHEWrZ3SLaBz1KjVxAhz5sHrewKl9Ke+5Dy1NygR1BczVlhadHSlqwtiE8DiJ0qiuiilMt21Fuw6f28AJYjswIJzfd1PrUulPmrH2v9bL15HHhTtJKPT2CoaaUAGAKFOcMywkjvSsCGcGtd2z2oOLDvAkAdqV3v2tKVJrzG5liwKFJhOAXLgTNKDhOIP21bOfoFaqKrdfUQEltwG2MvebDY0t9NYoCBJN8VTz6EohYIt6zZjGcrH1N06sCAt4y+c8Yli5NKeSc7vdaBP4G8oNSfLeFrHSqDMSoOE5vvBiVPrUvT9gTph6N/xOyvMaU6LBzhlPjWbQSaQq7mkU7ps4ZvJ8T6UucvxQZM/MiZpOk0rN7dAocBNM9oVsPfwuKWAVt7BH/d0y5LutT4/oqH89q/REyhekCiQTHwbLLdKb1xeUcHRzMdl9NrHKZezCuT2D+jFMREG3UMPQxF7neq7XhAvHYAILXK2V9it+b/nKj6RWZ9LD1n6ESkRDZC+qW9+nHIfZmakAa2GwnTLQT9IbaCPg5VLIBPcTDn3q+CUltfnLj9VzM4DNTJkcHiv0gHl44PscgI7zXnWs8+DltlQYAeGtQ32rr/dwgQMCPIhTqmY66Zjf+slk9XpL54ftFdX1ul1UsK+JQ4 xn+lyZU8 Ij7ZOKbnCx2eSbg/HTgLOVMbH89zIugkhOAdq9F2rRNkWdN/dTiOzEDoN5fxCb8vXhIrisB16xqoznd3YtmAI7bk1fx8h50DZyqU/iaB8vG1FyDYiV6BiNvnB9+ALXf35HSNd6wPRUwTZ7cQQwddMfwkLIdySAEs/6jh1ftsmVGOep3KHEPG6s0nhREOyF4GMmFrFcpDK8Yr8K0I3N9nbGrm+k+rP2oAkzuoKr/mSuIADavPaWwkPRebcoIoVm1w4ygB1twXl6whLktEqlC5+/Mw582rMmxEJwJa3gjffUvhzKyRne6LjsFFFSw== 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/5/10 1:23, Luis Henriques wrote: > On Thu 09 May 2024 12:39:53 PM -04, Theodore Ts'o wrote; > >> On Thu, May 09, 2024 at 04:16:34PM +0100, Luis Henriques wrote: >>> >>> It's looks like it's easy to trigger an infinite loop here using fstest >>> generic/039. If I understand it correctly (which doesn't happen as often >>> as I'd like), this is due to an integer overflow in the 'if' condition, >>> and should be fixed with the patch below. >> >> Thanks for the report. However, I can't reproduce the failure, and >> looking at generic/039, I don't see how it could be relevant to the >> code path in question. Generic/039 creates a test symlink with two >> hard links in the same directory, syncs the file system, and then >> removes one of the hard links, and then drops access to the block >> device using dmflakey. So I don't see how the extent code would be >> involved at all. Are you sure that you have the correct test listed? > > Yep, I just retested and it's definitely generic/039. I'm using a simple > test environment, with virtme-ng. > >> Looking at the code in question in fs/ext4/extents.c: >> >> again: >> ext4_es_find_extent_range(inode, &ext4_es_is_delayed, hole_start, >> hole_start + len - 1, &es); >> if (!es.es_len) >> goto insert_hole; >> >> * There's a delalloc extent in the hole, handle it if the delalloc >> * extent is in front of, behind and straddle the queried range. >> */ >> - if (lblk >= es.es_lblk + es.es_len) { >> + if (lblk >= ((__u64) es.es_lblk) + es.es_len) { >> /* >> * The delalloc extent is in front of the queried range, >> * find again from the queried start block. >> len -= lblk - hole_start; >> hole_start = lblk; >> goto again; >> >> lblk and es.es_lblk are both __u32. So the infinite loop is >> presumably because es.es_lblk + es.es_len has overflowed. This should >> never happen(tm), and in fact we have a test for this case which > > If I instrument the code, I can see that es.es_len is definitely set to > EXT_MAX_BLOCKS, which will overflow. > Thanks for the report. After looking at the code, I think the root cause of this issue is the variable es was not initialized on replaying fast commit. ext4_es_find_extent_range() will return directly when EXT4_FC_REPLAY flag is set, and then the es.len becomes stall. I can always reproduce this issue on generic/039 with MKFS_OPTIONS="-O fast_commit". This uninitialization problem originally existed in the old ext4_ext_put_gap_in_cache(), but it didn't trigger any real problem since we never check and use extent cache when replaying fast commit. So I suppose the correct fix would be to unconditionally initialize the es variable. Thanks, Yi. >> *should* have gotten tripped when ext4_es_find_extent_range() calls >> __es_tree_search() in fs/ext4/extents_status.c: >> >> static inline ext4_lblk_t ext4_es_end(struct extent_status *es) >> { >> BUG_ON(es->es_lblk + es->es_len < es->es_lblk); >> return es->es_lblk + es->es_len - 1; >> } >> >> So the patch is harmless, and I can see how it might fix what you were >> seeing --- but I'm a bit nervous that I can't reproduce it and the >> commit description claims that it reproduces easily; and we should >> have never allowed the entry to have gotten introduced into the >> extents status tree in the first place, and if it had been introduced, >> it should have been caught before it was returned by >> ext4_es_find_extent_range(). >> >> Can you give more details about the reproducer; can you double check >> the test id, and how easily you can trigger the failure, and what is >> the hardware you used to run the test? > > So, here's few more details that may clarify, and that I should have added > to the commit description: > > When the test hangs, the test is blocked mounting the flakey device: > > mount -t ext4 -o acl,user_xattr /dev/mapper/flakey-test /mnt/scratch > > which will eventually call into ext4_ext_map_blocks(), triggering the bug. > > Also, some more code instrumentation shows that after the call to > ext4_ext_find_hole(), the 'hole_start' will be set to '1' and 'len' to > '0xfffffffe'. This '0xfffffffe' value is a bit odd, but it comes from the > fact that, in ext4_ext_find_hole(), the call to > ext4_ext_next_allocated_block() will return EXT_MAX_BLOCKS and 'len' will > thus be set to 'EXT_MAX_BLOCKS - 1'. > > Does this make sense? > > Cheers, >