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 1AA08C28D13 for ; Thu, 25 Aug 2022 10:57:08 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 8C987940008; Thu, 25 Aug 2022 06:57:07 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 87A19940007; Thu, 25 Aug 2022 06:57:07 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 76882940008; Thu, 25 Aug 2022 06:57:07 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0013.hostedemail.com [216.40.44.13]) by kanga.kvack.org (Postfix) with ESMTP id 688CF940007 for ; Thu, 25 Aug 2022 06:57:07 -0400 (EDT) Received: from smtpin26.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay09.hostedemail.com (Postfix) with ESMTP id 34A3880508 for ; Thu, 25 Aug 2022 10:57:07 +0000 (UTC) X-FDA: 79837812894.26.AF8647F Received: from smtp-out2.suse.de (smtp-out2.suse.de [195.135.220.29]) by imf18.hostedemail.com (Postfix) with ESMTP id A59E91C000D for ; Thu, 25 Aug 2022 10:57:06 +0000 (UTC) Received: from imap2.suse-dmz.suse.de (imap2.suse-dmz.suse.de [192.168.254.74]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature ECDSA (P-521) server-digest SHA512) (No client certificate requested) by smtp-out2.suse.de (Postfix) with ESMTPS id 44A855C446; Thu, 25 Aug 2022 10:57:05 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=suse.cz; s=susede2_rsa; t=1661425025; h=from:from:reply-to:date:date:message-id:message-id:to:to:cc:cc: mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=s8SR+gQMDrofSbIUhaVQPDx4KdCnSHqChKukc6toPhQ=; b=zNcQo8pgYOyT64drYJ4KsptavsKSn/4hSmJPIGPZru1XLtadGjWmZ4sElVNQ1c2UOwafoj WCuiE6Q21TwWNKPhYN1ke/E2MrqeWID8GbgtumvfGHt3iF9LMhFKy53+5Q2bv0S68xq+8E fIEpFea4EEQf8efz+xn6ZlKL/IESfyw= DKIM-Signature: v=1; a=ed25519-sha256; c=relaxed/relaxed; d=suse.cz; s=susede2_ed25519; t=1661425025; h=from:from:reply-to:date:date:message-id:message-id:to:to:cc:cc: mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=s8SR+gQMDrofSbIUhaVQPDx4KdCnSHqChKukc6toPhQ=; b=DBRXnjRFbihD55jU3qG3QBzK4r7+40apmR5Aeo6f82wLz/CIA9KFOpAep46zrxlE/SkTZX ArVReGQqIyOKEQDw== Received: from imap2.suse-dmz.suse.de (imap2.suse-dmz.suse.de [192.168.254.74]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature ECDSA (P-521) server-digest SHA512) (No client certificate requested) by imap2.suse-dmz.suse.de (Postfix) with ESMTPS id 1BF0013517; Thu, 25 Aug 2022 10:57:05 +0000 (UTC) Received: from dovecot-director2.suse.de ([192.168.254.65]) by imap2.suse-dmz.suse.de with ESMTPSA id b9PDBoFVB2PhbAAAMHmgww (envelope-from ); Thu, 25 Aug 2022 10:57:05 +0000 Received: by quack3.suse.cz (Postfix, from userid 1000) id 86879A0679; Thu, 25 Aug 2022 12:57:04 +0200 (CEST) Date: Thu, 25 Aug 2022 12:57:04 +0200 From: Jan Kara To: Zhihao Cheng Cc: linux-ext4@vger.kernel.org, akpm@linux-foundation.org, jack@suse.cz, mgorman@suse.de, linux-mm@kvack.org, linux-kernel@vger.kernel.org, yukuai3@huawei.com, yi.zhang@huawei.com Subject: Re: [PATCH] mm: migrate: buffer_migrate_folio_norefs() fallback migrate not uptodate pages Message-ID: <20220825105704.e46hz6dp6opawsjk@quack3> References: <20220825080146.2021641-1-chengzhihao1@huawei.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20220825080146.2021641-1-chengzhihao1@huawei.com> ARC-Authentication-Results: i=1; imf18.hostedemail.com; dkim=pass header.d=suse.cz header.s=susede2_rsa header.b=zNcQo8pg; dkim=pass header.d=suse.cz header.s=susede2_ed25519 header.b=DBRXnjRF; spf=pass (imf18.hostedemail.com: domain of jack@suse.cz designates 195.135.220.29 as permitted sender) smtp.mailfrom=jack@suse.cz; dmarc=none ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1661425026; a=rsa-sha256; cv=none; b=d+gsYFjz0yjYCX2+789+839Xw6hxx8Kk7/xwYoeJ9ZOnPUI6QbiYtzgqbNUtNXJiY7KTg4 J8rF5ceLpmXDwuYPTurS2b6tJ0INwivrxfmIDUQe11ssKMfS9E/Jo/Jv5g07jNovmxJyY6 cRBvFu1s3FqkLQNWoMaruTfNwZqWp5w= ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1661425026; 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: in-reply-to:in-reply-to:references:references:dkim-signature; bh=s8SR+gQMDrofSbIUhaVQPDx4KdCnSHqChKukc6toPhQ=; b=8GdL057+nux0ROFSSvkobMJk2lPFyRqliJ3iqnEV+Q03dzZ0vbUYZIGfMOBY26ylhBsUOr LUTnTwnHQeHK+fK4PHfADZljSUiK01Kzpq00H6dLFmTjwMICJsM9WlFIdnd8D4QmqwJYQd TDNhvtHQK/94f1XWFnU2frR4xlGZ71M= X-Rspam-User: X-Rspamd-Queue-Id: A59E91C000D X-Stat-Signature: ru48ya64t6wc97ebpiuuj1kz7a8d6c87 Authentication-Results: imf18.hostedemail.com; dkim=pass header.d=suse.cz header.s=susede2_rsa header.b=zNcQo8pg; dkim=pass header.d=suse.cz header.s=susede2_ed25519 header.b=DBRXnjRF; spf=pass (imf18.hostedemail.com: domain of jack@suse.cz designates 195.135.220.29 as permitted sender) smtp.mailfrom=jack@suse.cz; dmarc=none X-Rspamd-Server: rspam01 X-HE-Tag: 1661425026-426664 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: On Thu 25-08-22 16:01:46, Zhihao Cheng wrote: > From: Zhang Yi > > Recently we notice that ext4 filesystem occasionally fail to read > metadata from disk and report error message, but the disk and block > layer looks fine. After analyse, we lockon commit 88dbcbb3a484 > ("blkdev: avoid migration stalls for blkdev pages"). It provide a > migration method for the bdev, we could move page that has buffers > without extra users now, but it will lock the buffers on the page, which > breaks a lot of current filesystem's fragile metadata read operations, > like ll_rw_block() for common usage and ext4_read_bh_lock() for ext4, > these helpers just trylock the buffer and skip submit IO if it lock > failed, many callers just wait_on_buffer() and conclude IO error if the > buffer is not uptodate after buffer unlocked. > > This issue could be easily reproduced by add some delay just after > buffer_migrate_lock_buffers() in __buffer_migrate_folio() and do > fsstress on ext4 filesystem. > > EXT4-fs error (device pmem1): __ext4_find_entry:1658: inode #73193: > comm fsstress: reading directory lblock 0 > EXT4-fs error (device pmem1): __ext4_find_entry:1658: inode #75334: > comm fsstress: reading directory lblock 0 > > Something like ll_rw_block() should be used carefully and seems could > only be safely used for the readahead case. So the best way is to fix > the read operations in filesystem in the long run, but now let us avoid > this issue first. This patch avoid this issue by fallback to migrate > pages that are not uotodate like fallback_migrate_folio(), those pages > that has buffers may probably do read operation soon. > > Fixes: 88dbcbb3a484 ("blkdev: avoid migration stalls for blkdev pages") > Signed-off-by: Zhang Yi > Signed-off-by: Zhihao Cheng Thanks for the analysis and the fix! As you noted above this is actually a bug in the filesystems that they assume that locked buffer means it is under IO. Usually that is the case but there are other places that lock the buffer without doing IO. Page migration is one of them, jbd2 machinery another one, there may be others. So I think this really ought to be fixed in filesystems instead of papering over the bug in the migration code. I agree this is more work but we will reduce the technical debt, not increase it :). Honestly, ll_rw_block() should just die. It is actively dangerous to use. Instead we should have one call for readahead of bhs and the rest should be converted to submit_bh() or similar calls. There are like 25 callers remaining so it won't be even that hard. And then we have the same buggy code as in ll_rw_block() copied to ext4_bread_batch() (ext4_read_bh_lock() in particular) so that needs to be fixed as well... Honza > --- > mm/migrate.c | 32 ++++++++++++++++++++++++++++++++ > 1 file changed, 32 insertions(+) > > diff --git a/mm/migrate.c b/mm/migrate.c > index 6a1597c92261..bded69867619 100644 > --- a/mm/migrate.c > +++ b/mm/migrate.c > @@ -691,6 +691,38 @@ static int __buffer_migrate_folio(struct address_space *mapping, > if (!head) > return migrate_folio(mapping, dst, src, mode); > > + /* > + * If the mapped buffers on the page are not uptodate and has refcount, > + * some others may propably try to lock the buffer and submit read IO > + * through ll_rw_block(), but it will not submit IO once it failed to > + * lock the buffer, so try to fallback to migrate_folio() to prevent > + * false positive EIO. > + */ > + if (check_refs) { > + bool uptodate = true; > + bool invalidate = false; > + > + bh = head; > + do { > + if (buffer_mapped(bh) && !buffer_uptodate(bh)) { > + uptodate = false; > + if (atomic_read(&bh->b_count)) { > + invalidate = true; > + break; > + } > + } > + bh = bh->b_this_page; > + } while (bh != head); > + > + if (!uptodate) { > + if (invalidate) > + invalidate_bh_lrus(); > + if (filemap_release_folio(src, GFP_KERNEL)) > + return migrate_folio(mapping, dst, src, mode); > + return -EAGAIN; > + } > + } > + > /* Check whether page does not have extra refs before we do more work */ > expected_count = folio_expected_refs(mapping, src); > if (folio_ref_count(src) != expected_count) > -- > 2.31.1 > -- Jan Kara SUSE Labs, CR